Rename setAdditionalProperties to avoid confusing naive introspectors

[ddossot]

As shown here, the code generated for additionalproperties is not compliant with the JavaBeans standard: the setter and getter types do not match, which causes other libraries that introspect the code generated by jsonschema2pojo to go haywire.

Could setAdditionalProperties(Map<String, Object>) be used instead? If this is not OK for @org.codehaus.jackson.annotate.JsonAnySetter maybe the annotation could be added to another method like setAdditionalProperty(String name, Object value)?

[joelittlejohn]

David, could you give me some more detail on exactly why you need this? I don't see why any introspector should go haywire with the current code. As far as the JavaBean spec is concerned, 'additionalProperties' is simply a read-only property (a property with a getter but no setter). Is there some other reason that you need this property to be accessible by other tools?

I don't see any particular reason why this shouldn't be included. It wont cause anyone any problem but of course I'd still prefer to avoid adding unnecessary code to the generated types if possible.

[ddossot]

The only reason why I need this is that because the next codegen tool in my build (Mulesoft's DevKit) goes berserk with the code generated by jsonschema2pojo. I'm basically stuck between a rock and a hard place, trying to convince one of you guys to fix something each side thinks is not broken :)

The core issue IMO is that the DevKit generator assumes the POJO is JavaBeans compliant just by virtue of the matching method names, while if it was using proper JavaBeans introspection it would be clear that the getAdditionalProperties and setAdditionalProperties do not fully match, hence are not compliant and shouldn't be treated as if they were.

If I manually refactor the code generated by jsonschema2pojo into:

    public void setAdditionalProperties(final Map props)
    {
        this.additionalProperties.putAll(props);
    }
    @JsonAnySetter
    protected void handleUnknown(final String key, final Object value)
    {
        this.additionalProperties.put(key, value);
    }

everyone is happy!

So I reckon, if I can't convince you to output such code, I'll add a Maven plug-in to introduce this code change in between the two codegens...

[joelittlejohn]

If we added setAdditionalProperties(final Map props) but left the other in place, would the DevKit plugin still throw an error?

Oh, and one other question :) Do you actually require this property to be settable/understood by DevKit? If we changed this:

@JsonAnySetter
setAdditionalProperties(String name, Object value)

to this:

@JsonAnySetter
setAdditionalProperty(String name, Object value)

Would this suffice? Or do you require setAdditionalProperties(Map) to exist?

[ddossot]

Positive answers to both questions:

1. No, DevKit doesn't throw an error in that case.
2. DevKit doesn't require a setter so: yes it would suffice (and actually is a better named method).

Pick your poison :)

[joelittlejohn]

My heart says 2 but my head says 1 :)

I agree about the naming (this was simply a bad choice on my part when the project was created long ago), but I do try to introduce as few backward-compatibility issues as possible.

I'll put something into the next release.