javaee / jaxb-v2

Other
211 stars 100 forks source link

boolean attribute with default value XJC generates a wrong getter #733

Closed glassfishrobot closed 14 years ago

glassfishrobot commented 14 years ago

Recently my webapp stopped working when I tried using Glassfish V3 final. The reason was this issue:

http://forums.java.net/jive/thread.jspa?messageID=381181&#381181

The problem was originally thought to be in hyperjaxb3 and issue incorrectly filed there as:

http://jira.highsource.org/browse/HJIII-25

The problem seems to be that XJC in JAXB RI 2.1.12 generates the getter method for an xs:attribute of type xs:boolean incorrectly by returning boolean instead of Boolean if it has a default value.

To use some simplified examples from latest regrep 4 schema:

For the following schema:

XJC generates the following incorrect getter method:

public class ResponseOptionType implements CopyTo, Copyable, Equals, HashCode { ... @XmlAttribute protected Boolean returnComposedObjects;

@Basic @Column(name = "RETURNCOMPOSEDOBJECTS") public boolean isReturnComposedObjects() { if (returnComposedObjects == null)

{ return false; }

else

{ return returnComposedObjects; }

}

public void setReturnComposedObjects(Boolean value)

{ this.returnComposedObjects = value; }

... }

The reason for this bug to be a P2 is that it prevents apps to migrate to Glassgfish V3 Final. This means that a number of my customers who would otherwise move to FG V3 Final cannot do so until this bug is fixed.

Environment

Operating System: All Platform: All

Affected Versions

[2.1.12]

glassfishrobot commented 14 years ago

Reported by najmi

glassfishrobot commented 14 years ago

snajper said: Hmm, we can't change the signature of the method, otherwise we would break compatibility. I tried to compile the schema with older jaxb compilers, and all of them return boolean, so I'm not sure how this is a regression. You might have caught builds where we tried to deliver this fix first and then had to revert it.

glassfishrobot commented 14 years ago

najmi said: Hi, I am not sure what you are saying about changing signatures. I use maven so my dependency management is 100% reliable and it is JAXB RI 2.1.1 pom and its dependencies as posted in the maven repo for JAXB RI at:

http://download.java.net/maven/1/com.sun.xml.bind/poms/jaxb-impl-2.1.12.pom

The issue is 100% reproducable on demand.

Please let me know at farrukh@wellfleetsoftware.com if you need the precise xsd files and I can send them via email. Thanks.

glassfishrobot commented 14 years ago

snajper said: Let me explain more - I mean that all the JAXB versions generate "boolean" as a return type for the isReturnComposedObjects() method. You say your app suddenly broke - so I assume your application has different code generated, which returns Boolean - that's interesting - and might have been caused by us trying to deliver this kind of fix which we had to revert afterwards.

Since "boolean" is generated for all JAXB versions, we can't fix this because there are applications that rely on this.

Please let me know if it's not clear or if I misunderstood the issue.

glassfishrobot commented 14 years ago

lexi said: My 5 cents.

The problem is that in case of a primitive property with default values XJC will generate "inconsistent" getter/setter pair. That is, getter will return primitive type whereas setter will accept the boxed type:

boolean isFoo()

{...}

but:

setFoo(Boolean foo) {...}

As Farrukh reports, this effectively breaks in Glassfish V3 since it expectes getter/setter pair to be type-consistent.

I don't think it's regression, the problem comes from the attempt of porting the application to Glassfish.

Now there's clearly a problem between Glassfish and the code generated by XJC. The question is - where does it have to be fixed.

From one side, Glassfish could treat the "inconsistent" getter/setter pair more generously. It must be just a few lines of code there.

From the other side, XJC does generate inconsistent getter/setter pair. Is it OK or not? Me personally I would prefer consistent pair.

You argument that by changing the signature we would break compatibility. I question that:

glassfishrobot commented 14 years ago

snajper said: Hi, I understand the reasoning. And I'd like to have this fixed as well. As I mentioned, we tried to deliver the fix some time ago and failed because we were not allowed to integrate against jaxws/jdk tck and compatibility test suite failures and had to revert back. One thing to note is that getter in case of Boolean shall also be changed to "get..." instead of "is...". Which leads to another possibility of fixing this issue - maybe we could generate both getters: 'Boolean getSomething()' and 'boolean isSomething()'. That would still certainly break jdk/jaxb/jaxws signature tests, but shouldn't hurt any existing applications. However it might lead to confusion. Deprecating the "is" getter might help a bit here, but I need to think about it more.

However, I'm still puzzled by what is your actual failure with GlassFish. You say this is a problem wrt migrating to GlassFish. I assumed it's about migrating from older versions of GlassFish, but since JAXB generates same incorrect code all the time, I'm not sure how this could cause migration problems.

Would you please describe the actual failure in your application and explain environment in which correct code is generated?

glassfishrobot commented 14 years ago

najmi said: My app woked in Glassfish V3 Prelude and broke when same app was deployed to Glassfish V3 Final. I thin it is because GF V3 Final has more stringent validation checks. Here is the thread on that issue:

http://forums.java.net/jive/message.jspa?messageID=382212

Thanks for any help in getting my app working in GF V3 Final.

glassfishrobot commented 14 years ago

snajper said: I see, so the original issue is this: 'Exception Description: The getter method [method isReturnComposedObjects] on entity class [class xxx.ResponseOptionType] does not have a corresponding setter method defined.'

So it seems that if we would generate both methods (boolean isAbc, Boolean getAbc), it would still break Eclipselink because it seems to expect to have methods in getter/setter pairs. Very unfortunate if my assumption is true.

glassfishrobot commented 14 years ago

najmi said: So as I understand it the JAXB RI team acknowledges this as a JAXB RI issue? If so the validation in EclipseLInk should be relaxed to allow the mismatch between getter and setter to pass through with a warning. Otherwise no one would be able to use EclipseLink with a schema like the one in this issue.

glassfishrobot commented 14 years ago

najmi said: For details on the EclipseLink error see:

http://forums.java.net/jive/thread.jspa?messageID=381181&#381181

glassfishrobot commented 14 years ago

snajper said: Yes, this is a JAXB issue, but I don't see a way how we could fix this without breaking backwards compatibility, since the proper name of getter method for Boolean starts with "get". Also if some applications chose to override our own default implementation of the code and their implementation returns null in some cases, this may lead to unexpected NPEs.

glassfishrobot commented 13 years ago

marsupilami_ said: Due to the won't fix status i decided to write a plugin for this. You can get it here: http://s-weber.blogspot.com/2011/03/jaxb2-primitive-fixer-plugin.html

Hope i can help some people out there!

glassfishrobot commented 14 years ago

Issue-Links: duplicates JAXB-825

glassfishrobot commented 14 years ago

Was assigned to snajper

glassfishrobot commented 7 years ago

This issue was imported from java.net JIRA JAXB-733

glassfishrobot commented 14 years ago

Marked as won't fix on Monday, March 8th 2010, 8:50:23 pm