Question: Is it forbidden to use adapted types as constructor parameters?

[mkarg]

I want to deserialize JSON into a POJO. The POJO is using @JsonbConstructor as it does not contain any setter methods. One of the parameters currently is of type URI. Everything deserializes fine. 😄

For some reason, I want to map URI to X. So I wrote a JsonbAdapter<X, URI> and replaced the type URI in my POJO by X. Unfortunately, the result now is: javax.json.bind.JsonbException: Internal error: WELD-001456: Argument resolvedBean must not be null. 😢

Is it forbidden to use adapted types as constructor parameters, or is this a bug of my runtime (here: Yasson)?

at org.eclipse.yasson.internal.components.BeanManagerInstanceCreator.lambda$getOrCreateComponent$0(BeanManagerInstanceCreator.java:71)
at java.base/java.util.concurrent.ConcurrentHashMap.computeIfAbsent(Unknown Source)
at org.eclipse.yasson.internal.components.BeanManagerInstanceCreator.getOrCreateComponent(BeanManagerInstanceCreator.java:66)
at org.eclipse.yasson.internal.ComponentMatcher.introspectAdapterBinding(ComponentMatcher.java:296)
at org.eclipse.yasson.internal.AnnotationIntrospector.getAdapterBindingFromAnnotation(AnnotationIntrospector.java:243)
at org.eclipse.yasson.internal.AnnotationIntrospector.getAdapterBinding(AnnotationIntrospector.java:238)
at org.eclipse.yasson.internal.model.CreatorModel.(CreatorModel.java:55)
at org.eclipse.yasson.internal.AnnotationIntrospector.createJsonbCreator(AnnotationIntrospector.java:198)
at org.eclipse.yasson.internal.AnnotationIntrospector.getCreator(AnnotationIntrospector.java:159)
at org.eclipse.yasson.internal.AnnotationIntrospector.introspectCustomization(AnnotationIntrospector.java:737)
at org.eclipse.yasson.internal.MappingContext.lambda$createParseClassModelFunction$1(MappingContext.java:91)
at java.base/java.util.concurrent.ConcurrentHashMap.computeIfAbsent(Unknown Source)
at org.eclipse.yasson.internal.MappingContext.getOrCreateClassModel(MappingContext.java:81)
at org.eclipse.yasson.internal.Unmarshaller.deserializeItem(Unmarshaller.java:60)
at org.eclipse.yasson.internal.Unmarshaller.deserialize(Unmarshaller.java:51)
at org.eclipse.yasson.internal.JsonBinding.deserialize(JsonBinding.java:59)
at org.eclipse.yasson.internal.JsonBinding.fromJson(JsonBinding.java:99)

public final class C {

    @JsonbCreator
    public C(@JsonbProperty("x") final X x) {
        this.x = x;
    }

    private final X x;

    public final X x() {
        return this.x;
    }
}

@JsonbTypeAdapter(XAdapter.class)
public class X {
   ...
}

public class XAdapter implements JsonbAdapter<X, URI> {
   ...
}

[rmannibucau]

Hi @mkarg , did you try to enable CDI and make your adapter a bean (@Dependent on it should be sufficient), looks like in your env CDI is used even if your sample does not require it but sample looks correct.

[mkarg]

Hi @mkarg , did you try to enable CDI and make your adapter a bean (@Dependent on it should be sufficient), looks like in your env CDI is used even if your sample does not require it but sample looks correct.

Romain, thank you for picking this up. Good to hear that our code itself is correct, so apparently it is a bug of the implementation. This is what I needed to know. :-)

Regarding your tipps: I gave it a try, but adding @Dependent does not improve it, the error message is still the same (possibly because dependent scope is the default in CDI)? CDI was already enabled in the JAR as other beans in the same JAR are actually making use of dependency injection. In fact I assume that the problem is that we should rather tell WELD to explicitly exclude this adapter from CDI? Anyways, this feels more like a product specific topic now, not like a generic API discussion.

[rmannibucau]

Yes, this can be a yasson issue since the lookup should work (cdi or not there).

Reviewing more widely your sample I realize a part is maybe unclear in the spec.
X->URI, none of them is JSON friendly (= be JSON-P friendly more or less, ie have a matching signature in generator to simplify it):

It has a custom code to convert the “unmappable” type (Original) into another one that JSONB can handle (Adapted).

Adapters are not intended to loop IIRC (for good reasons) so it should directly match some JSON compatible type and URI has its own adapter and is not a direct mapping (JsonValue, BigDecimal, BigInteger, int, long, double, String from memory).
(yes serializer/deserializer are a more generic replacement and adapters are rarely a better choice)

So something more like that should work:

public class Test {
    public static void main(final String... args) throws Exception {
        try (final var jsonb = JsonbBuilder.create(new JsonbConfig()
                .withAdapters(new XAdapter()))) {
            final var c = jsonb.fromJson("{\"x\":\"http://foo\"}", C.class);
            // c.x.value == http://foo
        }
    }

    public static final class C {
        @JsonbCreator
        public C(final X x) {
            this.x = x;
        }

        private final X x;
    }

    public static class X {
        private final String value;

        public X(final String value) {
            this.value = value;
        }
    }

    public static class XAdapter implements JsonbAdapter<X, String> { // String not URI
        @Override
        public String adaptToJson(final X x) throws Exception {
            return x.value;
        }

        @Override
        public X adaptFromJson(final String uri) throws Exception {
            return new X(uri);
        }
    }
}

[mkarg]

I do not see that the spec is unclear. In fact, URI is JSON friendly (or with "friendly" you meant "JSON type" instead of "JSON-B type", because I read "friendly" as "explicitly handled by JSONB spec"?). The specification literally says: "convert the “unmappable” type (Original) into another one that JSONB can handle (Adapted)" in 4.7.1, and it says that URI MUST be handled by all implementations in 3.4.1. It might be the case that such a combination was not intended by the spec authors, but such a combination de-facto is not forbidden by the spec -- so how should the spec reader know? IMHO it would be pretty weird to limit adapters to map to numbers and Strings, while URI is explicitly mentioned in 3.4.1, so such an unwritten constraint would be rather unintuitive hence unexcpected by the reader (like: "You cannot put LEGO brick A ontop on LEGO brick B, even if they are contained in the same box and have compatible pins. Yes the construction manual didn't tell that, but everybody knows that.").

Having said that, need to tell you that your example does not solve the problem: When adapting to String instead of URI, the result is the very same error message. Hence, I would say, I did nothing wrong, there is nothing unclear or missing in the spec, and it is simply a bug in Yasson.

To open a bug report in Yasson, I just am waiting for the Yasson team to agree. @m0mus WDYT?

[rmannibucau]

@mkarg yes, I think we are in a blurry area of the spec so means it must be considered as unspecified by the reader. The clarification must decide if adapters are about JSON(-P) types or JSON(-B) types. Guess the intent was JSON-B but then it is also very weird from a design perspective because all not JSON-P types are handled by kind of adapters/(de)serializers by design since JSON-B is built on top of JSON-P and then you get to the loop issue of adapters which should clearly not be allowed so guess the sanest clarification from my window would be to stick to JSON-P at that level to be portable.

That said your error is not yet at the type level but adapter resolution so guess your env misses something which ends in yasson area but still something to clarify at spec level but likely in another ticket.

[mkarg]

FYI: This bug is fixed since Yasson 3.0.0. Since that version, Yasson correctly uses JsonbAdapter<X, URI> to deserialize the custom class X before passing it as the sole constructor value.

@rmannibucau @m0mus Do we still need a spec change or is the existence of that fix a proof that the spec actually is meant in the sense of JSON-B-Types?

[rmannibucau]

@mkarg a fix is never a proof so tempted to say we are either on clarification or deprecation of adapters topic (indeed im still for their drop at the end after a few cycles).

[mkarg]

@mkarg a fix is never a proof so tempted to say we are either on clarification or deprecation of adapters topic (indeed im still for their drop at the end after a few cycles).

Dropping would mean breaking lots of existing apps. I doubt that the Jakarta EE WG would allow that.

For the further discussion, do we need a new issue, or can we simply go on here?

[rmannibucau]

@mkarg once again you are the only one speaking of dropping anything for next release, Im more keen to deprecate and then plan a drop in one or two more releases as allowed by jakarta rules....even if they are not respected by several spec, Im one of the ones thinking stability is the key value of jakarta. Since adapter are not well defined nor their usage being as flexible as it looks, plus since it is superseeded easily by other api, i think it makes sense to aim at dropping them...but after deprecation etc...
+1 for an umbrella issue, makes sense to me

[mkarg]

@mkarg once again you are the only one speaking of dropping anything for next release, Im more keen to deprecate and then plan a drop in one or two more releases as allowed by jakarta rules....

Fully understood and appreciated, but: Actually I am not speaking of the next release. The Jakarta EE WG does not allow to even just deprecate things. We did the same fault at JAX-RS, as we did not know that (we went down all that deprecation road and now we received the complaints that this is not allowed). Indeed, it was @struberg who told me recently that removal and even just deprecation apparently both are forbidden, even in new major releases. :-(

[rmannibucau]

Hmm, it always had been allowed at javaee and early jakarta times to deprecate things, with caution. Anyway nothing prevents the spec to say it is not well defined enough to be used which would be a bad deprecation but at least explicit to users to not use it when they care of bzing vendor locked or limited.
Point stays to freeze the concept and ack its replacement/superseed officially.

[mkarg]

Let's continue here: #341