conjure-java icon indicating copy to clipboard operation
conjure-java copied to clipboard

A union type even in invalid format should deserialize to Unknown

Open gm2211 opened this issue 5 years ago • 1 comments

What happened?

Given:

    objects:
      Foo:
        fields:
          foo: string
      Bar:
        fields:
          bar: string
      FooBar:
        union:
          foo: Foo
          bar: Bar

the following fails:

        ObjectMapper mapper = ObjectMappers.createDefaultJsonObjectMapper(false);
        String good = "{\n"
                + "  \"type\" : \"bar\",\n"
                + "  \"bar\" : {\n"
                + "    \"bar\" : \"123\"\n"
                + "  }\n"
                + "}";
        String bad = "{\n"
                + "  \"type\" : \"new\",\n"
                + "  \"some-string\""
                + "}";

        assertThat(mapper.readValue(good, FooBar.class))
                .extracting("value")
                .extracting("value")
                .isInstanceOf(Bar.class);
        assertThat(mapper.readValue(bad, FooBar.class))
                .extracting("value")
                .extracting(Object::getClass)
                .extracting(Class::getSimpleName)
                .asString()
                .contains("UnknownWrapper");

What did you want to happen?

The test should pass.

We can achieve this by modifying the generated code for the unknown wrapper to look something like this:

image

this is obviously not completely correct since if type is missing or there exists no sub-object, we'd be setting the raw invalid json as type, so we'd probably have to do a little more work (like making type optional or nullable for the unknown wrapper)

gm2211 avatar Sep 18 '19 01:09 gm2211

I dunno - the whole 'unknown' thing is supposed to make conjure enums forward compatible, so that old code can be left running and it shouldn't break if a server starts sending new responses.

Your bad example has a random floating "some-string", which can't plausibly be emitted by anything complying with the current conjure spec.

I think changing this behaviour would be net negative, because it would make it harder to catch malformed inputs, without actually improving our forwards-comaptibility posture.

iamdanfox avatar Sep 20 '19 17:09 iamdanfox