Skip to content

Commit

Permalink
Add better deprecated property handling
Browse files Browse the repository at this point in the history
  • Loading branch information
mtdowling committed Apr 9, 2020
1 parent 6acba10 commit 19952b8
Show file tree
Hide file tree
Showing 5 changed files with 123 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -69,21 +69,13 @@ public byte getOrder() {

@Override
public ObjectNode updateNode(Context<? extends Trait> context, OpenApi openapi, ObjectNode node) {
if (!isDisabled(context)) {
if (!context.getConfig().getExtensions(ApiGatewayConfig.class).getDisableCloudFormationSubstitution()) {
return node.accept(new CloudFormationFnSubInjector(PATHS)).expectObjectNode();
}

return node;
}

private boolean isDisabled(Context<?> context) {
// Support the old name for backward compatibility.
return context.getConfig().getExtensions(ApiGatewayConfig.class).getDisableCloudFormationSubstitution()
|| context.getConfig()
.getExtensions()
.getBooleanMemberOrDefault("apigateway.disableCloudFormationSubstitution");
}

private static class CloudFormationFnSubInjector extends NodeVisitor.Default<Node> {
private final Deque<String> stack = new ArrayDeque<>();
private final List<String[]> paths;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -808,13 +808,13 @@ public Builder removeExtension(String key) {
}

/**
* Applies a "disableX" key to a schema builder.
* Disables a specific JSON schema property by name.
*
* @param disableKey Disable key to apply (e.g., "disablePropertyNames").
* @param propertyName Property name to remove (e.g., "propertyNames").
* @return Returns the builder.
*/
public Builder disableProperty(String disableKey) {
switch (disableKey) {
public Builder disableProperty(String propertyName) {
switch (propertyName) {
case "const":
return this.constValue(null);
case "default":
Expand Down Expand Up @@ -884,7 +884,7 @@ public Builder disableProperty(String disableKey) {
case "examples":
return this.examples(null);
default:
LOGGER.warning("Unknown JSON Schema config 'disable' property: " + disableKey);
LOGGER.warning("Unknown JSON Schema config 'disable' property: " + propertyName);
return this;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,20 @@
package software.amazon.smithy.openapi;

import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.logging.Logger;
import software.amazon.smithy.jsonschema.JsonSchemaConfig;
import software.amazon.smithy.model.node.Node;
import software.amazon.smithy.model.node.NodeMapper;
import software.amazon.smithy.model.node.ObjectNode;
import software.amazon.smithy.model.shapes.ShapeId;
import software.amazon.smithy.openapi.fromsmithy.OpenApiProtocol;
import software.amazon.smithy.openapi.fromsmithy.Smithy2OpenApiExtension;
import software.amazon.smithy.utils.ListUtils;
import software.amazon.smithy.utils.StringUtils;

/**
* "openapi" smithy-build plugin configuration settings.
Expand All @@ -46,6 +51,26 @@ public enum HttpPrefixHeadersStrategy {
/** The JSON pointer to where OpenAPI schema components should be written. */
private static final String SCHEMA_COMPONENTS_POINTER = "#/components/schemas";

private static final Logger LOGGER = Logger.getLogger(OpenApiConfig.class.getName());
private static final Map<String, String> DEPRECATED_PROPERTY_RENAMES = new HashMap<>();

static {
DEPRECATED_PROPERTY_RENAMES.put("openapi.tags", "tags");
DEPRECATED_PROPERTY_RENAMES.put("openapi.supportedTags", "supportedTags");
DEPRECATED_PROPERTY_RENAMES.put("openapi.defaultBlobFormat", "defaultBlobFormat");
DEPRECATED_PROPERTY_RENAMES.put("openapi.keepUnusedComponents", "keepUnusedComponents");
DEPRECATED_PROPERTY_RENAMES.put("openapi.aws.jsonContentType", "jsonContentType");
DEPRECATED_PROPERTY_RENAMES.put("openapi.forbidGreedyLabels", "forbidGreedyLabels");
DEPRECATED_PROPERTY_RENAMES.put("openapi.onHttpPrefixHeaders", "onHttpPrefixHeaders");
// There was a typo in the docs, so might as well fix that here.
DEPRECATED_PROPERTY_RENAMES.put("openapi.ignoreUnsupportedTrait", "ignoreUnsupportedTraits");
DEPRECATED_PROPERTY_RENAMES.put("openapi.ignoreUnsupportedTraits", "ignoreUnsupportedTraits");
DEPRECATED_PROPERTY_RENAMES.put("openapi.substitutions", "substitutions");
// Cheating a little here, but oh well.
DEPRECATED_PROPERTY_RENAMES.put("apigateway.disableCloudFormationSubstitution",
"disableCloudFormationSubstitution");
}

private ShapeId service;
private ShapeId protocol;
private boolean tags;
Expand Down Expand Up @@ -284,4 +309,49 @@ public List<String> getExternalDocs() {
public void setExternalDocs(List<String> externalDocs) {
this.externalDocs = Objects.requireNonNull(externalDocs);
}

/**
* Creates an OpenApiConfig from a Node value.
*
* <p>This method first converts deprecated keys into their new locations and
* formats, and then uses the {@link NodeMapper} on the converted input
* object. Note that this class can be deserialized using a NodeMapper too
* since the NodeMapper will look for a static, public, fromNode method.
*
* @param input Input to deserialize.
* @return Returns the deserialized OpenApiConfig.
*/
public static OpenApiConfig fromNode(Node input) {
NodeMapper mapper = new NodeMapper();
ObjectNode node = fixDeprecatedKeys(input.expectObjectNode());
OpenApiConfig config = new OpenApiConfig();
return mapper.deserializeInto(node, config);
}

private static ObjectNode fixDeprecatedKeys(ObjectNode node) {
ObjectNode mapped = node;

// Remove deprecated "openapi." prefixes from configuration settings.
for (Map.Entry<String, Node> entry : mapped.getStringMap().entrySet()) {
if (DEPRECATED_PROPERTY_RENAMES.containsKey(entry.getKey())) {
// Fixes specific renamed keys.
String rename = DEPRECATED_PROPERTY_RENAMES.get(entry.getKey());
LOGGER.warning("Deprecated `openapi` configuration setting found: " + entry.getKey()
+ ". Use " + rename + " instead");
mapped = mapped.withMember(rename, entry.getValue());
mapped = mapped.withoutMember(entry.getKey());
} else if (entry.getKey().startsWith("disable.")) {
// These are now added into the "disableFeatures" property.
String property = StringUtils.uncapitalize(entry.getKey().substring(8));
throw new OpenApiException("Unsupported `openapi` configuration setting found: " + entry.getKey()
+ ". Add `" + property + "` to the `disableFeatures` property instead");
} else if (entry.getKey().startsWith("openapi.use.")) {
throw new OpenApiException(String.format(
"The `%s` `openapi` plugin property is no longer supported. Use the "
+ "`disableFeatures` property instead to disable features.", entry.getKey()));
}
}

return mapped;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,8 @@

package software.amazon.smithy.openapi.fromsmithy;

import java.util.Map;
import java.util.logging.Logger;
import software.amazon.smithy.build.PluginContext;
import software.amazon.smithy.build.SmithyBuildPlugin;
import software.amazon.smithy.model.node.Node;
import software.amazon.smithy.model.node.NodeMapper;
import software.amazon.smithy.model.node.ObjectNode;
import software.amazon.smithy.model.shapes.ShapeId;
import software.amazon.smithy.openapi.OpenApiConfig;
Expand All @@ -36,8 +32,6 @@
*/
public final class Smithy2OpenApi implements SmithyBuildPlugin {

private static final Logger LOGGER = Logger.getLogger(Smithy2OpenApi.class.getName());

@Override
public String getName() {
return "openapi";
Expand All @@ -47,23 +41,7 @@ public String getName() {
public void execute(PluginContext context) {
OpenApiConverter converter = OpenApiConverter.create();
context.getPluginClassLoader().ifPresent(converter::classLoader);

// Remove deprecated "openapi." prefixes from configuration settings.
ObjectNode mapped = context.getSettings();
for (Map.Entry<String, Node> entry : mapped.getStringMap().entrySet()) {
if (entry.getKey().startsWith("openapi.")) {
String expected = entry.getKey().substring(8);
LOGGER.warning("Deprecated `openapi` configuration setting found: " + entry.getKey()
+ ". Use " + expected + " instead");
mapped = mapped.withoutMember(entry.getKey());
mapped = mapped.withMember(expected, entry.getValue());
}
}

NodeMapper mapper = new NodeMapper();
OpenApiConfig config = new OpenApiConfig();
mapper.deserializeInto(mapped, config);

OpenApiConfig config = OpenApiConfig.fromNode(context.getSettings());
ShapeId shapeId = config.getService();

if (shapeId == null) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package software.amazon.smithy.openapi.fromsmithy;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;

import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import software.amazon.smithy.model.node.Node;
import software.amazon.smithy.openapi.OpenApiConfig;
import software.amazon.smithy.openapi.OpenApiException;

public class OpenApiConfigTest {
@Test
public void throwsOnDisableProperties() {
Node disableTest = Node.objectNode().withMember("disable.additionalProperties", Node.from(true));

Exception thrown = Assertions.assertThrows(OpenApiException.class, () -> {
OpenApiConfig.fromNode(disableTest);
});

assertThat(thrown.getMessage(), containsString("disableFeatures"));
}

@Test
public void throwsOnOpenApiUseProperties() {
Node openApiUseTest = Node.objectNode().withMember("openapi.use.xml", Node.from(true));

Exception thrown = Assertions.assertThrows(OpenApiException.class, () -> {
OpenApiConfig.fromNode(openApiUseTest);
});

assertThat(thrown.getMessage(), containsString("disableFeatures"));
}

@Test
public void convertsExplicitlyMappedProperties() {
Node mappedTest = Node.objectNode()
.withMember("openapi.tags", Node.from(true))
.withMember("openapi.ignoreUnsupportedTraits", Node.from(true));
OpenApiConfig config = OpenApiConfig.fromNode(mappedTest);

assertThat(config.getTags(), equalTo(true));
assertThat(config.getIgnoreUnsupportedTraits(), equalTo(true));
}
}

0 comments on commit 19952b8

Please sign in to comment.