Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JSON schema with enum member with a name starting with a capital letter, causes a generation of a code that doesn't compile #126

Closed
yurama opened this issue Oct 15, 2013 · 14 comments
Milestone

Comments

@yurama
Copy link

yurama commented Oct 15, 2013

When trying to run jsonschema2pojo on a json schema that has an enum property with a name starting with a capital letter, the generated code doesn't compile.
The compilation error is: non-static variable 'x' cannot be referenced from a static context.

That error occurs because the generated code has a private field with the same name as the enum, that is later accessed by that name from a static code block.

According to RFC 4627 - there is no requirement for member names to start with lower case letters.

For example, for the JSON schema:


{
    "type": "object",
    "properties": {
        "ExampleObj": {
            "type": "object",
            "properties": {
                "TimeFormat": {
                    "enum": [ "12h", "24h" ],
                    "description": "The referred time display format, either 12 or 24 hour clock."
                }
            }
        }       
    }
}

That error occurs because the generated code has a private field with the name TimeFormat:


    @JsonProperty("TimeFormat")
    private ExampleObj.TimeFormat TimeFormat;

and an enum with the same name, that tries to access that field from within a static code block (note the ExampleObj.TimeFormat.values() within the foreach loop):


    @Generated("com.googlecode.jsonschema2pojo")
    public static enum TimeFormat {

        _12_H("12h"),
        _24_H("24h");
        private final String value;
        private static Map constants = new HashMap();

        static {
            for (ExampleObj.TimeFormat c: ExampleObj.TimeFormat.values()) {
                constants.put(c.value, c);
            }
        }
.
.
.
@khitrenovich
Copy link
Contributor

I suggest to append 'Enum' suffix to enum type names. This change should be turned on by configuration switch (off by default), since the generated code will not be backward compatible. I will try to implement this change and check if this goes well.

@yurama
Copy link
Author

yurama commented Oct 17, 2013

Another option is not to use the fully qualified enum name when calling the static method values.
For the previous example, it would create the line:


for (ExampleObj.TimeFormat c: values()) {

instead of the line:


for (ExampleObj.TimeFormat c: ExampleObj.TimeFormat.values()) {

khitrenovich added a commit to khitrenovich/jsonschema2pojo that referenced this issue Oct 17, 2013
@joelittlejohn
Copy link
Owner

I think this is actually a bug in the naming of the field. Your field should be named timeFormat not TimeFormat. I'm surprised by this but I think there has been a regression here.

So the resulting field should be:

@JsonProperty("TimeFormat")
private ExampleObj.TimeFormat timeFormat;

@khitrenovich
Copy link
Contributor

@joelittlejohn Do you think always lowercasing the first letter of the field name will do the job?

@joelittlejohn
Copy link
Owner

@khitrenovich yes, I think so. Can you think an scenario in which this might cause problems? I actually think this is a recent bug, I can't quite believe it has always been present but of course I may be wrong.

In simple terms, Java field names should always start with a lower-case letter and Java type names should always start with an upper-case letter. A conflict between the two should be impossible.

@khitrenovich
Copy link
Contributor

@joelittlejohn I fully agree about Java naming convention :)

Yet, I'm not fully confident that there are no marshaling/unmarshalling
packages that rely on field names via reflection (even more, I cannot say
if this is a valid flow or not). However, it is your decision as a
maintainer whether jsonschema2pojo will support such packages if they
exist.

@joelittlejohn
Copy link
Owner

@khitrenovich yes, I take your point, but I don't see this as too much of a problem. If a JSON binding library can't cope with Java field names being different to JSON names then it simply can't use these types. It's unreasonable to require that the naming is the same, I think. For example, what if your JSON property was called time-format? Clearly a name that cannot be accurately represented as a Java field.

Most data binding libraries allow some kind of annotation (or similar) to indicate the JSON name for a Java field or bean property. jsonschema2pojo generally takes the view that when we need to support a new binding library, we'll do so explicitly by adding a new config option. There's just too much sophistication in the types to expect additional binding libraries to work with no extra effort.

If people really do have simple JSON data that looks a lot like a set of Java beans, they will probably be able to generate types that are a good match. However if your data doesn't map well to Java conventions, then you'll need to choose a binding library that can at the very least map from one name to another.

khitrenovich added a commit to khitrenovich/jsonschema2pojo that referenced this issue Oct 22, 2013
Name normalization should differentiate between class names and property names to avoid name collisions and resulted compilation errors
khitrenovich added a commit to khitrenovich/jsonschema2pojo that referenced this issue Oct 22, 2013
Name normalization should differentiate between class names and property
names to avoid name collisions and resulted compilation errors
@khitrenovich
Copy link
Contributor

@joelittlejohn I've implemented a fix in the name normalization code that seems to solve the issue. Please take a look on the changes (c262bb7 - the later commit above).

joelittlejohn added a commit that referenced this issue Oct 22, 2013
@joelittlejohn
Copy link
Owner

Fixed by PR from @khitrenovich.

@khitrenovich
Copy link
Contributor

@joelittlejohn Do you plan to release 0.4.0 soon? If not, any chance to get this fix as 0.3.8 or something?

@joelittlejohn
Copy link
Owner

Hard to say. In particular I'd like to make some changes to potentially solve #22, but this is not a small piece of work.

Would it be possible for you to work with a dated snapshot? There is another user waiting for a snapshot of 0.4.0 too.

@joelittlejohn
Copy link
Owner

@khitrenovich I've built a snapshot here if you want to take a look:

https://oss.sonatype.org/content/repositories/snapshots/org/jsonschema2pojo/

@khitrenovich
Copy link
Contributor

@joelittlejohn Unfortunately, external snapshot is not an option due to organization policies. Looks like the best thing I can do for now is to build and deploy a local copy of the plugin under some "private" version.

Thank you anyway!

joelittlejohn added a commit that referenced this issue Jun 9, 2014
Closes #187. Somewhat reverts changes made for #126 in favour of a
different solution.
joelittlejohn added a commit that referenced this issue Jun 9, 2014
Closes #187. Somewhat reverts changes made for #126 in favour of a
different solution.
joelittlejohn added a commit that referenced this issue Jun 11, 2014
@deepaksama
Copy link

Is there any way to get enum values as numenric?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants