[MDSAL-49] Do not generate Builders for Union types Created: 31/Oct/14 Updated: 14/Apr/22 Resolved: 14/Apr/22 |
|
| Status: | Resolved |
| Project: | mdsal |
| Component/s: | Binding codegen |
| Affects Version/s: | None |
| Fix Version/s: | 10.0.0 |
| Type: | Task | ||
| Reporter: | Robert Varga | Assignee: | Robert Varga |
| Resolution: | Done | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Environment: |
Operating System: All |
||
| Issue Links: |
|
||||||||||||||||||||||||||||||||
| Description |
|
Our codegen requires users to manually instantiate the builder for union types, which parses a string. RFC6020, specifies the following: When a string representing a union data type is validated, the string is validated against each member type, in the order they are specified in the "type" statement, until a match is found. This removes enough ambiguity, such that we can auto-generate a proper builder. Generate the builder (still in src/main, as we need to maintain compatibility). Unfortunately this is not enough to implement the desired functionality: a union member can be either an instance-identifier, an identityref or a leafref (to one of those two types). Reconstructing those two types, even if we solve the ugly String format part (which has to end up not conflicting with previous listed members), requires loading classes. This cannot be achieved in a general way when we are in a JPMS/OSGi environment when the code implementing this functionality resides in the module/bundle defining the union, as it simply does not have access to the totality of classes. The only way to make that work is to have a runtime component which integrates with the environment in a manner similar to binding-dom-codec – e.g. it has proper access to BindingRuntimeContext and can make decisions on that level. So rather than attempting to fix this wart, let's remove the union builders altogether: they are not referenced from generated code at all and therefore are only utilities for code. Utilities are best hand-crafted for a particular use case without a straight-jacket and in the appropriate component, with proper visibility and implementation constraints. |
| Comments |
| Comment by Minna Hu [ 17/Feb/15 ] |
|
Hi Robert, What's your plan of fixing this bug? Do you have a date scheduled? Thanks! |
| Comment by Robert Varga [ 17/Feb/15 ] |
|
We don't currently have a timeline for this due to resource constraints. The latest when we can make this change is M4, which I don't think we can currently commit to. |
| Comment by Minna Hu [ 18/Feb/15 ] |
|
Robert, thank you for your reply! What does M4 mean? Would you mind giving more info? I can try to modify the code and contribute it back to the community. There is other thing I would like to get your comments on, please let me know your opinions. In our company, we want to use the JAVA classes generated by yang-tools and then add our specific annotations as well as change some generated code. Right now, I modify the BindingGeneratorImpl, BuilderTemplate, ClassTemplate, etc., do you know any better way to separate our code modification? Then we can merge in community updates easily and decouple our code change. Looking forward to your reply, thank you! |
| Comment by Robert Varga [ 05/Mar/15 ] |
|
M4 as in the milestone in Simultaneous Release plan, for yangtools that is March 19th. As for modifications, that largely depends on the scope of changes. Can you elaborate a bit more? |
| Comment by Minna Hu [ 06/Mar/15 ] |
|
You know that, for a yang container element, ODL generates an interface, a Builder class, and a private class inside the Builder class that implements the interface, right? The changes I made belong to following scope: Do you recommend any way so that I can make the code change flexible? Thank you, looking forward to your reply! |
| Comment by Robert Varga [ 18/Mar/15 ] |
|
Cannot make the API freeze deadline for Lithium, retarget to Beryllium. |
| Comment by Robert Varga [ 17/Oct/16 ] |
|
Codecs are fixed, generated code needs take this into account (rather than requiring a custom-coded builder class). |
| Comment by Martin Ciglan [ 04/Apr/17 ] |
|
blocking 7942, so untargeting for time being |
| Comment by Robert Varga [ 08/Apr/22 ] |
|
Changing the mapping identity mapping will require revisiting all the custom-coded builders. Let's remove them instead. |