Skip to content

Commit 4e003af

Browse files
committed
Warn when using jsonAdd with schemas
Using the jsonAdd feature with the Smithy to OpenAPI converter is probably never what you actually want to do. It means that clients, servers, and other artifacts generated from the Smithy model don't know about the shapes being defined only in OpenAPI. This PR adds a severe log warning when this is detected.
1 parent 31c66f2 commit 4e003af

File tree

2 files changed

+63
-4
lines changed

2 files changed

+63
-4
lines changed

smithy-openapi/src/main/java/software/amazon/smithy/openapi/fromsmithy/mappers/OpenApiJsonAdd.java

+8
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,14 @@ public ObjectNode updateNode(Context<? extends Trait> context, OpenApi openapi,
5858
for (Map.Entry<String, Node> entry : add.entrySet()) {
5959
try {
6060
LOGGER.info(() -> "OpenAPI `jsonAdd`: adding `" + entry.getKey() + "`");
61+
62+
if (entry.getKey().startsWith("/components/schemas")) {
63+
LOGGER.severe("Adding schemas to the generated OpenAPI model directly means that "
64+
+ "clients, servers, and other artifacts generated from your Smithy "
65+
+ "model don't know about all of the shapes used in the service. You "
66+
+ "almost certainly should not do this.");
67+
}
68+
6169
result = NodePointer.parse(entry.getKey())
6270
.addWithIntermediateValues(result, entry.getValue().toNode())
6371
.expectObjectNode();

smithy-openapi/src/test/java/software/amazon/smithy/openapi/fromsmithy/mappers/OpenApiJsonAddTest.java

+55-4
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,11 @@
1515

1616
package software.amazon.smithy.openapi.fromsmithy.mappers;
1717

18+
import java.util.logging.Handler;
19+
import java.util.logging.LogRecord;
20+
import java.util.logging.Logger;
1821
import org.junit.jupiter.api.Assertions;
22+
import org.junit.jupiter.api.BeforeAll;
1923
import org.junit.jupiter.api.Test;
2024
import software.amazon.smithy.model.Model;
2125
import software.amazon.smithy.model.node.Node;
@@ -26,16 +30,22 @@
2630
import software.amazon.smithy.openapi.fromsmithy.OpenApiConverter;
2731

2832
public class OpenApiJsonAddTest {
29-
@Test
30-
public void addsWithPointers() {
31-
Model model = Model.assembler()
33+
34+
private static Model MODEL;
35+
36+
@BeforeAll
37+
public static void before() {
38+
MODEL = Model.assembler()
3239
// Reusing another test cases's model, but that doesn't matter for the
3340
// purpose of this test.
3441
.addImport(RemoveUnusedComponentsTest.class.getResource("substitutions.smithy"))
3542
.discoverModels()
3643
.assemble()
3744
.unwrap();
45+
}
3846

47+
@Test
48+
public void addsWithPointers() {
3949
ObjectNode addNode = Node.objectNodeBuilder()
4050
.withMember("/info/description", "hello")
4151
.withMember("/info/foo", "bar")
@@ -49,7 +59,7 @@ public void addsWithPointers() {
4959

5060
ObjectNode openApi = OpenApiConverter.create()
5161
.config(config)
52-
.convertToNode(model);
62+
.convertToNode(MODEL);
5363

5464
String description = NodePointer.parse("/info/description").getValue(openApi).expectStringNode().getValue();
5565
String infoFoo = NodePointer.parse("/info/foo").getValue(openApi).expectStringNode().getValue();
@@ -61,4 +71,45 @@ public void addsWithPointers() {
6171
Assertions.assertEquals("nested", infoNested);
6272
Assertions.assertEquals("custom", infoTitle);
6373
}
74+
75+
private static final class SearchingHandler extends Handler {
76+
boolean found;
77+
String searchString;
78+
79+
SearchingHandler(String searchString) {
80+
this.searchString = searchString;
81+
}
82+
83+
@Override
84+
public void publish(LogRecord record) {
85+
if (record.getMessage().contains(searchString)) {
86+
found = true;
87+
}
88+
}
89+
90+
@Override
91+
public void flush() {}
92+
93+
@Override
94+
public void close() throws SecurityException {}
95+
}
96+
97+
@Test
98+
public void warnsWhenAddingSchemas() {
99+
Logger logger = Logger.getLogger(OpenApiJsonAdd.class.getName());
100+
SearchingHandler handler = new SearchingHandler("Adding schemas to the generated OpenAPI model directly");
101+
logger.addHandler(handler);
102+
103+
ObjectNode addNode = Node.objectNode()
104+
.withMember("/components/schemas/Merged", Node.objectNode().withMember("type", "string"));
105+
106+
OpenApiConfig config = new OpenApiConfig();
107+
config.setService(ShapeId.from("smithy.example#Service"));
108+
config.setJsonAdd(addNode.getStringMap());
109+
ObjectNode openApi = OpenApiConverter.create().config(config).convertToNode(MODEL);
110+
NodePointer.parse("/components/schemas/Merged").getValue(openApi).expectObjectNode();
111+
logger.removeHandler(handler);
112+
113+
Assertions.assertTrue(handler.found);
114+
}
64115
}

0 commit comments

Comments
 (0)