Avoid NPE when setting additionalProperties path

[MikeEdgar]

A schema's id is not guaranteed to be non-null. E.g.

jsonschema2pojo/jsonschema2pojo-core/src/main/java/org/jsonschema2pojo/SchemaMapper.java

Lines 89 to 91 in 80d827d

	return ruleFactory.getSchemaRule().apply(className, schemaNode, null, jpackage, new Schema(null, schemaNode, null));

[andreaTP]

any update on this fix?

[joelittlejohn]

Thanks for pinging @andreaTP. I saw the CI job failed and didn't get around to investigating that. I've kicked this off again and will merge if this passes.

[MikeEdgar]

I think the issue with CI may be this: https://github.blog/changelog/2022-08-09-github-actions-the-ubuntu-18-04-actions-runner-image-is-being-deprecated-and-will-be-removed-by-12-1-22/. Would you like me to bump the Ubuntu version to 20.04 or 22.04 as part of this PR?

[unkish]

Would it be possible to provide also test case(s) as outlined in https://github.com/joelittlejohn/jsonschema2pojo/blob/master/CONTRIBUTING.md

[MikeEdgar]

Unit test added. Between the new test and the existing integration tests, both null/non-null schema Ids are tested.

[joelittlejohn]

@MikeEdgar is it not possible to recreate this case via the integration tests?

[MikeEdgar]

@joelittlejohn , the integration tests all seem to be using a particular approach where the schemas are loaded from JSON (which always sets the id) using the Maven plugin Mojo. If that's the preferred location for new tests, it will likely need to be an alternate process.

[MikeEdgar]

@joelittlejohn , can this PR move forward as-is or are additional testing changes necessary? IMO the scope of the change itself is quite minor from a testing risk perspective and a unit test has been added of course.

[andreaTP]

Ping @joelittlejohn and @unkish , anything that should be done here to get the fix merged and released?

Thanks in advance 🙏

[unkish]

anything that should be done here to get the fix merged and released?

Release schedule & content is up to maintainer to decide (might be dependent on available free time/resources/...)

[MikeEdgar]

@joelittlejohn , @unkish - please have another look and let me know if any other changes are necessary.

[unkish]

From the change perspective itself: it looks perfectly fine and valid. There is also similar "guard" in ArrayRule

jsonschema2pojo/jsonschema2pojo-core/src/main/java/org/jsonschema2pojo/rules/ArrayRule.java

Line 84 in 98c85ce

if (schema.getId() == null || schema.getId().getFragment() == null) {

Latter was added with commit 8e97ddb

Additionally verified that NPE would also be thrown via public API "usage" in SchemaMapper and current change would fix it:

public class SchemaMapperTest {
    ...

    @Test
    public void generateCreatesTypeFromSchemaWithAdditionalProperties() throws Exception {
        String schema = "{\"type\":\"object\",\"additionalProperties\":{\"type\":\"integer\",\"maximum\":9}}";
        JType result = new SchemaMapper().generate(new JCodeModel(), "ExampleClass", "", schema);
        JType returnType = ((JDefinedClass) result).getMethod("getAdditionalProperties", new JType[0]).type();

        assertEquals("Map<String,Integer>", returnType.name());
    }

}