Community
Participate
Working Groups
Need to fix the borken links in the samples, sdk and other identified packages as @XmlRootBinding will be dropped as a result of discussions and implementation of Bug-355457
As part of Bug-355457 i will submit an annoatations/testcases etc., and please review and close the issue once thats done I will take up this bug and fix the broken links. Please let me know if you think otherwise.
We cannot do it this way. A change that breaks the build cannot be committed. I added some comments to Bug 355457 regarding how we can split this work into multiple phases. Lets repurpose this bug be the second phase of cleanup of root xml binding API
This is continuation of work started in Bug 355457. Copying relevant text: ========== I am finally remembering all aspects of this API that I've been intending to correct. Let me fill you in on that... @XmlRootBinding predates @XmlNamespace that provides for more flexible way specify namespaces. I've been meaning to clean this up. Might as well do all of this cleanup in one shot. If @XmlNamespace owns association of a prefix with a namespace uri and @XmlSchema owns association of namespace uri with schema location, then there is not much left for @XmlRootBinding (just the element name). Given that, I think we can just drop this annotation and use the existing generic @XmlBinding in its place. Declarations would look something like this: @XmlNamespace( prefix = "m", uri = "http://something.com" ) @XmlSchema( namespace = "http://something.com", location = "http://something.com/schema.xsd" ) @XmlBinding( path = "m:root" ) or @XmlDocType( ... ) @XmlBinding( path = "root" ) or just @XmlBinding( path = "root" ) To complete this change, I'd like to move StandardRootElementController into the internal package. It doesn't make for good API and will need to change extensively anyway for these changes. ========== This tracks the following changes: 1. Make StandardRootElementController internal. It will no longer be API. 2. Remove @RootXmlBinding annotation. 3. Add @XmlSchema( namespace, location ) annotation. 4. Entry in migration guide, enhancements doc and other documentation pages. 5. Tests, sample updates. Don't know if we need to add stuff here, but we will certainly need to update the existing stuff.
(In reply to comment #3) > This is continuation of work started in Bug 355457. Copying relevant text: > > ========== > > I am finally remembering all aspects of this API that I've been intending to > correct. Let me fill you in on that... @XmlRootBinding predates @XmlNamespace > that provides for more flexible way specify namespaces. I've been meaning to > clean this up. Might as well do all of this cleanup in one shot. If > @XmlNamespace owns association of a prefix with a namespace uri and @XmlSchema > owns association of namespace uri with schema location, then there is not much > left for @XmlRootBinding (just the element name). Given that, I think we can > just drop this annotation and use the existing generic @XmlBinding in its > place. > > Declarations would look something like this: > > @XmlNamespace( prefix = "m", uri = "http://something.com" ) > @XmlSchema( namespace = "http://something.com", location = > "http://something.com/schema.xsd" ) > @XmlBinding( path = "m:root" ) > > or > > @XmlDocType( ... ) > @XmlBinding( path = "root" ) > > or just > > @XmlBinding( path = "root" ) > > To complete this change, I'd like to move StandardRootElementController into > the internal package. It doesn't make for good API and will need to change > extensively anyway for these changes. > > ========== > > This tracks the following changes: > > 1. Make StandardRootElementController internal. It will no longer be API. > This is already done in Bug-355457, since i doesn't break the build i did that as part of previous ticket I will work on it please change the status.
I will work on these changes once I see you marking the 355457 as closed. Let me know if you wish me to anyting particular with respect to the changes for this bug, its mainly going to be cleaning.
Adding one more comment: For any Xmlbinding - i.e. Root we should allow either @XmlDocumentType or @XmlSchema, is there is anyway we can valdiate this ? So that user does not added both of them to the class - Sapphire should restrict declaratively rather than @ runtime. Hope I make some sense here ?
> For any Xmlbinding - i.e. Root we should allow either @XmlDocumentType or > @XmlSchema, That's right. > is there is anyway we can valdiate this ? So that user does not added both of > them to the class - Sapphire should restrict declaratively rather than > @ runtime. Hope I make some sense here ? There are ways to validate this at build-time, but they are expensive to implement and cost/benefit is unlikely to balance here as the user will quickly discover their mistake at runtime.
so ideally when we discover the user has both these annotations we can throw an IllgealStateException and allow the user to correct it. is this what you mean ?
so how to you want to priortise between bugs 355751 and 354199 , i mean to say which one you want to close first. if you are fine with either ones i will work on them parllely. Let me know your thoughts .
> so ideally when we discover the user has both these annotations we can throw an > IllgealStateException and allow the user to correct it. is this what you mean ? Throwing exceptions is reserved for situations where there isn't a reasonable fallback. In this case, I would expect that the code would either detect @XmlSchema or @XmlDocumentType first and go with that. > so how to you want to priortise between bugs 355751 and 354199 , i mean to say > which one you want to close first. if you are fine with either ones i will work > on them parllely. Either way is fine with me.
Ok then let me work on 354199 as that sounds intresting and might need more time compared to this. Will take this up once I am done with 354199
Let me know if this item feels too daunting.
Let me try to work on other bugs and close them and come to this. I guess it should not be too daunting and all depends on how quickly i close the other ones :)
Started to work on this issue, from where we left off. Tasks 1. define @XmlSchema, 2. drop @XmlRootBinding 3. update samples 4. update test cases 5. update docs Let me know if you have anything specific with the Tasks and anything you want to add
Created attachment 206118 [details] Patch_v0.1 Attached is the first patch fore your review, it contains a lots of changes as we need to touch files which were using @XmlRootBinding and replace them with @XmlBinding. Please have a look at it and let me know if I am going in the right direction. Note: some of the test cases failed roughly 7 of them, in which 2 are related to XML not sure that is the problem with this changes, but your thought needed to see if its this fix caused issue Query: Do we need to support multiple schema declarations ? @XmlSchemas(@XmlSchma[]), because some of the XML files e.g. Spring beans will have multiple schema declarations. Let me know your thoughts on this too, may be we could take this up as separate enhancement too.
Did you get a chance to review these enhancements ?
I haven't tried running this patch, but I have not identified any obvious issues in the initial review. > Note: some of the test cases failed roughly 7 of them, in which 2 are related > to XML not sure that is the problem with this changes, but your thought needed > to see if its this fix caused issue All of the tests cases pass currently, so any failures are the result of this work. Note that many of the test cases in non-XML areas indirectly leverage XML functionality. > Do we need to support multiple schema declarations ? @XmlSchemas(@XmlSchma[]), > because some of the XML files e.g. Spring beans will have multiple schema > declarations. Let me know your thoughts on this too, may be we could take this > up as separate enhancement too. Let's go ahead an add @XmlSchemas as part of this one.
(In reply to comment #17) > > Do we need to support multiple schema declarations ? @XmlSchemas(@XmlSchma[]), > > because some of the XML files e.g. Spring beans will have multiple schema > > declarations. Let me know your thoughts on this too, may be we could take this > > up as separate enhancement too. > > Let's go ahead an add @XmlSchemas as part of this one. When I was thinking more on these lines I see that we can adopt the following Annotation structure for the @XmlSchema @XmlSchema( rootElementName="root", @XmlNamespace(uri="http://www.eclipse.org/sapphire",prefix="sph"), location="http://www/eclipse.org/sapphire/test.xsd", target=true) Since @XmlNamespace will not have any use unless and until we use schema, i personally feel its better to make the annotation come under @XmlSchema and we shall even drop the @XmlNamespaces, since we are going to introduce @XmlSchemas(@XmlSchma[])that will take care of handling multiple namespaces. This will also in kind provide a simpler declarative for users and help them understand the context of @XmlNamespace. From the implementor point of view it there will be only Annotation processing from which we can get all the required properties for constructing the root element and we dont need to change the existing functionality of @XmlBinding by making it support @ Type level Please let me know your thoughts.
No. That does not work. The namespace concept is independent of the schema concept. Consider that you can have a document with namespaces that doesn't use a schema or uses another form of content description. The @XmlNamespace and @XmlSchema annotations need to stay separate.
(In reply to comment #19) > No. That does not work. The namespace concept is independent of the schema > concept. Consider that you can have a document with namespaces that doesn't use > a schema or uses another form of content description. > > The @XmlNamespace and @XmlSchema annotations need to stay separate. Agreed! thanks I have a small query, in one of the test cases which fails i see the failing test case model object (B) is extending from another model object(A),the A has a unqualified root binding, where B has qualified root binding, so as a result of extension and when we read from the parent properties we get a unqualified name but we are expecting a qualified name. Is there any place I need to look in to for this scenario ? Any updates for dropping @XmlRootBinding? Test Model : org.eclipse.sapphire.tests.modeling.xml.XmlBindingTestModel Failing test: testValueProperties3
Short of stepping through your code changes, I couldn't tell you where the bug might be...
I want to push this fix for 0.4, as per the forum post http://www.eclipse.org/forums/index.php/t/261466/ i see very little time left. I am still struck up with the last error i told you. I am trying debug and fix that today if you have time can you please have a look at it ? ~kamesh
Further analysis I found that when the child model which is extending from a parent model is not getting serialized with name spaces :( .. I guess am making some mistake when handling it @ the RootXmlResource class, becos right now we dont have @XmlRootBinding .. all is done by @XmlBinding whether at root or at property levels, In the scenario where test case is failing , parent does not have @XmlNamespaces or @XmlSchema and there is no prefix for the same. Ideally (am not sure), the annotations from the child class should override the parent ones right ? Correct me if am wrong .. Thats the reason its not able to compare the expected output which has namespace with the unqualified elements.
I am working on this now. The cause of unit test failures was largely in RootXmlResource changes. The logic in that class wasn't handling prefixes and namespace resolution correctly, so information passed to StandardRootElementController was incorrect.
(In reply to comment #24) > I am working on this now. The cause of unit test failures was largely in > RootXmlResource changes. The logic in that class wasn't handling prefixes and > namespace resolution correctly, so information passed to > StandardRootElementController was incorrect. I had the same doubt, let me if you need to take some rest, i will debug the class and try to fix it. Anyways thanks a lot for take some time to look into it.
(In reply to comment #24) > I am working on this now. The cause of unit test failures was largely in > RootXmlResource changes. The logic in that class wasn't handling prefixes and > namespace resolution correctly, so information passed to > StandardRootElementController was incorrect. I kind of corrected to make the test case work :) as is .. fixed some cases in RootXmlResource, but feel there needs some more clean up. I will fix it and send it for your review. you can then review and update if needed.. Thanks once again for pointing it out and authenticating my doubt.
i guess i made some mess with the debugging and testing, esp. on code format and other stuff. will reset my local copy yo HEAD , apply patch_v0.1 and then will reapply my current changes to avoid any discrepancies while merging. Sorry about it :(
Don't worry about trying to make another patch for now. I have a working set of changes based on your original patch. I will try to wrap it up tomorrow. If I have to set it down and hand it back to you, I will attach a revised patch so that you can pick up where I left off.
(In reply to comment #28) > Don't worry about trying to make another patch for now. I have a working set of > changes based on your original patch. I will try to wrap it up tomorrow. If I > have to set it down and hand it back to you, I will attach a revised patch so > that you can pick up where I left off. Thanks a lot. I will wait for your revised patch and will work on it starting from there. thanks once again for the help.
I fixed the logic error in RootXmlResource, reverted improper changes to unit test expected files, added @XmlSchemas annotation and related logic, and done some general cleanup. All unit tests pass. The code changes have now been committed. The only thing left to do here is to write a section for the migration guide. I will do that tomorrow.
(In reply to comment #30) > I fixed the logic error in RootXmlResource, reverted improper changes to unit > test expected files, added @XmlSchemas annotation and related logic, and done > some general cleanup. > > All unit tests pass. The code changes have now been committed. The only thing > left to do here is to write a section for the migration guide. I will do that > tomorrow. Wonderful and Thanks! I will work on the the doc changes for @XmlSchema and @XmlSchemas that needs to get into 0.4/enhancements.html and send you a patch of it.
I have a small question: whats the use of hudson-build target in the build?
Created attachment 206780 [details] Enhancement Documentation The 0.4 enhancements documentation updated for @XmlSchema and @XmlSchemas annotations
I accepted half of the patch for the enhancement document (the @XmlSchema section) with a few changes. I replaced the section on @XmlSchemas with a single sentence. In addition to the above, I added content to the migration guide and fixed references @XmlRootBinding in the documentation. This item can be considered complete. Please verify.
*** Bug 342487 has been marked as a duplicate of this bug. ***
(In reply to comment #34) > I accepted half of the patch for the enhancement document (the @XmlSchema > section) with a few changes. I replaced the section on @XmlSchemas with a > single sentence. > > In addition to the above, I added content to the migration guide and fixed > references @XmlRootBinding in the documentation. > > This item can be considered complete. Please verify. Looks good, but a small suggestion in the migration guide, can we add a word saying the @XmlRootBinding is dropped and @XmlBinding is used in place. Tough its implied by the Before/After section but we can be explicit here to avoid any ambiguity.
The migration guide already contains essentially such wording: "Scenarios previously handled by @XmlRootBinding annotation are now handled by different combinations of @XmlBinding, @XmlNamespace, @XmlSchema and @XmlDocumentType annotations." Between this and the before/after table, I don't think we have any ambiguity issues to worry about.
(In reply to comment #37) > The migration guide already contains essentially such wording: > > "Scenarios previously handled by @XmlRootBinding annotation are now handled by > different combinations of @XmlBinding, @XmlNamespace, @XmlSchema and > @XmlDocumentType annotations." > > Between this and the before/after table, I don't think we have any ambiguity > issues to worry about. Great then, I have verified it. Thanks.
Closing.
