Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider the @JsonSubTypes for generating the discriminator mapping #3411

Open
SabhtarshaMojo opened this issue Jan 14, 2020 · 7 comments
Open

Comments

@SabhtarshaMojo
Copy link

SabhtarshaMojo commented Jan 14, 2020

To generate the appropriate mapping for a discriminator of a parent class, we are required to add the discriminatorMapping property to @Schema, which is almost always redundant if the class already has a @JsonSubTypes annotation.
As the discriminatorProperty is read from the @JsonTypeInfo.property can the discriminatorMappings also be read from the JsonSubTypes if available?

As a workaround, I have registered a CustomModelResolver and extended the method resolveDiscriminator as follows,

@Override
protected Discriminator resolveDiscriminator(JavaType type, ModelConverterContext context) {
	Discriminator discriminator = super.resolveDiscriminator(type, context);
	if (discriminator != null && discriminator.getPropertyName() != null &&
			(discriminator.getMapping() == null || discriminator.getMapping().isEmpty())) {
		JsonSubTypes jsonSubTypes = type.getRawClass().getDeclaredAnnotation(JsonSubTypes.class);
		if (jsonSubTypes != null) {
			Arrays.stream(jsonSubTypes.value()).forEach(subtype -> {
				discriminator.mapping(subtype.name(), RefUtils.constructRef(
						context.resolve(new AnnotatedType().type(subtype.value())).getName()));
			});
		}
	}
	return discriminator;
}

Additional Info: Using the swagger-maven-plugin v2.1.1 for generating the OAS files during the compile phase.

@copitz
Copy link

copitz commented Feb 20, 2020

Additionally to @SabhtarshaMojo's solution it would be good to add the subtypes registered by objectMapper.registerSubTypes() - here's what I ended up with:

@Override
protected Discriminator resolveDiscriminator(JavaType type, ModelConverterContext context) {
  Discriminator discriminator = super.resolveDiscriminator(type, context);
  if (discriminator != null && discriminator.getPropertyName() != null) {
    addResolvedSubTypeMappings(discriminator, type, context);
    addAnnotatedSubTypeMappings(discriminator, type, context);
  }

  return discriminator;
}

private void addResolvedSubTypeMappings(Discriminator discriminator, JavaType type, ModelConverterContext context) {
  MapperConfig<?> config = _mapper.getSerializationConfig();
  _mapper.getSubtypeResolver()
    .collectAndResolveSubtypesByClass(config, AnnotatedClassResolver.resolveWithoutSuperTypes(config, type, config))
    .stream()
    .filter(namedType -> !namedType.getType().equals(type.getRawClass()))
    .forEach(namedType -> addMapping(discriminator, namedType.getName(), namedType.getType(), context));
}

private void addAnnotatedSubTypeMappings(Discriminator discriminator, JavaType type, ModelConverterContext context) {
  JsonSubTypes jsonSubTypes = type.getRawClass().getDeclaredAnnotation(JsonSubTypes.class);
  if (jsonSubTypes != null) {
    Arrays.stream(jsonSubTypes.value())
      .forEach(subtype -> addMapping(discriminator, subtype.name(), subtype.value(), context));
  }
}

private void addMapping(Discriminator discriminator, String name, Type type, ModelConverterContext context) {
  boolean isNamed = name != null && !name.isBlank();
  String schemaName = context.resolve(new AnnotatedType().type(type)).getName();
  String ref = RefUtils.constructRef(schemaName);
  Map<String, String> mappings = Optional.ofNullable(discriminator.getMapping()).orElseGet(Map::of);

  if (!isNamed && mappings.containsValue(ref)) {
    // Skip adding the unnamed mapping
    return;
  }

  discriminator.mapping(isNamed ? name : schemaName, ref);

  if (isNamed && ref.equals(mappings.get(schemaName))) {
    // Remove previous unnamed mapping
    discriminator.getMapping().remove(schemaName);
  }
}

(This combines all sub type mappings but removes unnamed ones when named ones are present)

@MartinHaeusler
Copy link

Sorry to necro-post this issue but I'd like to add my voice to this as well. My team developed an application with a REST API which extensively relies on @JsonSubTypes for inheritance. At the moment, even a relatively simple schema like this:

@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.PROPERTY, property = "type")
@JsonSubTypes(
    JsonSubTypes.Type(value = CatDTO::class, name = "cat"),
    JsonSubTypes.Type(value = DogDTO::class, name = "dog"),
)
sealed interface AnimalDTO

class CatDTO(val catProperty: String): AnimalDTO

class DogDTO(val dogProperty: String) : AnimalDTO

... cannot be used with OpenAPI, because in the resulting JSON definition the subtype names (cat and dog) are not even included. OpenAPI seems to assume that the discriminator name is always equal to the class name, which clearly isn't always the case.

Since we're using polymorphism like this also for POST request bodies, it's an absolute necessity for the client to be aware of these discriminators and their proper names.

@MartinHaeusler
Copy link

Are there any news on this matter?

@Schwaller
Copy link

I would also love to see some progress on this. Apparently its quite helpful when generating TS union types from it ... for now we have to duplicate these mappings for swagger style which is error prone. If it would determine the mappings by instantiating all possible subclasses that would also be fine. But this might be trick to get reliably done when constructor is complex but maybe you use some unsafe tricks to solve that.

@jeffreyschultz
Copy link

Please make this feature generally available. Unless I override functionality, this breaks the OpenAPI specs as they are not legal values for each sub-type.

@ondrejkrpec
Copy link

Are there any updates on this?

@jeffreyschultz
Copy link

Any update on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants