swagger-api / swagger-api/swagger-core

Making primitive types required via customizers?

Open
#4,444 0 comments 4 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

backlog
Dominant language
Java
Stars
7.5k
Forks
2.3k
Avg merge
18h 1m
Merged PRs (30d)
10

Description

First off, thank you very much for doing all this hard work and maintaining Swagger. I ran into a peculiar problem with 2.2.0, which, by the look of it seems to be still relevant for the newer versions, and thought it might be an opportunity for improvement.

Original problem

By default swagger-core (2.2.0) does not mark scalar properties (boolean, int, ...) as required, unless they are annotated with one of the ModelResolver.NOT_NULL_ANNOTATIONS annotations. The following will result in getPropertyA() being optional:

public interface ScalarTypesIssue {
  boolean getPropertyA();
 }

This is not incorrect per se (the implementation could be done in such way that the property will not be serialized if it isn't set, and so from the REST API's perspective it may still be optional). However, it is rather unpractical since most POJOs have the getters and setters of the same type, and boolean or int typically imply a required value.

This can be worked around by adding one of the not-null annotations:

import javax.validation.constraints.NotNull;

public interface ScalarTypesIssue {
  @NonNull
  boolean getPropertyA();
}

This workaround, however, is hard to apply when you do not control the type definitions (in our case they were generated by another tool that treats the scalar types as required). This may also not be feasible if the required-ness of a property is determined elsewhere, i.e. by a naming convention, various monadic types, and suchlike madness.

Attempted solution

I tried using the customizers for fixing this behavior as they seemed to be a good fit. The customizer would inspect the type and, if it is indeed primitive, append to the parent's required.

This, however, did not work as expected. Namely in this situation:

public interface ScalarTypesIssue {
  boolean getPropertyA();
  boolean getPropertyB();
}

only the propertyA would become required.

This happens for two reasons:

  1. the required-ness in OpenAPI is not context-free (unfortunately). Only the parent schema knows which properties are required, so making this decision while inspecting the property looks a bit sketchy.
  2. the ModelResolver and ModelContext short-circuit the type inspection by caching already discovered schemas with their AnnotatedTypes as cache keys. When the getPropertyB() is introspected, the boolean type is already present in the ModelContext's cache, therefore it is just re-used and customizers are not called on it. The ModelResolver applies the required-ness detection logic based on annotations (here is where ModelResolver.NOT_NULL_ANNOTATIONS comes into play), but that does not seem to be customizable to the point needed for my use case.

Eventually I made it work by creating a customizer that introspects the type again, re-discovers all of its properties, and populates the required as necessary. This effectively double-scans every POJO and is less than ideal.

The code is pretty long, but I can come up with a distilled version if this is required.

Questions / suggestions

  1. Is there a better approach to implementing customizations like this?
  2. Does the introspection short-circuiting in ModelContext.resolve work as expected here? It appears that AnnotatedType and by extension customizers are designed to be contextual, however the ModelContext.resolve implementation treats the types as context-free (type equivalency is determined by a subset of annotations and type alone, see AnnotatedType.hashCode, and ignores the context in which the type is used). This makes the result of introspection dependent on which path along the schema graph is traversed first, which is non-deterministic in practice. Perhaps the customizers should be context-free (type customizations are not dependent on the context where the type is used), or perhaps there should be a way to "post-process" a type in the context where it is used independently of other uses?
  3. Perhaps treating primitive types as required could be added to the config as an option?

Thank you very much for looking into this!

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reading ModelResolver, ModelContext.resolve, and AnnotatedType.hashCode to understand schema caching, customizer invocation, and required-property detection. Reproduce the two-primitive-property case, then determine whether the intended outcome is configurable primitive requiredness, context-aware post-processing, or another customization approach; completion requires a decided design and behavior that no longer depends on traversal order.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
api, backend-api-design
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.