[#1440] handle adapter in mark-generated plugin#1864
[#1440] handle adapter in mark-generated plugin#1864laurentschoelens wants to merge 1 commit intoeclipse-ee4j:masterfrom
Conversation
lukasj
left a comment
lukasj
left a comment
There was a problem hiding this comment.
Good start but I believe the fix should be done on a different place.
| JPackage pkg = Ring.get(ClassSelector.class).getClassScope().getOwnerPackage(); | ||
| adapter = pkg._class("Adapter"+id); | ||
| // reference adapter in the CodeModel instance | ||
| pkg.owner()._adapter(adapter); |
There was a problem hiding this comment.
adapter is a class belonging to the package, therefore adding it to the core codemodel just doubles pointers to this class - and so it is not the right thing to do.
| private final HashMap<String,JPackage> packages = new HashMap<>(); | ||
|
|
||
| /** The adapters that this JCodeWriter contains. */ | ||
| private final HashMap<String,JDefinedClass> adapters = new HashMap<>(); |
There was a problem hiding this comment.
if it is to stay, it should be Map instead, unless there is a will to update both fields in this class at once through the new PR (together with checking the rest of the codebase)
| * Returns an iterator that walks the adapters defined using this code writer. | ||
| * @return an iterator of the adapters used by this code writer | ||
| */ | ||
| public Iterator<JDefinedClass> adapters() { |
There was a problem hiding this comment.
Why are these methods needed? each adapter should be already available through packages().classes(). If it is not there, then the bug is likely to be somewhere around lines where the adapter class is being defined. That can be a place where an instance of JCodeModel and an instance of Model become out of sync.
| } | ||
|
|
||
| // annotate XmlAdapter generated-classes | ||
| Iterator<JDefinedClass> it = model.getCodeModel().adapters(); |
There was a problem hiding this comment.
with the right fix, this should be already covered by model.getClasses() part few lines above
There was a problem hiding this comment.
I think adapters class are just referenced in package and not model itself that's why I was going this way all around
There was a problem hiding this comment.
if they are referenced in a package only then making them available through the package sounds like better thing to do
There was a problem hiding this comment.
Like having public Iterator<JDefinedClass> adapters() but in PackageOutline ?
There was a problem hiding this comment.
It makes more sense there on the package than on the (module?) model to me. Yet it would be good to understand why it is not or it cannot be just taken from classes.
There was a problem hiding this comment.
I'd start debugging from the BIConversion - that's where the adapter gets created as you've already found out or from the BeanGenerator (around line 169) - that's where the adapter is still available in the codemodel.packages().classes() to figure out why it is not available in the outline/model passed later to a plugin(s)
There was a problem hiding this comment.
Since I have unit test to reproduice the issue and test the fix, I'll take another round on it
There was a problem hiding this comment.
In any case it's not available in the classes of the model since the issue is here 😄
There was a problem hiding this comment.
that is likely because outline.classes != codemodel.classes - codemodel has it, outline does not
There was a problem hiding this comment.
The only way to get to the Adapter class without that PR is to go through :
model(Outline) ->packageContexts
iterate over each PackageOutline -> _package (JPackage) -> classes (JDefinedClass)
This also returns the xsd generated classes (which are and are not availables in model.getClasses() like XSD generated classes and ObjectFactory)
Don't know if going through this will work for "all generated code" ?
|
Closing in favor of #1902 |
2 participants
Fixes #1440
Generated XmlAdapter classes are now referenced in CodeModel and can be used in plugin like the
mark-generatedoneAdded test case too