Repository: smithy-lang/smithy (java)

Issue:
Inline {map,list}_statements in conversion to OpenAPI
Hi everyone, is this something that can be accomplished relatively easily?

I understand that lists and maps need to be defined verbosely and that there aren't any inline structures of this kind;
but this is very suboptimal when schemas such as "AdditionalConfigMap" are created in generation, where this is just supposed to be a `Map<String, String>`.

This also makes it hard to combine with other codegen tools such as `datamodel-codegen`.

I would ideally like to be able to replace them with simple "object" or "array" OAS types.

Pull request description:
Add config for inline JSON Schema maps
Resolves #2443 

This commit adds the `useInlineMaps` setting to allow users to configure JSON Schema conversion to inline converted `map` shapes instead of creating references.

This is distinct from the `mapStrategy` setting in that it's about the placement of the generated type and not how the type is generated. Combining the two would require two strategies for each type of map converted.

---
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/model-translations/converting-to-openapi.rst b/docs/source-2.0/guides/model-translations/converting-to-openapi.rst
index 81782ae64..b9c2a48e3 100644
--- a/docs/source-2.0/guides/model-translations/converting-to-openapi.rst
+++ b/docs/source-2.0/guides/model-translations/converting-to-openapi.rst
@@ -804,6 +804,26 @@ mapStrategy (``string``)
             }
         }
 
+.. _generate-openapi-jsonschema-setting-useInlineMaps:
+
+useInlineMaps (``boolean``)
+    Configures Smithy to generate ``map`` shapes inline instead of as
+    references. This is necessary for some code generators to distinguish
+    between ``maps`` and ``structure`` shapes when generating.
+
+    .. code-block:: json
+        :caption: smithy-build.json
+
+        {
+            "version": "1.0",
+            "plugins": {
+                "openapi": {
+                    "service": "example.weather#Weather",
+                    "useInlineMaps": true
+                }
+            }
+        }
+
 .. _generate-openapi-jsonschema-setting-schemaDocumentExtensions:
 
 schemaDocumentExtensions (``Map<String, any>``)
diff --git a/smithy-jsonschema/src/main/java/software/amazon/smithy/jsonschema/DeconflictingStrategy.java b/smithy-jsonschema/src/main/java/software/amazon/smithy/jsonschema/DeconflictingStrategy.java
index 09f84243f..cb0f47e25 100644
--- a/smithy-jsonschema/src/main/java/software/amazon/smithy/jsonschema/DeconflictingStrategy.java
+++ b/smithy-jsonschema/src/main/java/software/amazon/smithy/jsonschema/DeconflictingStrategy.java
@@ -17,6 +17,7 @@ package software.amazon.smithy.jsonschema;
 
 import java.util.HashMap;
 import java.util.Map;
+import java.util.TreeSet;
 import java.util.function.Predicate;
 import java.util.logging.Logger;
 import java.util.regex.Pattern;
@@ -60,24 +61,30 @@ final class DeconflictingStrategy implements RefStrategy {
         // to make the result deterministic. Eliminate trait definitions
         // ahead of that computation to eliminate potential conflicts with
         // shapes that won't go into JSON Schema anyway.
-        Model scrubbedModel = ModelTransformer.create().scrubTraitDefinitions(model);
-        scrubbedModel.shapes()
-                .filter(shapePredicate.and(FunctionalUtils.not(this::isIgnoredShape)))
-                .sorted()
-                .forEach(shape -> {
+        Model scrubbedModel = null;
+        for (Shape shape : new TreeSet<>(model.toSet())) {
+            if (!shapePredicate.and(FunctionalUtils.not(this::isIgnoredShape)).test(shape)) {
+                continue;
+            }
+
             String pointer = delegate.toPointer(shape.getId());
             if (!reversePointers.containsKey(pointer)) {
                 pointers.put(shape.getId(), pointer);
                 reversePointers.put(pointer, shape.getId());
             } else {
-                String deconflictedPointer = deconflict(shape, pointer, reversePointers);
-                LOGGER.info(() -> String.format(
-                        "De-conflicted `%s` JSON schema pointer from `%s` to `%s`",
-                        shape.getId(), pointer, deconflictedPointer));
-                pointers.put(shape.getId(), deconflictedPointer);
-                reversePointers.put(deconflictedPointer, shape.getId());
+                if (scrubbedModel == null) {
+                    scrubbedModel = ModelTransformer.create().scrubTraitDefinitions(model);
+                }
+                if (scrubbedModel.getShape(reversePointers.get(pointer)).isPresent()) {
+                    String deconflictedPointer = deconflict(shape, pointer, reversePointers);
+                    LOGGER.info(() -> String.format(
+                            "De-conflicted `%s` JSON schema pointer from `%s` to `%s`",
+                            shape.getId(), pointer, deconflictedPointer));
+                    pointers.put(shape.getId(), deconflictedPointer);
+                    reversePointers.put(deconflictedPointer, shape.getId());
+                }
             }
-        });
+        }
     }
 
     // Some shapes aren't converted to JSON schema at all because they
diff --git a/smithy-jsonschema/src/main/java/software/amazon/smithy/jsonschema/DefaultRefStrategy.java b/smithy-jsonschema/src/main/java/software/amazon/smithy/jsonschema/DefaultRefStrategy.java
index f082670b8..0ea8d54e4 100644
--- a/smithy-jsonschema/src/main/java/software/amazon/smithy/jsonschema/DefaultRefStrategy.java
+++ b/smithy-jsonschema/src/main/java/software/amazon/smithy/jsonschema/DefaultRefStrategy.java
@@ -148,6 +148,17 @@ final class DefaultRefStrategy implements RefStrategy {
             return true;
         }
 
+        // Maps are not inlined by default, but can be if configured.
+        // Maps are usually not a generated type in programming languages,
+        // but JSON schema represents them as "object" types which code
+        // generators may not distinguish from converted structures.
+        //
+        // Some code generators, however, will treat inline "object" types
+        // as maps and referenced ones as structures.
+        if (shape.isMapShape() && config.getUseInlineMaps()) {
+            return true;
+        }
+
         // Strings with the enum trait are never inlined. This helps to ensure
         // that the name of an enum string can be round-tripped from
         // Smithy -> JSON Schema -> Smithy, helps OpenAPI code generators to
diff --git a/smithy-jsonschema/src/main/java/software/amazon/smithy/jsonschema/JsonSchemaConfig.java b/smithy-jsonschema/src/main/java/software/amazon/smithy/jsonschema/JsonSchemaConfig.java
index 6656647ec..d35a297ad 100644
--- a/smithy-jsonschema/src/main/java/software/amazon/smithy/jsonschema/JsonSchemaConfig.java
+++ b/smithy-jsonschema/src/main/java/software/amazon/smithy/jsonschema/JsonSchemaConfig.java
@@ -115,6 +115,7 @@ public class JsonSchemaConfig {
     private boolean disableDefaultValues = false;
     private boolean disableIntEnums = false;
     private boolean addReferenceDescriptions = false;
+    private boolean useInlineMaps = false;
 
     public JsonSchemaConfig() {
         nodeMapper.setWhenMissingSetter(NodeMapper.WhenMissing.IGNORE);
@@ -479,4 +480,26 @@ public class JsonSchemaConfig {
     public void setAddReferenceDescriptions(boolean addReferenceDescriptions) {
         this.addReferenceDescriptions = addReferenceDescriptions;
     }
+
+    /**
+     * Whether to inline map shapes when creating JSON Schema object types
+     * from them.
+     *
+     * <p>Defaults to {@code false}.</p>
+     *
+     * @return Whether to inline map shapes in the resulting schema
+     */
+    public boolean getUseInlineMaps() {
+        return useInlineMaps;
+    }
+
+    /**
+     * Sets whether to inline map shapes when creating JSON schema object types
+     * from them.
+     *
+     * @param useInlineMaps Whether to inline map shapes in the resulting schema.
+     */
+    public void setUseInlineMaps(boolean useInlineMaps) {
+        this.useInlineMaps = useInlineMaps;
+    }
 }


Test patch:
diff --git a/smithy-jsonschema/src/test/java/software/amazon/smithy/jsonschema/JsonSchemaConverterTest.java b/smithy-jsonschema/src/test/java/software/amazon/smithy/jsonschema/JsonSchemaConverterTest.java
index 9a9a0f3a5..b3c74ad02 100644
--- a/smithy-jsonschema/src/test/java/software/amazon/smithy/jsonschema/JsonSchemaConverterTest.java
+++ b/smithy-jsonschema/src/test/java/software/amazon/smithy/jsonschema/JsonSchemaConverterTest.java
@@ -573,6 +573,32 @@ public class JsonSchemaConverterTest {
         assertThat(schema.getAdditionalProperties().get().getType().get(), equalTo("string"));
     }
 
+    @Test
+    public void supportsInlineMapPropertyNames() {
+        ShapeId shapeId = ShapeId.from("smithy.api#String");
+        StringShape string = StringShape.builder().id(shapeId).build();
+        MapShape map = MapShape.builder().id("a.b#Map").key(shapeId).value(shapeId).build();
+        StructureShape container = StructureShape.builder().id("a.b#Container").addMember("map", map.getId()).build();
+        Model model = Model.builder().addShapes(container, map, string).build();
+        JsonSchemaConfig config = new JsonSchemaConfig();
+        config.setUseInlineMaps(true);
+        SchemaDocument document = JsonSchemaConverter.builder()
+                .config(config)
+                .model(model)
+                .build()
+                .convertShape(container);
+        Schema schema = document.getRootSchema();
+
+        assertThat(schema.getProperties().containsKey("map"), equalTo(true));
+        Schema mapMember = schema.getProperties().get("map");
+        assertThat(mapMember.getRef().isPresent(), equalTo(false));
+        assertThat(mapMember.getType().get(), equalTo("object"));
+        assertTrue(mapMember.getPropertyNames().isPresent());
+        assertThat(mapMember.getPropertyNames().get().getType().get(), equalTo("string"));
+        assertTrue(mapMember.getAdditionalProperties().isPresent());
+        assertThat(mapMember.getAdditionalProperties().get().getType().get(), equalTo("string"));
+    }
+
     @Test
     public void supportsMapPatternProperties() {
         ShapeId shapeId = ShapeId.from("smithy.api#String");
@@ -596,6 +622,35 @@ public class JsonSchemaConverterTest {
         assertThat(schema.getPatternProperties().get(pattern).getType().get(), equalTo("string"));
     }
 
+    @Test
+    public void supportsInlineMapPatternProperties() {
+        ShapeId shapeId = ShapeId.from("smithy.api#String");
+        StringShape string = StringShape.builder().id(shapeId).build();
+        String pattern = "[a-z]{1,16}";
+        StringShape key = StringShape.builder().id("a.b#Key")
+                .addTrait(new PatternTrait(pattern)).build();
+        MapShape map = MapShape.builder().id("a.b#Map").key(key.getId()).value(shapeId).build();
+        StructureShape container = StructureShape.builder().id("a.b#Container").addMember("map", map.getId()).build();
+        Model model = Model.builder().addShapes(container, map, key, string).build();
+        JsonSchemaConfig config = new JsonSchemaConfig();
+        config.setUseInlineMaps(true);
+        config.setMapStrategy(JsonSchemaConfig.MapStrategy.PATTERN_PROPERTIES);
+        SchemaDocument document = JsonSchemaConverter.builder()
+                .config(config)
+                .model(model)
+                .build()
+                .convertShape(container);
+        Schema schema = document.getRootSchema();
+
+        assertThat(schema.getProperties().containsKey("map"), equalTo(true));
+        Schema mapMember = schema.getProperties().get("map");
+        assertThat(mapMember.getRef().isPresent(), equalTo(false));
+        assertThat(mapMember.getType().get(), equalTo("object"));
+        assertThat(mapMember.getPatternProperties().size(), equalTo(1));
+        assertTrue(mapMember.getPatternProperties().containsKey(pattern));
+        assertThat(mapMember.getPatternProperties().get(pattern).getType().get(), equalTo("string"));
+    }
+
     @Test
     public void supportsMapPatternPropertiesWithDefaultPattern() {
         ShapeId shapeId = ShapeId.from("smithy.api#String");