Community
Participate
Working Groups
Created attachment 131221 [details] JavaModelException stacktrace I have a Java annotation that looks like this: @SecondaryTables({@SecondaryTable(name = "FOO"),@SecondaryTable(name = "BAR")}) When I try to access the value of the name element of either of the nested SecondaryTable annotations I get a JavaModelException indicating the annotation does not exist: Java Model Status [@javax.persistence.SecondaryTable [in @javax.persistence.SecondaryTables [in Employee [in Employee.class [in model [in lib/myJPA.jar [in Bar]]]]]] does not exist] We have traced this to a problem in o.e.jdt.internal.core.ClassFileInfo#generateAnnotationInfo(JavaElement, HashMap, IBinaryAnnotation). This method has a check to see if the value of any of the new annotation's element value pairs is yet another annotation, so the corresponding handle for that binary annotation can be generated also. Unfortunately, it does not check for an array and whether that array contains yet more annotations. Handles for these binary annotation need to be generated also. I tried to add this check for an array; but ran into a problem. The annotation handle's "value" is based on the annotation's name plus the names of the annotation's "ancestors" (parent etc.). As a result, all the annotations in the array have the same handle; resulting in only a single handle being added to the 'newElements' map. I added a bit more code, incrementing the annotation handle's 'occurrenceCount'. This felt a bit hacky, but seemed to at least get all the annotations in the array added to 'newElements'. This "fixed" the JavaModelException; but then other behavior was broken: When I call Annotation#getMemberValuePairs() on the "outer" annotation (in the above example, SecondaryTables), the value of the "value" member value pair is an array of two identical annotation handles. They both have the same name "SecondaryTable") and same occurrence count (1). As a result, I can only retrieve the settings for the first nested annotation; all others are inaccessible. These handles appear to be incorrectly constructed in o.e.jdt.internal.core.util.Util#getAnnotationMemberValue(JavaElement, MemberValuePair, Object). When retrieving the nested annotations, if I hack the handle's occurrence count to be 2, the JavaModelCache does find the appropriate AnnotationInfo. It's just not clear how I should change Util and whether there are other places that need to be modified. Nor is it clear that changing the 'occurrenceCount' is the correct approach....
Created attachment 131226 [details] incomplete attempt at fix This patch hacks a fix for the example in the original description; but does not handle something like this correctly: @JoinTable(name="EMP_PROJ", joinColumns={ @JoinColumn(name="EMP_ID", referencedColumnName="EMP_ID") }, inverseJoinColumns={ @JoinColumn(name="PROJ_ID", referencedColumnName="PROJ_ID") } ) private Collection<Project> projects = new HashSet<Project>(); The nested annotations cannot simply differentiated by their occurrence counts within the "outer" annotation; they must also be differentiated by the name of the "outer" annotation's member that references them. For example, the first JoinColumn above is occurrence 1 inside the "joinColumns" array inside the JoinTable annotation inside the member "projects" etc.... FYI: These examples are using annotations specified by the JPA spec.
Could you please provide a test case that reproduces this issue?
Never mind. Reproduced.
I'm guessing the "parent" of the nested annotations needs to be the member value pair, not the outer annotation. Occurrence counts are still significant though (for both the member value pair and the nested annotation). Although, in our case, where we are only interested in binary stuff, there *shouldn't* be any duplicate member value pairs. :-) There are still duplicate nested annotations in arrays. Should I continue to try to create a patch? Or would that be duplicate effort?
Created attachment 131310 [details] First draft Could you please try this patch? I'll ask Jérôme to review it if it is fine.
(In reply to comment #5) > Could you please try this patch? As I mentioned in comment 1, this does not work when you have the same-named nested annotation it two different member value pairs of the same outer annotation. See the example in comment 1. Using this example: When I call IAnnotation.getMemberValuePairs() on the outer annotation (JoinTable) I get 3 pairs: name = "EMP_PROJ" joinColumns = [Annotation(name="JoinColumn", occurrence=1)] inverseJoinColumns = [Annotation(name="JoinColumn", occurrence=1)] When I take *either* of the nested annotations (JoinColumn) and call getMemberValuePairs() I get the same pairs: name = "EMP_ID" referencedColumnName = "EMP_ID" This because the look-up in JavaModelCache (with the current patch applied) uses only the annotation names and occurrence counts to resolve what AnnotationInfo to return. It does *not* use the member value pair key to find the correct AnnotationInfo. As a result the *same* AnnotationInfo is returned for both JoinColumns, irrespective of whether the JoinColumn is under "joinColumns" or "inverseJoinColumns".
Ok, I'll investigate that case. I'll let you know where I get with this. No need for you to continue to create a patch. It is up to you if you want to continue.
Created attachment 131318 [details] Proposed fix
Brian, Could you please try this one? Thanks.
Olivier, this seems to fix the example I discussed; but it might have one little hole remaining. I think the current patch might still misbehave when you have two nested annotations of the same type but as parts of different member value pairs and *not* in arrays. Maybe line 68 of ClassFileInfo should be changed from: generateAnnotationInfo(annotation, newElements, (IBinaryAnnotation) value); to: generateAnnotationInfo(annotation, newElements, (IBinaryAnnotation) value, new String(pairs[i].getName())); So, if the example were changed to something like this: @JoinTable(name="EMP_PROJ", joinColumn=@JoinColumn(name="EMP_ID", referencedColumnName="EMP_ID"), inverseJoinColumn=@JoinColumn(name="PROJ_ID", referencedColumnName="PROJ_ID") ) It might cause problems? Note the two problematic members are no longer arrays, just single annotations. The declaration would look something like this: public @interface JoinTable { String name() default ""; JoinColumn joinColumn; JoinColumn inverseJoinColumn; } I'm not positive of this because I don't have a test case. Just looking at the code and comparing it to what I came up with. I'm sorry, I'm just a bit short on time at the moment... Thanks!
Ok, I'll add a test case for this. I am always adding test case for source and binary forms to make sure both works as expected. I'll try to come up with a new patch later today or tomorrow if required.
Created attachment 131335 [details] Proposed fix Ok, your case was still an issue. Should be fixed now. All annotations have a reference to the member pair they belong to and an optional occurence count. Let me know what you think.
Thanks a lot, Olivier. Looks good. I only see one little thing: I originally put in lines 60-62 of ClassFileInfo in my first patch. I don't think they are needed any more, since the parent of the annotation at the point is now always a BinaryMember, and members can only have a single occurrence of each annotation type. Of course, source members can have multiple.... Anyway, I'm glad you are testing the source stuff too. I kept meaning to ask you about that. Our stuff is focused on only the binary model. (When dealing with source, we use the AST; since we manipulate it extensively.) Thanks.
(In reply to comment #13) > Looks good. I only see one little thing: I originally put in lines 60-62 of > ClassFileInfo in my first patch. I don't think they are needed any more, since > the parent of the annotation at the point is now always a BinaryMember, and > members can only have a single occurrence of each annotation type. Of course, > source members can have multiple.... I think you still need it for the array case. @SecondaryTables({@SecondaryTable(name = "FOO"),@SecondaryTable(name = "BAR")}) Both SecondaryTable annotations would have the same member value pair name, but they have a difference occurrence. I think both are needed (a member value pair name and the optional occurrence).
(In reply to comment #14) > Both SecondaryTable annotations would have the same member value pair name, but > they have a difference occurrence. > I think both are needed (a member value pair name and the optional occurrence). Ahh, right. Duh. :-) I must be distracted. :-) Anyway, it looks good. Our stuff seems to work fine when this patch is in place. Thanks.
(In reply to comment #15) > (In reply to comment #14) > > Both SecondaryTable annotations would have the same member value pair name, but > > they have a difference occurrence. > > I think both are needed (a member value pair name and the optional occurrence). > > Ahh, right. Duh. :-) I must be distracted. :-) Anyway, it looks good. Our stuff > seems to work fine when this patch is in place. > > Thanks. > But, wait. Won't those guys be processed by the new method that is used for any nested annotations? The first method should only be called when the parent is a member? Sorry to be so confused. I'm just a bit over-tired....
Toplevel annotations don't need a member type value pair name. I believe the current code works fine. I'll release it shortly. I did some refactoring a bit. Released for 3.5M7. Regression tests added in: org.eclipse.jdt.core.tests.dom.ASTConverter15Test#test0325 org.eclipse.jdt.core.tests.dom.ASTConverter15Test#test0326 org.eclipse.jdt.core.tests.dom.ASTConverter15Test#test0327 org.eclipse.jdt.core.tests.dom.ASTConverter15Test#test0328 org.eclipse.jdt.core.tests.dom.ASTConverter15Test#test0329 org.eclipse.jdt.core.tests.dom.ASTConverter15Test#test0330 org.eclipse.jdt.core.tests.dom.ASTConverter15Test#test0331 org.eclipse.jdt.core.tests.dom.ASTConverter15Test#test0332 org.eclipse.jdt.core.tests.dom.ASTConverter15Test#test0333 Let me know if you have any issue once you get a build with this fix in.
verified fixed in the 4/16 platform build, thanks!
