[WIP] Add support for custom media types #101
Conversation
In order to support custom media types a registry is added that allows registration of these. With the registry comes a need for a refactoring of the ContentAnalyser. As it becomes part of the public module api, an interface is extracted and is moved to the new mediatype package.
| * @return The builder, for a fluent interface | ||
| */ | ||
| public OpenAPIContractBuilder contract(String contractPath) { | ||
| if (this.contract != null) |
| * @param path The path to the contract file. | ||
| * @return The builder, for a fluent interface | ||
| */ | ||
| public OpenAPIContractBuilder addAdditionalContent(String key, String path) { |
There was a problem hiding this comment.
putAdditionalContract <- then you don't have to check for duplicates. It also allows you to override a single contract which was maybe added by "addAdditionalContentFiles".
| return this; | ||
| } | ||
|
|
||
| public OpenAPIContractBuilder addAdditionalContentFiles(Map<String, String> otherContractFiles) { |
There was a problem hiding this comment.
setAdditionalContractFiles <- then you don't have to check for duplicates. And you are able to remove something. This would require to reset the other Map of course.
| private final Vertx vertx; | ||
| private String contractFile; | ||
| private JsonObject contract; | ||
| private final Map<String, String> additionalContentFiles = new HashMap<>(); |
| private String contractFile; | ||
| private JsonObject contract; | ||
| private final Map<String, String> additionalContentFiles = new HashMap<>(); | ||
| private final Map<String, JsonObject> additionalContent = new HashMap<>(); |
| */ | ||
| public Future<OpenAPIContract> build() { | ||
| if (contractFile == null && contract == null) { | ||
| return Future.failedFuture(new OpenAPIContractBuilderException("Neither a contract file or a contract is set. One of them must be set.")); |
There was a problem hiding this comment.
This line clashes with
return Future.failedFuture(OpenAPIContractException.createInvalidContract("Spec must not be null"));
from OpenAPIContract
Same issue, but different Exceptions. Maybe this Exception makes more sense in the Builder. It would be a breaking change, but it is okay as Vert.x OpenAPI is in Technical Preview. Let's discuss it more deeply.
| }); | ||
| } | ||
|
|
||
| private Future<OpenAPIContract> from(JsonObject unresolvedContract, |
There was a problem hiding this comment.
In my opinion from doesn't make sense in the scope of Builder. This is the real "build" method. The content of the current "build" method is more or less a consolidation or pre-work / init for the real "build"
There was a problem hiding this comment.
Maybe this method could be stay in OpenAPIContract and the build method in Builder is calling it. This would also solve the issue with different Exceptions, because then the Builder really just aggregates settings for the Contract and has no own logic.
| * @return The registry. | ||
| */ | ||
| @GenIgnore | ||
| MediaTypeRegistry mediaTypes(); |
There was a problem hiding this comment.
| MediaTypeRegistry mediaTypes(); | |
| MediaTypeRegistry getMediaTypeRegistry(); |
| static DefaultMediaTypeRegistry createDefault() { | ||
| return new DefaultMediaTypeRegistry() | ||
| .register(MediaTypeRegistration.TEXT_PLAIN) | ||
| .register(MediaTypeRegistration.MULTIPART_FORM_DATA) | ||
| .register(MediaTypeRegistration.APPLICATION_JSON) | ||
| .register(MediaTypeRegistration.APPLICATION_OCTET_STREAM); | ||
| } | ||
|
|
||
| /** | ||
| * Creates an empty registry. | ||
| * | ||
| * @return An empty registry. | ||
| */ | ||
| static DefaultMediaTypeRegistry createEmpty() { | ||
| return new DefaultMediaTypeRegistry(); | ||
| } | ||
|
|
||
| /** | ||
| * Registers a new MediaTypeHandler | ||
| * | ||
| * @param registration The mediatype registration. | ||
| * @return This registry for a fluent interface. | ||
| */ | ||
| DefaultMediaTypeRegistry register(MediaTypeRegistration registration); |
There was a problem hiding this comment.
Keep this in DefaultMediaTypeRegistry
| * @param context Whether the analyser is for a request or response. | ||
| * @return A fresh content analyser instance. | ||
| */ | ||
| ContentAnalyser createContentAnalyser(String contentType, Buffer content, ValidationContext context); |
Motivation:
At the moment it is not possible to use media types that are not included in the static supported mediatypes list in requests and responses. This forces the users to use generic media types where more specific ones exist in their openapi specs.
This pull-request replaces the current static list of mediatypes with a registry that allows the registration of additional mediatypes.
At the moment this is an early draft of the feature and the pull request is used to provide a place for a first review.
Conformance:
The Eclipse Contributor Agreement is signed by me.
The code style will be enforced once this feature reaches a mature state.