jsonb-api icon indicating copy to clipboard operation
jsonb-api copied to clipboard

Configure impl to interface mappings

Open jsonbrobot opened this issue 7 years ago • 14 comments

I have a interface model like:

public interface Customer {

    String getName();
    Address getAddress();
}

public interface Address {

    String getStreet();
    String getCity();
}

For each interface there is a single implementation. Important to note that the model contains nested properties/types using (again) interfaces.

How can this be parsed?

JsonbConfig config = new JsonbConfig()
// TODO: what configuration to add to map interfaces to concrete implementations?
Jsonb jsonb = JsonbBuilder.create(config)
jsonb.fromJson(json, Customer.class);

Above gives exception: javax.json.bind.JsonbException: interface com.acme.Customer not instantiable

Note: I cannot change the interfaces nor the concrete implementations as these are provided.

jsonbrobot avatar Oct 22 '17 18:10 jsonbrobot

  • Issue Imported From: https://github.com/javaee/jsonb-spec/issues/65
  • Original Issue Raised By:@marceloverdijk
  • Original Issue Assigned To: Unassigned

jsonbrobot avatar May 23 '18 22:05 jsonbrobot

@bravehorsie Commented There is already a pull request for this in the Yasson RI. https://github.com/eclipse/yasson/pull/64 @m0mus Can we move annotation and JsonbConfig in the above pull request to the spec for next release? Looks like a handy feauture.

jsonbrobot avatar Nov 22 '17 10:11 jsonbrobot

@marceloverdijk Commented Can that solution be used without an @Annotation? In my case I cannot change the classes itself.

jsonbrobot avatar Nov 22 '17 10:11 jsonbrobot

@bravehorsie Commented You can also add mapping with JsonbConfig see https://github.com/eclipse/yasson/pull/64/files#diff-42990ea8091951c02b2166a51a23f6a6 second test method. In your case it is also enough to call: Customer customer = jsonb.fromJson(json, CustomerImpl.class);

jsonbrobot avatar Nov 22 '17 11:11 jsonbrobot

@marceloverdijk Commented Thanks! Looking forward to see this incorporated in the jsonb spec.

jsonbrobot avatar Nov 22 '17 12:11 jsonbrobot

I think this is a good feature and we should consider it for the next version of the spec.

Additionally, CDI already has the knowledge to know which impl classes map to which interfaces, so we could have the spec define that if CDI is present, it is used to discover a default impl->ifc mapping.

aguibert avatar Jul 22 '19 22:07 aguibert

Hi Johnzon has a property for that ([1])

Side note: cdi does not have that info for two reason:

  • models are likely nlt cdi beans
  • you have the contract in the API, not the impl, dont forget it can be made of multiple beans with decorators or of proxies with producers and custom beans

So at the end you need to use an explicit mapping

[1] https://github.com/apache/geronimo-openapi/blob/master/geronimo-openapi-impl/src/main/java/org/apache/geronimo/microprofile/openapi/impl/loader/DefaultLoader.java#L46

rmannibucau avatar Jul 23 '19 04:07 rmannibucau

models are likely nlt cdi beans

Good point, usually the app directly creates model objects (or uses JSON-B to create them) as POJOs.

How about an explicit mapping through the API then? A combination of what you've done with system properties / JsonbConfig and what Roman has done in https://github.com/eclipse-ee4j/yasson/pull/64/files sounds good.

aguibert avatar Jul 23 '19 21:07 aguibert

Agree, both work. The @Impl option is weird in practise cause then it bounds the api to an impl which is never the case or you created an interface for nothing. If you take microprofike openapi, the api are not modifiable too so the property in jsonbconfig is the best option IMHO. In other words, if you can use @Impl, you can directly map the impl cause it is your own code and we just get back to the polymorphic issue which has another api to solve that.

rmannibucau avatar Jul 24 '19 04:07 rmannibucau

I agree that some sort of @Impl annotation on the field/method defeats the purpose of the application using an interface to begin with.

The main thing I'm after is type-safety with the config. Something like:

    private static final Jsonb jsonb = JsonbBuilder.create(new JsonbConfig()
                    .withInterfaceMapping(Vehicle.class, Truck.class);

But this is not quite as flexible as Roman's @Impl approach because we may want to map Vehicle.class -> Truck.class in one place, but Vehicle.class -> Van.class in another place. To solve this, we could allow for overloading withInterfaceMapping such as:

// Maps deserialization of an interface to a specific implementation for all JSON-B models, 
// unless specified at a more specific scope using withInterfaceMapping(Class, Class, Class)
public JsonbConfig withInterfaceMapping(Class<?> interface, Class<?> impl);

// Maps deserialization of an interface to a specific implementation for the given JSON-B model class
public JsonbConfig withInterfaceMapping(Class<?> interface, Class<?> impl, Class<?> jsonbClass);

aguibert avatar Jul 24 '19 15:07 aguibert

Woudlnt it be broken in array of object case? What about starting simple with a direct mapping, all others cases can be solved by deserializers right?

rmannibucau avatar Jul 24 '19 15:07 rmannibucau

Woudlnt it be broken in array of object case?

Not sure what you are referring to here -- can you clarify what you mean by this?

What about starting simple with a direct mapping, all others cases can be solved by deserializers right?

I see ifc->impl mapping as a cross-cutting concept to deserializers. For example, what if I have 20 objects that all reference the Vehicle interface, and I want to map them to the Truck impl class. With the deserializer approach, I would have to write 20 deserializers just to say Vehicle-->Truck 20 times over.

aguibert avatar Jul 24 '19 16:07 aguibert

@aguibert if you have an array of Foo and Foo being an interface, how do you map it in your last signature?

I would have to write 20 deserializers

No? You only need to map a deserializer (you write it once and reuse it in your model) if the default global mapping does not match and your field is nested which is likely rare. At least we pass microprofile-openapi with that option without custom deserializers for the interface mapping.

rmannibucau avatar Jul 24 '19 17:07 rmannibucau

I've implemented simple interface-to-implementation mapping for embedded interface fields using adapters. I wish it could be easier as those adapters do not really convert anything.

kazssym avatar Jun 26 '20 01:06 kazssym