JsonSchemaFactory can potentially and silently break the application

[timocov]

JsonSchemaFactory has a cache of schemas that is enabled by default.

If a requested schema is in the cache, then cached values is returned and the validation context is updated with the provided SchemaValidatorsConfig value.

Now the issue is that some of the values in SchemaValidatorsConfig affect the result of some operations with the schema.

The "wrong" usage is pretty simple (the code is similar to the library's unit tests):

public class SharedConfigTest {
    private static class AllKeywordListener implements JsonSchemaWalkListener {
        public boolean wasCalled = false;

        @Override
        public WalkFlow onWalkStart(WalkEvent walkEvent) {
            wasCalled = true;
            return WalkFlow.CONTINUE;
        }

        @Override
        public void onWalkEnd(WalkEvent walkEvent, Set<ValidationMessage> validationMessages) {
        }
    }

    @Test
    public void shouldCallAllKeywordListenerOnWalkStart() throws Exception {
        JsonSchemaFactory factory = JsonSchemaFactory.getInstance(SpecVersion.VersionFlag.V7);

        SchemaValidatorsConfig schemaValidatorsConfig = new SchemaValidatorsConfig();
        AllKeywordListener allKeywordListener = new AllKeywordListener();
        schemaValidatorsConfig.addKeywordWalkListener(allKeywordListener);

        URI draft07Schema = new URI("resource:/draft-07/schema#");

        // depending on this line the test either passes or fails:
        // - if this line is executed, then it passes
        // - if this line is not executed (just comment it) - it fails
        JsonSchema firstSchema = factory.getSchema(draft07Schema);

        // note that only second schema takes overridden schemaValidatorsConfig
        JsonSchema secondSchema = factory.getSchema(draft07Schema, schemaValidatorsConfig);

        secondSchema.walk(new ObjectMapper().readTree("{ \"id\": 123 }"), true);
        Assertions.assertTrue(allKeywordListener.wasCalled);
    }
}

Note here that currently the test will fail, but if you comment JsonSchema firstSchema = factory.getSchema(draft07Schema); line it will pass.

addKeywordWalkListener is not the only value that is affected. It feels like this behavior is kinda very dangerous, because there is no way to easily say that something went wrong, it doesn't throw an exception either, it just silently behaves not like you expected or configured it (but I agree that it depends on the settings you overrode in SchemaValidatorsConfig - some of them might not be that dangerous). The reason why I think so is that you might want to set up a library to collect and extract some values from a json object based on your schema, and if "no values extracted" is one of possible options, there is no way to say that the returned empty value (or no value) is because a json value doesn't have anything to extract or the library isn't working properly.

If that SchemaValidatorsConfig actually should be stored only once, then probably it is worth it to store in the factory and require to be a factory's parameter instead of the schema? But I assume it is not true (based on this comment)

As a workaround for now I found to have schema factory per config (in my case - one factory per one usage even tho factories are exactly the same - they just shouldn't share the cache).