From 61467e3376c9927cbd01bc8b1e71fbd75836e029 Mon Sep 17 00:00:00 2001 From: "tim.quinn@oracle.com" Date: Thu, 2 Dec 2021 13:24:06 -0600 Subject: [PATCH 01/12] New versions of Jandex and SmallRye OpenAPI --- dependencies/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dependencies/pom.xml b/dependencies/pom.xml index 0c2a6d10fdd..56dfdf77578 100644 --- a/dependencies/pom.xml +++ b/dependencies/pom.xml @@ -137,7 +137,7 @@ 42.2.18 0.9.0 1.7.32 - 2.1.15 + 2.1.16 1.27 1.4.1 2.0.1 From f8d65112b539fec9469edced03674d89a85e8da8 Mon Sep 17 00:00:00 2001 From: "tim.quinn@oracle.com" Date: Thu, 2 Dec 2021 13:26:14 -0600 Subject: [PATCH 02/12] Revise customizations to SnakeYAML parsing to reflect changes in MP OpenAPI API --- .../io/helidon/openapi/CustomConstructor.java | 185 ++++++++++++++---- .../openapi/ExpandedTypeDescription.java | 114 ++++++++++- 2 files changed, 255 insertions(+), 44 deletions(-) diff --git a/openapi/src/main/java/io/helidon/openapi/CustomConstructor.java b/openapi/src/main/java/io/helidon/openapi/CustomConstructor.java index 826a8a9749e..091593950b1 100644 --- a/openapi/src/main/java/io/helidon/openapi/CustomConstructor.java +++ b/openapi/src/main/java/io/helidon/openapi/CustomConstructor.java @@ -16,9 +16,9 @@ package io.helidon.openapi; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.function.Function; import java.util.logging.Level; import java.util.logging.Logger; @@ -40,56 +40,141 @@ import org.yaml.snakeyaml.nodes.Tag; /** - * Specialized SnakeYAML constructor for modifying {@code Node} objects for OpenAPI types that extend {@code Map} to adjust the - * type of the child nodes of such nodes. + * Specialized SnakeYAML constructor for modifying {@code Node} objects for OpenAPI types needing special attention. *

- * Several MicroProfile OpenAPI interfaces extend {@code Map}. For example, {@code Paths} extends {@code Map - * } and {@code SecurityRequirement} extends {@code Map>}. When SnakeYAML builds the node - * corresponding to one of these types, it correctly creates each child node as a {@code MappingNode} but it assigns those - * child nodes a type of {@code Object} instead of the mapped type -- {@code PathItem} in the example above. - *

- *

- * This class customizes the preparation of the node tree in these situations by setting the types for the child nodes explicitly - * to the corresponding child type. In OpenAPI 1.1.2 there are two situations, depending on whether the mapped-to type is a - * {@code List} or not. - *

- *

- * The MicroProfile OpenAPI 2.0 versions of the interfaces no longer use this construct of an interface extending {@code Map}, so - * ideally we can remove this workaround when we adopt 2.0. + * Several MP OpenAPI types resemble maps with strings for keys and various child types as values. Such interfaces + * expose an {@code addX} method, where X is the child type (e.g., {@link Paths} exposes {@link Paths#addPathItem}. + * SnakeYAML parsing, left to itself, would incorrectly attempt to use the string keys as property names in converting OpenAPI + * documents to and from the in-memory POJO model. To prevent that, this custom constructor takes over the job of + * creating these parent instances and populating the children from the SnakeYAML node graph. *

*/ final class CustomConstructor extends Constructor { - // maps OpenAPI interfaces which extend Map to the mapped-to type where that mapped-to type is NOT List - private static final Map, Class> CHILD_MAP_TYPES = new HashMap<>(); + // OpenAPI interfaces which resemble Map, linked to info used to prepare the type description for that type where + // the mapped-to type is NOT a list. For typing reasons (in ExpandedTypeDescription$MapLikeTypeDescription#create) + // we provide type-specific factory functions as part of the type metadata here where we can specify the actual parent + // and child types. + static final Map, ChildMapType> CHILD_MAP_TYPES = Map.of( + APIResponses.class, new ChildMapType<>(APIResponses.class, + APIResponse.class, + APIResponses::addAPIResponse, + impl -> ExpandedTypeDescription.MapLikeTypeDescription.create( + APIResponses.class, + impl, + APIResponse.class, + APIResponses::addAPIResponse)), + Callback.class, new ChildMapType<>(Callback.class, + PathItem.class, + Callback::addPathItem, + impl -> ExpandedTypeDescription.MapLikeTypeDescription.create( + Callback.class, + impl, + PathItem.class, + Callback::addPathItem)), + Content.class, new ChildMapType<>(Content.class, + MediaType.class, + Content::addMediaType, + impl -> ExpandedTypeDescription.MapLikeTypeDescription.create( + Content.class, + impl, + MediaType.class, + Content::addMediaType)), + Paths.class, new ChildMapType<>(Paths.class, + PathItem.class, + Paths::addPathItem, + impl -> ExpandedTypeDescription.MapLikeTypeDescription.create( + Paths.class, + impl, + PathItem.class, + Paths::addPathItem))); - // maps OpenAPI interfaces which extend Map> to the type that appears in the list - private static final Map, Class> CHILD_MAP_OF_LIST_TYPES = new HashMap<>(); + // OpenAPI interfaces which resemble Map>, linked to info used to prepare the type description for that type + // where the mapped-to type IS a list. + static final Map, ChildMapListType> CHILD_MAP_OF_LIST_TYPES = Map.of( + SecurityRequirement.class, new ChildMapListType<>(SecurityRequirement.class, + String.class, + SecurityRequirement::addScheme, + SecurityRequirement::addScheme, + SecurityRequirement::addScheme, + impl -> ExpandedTypeDescription.ListMapLikeTypeDescription.create( + SecurityRequirement.class, + impl, + String.class, + SecurityRequirement::addScheme, + SecurityRequirement::addScheme, + SecurityRequirement::addScheme))); - private static final Logger LOGGER = Logger.getLogger(CustomConstructor.class.getName()); + /** + * Adds a single named child to the parent. + * + * @param

parent type + * @param child type + */ + @FunctionalInterface + interface ChildAdder { + Object addChild(P parent, String name, C child); + } + + /** + * Adds a list of children to the parent. + * + * @param

parent type + * @param child type + */ + @FunctionalInterface + interface ChildListAdder { + Object addChildren(P parent, String name, List children); + } - static { - CHILD_MAP_TYPES.put(Paths.class, PathItem.class); - CHILD_MAP_TYPES.put(Callback.class, PathItem.class); - CHILD_MAP_TYPES.put(Content.class, MediaType.class); - CHILD_MAP_TYPES.put(APIResponses.class, APIResponse.class); - /* - TODO 3.0.0-JAKARTA - CHILD_MAP_TYPES.put(ServerVariables.class, ServerVariable.class); - CHILD_MAP_TYPES.put(Scopes.class, String.class); - */ - CHILD_MAP_OF_LIST_TYPES.put(SecurityRequirement.class, String.class); + /** + * Adds a valueless child name to the parent. + * + * @param

parent type + */ + @FunctionalInterface + interface ChildNameAdder

{ + P addChild(P parent, String name); } + /** + * Type information about a map-resembling interface. + * + * @param

parent type + * @param child type + */ + record ChildMapType(Class

parentType, + Class childType, + ChildAdder childAdder, + Function, ExpandedTypeDescription.MapLikeTypeDescription> typeDescriptionFactory) { } + + /** + * Type information about a map-resembling interface in which a child can have 0, 1, or more values i.e., the child is + * a list). + * + * @param

parent type + * @param child type + */ + record ChildMapListType( + Class

parentType, + Class childType, + ChildAdder childAdder, + ChildListAdder childListAdder, + ChildNameAdder

childNameAdder, + Function, ExpandedTypeDescription.ListMapLikeTypeDescription> typeDescriptionFunction) { } + + private static final Logger LOGGER = Logger.getLogger(CustomConstructor.class.getName()); + CustomConstructor(TypeDescription td) { super(td); + yamlClassConstructors.put(NodeId.mapping, new ConstructMapping()); } @Override protected void constructMapping2ndStep(MappingNode node, Map mapping) { Class parentType = node.getType(); if (CHILD_MAP_TYPES.containsKey(parentType)) { - Class childType = CHILD_MAP_TYPES.get(parentType); + Class childType = CHILD_MAP_TYPES.get(parentType).childType; node.getValue().forEach(tuple -> { Node valueNode = tuple.getValueNode(); if (valueNode.getType() == Object.class) { @@ -97,7 +182,7 @@ protected void constructMapping2ndStep(MappingNode node, Map map } }); } else if (CHILD_MAP_OF_LIST_TYPES.containsKey(parentType)) { - Class childType = CHILD_MAP_OF_LIST_TYPES.get(parentType); + Class childType = CHILD_MAP_OF_LIST_TYPES.get(parentType).childType; node.getValue().forEach(tuple -> { Node valueNode = tuple.getValueNode(); if (valueNode.getNodeId() == NodeId.sequence) { @@ -125,9 +210,35 @@ private void convertIntHttpStatuses(MappingNode node) { }); if (!numericHttpStatusMarks.isEmpty()) { LOGGER.log(Level.WARNING, - "Numeric HTTP status value(s) should be quoted. " - + "Please change the following; unquoted numeric values might be rejected in a future release: {0}", - numericHttpStatusMarks); + "Numeric HTTP status value(s) should be quoted. " + + "Please change the following; unquoted numeric values might be rejected in a future release: " + + "{0}", + numericHttpStatusMarks); + } + } + + /** + * Override of SnakeYAML logic which constructs an object from a node. + *

+ * This class makes sure that parent/child relationships that resemble maps are handled correctly and defers to the + * superclass implementation in other cases. + *

+ */ + class ConstructMapping extends Constructor.ConstructMapping { + + @Override + public Object construct(Node node) { + Class parentType = node.getType(); + if (CHILD_MAP_TYPES.containsKey(parentType) || CHILD_MAP_OF_LIST_TYPES.containsKey(parentType)) { + // Following is inspired by SnakeYAML Constructor$ConstructMapping#construct. + MappingNode mappingNode = (MappingNode) node; + if (node.isTwoStepsConstruction()) { + return newMap(mappingNode); + } else { + return constructMapping(mappingNode); + } + } + return super.construct(node); } } } diff --git a/openapi/src/main/java/io/helidon/openapi/ExpandedTypeDescription.java b/openapi/src/main/java/io/helidon/openapi/ExpandedTypeDescription.java index 8618fdccc08..eb43aed5f8b 100644 --- a/openapi/src/main/java/io/helidon/openapi/ExpandedTypeDescription.java +++ b/openapi/src/main/java/io/helidon/openapi/ExpandedTypeDescription.java @@ -68,7 +68,7 @@ class ExpandedTypeDescription extends TypeDescription { private static final PropertyUtils PROPERTY_UTILS = new PropertyUtils(); - private Class impl; + private final Class impl; /** * Factory method for ease of chaining other method invocations. @@ -77,15 +77,25 @@ class ExpandedTypeDescription extends TypeDescription { * @param impl implementation class for the interface * @return resulting TypeDescription */ - static ExpandedTypeDescription create(Class clazz, Class impl) { - - ExpandedTypeDescription result = clazz.equals(Schema.class) - ? new SchemaTypeDescription(clazz, impl) : new ExpandedTypeDescription(clazz, impl); + static ExpandedTypeDescription create(Class clazz, Class impl) { + + ExpandedTypeDescription result; + if (clazz.equals(Schema.class)) { + result = new SchemaTypeDescription(clazz, impl); + } else if (CustomConstructor.CHILD_MAP_TYPES.containsKey(clazz)) { + CustomConstructor.ChildMapType childMapType = CustomConstructor.CHILD_MAP_TYPES.get(clazz); + result = childMapType.typeDescriptionFactory().apply(impl); + } else if (CustomConstructor.CHILD_MAP_OF_LIST_TYPES.containsKey(clazz)) { + CustomConstructor.ChildMapListType childMapListType = CustomConstructor.CHILD_MAP_OF_LIST_TYPES.get(clazz); + result = childMapListType.typeDescriptionFunction().apply(impl); + } else { + result = new ExpandedTypeDescription(clazz, impl); + } result.setPropertyUtils(PROPERTY_UTILS); return result; } - private ExpandedTypeDescription(Class clazz, Class impl) { + private ExpandedTypeDescription(Class clazz, Class impl) { super(clazz, null, impl); this.impl = impl; } @@ -260,7 +270,7 @@ private static PropertyDescriptor preparePropertyDescriptor() { } } - private SchemaTypeDescription(Class clazz, Class impl) { + private SchemaTypeDescription(Class clazz, Class impl) { super(clazz, impl); } @@ -270,6 +280,96 @@ public Property getProperty(String name) { } } + /** + * Type description for parent/child model objects which resemble maps. + * + * @param

parent type + * @param child type + */ + static class MapLikeTypeDescription extends ExpandedTypeDescription { + + private final Class

parentType; + private final Class childType; + private final CustomConstructor.ChildAdder childAdder; + + static MapLikeTypeDescription create(Class

parentType, + Class impl, + Class childType, + CustomConstructor.ChildAdder childAdder) { + return new MapLikeTypeDescription<>(parentType, impl, childType, childAdder); + } + + MapLikeTypeDescription(Class

parentType, + Class impl, + Class childType, + CustomConstructor.ChildAdder childAdder) { + super(parentType, impl); + this.childType = childType; + this.parentType = parentType; + this.childAdder = childAdder; + } + + @Override + public boolean setProperty(Object targetBean, String propertyName, Object value) throws Exception { + P parent = parentType.cast(targetBean); + C child = childType.cast(value); + childAdder.addChild(parent, propertyName, child); + return true; + } + + protected Class

parentType() { + return parentType; + } + + protected Class childType() { + return childType; + } + + protected CustomConstructor.ChildAdder childAdder() { + return childAdder; + } + } + + static class ListMapLikeTypeDescription extends MapLikeTypeDescription { + + private final CustomConstructor.ChildNameAdder

childNameAdder; + private final CustomConstructor.ChildListAdder childListAdder; + + static ListMapLikeTypeDescription create(Class

parentType, + Class impl, + Class childType, + CustomConstructor.ChildAdder childAdder, + CustomConstructor.ChildNameAdder

childNameAdder, + CustomConstructor.ChildListAdder childListAdder) { + return new ListMapLikeTypeDescription<>(parentType, impl, childType, childAdder, childNameAdder, childListAdder); + } + + private ListMapLikeTypeDescription(Class

parentType, + Class impl, + Class childType, + CustomConstructor.ChildAdder childAdder, + CustomConstructor.ChildNameAdder

childNameAdder, + CustomConstructor.ChildListAdder childListAdder) { + super(parentType, impl, childType, childAdder); + this.childNameAdder = childNameAdder; + this.childListAdder = childListAdder; + } + + @Override + @SuppressWarnings("unchecked") + public boolean setProperty(Object targetBean, String propertyName, Object value) throws Exception { + P parent = parentType().cast(targetBean); + if (value == null) { + childNameAdder.addChild(parent, propertyName); + } else if (value instanceof List) { + childListAdder.addChildren(parent, propertyName, (List) value); + } else { + childAdder().addChild(parent, propertyName, childType().cast(value)); + } + return true; + } + } + /** * Property description for an extension subnode. */ From 462b8c4901e6e837ed0df625dcd56aa547b5c247 Mon Sep 17 00:00:00 2001 From: "tim.quinn@oracle.com" Date: Thu, 2 Dec 2021 13:27:43 -0600 Subject: [PATCH 03/12] Conform to new standard builder pattern --- .../io/helidon/microprofile/openapi/MPOpenAPIBuilder.java | 4 ---- .../main/java/io/helidon/openapi/SEOpenAPISupportBuilder.java | 4 ---- 2 files changed, 8 deletions(-) diff --git a/microprofile/openapi/src/main/java/io/helidon/microprofile/openapi/MPOpenAPIBuilder.java b/microprofile/openapi/src/main/java/io/helidon/microprofile/openapi/MPOpenAPIBuilder.java index e1f3d867123..f4d46477bea 100644 --- a/microprofile/openapi/src/main/java/io/helidon/microprofile/openapi/MPOpenAPIBuilder.java +++ b/microprofile/openapi/src/main/java/io/helidon/microprofile/openapi/MPOpenAPIBuilder.java @@ -68,10 +68,6 @@ public final class MPOpenAPIBuilder extends OpenAPISupport.Builder From a7d31441a84b32f3a66ea16e93a44460c06e26e3 Mon Sep 17 00:00:00 2001 From: "tim.quinn@oracle.com" Date: Thu, 2 Dec 2021 13:29:38 -0600 Subject: [PATCH 04/12] Conform to new builder pattern; add support for optional query param 'format' on GETs to /openapi --- .../io/helidon/openapi/OpenAPISupport.java | 66 ++++++++++++++----- 1 file changed, 49 insertions(+), 17 deletions(-) diff --git a/openapi/src/main/java/io/helidon/openapi/OpenAPISupport.java b/openapi/src/main/java/io/helidon/openapi/OpenAPISupport.java index 86c83611880..381192bf2a4 100644 --- a/openapi/src/main/java/io/helidon/openapi/OpenAPISupport.java +++ b/openapi/src/main/java/io/helidon/openapi/OpenAPISupport.java @@ -106,9 +106,26 @@ public abstract class OpenAPISupport implements Service { */ public static final MediaType DEFAULT_RESPONSE_MEDIA_TYPE = MediaType.APPLICATION_OPENAPI_YAML; - /** - * Path to the Jandex index file. - */ + private enum QueryParameterRequestedFormat { + JSON(MediaType.APPLICATION_JSON), YAML(MediaType.APPLICATION_OPENAPI_YAML); + + static QueryParameterRequestedFormat chooseFormat(String format) { + return QueryParameterRequestedFormat.valueOf(format); + } + + private final MediaType mt; + + QueryParameterRequestedFormat(MediaType mt) { + this.mt = mt; + } + + MediaType mediaType() { + return mt; + } + } + + private static final String OPENAPI_ENDPOINT_FORMAT_QUERY_PARAMETER = "format"; + private static final Logger LOGGER = Logger.getLogger(OpenAPISupport.class.getName()); private static final String DEFAULT_STATIC_FILE_PATH_PREFIX = "META-INF/openapi."; @@ -139,6 +156,11 @@ public abstract class OpenAPISupport implements Service { private final OpenApiStaticFile openApiStaticFile; private final Supplier> indexViewsSupplier; + /** + * Creates a new instance of {@code OpenAPISupport}. + * + * @param builder the builder to use in constructing the instance + */ protected OpenAPISupport(Builder builder) { adjustTypeDescriptions(helper().types()); implsToTypes = buildImplsToTypes(helper()); @@ -368,7 +390,7 @@ private void prepareResponse(ServerRequest req, ServerResponse resp) { resp.send(openAPIDocument); } catch (Exception ex) { resp.status(Http.Status.INTERNAL_SERVER_ERROR_500); - resp.send("Error serializing OpenAPI document"); + resp.send("Error serializing OpenAPI document; " + ex.getMessage()); LOGGER.log(Level.SEVERE, "Error serializing OpenAPI document", ex); } } @@ -422,6 +444,20 @@ private MediaType chooseResponseMediaType(ServerRequest req) { * Response media type default is application/vnd.oai.openapi (YAML) * unless otherwise specified. */ + Optional queryParameterFormat = req.queryParams() + .first(OPENAPI_ENDPOINT_FORMAT_QUERY_PARAMETER); + if (queryParameterFormat.isPresent()) { + String queryParameterFormatValue = queryParameterFormat.get(); + try { + return QueryParameterRequestedFormat.chooseFormat(queryParameterFormatValue).mediaType(); + } catch (IllegalArgumentException e) { + throw new IllegalArgumentException( + "Query parameter 'format' had value '" + + queryParameterFormatValue + + "' but expected " + Arrays.toString(QueryParameterRequestedFormat.values())); + } + } + final Optional requestedMediaType = req.headers() .bestAccepted(OpenAPIMediaType.preferredOrdering()); @@ -678,19 +714,10 @@ public abstract static class Builder> implements io.helidon */ public static final String CONFIG_KEY = "openapi"; - private final Class builderClass; - private Optional webContext = Optional.empty(); private Optional staticFilePath = Optional.empty(); private CrossOriginConfig crossOriginConfig = null; - protected Builder(Class builderClass) { - this.builderClass = builderClass; - } - - protected B me() { - return builderClass.cast(this); - } /** * Set various builder attributes from the specified {@code Config} object. @@ -712,7 +739,7 @@ public B config(Config config) { config.get(CORS_CONFIG_KEY) .as(CrossOriginConfig::create) .ifPresent(this::crossOriginConfig); - return me(); + return identity(); } /** @@ -774,7 +801,7 @@ public B webContext(String path) { path = "/" + path; } this.webContext = Optional.of(path); - return me(); + return identity(); } /** @@ -786,7 +813,7 @@ public B webContext(String path) { public B staticFile(String path) { Objects.requireNonNull(path, "path to static file must be non-null"); staticFilePath = Optional.of(path); - return me(); + return identity(); } /** @@ -798,9 +825,14 @@ public B staticFile(String path) { public B crossOriginConfig(CrossOriginConfig crossOriginConfig) { Objects.requireNonNull(crossOriginConfig, "CrossOriginConfig must be non-null"); this.crossOriginConfig = crossOriginConfig; - return me(); + return identity(); } + /** + * Returns the supplier of index views. + * + * @return index views supplier + */ protected Supplier> indexViewsSupplier() { // Only in MP can we have possibly multiple index views, one per app, from scanning classes (or the Jandex index). return () -> Collections.emptyList(); From ab0480e7a3fe1123085ed7c58756958301451f72 Mon Sep 17 00:00:00 2001 From: "tim.quinn@oracle.com" Date: Thu, 2 Dec 2021 13:31:19 -0600 Subject: [PATCH 05/12] Support new feature allowing config to define schema for type --- .../openapi/internal/OpenAPIConfigImpl.java | 83 +++++++++++-------- .../io/helidon/openapi/OpenAPIConfigTest.java | 49 +++++++++++ 2 files changed, 96 insertions(+), 36 deletions(-) diff --git a/openapi/src/main/java/io/helidon/openapi/internal/OpenAPIConfigImpl.java b/openapi/src/main/java/io/helidon/openapi/internal/OpenAPIConfigImpl.java index 9b0aa38ca6e..08aef84bd84 100644 --- a/openapi/src/main/java/io/helidon/openapi/internal/OpenAPIConfigImpl.java +++ b/openapi/src/main/java/io/helidon/openapi/internal/OpenAPIConfigImpl.java @@ -15,16 +15,14 @@ */ package io.helidon.openapi.internal; -import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; -import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.atomic.AtomicReference; import java.util.function.BiFunction; import java.util.function.Function; -import java.util.logging.Level; import java.util.logging.Logger; import java.util.regex.Pattern; @@ -36,10 +34,6 @@ * Helidon-specific implementation of the smallrye OpenApiConfig interface, * loadable from a Helidon {@link Config} object as well as individual items * settable programmatically. - *

- * Helidon SE does not support annotation scanning, so we do not need - * a way to set the scanning-related values. We just initialize them - * appropriately. */ public class OpenAPIConfigImpl implements OpenApiConfig { @@ -47,7 +41,7 @@ public class OpenAPIConfigImpl implements OpenApiConfig { private final String filter; private final Map> operationServers; private final Map> pathServers; - private Boolean scanDisable = Boolean.TRUE; + private Boolean scanDisable; private final Pattern scanPackages; private final Pattern scanClasses; private final Pattern scanExcludePackages; @@ -57,6 +51,7 @@ public class OpenAPIConfigImpl implements OpenApiConfig { private final Set scanDependenciesJars = Collections.emptySet(); private final String customSchemaRegistryClass; private final Boolean applicationPathDisable; + private final Map schemas; private OpenAPIConfigImpl(Builder builder) { modelReader = builder.modelReader; @@ -71,6 +66,7 @@ private OpenAPIConfigImpl(Builder builder) { scanExcludeClasses = builder.scanExcludeClasses; customSchemaRegistryClass = builder.customSchemaRegistryClass; applicationPathDisable = builder.applicationPathDisable; + schemas = Collections.unmodifiableMap(builder.schemas); } /** @@ -142,17 +138,6 @@ public Set scanDependenciesJars() { return scanDependenciesJars; } - /** - * Reports whether schema-reference following is enabled; note this is now always {$code true}. Provided for backward - * compatibility only. - * - * @return true - */ - @Deprecated - public boolean schemaReferencesEnable() { - return true; - } - @Override public String customSchemaRegistryClass() { return customSchemaRegistryClass; @@ -163,6 +148,11 @@ public boolean applicationPathDisable() { return applicationPathDisable; } + @Override + public Map getSchemas() { + return schemas; + } + private static Set chooseEntry(Map> map, T key) { if (map.containsKey(key)) { return map.get(key); @@ -190,6 +180,11 @@ private static Set chooseEntry(Map> map, T key) { */ public static final class Builder implements io.helidon.common.Builder { + /** + * Config key prefix for schema overrides for specified classes. + */ + public static final String SCHEMA = "schema"; + private static final Logger LOGGER = Logger.getLogger(Builder.class.getName()); private static final Pattern MATCH_EVERYTHING = Pattern.compile(".*"); @@ -201,14 +196,9 @@ public static final class Builder implements io.helidon.common.Builder CONFIG_KEYS = Arrays.asList(new String[] {MODEL_READER, FILTER, SERVERS}); - private String modelReader; private String filter; private final Map> operationServers = new HashMap<>(); @@ -222,6 +212,7 @@ public static final class Builder implements io.helidon.common.Builder schemas = new HashMap<>(); private Builder() { } @@ -244,9 +235,9 @@ public Builder config(Config config) { stringFromConfig(config, SERVERS, this::servers); listFromConfig(config, SERVERS_PATH, this::pathServers); listFromConfig(config, SERVERS_OPERATION, this::operationServers); - booleanFromConfig(config, SCHEMA_REFERENCES_ENABLE, this::schemaReferencesEnable); stringFromConfig(config, CUSTOM_SCHEMA_REGISTRY_CLASS, this::customSchemaRegistryClass); booleanFromConfig(config, APPLICATION_PATH_DISABLE, this::applicationPathDisable); + mapFromConfig(config, SCHEMA, this::schemas); return this; } @@ -346,27 +337,36 @@ public Builder addServer(String server) { } /** - * Sets whether annotation scanning should be disabled. + * Sets schemas. * - * @param value new setting for annotation scanning disabled flag + * @param schemas map of FQ class name to JSON string depicting the schema * @return updated builder */ - public Builder scanDisable(boolean value) { - scanDisable = value; + public Builder schemas(Map schemas) { + this.schemas = new HashMap<>(schemas); return this; } /** - * NO LONGER FUNCTIONAL; sets whether schema references are enabled. + * Adds a schema for a class. * - * @param value new setting for schema references enabled + * @param fullyQualifiedClassName name of the class the schema describes + * @param schema JSON text definition of the schema * @return updated builder */ - @Deprecated - public Builder schemaReferencesEnable(Boolean value) { - LOGGER.log(Level.WARNING, - String.format("OpenAPI configuration setting %s is no longer supported but was set to '%b'; " - + "it is always treated as 'true'", SCHEMA_REFERENCES_ENABLE, value)); + public Builder addSchema(String fullyQualifiedClassName, String schema) { + schemas.put(fullyQualifiedClassName, schema); + return this; + } + + /** + * Sets whether annotation scanning should be disabled. + * + * @param value new setting for annotation scanning disabled flag + * @return updated builder + */ + public Builder scanDisable(boolean value) { + scanDisable = value; return this; } @@ -408,6 +408,17 @@ private static void listFromConfig(Config config, })); } + private static void mapFromConfig(Config config, + String keyPrefix, + Function, Builder> assignment) { + AtomicReference> schemas = new AtomicReference<>(new HashMap<>()); + config.get(keyPrefix) + .detach() + .ifExists(configNode -> schemas.set(configNode.asMap().get())); + + assignment.apply(schemas.get()); + } + private static void booleanFromConfig(Config config, String key, Function assignment) { diff --git a/openapi/src/test/java/io/helidon/openapi/OpenAPIConfigTest.java b/openapi/src/test/java/io/helidon/openapi/OpenAPIConfigTest.java index de1e24c103a..5bcaf18b546 100644 --- a/openapi/src/test/java/io/helidon/openapi/OpenAPIConfigTest.java +++ b/openapi/src/test/java/io/helidon/openapi/OpenAPIConfigTest.java @@ -16,6 +16,9 @@ package io.helidon.openapi; import java.nio.file.Paths; +import java.util.List; +import java.util.Map; +import java.util.StringJoiner; import io.helidon.config.Config; import io.helidon.config.ConfigSources; @@ -26,7 +29,9 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.stringContainsInOrder; /** * @@ -35,6 +40,36 @@ public class OpenAPIConfigTest { private final static String TEST_CONFIG_DIR = "src/test/resources"; + private static final List SCHEMA_OVERRIDE_CONTENTS = List.of( + "\"name\": \"EpochMillis\"", + "\"type\": \"number\",", + "\"format\": \"int64\",", + "\"description\": \"Milliseconds since January 1, 1970, 00:00:00 GMT\""); + + private static final Map SCHEMA_OVERRIDE_VALUES = Map.of( + "name", "EpochMillis", + "type", "number", + "format", "int64", + "description", "Milliseconds since January 1, 1970, 00:00:00 GMT"); + + private static final String SCHEMA_OVERRIDE_JSON = prepareSchemaOverrideJSON(); + + private static final String SCHEMA_OVERRIDE_CONFIG_FQCN = "java.util.Date"; + + private static final Map SCHEMA_OVERRIDE_CONFIG = Map.of( + OpenAPISupport.Builder.CONFIG_KEY + + "." + + OpenAPIConfigImpl.Builder.SCHEMA + + "." + + SCHEMA_OVERRIDE_CONFIG_FQCN, + SCHEMA_OVERRIDE_JSON); + + private static String prepareSchemaOverrideJSON() { + StringJoiner sj = new StringJoiner(",\n", "{\n", "\n}"); + SCHEMA_OVERRIDE_VALUES.forEach((key, value) -> sj.add("\"" + key + "\": \"" + value + "\"")); + return sj.toString(); + } + public OpenAPIConfigTest() { } @@ -69,4 +104,18 @@ public void checkUnconfiguredValues() { assertThat("scan disable mismatch", openAPIConfig.scanDisable(), is(true)); } + + @Test + void checkSchemaConfig() { + Config config = Config.just(ConfigSources.file(Paths.get(TEST_CONFIG_DIR, "simple.properties").toString()), + ConfigSources.create(SCHEMA_OVERRIDE_CONFIG)); + OpenApiConfig openAPIConfig = OpenAPIConfigImpl.builder() + .config(config.get(OpenAPISupport.Builder.CONFIG_KEY)) + .build(); + + assertThat("Schema override", openAPIConfig.getSchemas(), hasKey(SCHEMA_OVERRIDE_CONFIG_FQCN)); + assertThat("Schema override value for " + SCHEMA_OVERRIDE_CONFIG_FQCN, + openAPIConfig.getSchemas().get(SCHEMA_OVERRIDE_CONFIG_FQCN), + is(SCHEMA_OVERRIDE_JSON)); + } } From db4f877ffb1e28b2edc0b45940a4f9f525f7c15c Mon Sep 17 00:00:00 2001 From: "tim.quinn@oracle.com" Date: Thu, 2 Dec 2021 13:31:55 -0600 Subject: [PATCH 06/12] Add/revise tests --- .../java/io/helidon/openapi/ParserTest.java | 2 -- .../io/helidon/openapi/SerializerTest.java | 2 -- .../openapi/ServerModelReaderTest.java | 4 +-- .../java/io/helidon/openapi/ServerTest.java | 17 +++++++++-- .../java/io/helidon/openapi/TestCors.java | 2 -- .../java/io/helidon/openapi/TestUtil.java | 30 ++++++++++++++++++- 6 files changed, 44 insertions(+), 13 deletions(-) diff --git a/openapi/src/test/java/io/helidon/openapi/ParserTest.java b/openapi/src/test/java/io/helidon/openapi/ParserTest.java index 1fbffa2f306..c0238e293d5 100644 --- a/openapi/src/test/java/io/helidon/openapi/ParserTest.java +++ b/openapi/src/test/java/io/helidon/openapi/ParserTest.java @@ -23,7 +23,6 @@ import org.eclipse.microprofile.openapi.models.OpenAPI; import org.eclipse.microprofile.openapi.models.Paths; import org.eclipse.microprofile.openapi.models.parameters.Parameter; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import static org.hamcrest.CoreMatchers.equalTo; @@ -31,7 +30,6 @@ import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; -@Disabled("3.0.0-JAKARTA") class ParserTest { private static SnakeYAMLParserHelper helper = OpenAPISupport.helper(); diff --git a/openapi/src/test/java/io/helidon/openapi/SerializerTest.java b/openapi/src/test/java/io/helidon/openapi/SerializerTest.java index 56e88c9b8c3..515e2825453 100644 --- a/openapi/src/test/java/io/helidon/openapi/SerializerTest.java +++ b/openapi/src/test/java/io/helidon/openapi/SerializerTest.java @@ -33,7 +33,6 @@ import jakarta.json.JsonValue; import org.eclipse.microprofile.openapi.models.OpenAPI; import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import static org.hamcrest.CoreMatchers.equalTo; @@ -43,7 +42,6 @@ import static org.hamcrest.CoreMatchers.startsWith; import static org.hamcrest.MatcherAssert.assertThat; -@Disabled("3.0.0-JAKARTA") class SerializerTest { private static SnakeYAMLParserHelper helper; diff --git a/openapi/src/test/java/io/helidon/openapi/ServerModelReaderTest.java b/openapi/src/test/java/io/helidon/openapi/ServerModelReaderTest.java index ebc36ae724f..8618636b646 100644 --- a/openapi/src/test/java/io/helidon/openapi/ServerModelReaderTest.java +++ b/openapi/src/test/java/io/helidon/openapi/ServerModelReaderTest.java @@ -29,7 +29,6 @@ import jakarta.json.JsonValue; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -40,12 +39,11 @@ * Makes sure that the app-supplied model reader participates in constructing * the OpenAPI model. */ -@Disabled("3.0.0-JAKARTA") public class ServerModelReaderTest { private static final String SIMPLE_PROPS_PATH = "/openapi"; - private static final OpenAPISupport.Builder OPENAPI_SUPPORT_BUILDER = + private static final OpenAPISupport.Builder OPENAPI_SUPPORT_BUILDER = OpenAPISupport.builderSE() .config(Config.create(ConfigSources.classpath("simple.properties")).get(OpenAPISupport.Builder.CONFIG_KEY)); diff --git a/openapi/src/test/java/io/helidon/openapi/ServerTest.java b/openapi/src/test/java/io/helidon/openapi/ServerTest.java index 9e5e3107551..3833f2d447d 100644 --- a/openapi/src/test/java/io/helidon/openapi/ServerTest.java +++ b/openapi/src/test/java/io/helidon/openapi/ServerTest.java @@ -27,7 +27,6 @@ import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -38,7 +37,6 @@ * Starts a server with the default OpenAPI endpoint to test a static OpenAPI * document file in various ways. */ -@Disabled("3.0.0-JAKARTA") public class ServerTest { private static WebServer greetingWebServer; @@ -149,13 +147,26 @@ public void testGreetingAsConfig() throws Exception { * response */ @Test - public void checkExplicitResponseMediaType() throws Exception { + public void checkExplicitResponseMediaTypeViaHeaders() throws Exception { connectAndConsumePayload(MediaType.APPLICATION_OPENAPI_YAML); connectAndConsumePayload(MediaType.APPLICATION_YAML); connectAndConsumePayload(MediaType.APPLICATION_OPENAPI_JSON); connectAndConsumePayload(MediaType.APPLICATION_JSON); } + @Test + void checkExplicitResponseMediaTypeViaQueryParameter() throws Exception { + TestUtil.connectAndConsumePayload(greetingWebServer.port(), + GREETING_PATH, + "format=JSON", + MediaType.APPLICATION_JSON); + + TestUtil.connectAndConsumePayload(greetingWebServer.port(), + GREETING_PATH, + "format=YAML", + MediaType.APPLICATION_OPENAPI_YAML); + } + /** * Makes sure that the response is correct if the request specified no * explicit Accept. diff --git a/openapi/src/test/java/io/helidon/openapi/TestCors.java b/openapi/src/test/java/io/helidon/openapi/TestCors.java index d2f4ba4a787..6ed5292ff04 100644 --- a/openapi/src/test/java/io/helidon/openapi/TestCors.java +++ b/openapi/src/test/java/io/helidon/openapi/TestCors.java @@ -23,14 +23,12 @@ import io.helidon.webserver.WebServer; import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import static io.helidon.openapi.ServerTest.GREETING_OPENAPI_SUPPORT_BUILDER; import static io.helidon.openapi.ServerTest.TIME_OPENAPI_SUPPORT_BUILDER; import static org.junit.jupiter.api.Assertions.assertEquals; -@Disabled("3.0.0-JAKARTA") public class TestCors { private static WebServer greetingWebServer; diff --git a/openapi/src/test/java/io/helidon/openapi/TestUtil.java b/openapi/src/test/java/io/helidon/openapi/TestUtil.java index 7acca5b0fe2..f307c57bdb5 100644 --- a/openapi/src/test/java/io/helidon/openapi/TestUtil.java +++ b/openapi/src/test/java/io/helidon/openapi/TestUtil.java @@ -116,6 +116,22 @@ public static MediaType connectAndConsumePayload( return actualMT; } + static MediaType connectAndConsumePayload( + int port, String path, String queryParameter, MediaType expectedMediaType) throws Exception { + HttpURLConnection cnx = getURLConnection(port, "GET", path, queryParameter); + MediaType actualMT = validateResponseMediaType(cnx, expectedMediaType); + if (actualMT.test(MediaType.APPLICATION_OPENAPI_YAML) || actualMT.test(MediaType.APPLICATION_YAML)) { + yamlFromResponse(cnx); + } else if (actualMT.test(MediaType.APPLICATION_OPENAPI_JSON) + || actualMT.test(MediaType.APPLICATION_JSON)) { + jsonFromResponse(cnx); + } else { + throw new IllegalArgumentException( + "Expected either JSON or YAML response but received " + actualMT.toString()); + } + return actualMT; + } + /** * Returns the {@code MediaType} instance conforming to the HTTP response * content type. @@ -293,6 +309,18 @@ public static HttpURLConnection getURLConnection( return conn; } + static HttpURLConnection getURLConnection( + int port, + String method, + String path, + String queryParameter) throws Exception { + URL url = new URL("http://localhost:" + port + path + "?" + queryParameter); + HttpURLConnection conn = (HttpURLConnection) url.openConnection(); + conn.setRequestMethod(method); + System.out.println("Connectiong: " + method + " " + url); + return conn; + } + /** * Stop the web server. * @@ -322,7 +350,7 @@ public static void stopServer(WebServer server) throws */ public static WebServer startServer( int port, - OpenAPISupport.Builder... openAPIBuilders) throws + OpenAPISupport.Builder... openAPIBuilders) throws InterruptedException, ExecutionException, TimeoutException { WebServer result = WebServer.builder(Routing.builder() .register(openAPIBuilders) From ac965b95b0198a1f7ccea7b749212129d1596d47 Mon Sep 17 00:00:00 2001 From: "tim.quinn@oracle.com" Date: Thu, 2 Dec 2021 13:32:46 -0600 Subject: [PATCH 07/12] TCK depends on RestAssured which relies on javax JAX-B; add relevant dependencies explicitly --- microprofile/tests/tck/tck-openapi/pom.xml | 23 +++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/microprofile/tests/tck/tck-openapi/pom.xml b/microprofile/tests/tck/tck-openapi/pom.xml index cbde49cbea2..d68b2d3228e 100644 --- a/microprofile/tests/tck/tck-openapi/pom.xml +++ b/microprofile/tests/tck/tck-openapi/pom.xml @@ -30,11 +30,6 @@ tck-openapi Helidon Microprofile Tests TCK OpenAPI - - - true - - io.helidon.microprofile.tests @@ -72,6 +67,24 @@ jakarta.activation-api test + + + + jakarta.xml.bind + jakarta.xml.bind-api + 2.3.2 + test + + + + + org.glassfish.jaxb + jaxb-runtime + 2.3.2 + test + From 0709f628cdd8d93fe74c70ab0532d09facb3435e Mon Sep 17 00:00:00 2001 From: "tim.quinn@oracle.com" Date: Thu, 2 Dec 2021 13:33:50 -0600 Subject: [PATCH 08/12] Re-enable tests disabled during initial 3.0 merge --- .../test/java/io/helidon/examples/quickstart/mp/MainTest.java | 2 -- .../test/java/io/helidon/examples/quickstart/mp/MainTest.java | 2 -- 2 files changed, 4 deletions(-) diff --git a/examples/quickstarts/helidon-quickstart-mp/src/test/java/io/helidon/examples/quickstart/mp/MainTest.java b/examples/quickstarts/helidon-quickstart-mp/src/test/java/io/helidon/examples/quickstart/mp/MainTest.java index f8feae5efa5..00bdcab6d12 100644 --- a/examples/quickstarts/helidon-quickstart-mp/src/test/java/io/helidon/examples/quickstart/mp/MainTest.java +++ b/examples/quickstarts/helidon-quickstart-mp/src/test/java/io/helidon/examples/quickstart/mp/MainTest.java @@ -32,8 +32,6 @@ import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; -@Disabled("3.0.0-JAKARTA") // OpenAPI: Caused by: java.lang.NoSuchMethodError: - // 'java.util.List org.jboss.jandex.ClassInfo.unsortedFields()' class MainTest { private static Server server; diff --git a/examples/quickstarts/helidon-standalone-quickstart-mp/src/test/java/io/helidon/examples/quickstart/mp/MainTest.java b/examples/quickstarts/helidon-standalone-quickstart-mp/src/test/java/io/helidon/examples/quickstart/mp/MainTest.java index f8feae5efa5..00bdcab6d12 100644 --- a/examples/quickstarts/helidon-standalone-quickstart-mp/src/test/java/io/helidon/examples/quickstart/mp/MainTest.java +++ b/examples/quickstarts/helidon-standalone-quickstart-mp/src/test/java/io/helidon/examples/quickstart/mp/MainTest.java @@ -32,8 +32,6 @@ import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; -@Disabled("3.0.0-JAKARTA") // OpenAPI: Caused by: java.lang.NoSuchMethodError: - // 'java.util.List org.jboss.jandex.ClassInfo.unsortedFields()' class MainTest { private static Server server; From cb216b8aaf87f3ff6367986d455e0df0dfe11263 Mon Sep 17 00:00:00 2001 From: "tim.quinn@oracle.com" Date: Thu, 2 Dec 2021 13:58:25 -0600 Subject: [PATCH 09/12] Minor doc updats: new config setting and new (optional) feature allowing 'format' query parameter on GET to /openapi to control format --- docs/mp/openapi/01_openapi.adoc | 6 +++--- docs/se/openapi/01_openapi.adoc | 5 +++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/docs/mp/openapi/01_openapi.adoc b/docs/mp/openapi/01_openapi.adoc index f2b6671ee06..cf6efb8eaa7 100644 --- a/docs/mp/openapi/01_openapi.adoc +++ b/docs/mp/openapi/01_openapi.adoc @@ -178,7 +178,7 @@ link:{mp-openapi-spec}#configuration[configuration section] of the MicroProfile OpenAPI spec. == Accessing the OpenAPI document -Now your Helidon MP application will automatially respond to an additional endpoint -- +Now your Helidon MP application will automatically respond to an additional endpoint -- `/openapi` -- and it will return the OpenAPI document describing the endpoints in your application. @@ -187,5 +187,5 @@ There is not yet an adopted IANA YAML media type, but a proposed one specificall for OpenAPI documents that has some support is `application/vnd.oai.openapi`. That is what Helidon returns, by default. -A client can specify `Accept:` as either `application/vnd.oai.openapi+json` or `application/json` -to request JSON. +In addition a client can specify the HTTP header `Accept:` as either `application/vnd.oai.openapi+json` or `application/json` +to request JSON. Alternatively, the client can pass the query parameter `format` as either `JSON` or `YAML` to receive `application/json` or `application/vnd.oai.openapi` (YAML) output, respectively. diff --git a/docs/se/openapi/01_openapi.adoc b/docs/se/openapi/01_openapi.adoc index 3214d5a837c..451f1d1f3be 100644 --- a/docs/se/openapi/01_openapi.adoc +++ b/docs/se/openapi/01_openapi.adoc @@ -152,6 +152,7 @@ those described in the MicroProfile OpenAPI spec, two of which were mentioned ab servers for given paths |`openapi.servers.operation` |Prefix for config properties specifying alternative servers for given operations +|`openapi.schema` |Prefix for config properties defining the schema for a class |=== For more information on what these settings do consult the MicroProfile OpenAPI spec. @@ -182,5 +183,5 @@ There is not yet an adopted IANA YAML media type, but a proposed one specificall for OpenAPI documents that has some support is `application/vnd.oai.openapi`. That is what Helidon returns, by default. -In addition a client can specify `Accept:` as either `application/vnd.oai.openapi+json` or `application/json` -to request JSON. +In addition a client can specify the HTTP header `Accept:` as either `application/vnd.oai.openapi+json` or `application/json` +to request JSON. Alternatively, the client can pass the query parameter `format` as either `JSON` or `YAML` to receive `application/json` or `application/vnd.oai.openapi` (YAML) output, respectively. From bac0d5614fd2028941745f6ac7080352d506b9fd Mon Sep 17 00:00:00 2001 From: "tim.quinn@oracle.com" Date: Thu, 2 Dec 2021 15:40:56 -0600 Subject: [PATCH 10/12] Change to jandex version got lost during a merge --- dependencies/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dependencies/pom.xml b/dependencies/pom.xml index 56dfdf77578..2f13453985e 100644 --- a/dependencies/pom.xml +++ b/dependencies/pom.xml @@ -81,7 +81,7 @@ 3.0.0 2.0.0 3.0.1 - 2.3.1.Final + 2.4.1.Final 3.0.2 3.0.2 1.2.5.Final From ad50acec3dc252795b4e5423f5e8f2c1de971f01 Mon Sep 17 00:00:00 2001 From: "tim.quinn@oracle.com" Date: Mon, 6 Dec 2021 14:15:33 -0600 Subject: [PATCH 11/12] Remove System.out from test --- openapi/src/test/java/io/helidon/openapi/TestUtil.java | 1 - 1 file changed, 1 deletion(-) diff --git a/openapi/src/test/java/io/helidon/openapi/TestUtil.java b/openapi/src/test/java/io/helidon/openapi/TestUtil.java index f307c57bdb5..00f46d057a1 100644 --- a/openapi/src/test/java/io/helidon/openapi/TestUtil.java +++ b/openapi/src/test/java/io/helidon/openapi/TestUtil.java @@ -317,7 +317,6 @@ static HttpURLConnection getURLConnection( URL url = new URL("http://localhost:" + port + path + "?" + queryParameter); HttpURLConnection conn = (HttpURLConnection) url.openConnection(); conn.setRequestMethod(method); - System.out.println("Connectiong: " + method + " " + url); return conn; } From 58f65e8fb205cf0b69501085e7f570c4c96d132d Mon Sep 17 00:00:00 2001 From: "tim.quinn@oracle.com" Date: Tue, 7 Dec 2021 10:50:30 -0600 Subject: [PATCH 12/12] Forward-port of 2.x additionalProperties handling fix to 3.0 --- .../openapi/ExpandedTypeDescription.java | 60 +++++++-- .../java/io/helidon/openapi/Serializer.java | 21 ++- .../openapi/TestAdditionalProperties.java | 123 ++++++++++++++++++ .../test/resources/withBooleanAddlProps.yml | 45 +++++++ .../test/resources/withSchemaAddlProps.yml | 51 ++++++++ 5 files changed, 287 insertions(+), 13 deletions(-) create mode 100644 openapi/src/test/java/io/helidon/openapi/TestAdditionalProperties.java create mode 100644 openapi/src/test/resources/withBooleanAddlProps.yml create mode 100644 openapi/src/test/resources/withSchemaAddlProps.yml diff --git a/openapi/src/main/java/io/helidon/openapi/ExpandedTypeDescription.java b/openapi/src/main/java/io/helidon/openapi/ExpandedTypeDescription.java index eb43aed5f8b..582c295851d 100644 --- a/openapi/src/main/java/io/helidon/openapi/ExpandedTypeDescription.java +++ b/openapi/src/main/java/io/helidon/openapi/ExpandedTypeDescription.java @@ -33,13 +33,15 @@ import org.yaml.snakeyaml.introspector.PropertyUtils; import org.yaml.snakeyaml.nodes.Node; import org.yaml.snakeyaml.nodes.ScalarNode; +import org.yaml.snakeyaml.nodes.Tag; /** * Extension of {@link TypeDescription} that handles: *

    *
  • nested enums,
    • - *
  • extensible types, and
    • - *
  • references.
    • + *
  • extensible types,
    • + *
  • references, and
    • + *
  • additional properties (which can be either Boolean or Schema).
    • *
*

* The OpenAPI document format uses lower-case enum names and values, while the SmallRye @@ -59,6 +61,14 @@ * description simplifies defining the {@code $ref} property to those types that support it. *

*

+ * In schemas, the {@code additionalProperties} value can be either a boolean or a schema. The MicroProfile + * {@link org.eclipse.microprofile.openapi.models.media.Schema} type exposes {@code getAdditionalPropertiesBoolean}, + * {@code setAdditionalPropertiesBoolean}, {@code getAdditionalPropertiesSchema}, and {@code setAdditionalPropertiesSchema} + * methods. We do not know until runtime and the value is available for each {@code additionalProperties} instance which + * type (Boolean or Schema) to use, so we cannot just prepare a smart SnakeYAML {@code Property} implementation. Instead + * we augment the schema-specific {@code TypeDescription} so it knows how to decide, at runtime, what to do. + *

+ *

* We use this expanded version of {@code TypeDescription} with the generated SnakeYAMLParserHelper class. *

*/ @@ -66,7 +76,7 @@ class ExpandedTypeDescription extends TypeDescription { private static final String EXTENSION_PROPERTY_PREFIX = "x-"; - private static final PropertyUtils PROPERTY_UTILS = new PropertyUtils(); + static final PropertyUtils PROPERTY_UTILS = new PropertyUtils(); private final Class impl; @@ -230,10 +240,11 @@ private static boolean isRef(String name) { * Specific type description for {@code Schema}. *

* The {@code Schema} node allows the {@code additionalProperties} subnode to be either - * {@code Boolean} or another {@code Schema}. This type description provides a customized - * property description for {@code additionalProperties} that infers which variant a - * specific node in the document actually uses and then processes it accordingly. + * {@code Boolean} or another {@code Schema}, and the {@code Schema} class exposes getters and setters for + * {@code additionalPropertiesBoolean}, and {@code additionalPropertiesSchema}. + * This type description customizes the handling of {@code additionalProperties} to account for all that. *

+ * @see io.helidon.openapi.Serializer (specifically doRepresentJavaBeanProperty) for output handling for additionalProperties */ static final class SchemaTypeDescription extends ExpandedTypeDescription { @@ -261,23 +272,52 @@ public Object get(Object object) { }; private static PropertyDescriptor preparePropertyDescriptor() { + /* + * The PropertyDescriptor here is just a placeholder. We will not know until we are mapping a node in the model + * whether the additionalProperties is a boolean or a Schema. That is handled explicitly in setProperty below. + */ try { return new PropertyDescriptor("additionalProperties", - Schema.class.getMethod("getAdditionalPropertiesSchema"), - Schema.class.getMethod("setAdditionalPropertiesSchema", Schema.class)); + Schema.class.getMethod("getAdditionalPropertiesSchema"), + Schema.class.getMethod("setAdditionalPropertiesSchema", Schema.class)); } catch (IntrospectionException | NoSuchMethodException e) { throw new RuntimeException(e); } } - private SchemaTypeDescription(Class clazz, Class impl) { - super(clazz, impl); + @Override + public boolean setupPropertyType(String key, Node valueNode) { + if (key.equals("additionalProperties")) { + valueNode.setType(valueNode.getTag().equals(Tag.BOOL) ? Boolean.class : Schema.class); + return true; + } + return super.setupPropertyType(key, valueNode); } @Override public Property getProperty(String name) { return name.equals("additionalProperties") ? ADDL_PROPS_PROPERTY : super.getProperty(name); } + + @Override + public boolean setProperty(Object targetBean, String propertyName, Object value) throws Exception { + if (!(targetBean instanceof Schema schema) || !propertyName.equals("additionalProperties")) { + return super.setProperty(targetBean, propertyName, value); + } + if (value instanceof Boolean) { + schema.setAdditionalPropertiesBoolean((Boolean) value); + } else if (value instanceof Schema) { + schema.setAdditionalPropertiesSchema((Schema) value); + } else { + throw new IllegalArgumentException("Expected additionalProperties as Boolean or Schema but was " + + value.getClass().getName()); + } + return true; + } + + private SchemaTypeDescription(Class clazz, Class impl) { + super(clazz, impl); + } } /** diff --git a/openapi/src/main/java/io/helidon/openapi/Serializer.java b/openapi/src/main/java/io/helidon/openapi/Serializer.java index 59e23cfcb97..174b80bcc1a 100644 --- a/openapi/src/main/java/io/helidon/openapi/Serializer.java +++ b/openapi/src/main/java/io/helidon/openapi/Serializer.java @@ -31,6 +31,7 @@ import org.eclipse.microprofile.openapi.models.Extensible; import org.eclipse.microprofile.openapi.models.OpenAPI; import org.eclipse.microprofile.openapi.models.Reference; +import org.eclipse.microprofile.openapi.models.media.Schema; import org.eclipse.microprofile.openapi.models.parameters.Parameter; import org.yaml.snakeyaml.DumperOptions; import org.yaml.snakeyaml.Yaml; @@ -155,9 +156,23 @@ protected NodeTuple representJavaBeanProperty(Object javaBean, Property property private NodeTuple doRepresentJavaBeanProperty(Object javaBean, Property property, Object propertyValue, Tag customTag) { NodeTuple defaultTuple = super.representJavaBeanProperty(javaBean, property, propertyValue, customTag); - return (javaBean instanceof Reference) && property.getName().equals("ref") - ? new NodeTuple(representData("$ref"), defaultTuple.getValueNode()) - : defaultTuple; + if ((javaBean instanceof Reference) && property.getName().equals("ref")) { + return new NodeTuple(representData("$ref"), defaultTuple.getValueNode()); + } + if (javaBean instanceof Schema) { + /* + * At most one of additionalPropertiesBoolean and additionalPropertiesSchema will return a non-null value. + * Whichever one does (if either), replace the name with "additionalProperties" for output. Skip whatever is + * returned from the deprecated additionalProperties method itself. + */ + String propertyName = property.getName(); + if (propertyName.equals("additionalProperties")) { + return null; + } else if (propertyName.startsWith("additionalProperties")) { + return new NodeTuple(representData("additionalProperties"), defaultTuple.getValueNode()); + } + } + return defaultTuple; } private Object adjustPropertyValue(Object propertyValue) { diff --git a/openapi/src/test/java/io/helidon/openapi/TestAdditionalProperties.java b/openapi/src/test/java/io/helidon/openapi/TestAdditionalProperties.java new file mode 100644 index 00000000000..6153a4f968b --- /dev/null +++ b/openapi/src/test/java/io/helidon/openapi/TestAdditionalProperties.java @@ -0,0 +1,123 @@ +/* + * Copyright (c) 2021 Oracle and/or its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.helidon.openapi; + + +import java.io.IOException; +import java.io.StringWriter; +import java.util.Map; + +import io.smallrye.openapi.runtime.io.Format; +import org.eclipse.microprofile.openapi.models.OpenAPI; +import org.eclipse.microprofile.openapi.models.media.Schema; +import org.junit.jupiter.api.Test; +import org.yaml.snakeyaml.Yaml; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.hasKey; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.nullValue; + +class TestAdditionalProperties { + + private static SnakeYAMLParserHelper helper = OpenAPISupport.helper(); + + + @Test + void checkParsingBooleanAdditionalProperties() throws IOException { + OpenAPI openAPI = ParserTest.parse(helper, "/withBooleanAddlProps.yml", OpenAPISupport.OpenAPIMediaType.YAML); + Schema itemSchema = openAPI.getComponents().getSchemas().get("item"); + + Schema additionalPropertiesSchema = itemSchema.getAdditionalPropertiesSchema(); + Boolean additionalPropertiesBoolean = itemSchema.getAdditionalPropertiesBoolean(); + + assertThat("Additional properties as schema", additionalPropertiesSchema, is(nullValue())); + assertThat("Additional properties as boolean", additionalPropertiesBoolean, is(notNullValue())); + assertThat("Additional properties value", additionalPropertiesBoolean.booleanValue(), is(false)); + } + + @Test + void checkParsingSchemaAdditionalProperties() throws IOException { + OpenAPI openAPI = ParserTest.parse(helper, "/withSchemaAddlProps.yml", OpenAPISupport.OpenAPIMediaType.YAML); + Schema itemSchema = openAPI.getComponents().getSchemas().get("item"); + + Schema additionalPropertiesSchema = itemSchema.getAdditionalPropertiesSchema(); + Boolean additionalPropertiesBoolean = itemSchema.getAdditionalPropertiesBoolean(); + + assertThat("Additional properties as boolean", additionalPropertiesBoolean, is(nullValue())); + assertThat("Additional properties as schema", additionalPropertiesSchema, is(notNullValue())); + + Map additionalProperties = additionalPropertiesSchema.getProperties(); + assertThat("Additional property 'code'", additionalProperties, hasKey("code")); + assertThat("Additional property 'text'", additionalProperties, hasKey("text")); + } + + @Test + void checkWritingSchemaAdditionalProperties() throws IOException { + OpenAPI openAPI = ParserTest.parse(helper, "/withSchemaAddlProps.yml", OpenAPISupport.OpenAPIMediaType.YAML); + String document = formatModel(openAPI); + + /* + * Expected output (although the + additionalProperties: + type: object + properties: + code: + type: integer + text: + type: string + */ + Yaml yaml = new Yaml(); + Map model = yaml.load(document); + Map item = asMap(model, "components", "schemas", "item"); + + Object additionalProperties = item.get("additionalProperties"); + + assertThat("Additional properties node type", additionalProperties, is(instanceOf(Map.class))); + + } + + private static Map asMap(Map map, String... keys) { + Map m = map; + for (String key : keys) { + m = (Map) m.get(key); + } + return m; + } + + @Test + void checkWritingBooleanAdditionalProperties() throws IOException { + OpenAPI openAPI = ParserTest.parse(helper, "/withBooleanAddlProps.yml", OpenAPISupport.OpenAPIMediaType.YAML); + String document = formatModel(openAPI); + + /* + * Expected output: additionalProperties: false + */ + + assertThat("Formatted OpenAPI document matches expected pattern", + document, containsString("additionalProperties: false")); + } + + private String formatModel(OpenAPI model) { + StringWriter sw = new StringWriter(); + Map, ExpandedTypeDescription> implsToTypes = OpenAPISupport.buildImplsToTypes(helper); + Serializer.serialize(helper.types(), implsToTypes, model, Format.YAML, sw); + return sw.toString(); + } +} diff --git a/openapi/src/test/resources/withBooleanAddlProps.yml b/openapi/src/test/resources/withBooleanAddlProps.yml new file mode 100644 index 00000000000..bb47d220b4f --- /dev/null +++ b/openapi/src/test/resources/withBooleanAddlProps.yml @@ -0,0 +1,45 @@ +# +# Copyright (c) 2021 Oracle and/or its affiliates. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +openapi: 3.1.0 + +info: + title: Some service + version: 0.1.0 + +components: + schemas: + item: + type: object + additionalProperties: false + properties: + id: + type: string + title: + type: string + +paths: + /items: + get: + responses: + '200': + description: Get items + content: + application/json: + schema: + type: array + items: + $ref: '#/components/schemas/item' diff --git a/openapi/src/test/resources/withSchemaAddlProps.yml b/openapi/src/test/resources/withSchemaAddlProps.yml new file mode 100644 index 00000000000..232d598c87e --- /dev/null +++ b/openapi/src/test/resources/withSchemaAddlProps.yml @@ -0,0 +1,51 @@ +# +# Copyright (c) 2021 Oracle and/or its affiliates. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +openapi: 3.1.0 + +info: + title: Some service + version: 0.1.0 + +components: + schemas: + item: + type: object + additionalProperties: + type: object + properties: + code: + type: integer + text: + type: string + properties: + id: + type: string + title: + type: string + +paths: + /items: + get: + responses: + '200': + description: Get items + content: + application/json: + schema: + type: array + items: + $ref: '#/components/schemas/item'