- improved support for multiple schemas

- test coverage for uri fragments

Author: bartoszm

pom.xml

@@ -25,7 +25,7 @@
 <groupId>com.amartus.sonata</groupId>
 <artifactId>blender</artifactId>
 <description>SonataBlendingTool</description>
-<version>1.10.2</version>
+<version>1.11.0-SNAPSHOT</version>
 <properties>
 <openapi.generator.version>6.2.0</openapi.generator.version>
 <jackson.version>2.14.2</jackson.version>
@@ -84,6 +84,12 @@
 <version>1.18.26</version>
 <scope>provided</scope>
 </dependency>
+<dependency>
+<groupId>org.junit.jupiter</groupId>
+<artifactId>junit-jupiter-api</artifactId>
+<version>5.9.0</version>
+<scope>test</scope>
+</dependency>
 <dependency>
 <groupId>org.junit.jupiter</groupId>
 <artifactId>junit-jupiter-engine</artifactId>
@@ -123,7 +129,7 @@
 <path>
 <groupId>org.projectlombok</groupId>
 <artifactId>lombok</artifactId>
-<version>1.18.22</version>
+<version>1.18.26</version>
 </path>
 </annotationProcessorPaths>
 </configuration>
@@ -146,6 +152,12 @@
 </archive>
 </configuration>
 </plugin>
+<plugin>
+<groupId>org.apache.maven.plugins</groupId>
+<artifactId>maven-surefire-plugin</artifactId>
+<version>3.1.0</version>
+
+</plugin>
 <plugin>
 <groupId>org.apache.maven.plugins</groupId>
 <artifactId>maven-shade-plugin</artifactId>

src/main/java/com/amartus/sonata/blender/impl/MergeSchemasAction.java

@@ -21,19 +21,16 @@
 import com.amartus.sonata.blender.cmd.AbstractBlend;
 import com.amartus.sonata.blender.impl.util.OasUtils;
 import com.amartus.sonata.blender.impl.util.OasWrapper;
-import io.swagger.models.properties.Property;
 import io.swagger.v3.oas.models.OpenAPI;
 import io.swagger.v3.oas.models.media.Discriminator;
 import io.swagger.v3.oas.models.media.Schema;
 import io.swagger.v3.oas.models.media.StringSchema;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.util.Collection;
-import java.util.Map;
-import java.util.Objects;
-import java.util.Optional;
+import java.util.*;
 import java.util.function.Function;
+import java.util.stream.Collectors;
 
 
 public class MergeSchemasAction {
@@ -50,7 +47,7 @@ public enum Mode {
 private OasWrapper wrapper;
 
 private interface Handler {
-void handle(Discriminator disc, Schema prop);
+void handle(Discriminator disc, Schema prop, Schema subject);
 }
 
 public MergeSchemasAction(String modelToAugment, Mode mode) {
@@ -62,7 +59,7 @@ public MergeSchemasAction(String modelToAugment, Mode mode) {
 private Handler handler(Mode mode, String model) {
 switch (mode) {
 case FIX: return this::fixTargetSchema;
-case RELAXED: return (d, p) -> {
+case RELAXED: return (d, p, t) -> {
 var issues = 0;
 if(d == null) {
 issues += 1;
@@ -77,10 +74,10 @@ private Handler handler(Mode mode, String model) {
 throw new IllegalStateException("Discriminator property not found");
 }
 
-fixTargetSchema(d, p);
+fixTargetSchema(d, p, t);
 
 };
-default: return (s, p) -> {
+default: return (s, p, _n) -> {
 if(s == null || p == null) {
 log.error("No discriminator defined for {} ", modelToAugment);
 throw new IllegalStateException("Discriminator not found");
@@ -103,45 +100,60 @@ public MergeSchemasAction target(OpenAPI api) {
 }
 
 public void execute() {
+if (schemasToInject.isEmpty()) return;
+
 log.debug("Injecting {} schemas",
 schemasToInject.size());
+Set<String> targets = findTargets();
+
+targets.forEach(targetName -> {
+validateTargetExists(targetName);
+Schema schema = this.openAPI.getComponents().getSchemas().get(targetName);
+log.info("Evaluating target {}", targetName);
+handler.handle(discriminator(schema), prop(schema), schema);
+});
 
-if (!schemasToInject.isEmpty()) {
-validateTargetExists();
-Schema schema = this.openAPI.getComponents().getSchemas().get(modelToAugment);
-handler.handle(discriminator(schema), prop(schema));
-}
 this.openAPI.getComponents().getSchemas().putAll(schemasToInject);
 }
 
-private void validateTargetExists() {
-if (getTargetSchema().isEmpty()) {
+private Set<String> findTargets() {
+Set<String> targets = schemasToInject.values().stream()
+.map(s -> Optional.ofNullable(s.getExtensions())
+.map(em -> (String) em.get(ProductSpecReader.TARGET_NAME)).orElse(null))
+.filter(Objects::nonNull)
+.collect(Collectors.toSet());
+targets.add(modelToAugment);
+return targets;
+}
+
+private void validateTargetExists(String targetName) {
+if (getTargetSchema(targetName).isEmpty()) {
 log.error("Schema with name '{}' is not present in the API", modelToAugment);
 throw new IllegalStateException(String.format("Schema '%s' not found in the specification", modelToAugment));
 }
 }
 
 @SuppressWarnings("OptionalGetWithoutIsPresent")
-private void fixTargetSchema(Discriminator discriminator, Schema property) {
-var schema = getTargetSchema().get();
+private void fixTargetSchema(Discriminator discriminator, Schema property, Schema targetSchema) {
+
 if(property == null) {
 log.info("Adding field {} to the {}", DISCRIMINATOR_NAME, modelToAugment);
-schema.addProperty(DISCRIMINATOR_NAME,
+targetSchema.addProperty(DISCRIMINATOR_NAME,
 new StringSchema().description("Used as a discriminator to support polymorphic definitions"));
 }
 if(discriminator == null) {
 log.info("Adding discriminator to the {}", modelToAugment);
-schema.setDiscriminator(new Discriminator().propertyName(DISCRIMINATOR_NAME));
+targetSchema.setDiscriminator(new Discriminator().propertyName(DISCRIMINATOR_NAME));
 }
 }
 
 @SuppressWarnings("rawtypes")
-private Optional<Schema> getTargetSchema() {
+private Optional<Schema> getTargetSchema(String modelName) {
 var allSchemas = Optional.ofNullable(this.openAPI.getComponents())
 .flatMap(c -> Optional.ofNullable(c.getSchemas()));
 
 return allSchemas
-.map(all -> Optional.ofNullable(all.get(modelToAugment)))
+.map(all -> Optional.ofNullable(all.get(modelName)))
 .orElseThrow(() -> new IllegalStateException("API schemas are not resolved"));
 }
 
@@ -165,7 +177,7 @@ private <T> T recursiveSearch(Schema schema, Function<Schema, T> extractor) {
 }
 
 private Discriminator discriminator(Schema schema) {
-return recursiveSearch(schema, Schema::getDiscriminator);
+return schema.getDiscriminator();
 }
 
 private Schema prop(Schema schema) {

src/main/java/com/amartus/sonata/blender/impl/postprocess/UpdateDiscriminatorMapping.java

@@ -43,7 +43,7 @@ protected void process(String name, Schema schema) {
 .findFirst();
 
 parentCandidate.ifPresent(parent -> {
-var discValue = discrininatorValue(schema).orElse(name);
+var discValue = discriminatorValue(schema).orElse(name);
 log.debug("Trying {} as parent discriminator update for {}", parent.get$ref(), name);
 updateMapping(name, discValue, parent.get$ref());
 });
@@ -60,13 +60,13 @@ private void updateMapping(String schemaName, String discriminator, String paren
 .flatMap(s -> Optional.ofNullable(s.getDiscriminator()))
 .filter(d -> d.getPropertyName() != null);
 
-disc.ifPresent(d -> {
+disc.ifPresentOrElse(d -> {
 log.info("Updating {} discriminator mapping for {} with value '{}'", parentRef, schemaName, discriminator);
 d.mapping(discriminator, OasUtils.toSchemRef(schemaName));
-});
+}, () -> log.warn("Discriminator definition for '{}' not present at {}", schemaName, discriminator));
 }
 
-private Optional<String> discrininatorValue(Schema schema) {
+private Optional<String> discriminatorValue(Schema schema) {
 return Optional.ofNullable(schema.getExtensions())
 .flatMap(e -> Optional.ofNullable((String) e.get("x-discriminator-value")));
 }

src/test/java/com/amartus/sonata/blender/impl/ProductSpecReaderTest.java

@@ -32,6 +32,7 @@
 import java.util.stream.Stream;
 
 import static com.amartus.sonata.blender.impl.ProductSpecReader.DISCRIMINATOR_VALUE;
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 
@@ -53,6 +54,7 @@ public void testReaderForSchemaUsingRelativePaths() {
 var schemas = new ProductSpecReader("testToAugment", dirPath.resolve("model-js.json"))
 .readSchemas();
 singleRootSchema(schemas);
+validateDiscriminatedModel(schemas.get("model-js.json"), "model-js.json");
 assertEquals(7, schemas.size());
 }
 
@@ -62,6 +64,16 @@ public void testReaderForSchemaOas() {
 var schemas = new ProductSpecReader(ProductSpecReader.Options.forName("testToAugment"), dirPath.resolve("model-oas.yaml"), "#/components/schemas/ModelOAS", deserializerProvider, ProductSpecReader.defaultOptions())
 .readSchemas();
 singleRootSchema(schemas);
+validateDiscriminatedModel(schemas.get("ModelOAS"), "ModelOAS");
+assertEquals(7, schemas.size());
+}
+@Test
+public void testReaderForSchemaOasWithXDiscriminator() {
+var dirPath = Utils.toPath("mini-model");
+var schemas = new ProductSpecReader(ProductSpecReader.Options.forName("testToAugment"), dirPath.resolve("model-oas.yaml"), "#/components/schemas/ModelOASWithDiscriminator", deserializerProvider, ProductSpecReader.defaultOptions())
+.readSchemas();
+singleRootSchema(schemas);
+validateDiscriminatedModel(schemas.get("ModelOASWithDiscriminator"), "urn:mef:lso:spec:cantata-sonata:model-with-discriminator:v0.3.0:all");
 assertEquals(7, schemas.size());
 }
 
@@ -100,4 +112,12 @@ private Stream<String> markedSchemas(Map<String, Schema<?>> allSchemas) {
 .map(Map.Entry::getKey);
 }
 
+private void validateDiscriminatedModel(Schema<?> modelOAS, String discriminator) {
+assertThat(modelOAS)
+.isNotNull()
+.extracting(Schema::getExtensions)
+.satisfies(m -> assertThat(m).containsEntry("x-discriminator-value", discriminator));
+
+}
+
 }

src/test/java/com/amartus/sonata/blender/impl/ValidateSpecificationTest.java

@@ -32,20 +32,18 @@
 
 public class ValidateSpecificationTest {
 private static final Logger log = LoggerFactory.getLogger(ValidateSpecificationTest.class);
-private static final Path source;
+private static final Path source = classPath("/oas/test-spec.yaml");
+private Path target;
 
-static {
+private static Path classPath(String path) {
 try {
-//noinspection DataFlowIssue
-source = Paths.get(ValidateSpecificationTest.class.getResource("/oas/test-spec.yaml").toURI());
+return Paths.get(ValidateSpecificationTest.class.getResource(path).toURI());
 } catch (URISyntaxException e) {
 throw new RuntimeException(e);
 }
 }
 
-private Path target;
-
-private static Stream<String> args(Path target) {
+private static Stream<String> commonArgs(Path target) {
 var api = source.toAbsolutePath();
 var search = api.getParent();
 return Stream.of(
@@ -54,15 +52,41 @@ private static Stream<String> args(Path target) {
 api.toString(),
 "-d",
 search.toString(),
-"--model-name",
-"Placeholder",
-"--all-schemas",
-target.getParent().toAbsolutePath().toString(),
 "-o",
 target.toAbsolutePath().toString()
 );
 }
 
+private static Stream<String> args(Path target) {
+return args("Placeholder", target);
+}
+
+private static Stream<String> args(String model, Path target) {
+return Stream.concat(
+commonArgs(target),
+Stream.of(
+"--model-name",
+model,
+"--all-schemas",
+"all"
+)
+);
+}
+
+
+
+private static Stream<String> args(Path target, String file) {
+return Stream.concat(
+commonArgs(target),
+Stream.of(
+"--model-name",
+"Placeholder",
+"-b",
+file
+)
+);
+}
+
 @BeforeEach
 public void setup() throws IOException {
 var tempDir = Files.createTempDirectory("oas");
@@ -104,9 +128,32 @@ public void smokeTest() throws Exception {
 );
 }
 
+@Test
+public void checkFragmentSchema() throws Exception {
+var fileFragment = classPath("/mini-model/model-oas.yaml") + "#/components/schemas/ModelOASWithDiscriminator";
+Stream<String> args = args(target, fileFragment);
+
+var builder = builder();
+
+builder.build().parse(args.toArray(String[]::new)).run();
+
+var generated = SerializationUtils.yamlMapper().readTree(target.toFile());
+assertThatJson(generated)
+.inPath("$.components.schemas.ModelOASWithDiscriminator")
+.isObject()
+.satisfies(s ->
+assertThatJson(s).node("allOf[0].$ref").isEqualTo("#/components/schemas/Placeholder")
+);
+assertThatJson(generated)
+.inPath("$.components.schemas.Placeholder.discriminator.mapping")
+.isObject()
+.satisfies(e ->
+assertThat(e).containsEntry("urn:mef:lso:spec:cantata-sonata:model-with-discriminator:v0.3.0:all", "#/components/schemas/ModelOASWithDiscriminator"));
+}
+
 @Test
 public void strictModeSupportsInheritance() throws IOException {
-Stream<String> args = Stream.concat(args(target), Stream.of("--strict-mode"));
+Stream<String> args = Stream.concat(args("TargetParentName", target), Stream.of("--strict-mode"));
 var builder = builder();
 
 builder.build().parse(args.toArray(String[]::new)).run();
@@ -116,12 +163,12 @@ public void strictModeSupportsInheritance() throws IOException {
 .inPath("$.components.schemas.ToInject")
 .isObject()
 .satisfies(s ->
-assertThatJson(s).node("allOf[0].$ref").isEqualTo("#/components/schemas/Placeholder")
+assertThatJson(s).node("allOf[0].$ref").isEqualTo("#/components/schemas/TargetParentName")
 );
 }
 
 @Test
-public void doesNotHaveDefaultValuesForParameters() throws IOException {
+public void doesNotHaveDefaultValuesForParameters() {
 Stream<String> args = args(target);
 var builder = builder();
 builder.build().parse(args.toArray(String[]::new)).run();
@@ -143,6 +190,11 @@ public void respectTarget() throws Exception {
 .satisfies(s ->
 assertThatJson(s).node("allOf[0].$ref").isEqualTo("#/components/schemas/TargetParentName")
 );
+assertThatJson(generated)
+.inPath("$.components.schemas.TargetParentName.discriminator.mapping")
+.isObject()
+.satisfies(e ->
+assertThat(e).containsEntry("urn:mef:lso:spec:cantata-sonata:to-inject:v0.3.0:all", "#/components/schemas/ToInject"));
 }
 
 @Test