I have been puzzled that some of the keys which were being tested were no
where to be found in the source code. I used eclipse search, wrote a little
asm utility, used find and grep, to no avail.
Here is an example where i was using find to search for they key
xml.businessLocal.ejbObject
find -name *.java -exec grep "xml.businessLocal.ejbObject" '{}' \; -print
I got 1 result and that too was in a test case testing for this validation
key. This puzzled me because I thought, how can a key be tested, if it was
not even being used in our Validation code? So I started relaxing my search
criteria a bit i.e. just searched for the substring of the key, in this
case, here was the find command
find -name *.java -exec grep "businessLocal" '{}' \; -print
The above gave me multiple results, as you can guess, most of them were
irrelevant. However, i found one little code snippet in the result, which
solved the mystery for me. Here it is
fail(b, "xml." + tag + ".businessLocal", clazz.getName());
The key was being built by appending strings , there was no way the key
could have shown up in the original search.
In an earlier email I had mentioned that each key has a comment, which is
nothing but the code which uses it. Had that been the case for all the keys,
it would have taken me minutes to search for that code and figure out how
the key is being used and that would have helped figure out why my tests
were failing.
Anyways, I would like to propose a new addition to the Message.properties
file.
First, lets look at an example key entry in this file
# warn(b, "unused.ejb.create", b.getEjbClass(), ejbCreate.getName(),
paramString, create.toString());
1.unused.ejb.create Unused ejbCreate method
2.unused.ejb.create Unused ejbCreate method: {1}({2})
3.unused.ejb.create Create method will never be called. The bean
class {0} defines the create method {1}({2}), but there is no matching
{3}({2}) method in the home or local-home interfaces.
As you can see, the comment specifies the code where this key is being used.
Instead of this info being a comment, I would like to modify it as
0.unused.ejb.create warn(b, "unused.ejb.create", b.getEjbClass(),
ejbCreate.getName(), paramString, create.toString());
1.unused.ejb.create Unused ejbCreate method
.....
......
So instead of it being a comment, now i can load that info and write a
utility which will verify that the source code information for each key
exists and that the information is correct. Sometimes changes to source code
may be made, but this file might not be updated, in those scenarios, the
utility will be very helpful in keeping things in sync.
Thoughts?
On Fri, Jul 16, 2010 at 12:29 AM, David Blevins <[email protected]>wrote:
>
> On Jul 15, 2010, at 7:42 PM, Karan Malhi wrote:
>
> > The key persistenceContextRef.noMatches is also not used. Should i just
> > start deleting these keys from Messages.properties, or probably comment
> them
> > out?
>
> If it is something worth checking and not covered, we should just add
> checks for it if possible. Fine to comment it out with a TODO in the
> meantime.
>
> -David
>
> >
> > On Thu, Jul 15, 2010 at 9:23 PM, David Blevins <[email protected]
> >wrote:
> >
> >>
> >> On Jul 15, 2010, at 5:26 PM, David Jencks wrote:
> >>
> >>> I think that particular one's xml element/jee tree class got renamed to
> >> ConcurrencyManagementType late in the spec. I don't know if it's being
> >> handled in code yet...so this may be of no use :-\
> >>
> >> Renamed and relocated from assembly descriptor to under <session>
> >>
> >> Anyway, here's the code that drives concurrency-attribute:
> >>
> >> /*
> >> * @Lock
> >> */
> >> if (sessionBean.getConcurrencyManagementType() ==
> >> ConcurrencyManagementType.CONTAINER) {
> >> processAttributes(new
> >> ConcurrencyAttributeHandler(assemblyDescriptor, ejbName), clazz,
> >> inheritedClassFinder);
> >> } else {
> >> checkAttributes(new
> ConcurrencyAttributeHandler(assemblyDescriptor,
> >> ejbName), ejbName, ejbModule, classFinder,
> "invalidConcurrencyAttribute");
> >> }
> >>
> >> It uses the same abstraction for method based processing as transaction
> >> attribute does (and also revealing of why it's hard to change that code
> to
> >> the different element)
> >>
> >> /*
> >> * TransactionAttribute
> >> */
> >> if (bean.getTransactionType() == TransactionType.CONTAINER) {
> >> processAttributes(new
> >> TransactionAttributeHandler(assemblyDescriptor, ejbName), clazz,
> >> inheritedClassFinder);
> >> } else {
> >> checkAttributes(new
> TransactionAttributeHandler(assemblyDescriptor,
> >> ejbName), ejbName, ejbModule, classFinder,
> "invalidTransactionAttribute");
> >> }
> >>
> >>
> >> On the assembler side, the code for the above and security attributes
> all
> >> uses the same algorithm (MethodAttributeInfo +
> >> MethodInfoUtil.resolveAttributes).
> >>
> >> Anyway, skip that message key because the relating code will greatly
> change
> >> at some painful point :)
> >>
> >>
> >> -David
> >>
> >>
> >>>
> >>> On Jul 15, 2010, at 4:40 PM, Karan Malhi wrote:
> >>>
> >>>> What needs to be done with keys which are listed in
> Messages.properties
> >> but
> >>>> are not used anywhere in the java code.? For example,
> >>>> xml.invalidConcurrencyAttribute cannot be found in any .java file in
> the
> >>>> ejb-core project, so I am assuming that there is not validation for
> >> this.
> >>>>
> >>>> On Thu, Jul 15, 2010 at 5:57 PM, Karan Malhi <[email protected]>
> >> wrote:
> >>>>
> >>>>> In the messages.properties, some of the keys have nice comments above
> >> them,
> >>>>> like fail() or warn(). This gives a good idea to the test author
> >> whether to
> >>>>> test for a warning or failure. Please try and make sure if you add
> keys
> >> to
> >>>>> this file, also supplement the keys with the little comment.
> >>>>>
> >>>>>
> >>>>> On Wed, Jul 14, 2010 at 11:38 PM, David Blevins <
> >> [email protected]>wrote:
> >>>>>
> >>>>>>
> >>>>>> On Jun 10, 2010, at 10:48 PM, David Blevins wrote:
> >>>>>>
> >>>>>>>> We really should have 100% coverage. If there is a message key
> for
> >> it
> >>>>>> in ../config/rules/Messages.properties, than there really should be
> a
> >> test
> >>>>>> for that scenario.
> >>>>>>>
> >>>>>>> Basically, we need to go through this list and make sure there are
> >> tests
> >>>>>> to challenge every message key:
> >>>>>>
> >>>>>> FYI, to those who haven't seen it. This is pretty darn cool!!
> >>>>>>
> >>>>>>
> >>>>>>
> >>
> https://cwiki.apache.org/confluence/display/OPENEJB/Validation+Keys+Audit+Report
> >>>>>>
> >>>>>> Innovative stuff!
> >>>>>>
> >>>>>> -David
> >>>>>>
> >>>>>>
> >>>>>
> >>>>>
> >>>>> --
> >>>>> Karan Singh Malhi
> >>>>>
> >>>>
> >>>>
> >>>>
> >>>> --
> >>>> Karan Singh Malhi
> >>>
> >>>
> >>
> >>
> >
> >
> > --
> > Karan Singh Malhi
>
>
--
Karan Singh Malhi
