Repository: smithy-lang/smithy
Pull request #1904 (2023-08-02 18:16:22)

Allow disabling format on integers
Fixes: https://github.com/smithy-lang/smithy/issues/1899

This PR adds a configuration option, `disableIntegerFormat`, which removes the `format` property which is set when integer types are enabled with `useIntegerType`.

API Gateway does not support `int32` or `int64` formats on numbers as noted in [API Gateway Known Issues](https://docs.aws.amazon.com/apigateway/latest/developerguide/api-gateway-known-issues.html):

> Numbers of the Int32 or Int64 type are not supported.

These formats are disabled by default for API Gateway conversions, but are otherwise enabled when using the `useIntegerType` configuration.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.



Patch:
diff --git a/docs/source-2.0/guides/converting-to-openapi.rst b/docs/source-2.0/guides/converting-to-openapi.rst
index eb3009ad0..64b2e05c2 100644
--- a/docs/source-2.0/guides/converting-to-openapi.rst
+++ b/docs/source-2.0/guides/converting-to-openapi.rst
@@ -536,6 +536,48 @@ useIntegerType (``boolean``)
             }
         }
 
+
+.. _generate-openapi-setting-disableIntegerFormat:
+
+disableIntegerFormat (``boolean``)
+    Set to true to disable setting the ``format`` property when using the
+    "integer" type that is enabled by the :ref:`useIntegerType <generate-openapi-setting-useIntegerType>`
+    configuration setting.
+
+    .. code-block:: json
+
+        {
+            "version": "2.0",
+            "plugins": {
+                "openapi": {
+                    "service": "example.weather#Weather",
+                    "useIntegerType": true,
+                    "disableIntegerFormat": true
+                }
+            }
+        }
+
+    With this enabled (the default), the ``format`` property is set to ``int32``
+    or ``int64`` for Integer or Long shapes respectively.
+
+    .. code-block:: json
+
+        {
+            "Foo": {
+                "type": "object",
+                "properties": {
+                    "myInteger": {
+                        "type": "integer",
+                        "format": "int32"
+                    },
+                    "myLong": {
+                        "type": "integer",
+                        "format": "int64"
+                    }
+                }
+            }
+        }
+
 .. _generate-openapi-setting-onErrorStatusConflict:
 
 onErrorStatusConflict (``String``)
diff --git a/smithy-aws-apigateway-openapi/src/main/java/software/amazon/smithy/aws/apigateway/openapi/AddDefaultConfigSettings.java b/smithy-aws-apigateway-openapi/src/main/java/software/amazon/smithy/aws/apigateway/openapi/AddDefaultConfigSettings.java
index 4c95190b5..07e4d34b2 100644
--- a/smithy-aws-apigateway-openapi/src/main/java/software/amazon/smithy/aws/apigateway/openapi/AddDefaultConfigSettings.java
+++ b/smithy-aws-apigateway-openapi/src/main/java/software/amazon/smithy/aws/apigateway/openapi/AddDefaultConfigSettings.java
@@ -23,7 +23,8 @@ import software.amazon.smithy.openapi.OpenApiConfig;
  * Disables OpenAPI and JSON Schema features not supported by API Gateway.
  *
  * <p>API Gateway does not allow characters like "_". API Gateway
- * doesn't support the "default" trait.
+ * doesn't support the "default" trait or `int32` or `int64` "format"
+ * values.
  */
 final class AddDefaultConfigSettings implements ApiGatewayMapper {
     @Override
@@ -36,5 +37,9 @@ final class AddDefaultConfigSettings implements ApiGatewayMapper {
         config.setAlphanumericOnlyRefs(true);
         config.getDisableFeatures().add("default");
         config.setDisableDefaultValues(true);
+        // If the `useIntegerType` config has been set, this assures that
+        // `int32` and `int64` formats are not set on those integer types.
+        // See https://docs.aws.amazon.com/apigateway/latest/developerguide/api-gateway-known-issues.html
+        config.setDisableIntegerFormat(true);
     }
 }
diff --git a/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/ServiceTrait.java b/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/ServiceTrait.java
index 7d4c43afe..d56938b5a 100644
--- a/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/ServiceTrait.java
+++ b/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/ServiceTrait.java
@@ -232,9 +232,7 @@ public final class ServiceTrait extends AbstractTrait implements ToSmithyBuilder
                     && arnNamespace.equals(os.arnNamespace)
                     && cloudFormationName.equals(os.cloudFormationName)
                     && cloudTrailEventSource.equals(os.cloudTrailEventSource)
-                    && docId == null
-                        ? os.docId == null
-                        : docId.equals(os.docId)
+                    && Objects.equals(docId, os.docId)
                     && endpointPrefix.equals(os.endpointPrefix);
         }
     }
diff --git a/smithy-openapi/src/main/java/software/amazon/smithy/openapi/OpenApiConfig.java b/smithy-openapi/src/main/java/software/amazon/smithy/openapi/OpenApiConfig.java
index c2be5c196..43f6ea6b4 100644
--- a/smithy-openapi/src/main/java/software/amazon/smithy/openapi/OpenApiConfig.java
+++ b/smithy-openapi/src/main/java/software/amazon/smithy/openapi/OpenApiConfig.java
@@ -83,6 +83,7 @@ public class OpenApiConfig extends JsonSchemaConfig {
     private Map<String, Node> jsonAdd = Collections.emptyMap();
     private List<String> externalDocs = ListUtils.of(
             "Homepage", "API Reference", "User Guide", "Developer Guide", "Reference", "Guide");
+    private boolean disableIntegerFormat = false;
     private OpenApiVersion version = OpenApiVersion.VERSION_3_0_2;
 
     public OpenApiConfig() {
@@ -321,6 +322,20 @@ public class OpenApiConfig extends JsonSchemaConfig {
         super.setJsonSchemaVersion(version.getJsonSchemaVersion());
     }
 
+
+    public boolean getDisableIntegerFormat() {
+        return this.disableIntegerFormat;
+    }
+
+    /**
+     * Set to true to disable setting the `format` property on integer types.
+     *
+     * @param disableIntegerFormat True to disable setting format on integer types.
+     */
+    public void setDisableIntegerFormat(boolean disableIntegerFormat) {
+        this.disableIntegerFormat = disableIntegerFormat;
+    }
+
     /**
      * Creates an OpenApiConfig from a Node value.
      *
diff --git a/smithy-openapi/src/main/java/software/amazon/smithy/openapi/fromsmithy/OpenApiJsonSchemaMapper.java b/smithy-openapi/src/main/java/software/amazon/smithy/openapi/fromsmithy/OpenApiJsonSchemaMapper.java
index 581b74865..4a407eef0 100644
--- a/smithy-openapi/src/main/java/software/amazon/smithy/openapi/fromsmithy/OpenApiJsonSchemaMapper.java
+++ b/smithy-openapi/src/main/java/software/amazon/smithy/openapi/fromsmithy/OpenApiJsonSchemaMapper.java
@@ -62,7 +62,8 @@ public final class OpenApiJsonSchemaMapper implements JsonSchemaMapper {
         }
 
         boolean useOpenApiIntegerType = config instanceof OpenApiConfig
-                && ((OpenApiConfig) config).getUseIntegerType();
+                && ((OpenApiConfig) config).getUseIntegerType()
+                && !((OpenApiConfig) config).getDisableIntegerFormat();
 
         // Don't overwrite an existing format setting.
         if (!builder.getFormat().isPresent()) {


Test patch:
diff --git a/smithy-aws-traits/src/test/java/software/amazon/smithy/aws/traits/ServiceTraitTest.java b/smithy-aws-traits/src/test/java/software/amazon/smithy/aws/traits/ServiceTraitTest.java
index 1d2e870c5..9743c4b96 100644
--- a/smithy-aws-traits/src/test/java/software/amazon/smithy/aws/traits/ServiceTraitTest.java
+++ b/smithy-aws-traits/src/test/java/software/amazon/smithy/aws/traits/ServiceTraitTest.java
@@ -19,9 +19,9 @@ import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
-
 import java.util.Optional;
 import org.junit.jupiter.api.Test;
 import software.amazon.smithy.model.Model;
@@ -126,4 +126,22 @@ public class ServiceTraitTest {
         assertFalse(trait.getDocId().isPresent());
         assertThat(trait.resolveDocId(service), equalTo("some-value-2018-03-17"));
     }
+
+    @Test
+    public void equality() {
+        Node node1 = Node.parse("{\"sdkId\": \"Foo1\", \"arnNamespace\": \"service\", \"cloudFormationName\": \"Baz\", "
+                + "\"endpointPrefix\": \"endpoint-prefix\"}");
+
+        Node node2 = Node.parse("{\"sdkId\": \"Foo2\", \"arnNamespace\": \"service\", \"cloudFormationName\": \"Baz\", "
+                + "\"endpointPrefix\": \"endpoint-prefix\"}");
+
+        TraitFactory provider = TraitFactory.createServiceFactory();
+        Optional<Trait> trait1 = provider.createTrait(ServiceTrait.ID, ShapeId.from("ns.foo#foo1"), node1);
+        Optional<Trait> trait2 = provider.createTrait(ServiceTrait.ID, ShapeId.from("ns.foo#foo2"), node2);
+
+        ServiceTrait serviceTrait1 = (ServiceTrait) trait1.get();
+        ServiceTrait serviceTrait2 = (ServiceTrait) trait2.get();
+
+        assertNotEquals(serviceTrait1, serviceTrait2);
+    }
 }
diff --git a/smithy-openapi/src/test/java/software/amazon/smithy/openapi/fromsmithy/OpenApiJsonSchemaMapperTest.java b/smithy-openapi/src/test/java/software/amazon/smithy/openapi/fromsmithy/OpenApiJsonSchemaMapperTest.java
index 41fded1df..b1603c4b6 100644
--- a/smithy-openapi/src/test/java/software/amazon/smithy/openapi/fromsmithy/OpenApiJsonSchemaMapperTest.java
+++ b/smithy-openapi/src/test/java/software/amazon/smithy/openapi/fromsmithy/OpenApiJsonSchemaMapperTest.java
@@ -178,6 +178,29 @@ public class OpenApiJsonSchemaMapperTest {
         assertThat(document.getRootSchema().getFormat().get(), equalTo("int64"));
     }
 
+    @Test
+    public void canDisableIntegerFormats() {
+        IntegerShape integerShape = IntegerShape.builder().id("a.b#C").build();
+        LongShape longShape = LongShape.builder().id("a.b#D").build();
+        Model model = Model.builder().addShapes(integerShape, longShape).build();
+        OpenApiConfig config = new OpenApiConfig();
+        config.setUseIntegerType(true);
+        config.setDisableIntegerFormat(true);
+        JsonSchemaConverter converter = JsonSchemaConverter.builder()
+                .addMapper(new OpenApiJsonSchemaMapper())
+                .config(config)
+                .model(model)
+                .build();
+
+        SchemaDocument integerDocument = converter.convertShape(integerShape);
+        SchemaDocument longDocument = converter.convertShape(longShape);
+
+        assertThat(integerDocument.getRootSchema().getFormat().isPresent(), equalTo(false));
+        assertThat(integerDocument.getRootSchema().getType().get(), equalTo("integer"));
+        assertThat(longDocument.getRootSchema().getFormat().isPresent(), equalTo(false));
+        assertThat(longDocument.getRootSchema().getType().get(), equalTo("integer"));
+    }
+
     @Test
     public void supportsFloatFormat() {
         FloatShape shape = FloatShape.builder().id("a.b#C").build();