After specs comparison, all parameters are removed from new spec (#453)

Co-authored-by: justinas.bardauskas <justinas.bardauskas@multitude.com>

Author: justinasbardauskas

core/src/main/java/org/openapitools/openapidiff/core/compare/ParametersDiff.java

@@ -3,6 +3,7 @@
 import io.swagger.v3.oas.models.Components;
 import io.swagger.v3.oas.models.parameters.Parameter;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
@@ -16,6 +17,7 @@
 
 /** compare two parameter */
 public class ParametersDiff {
+
   private static final RefPointer<Parameter> refPointer = new RefPointer<>(RefType.PARAMETERS);
 
   private final Components leftComponents;
@@ -47,29 +49,27 @@ public static boolean same(Parameter left, Parameter right) {
   }
 
   public DeferredChanged<ChangedParameters> diff(
-      List<Parameter> left, List<Parameter> right, DiffContext context) {
-
-    DeferredBuilder<Changed> builder = new DeferredBuilder<>();
-    ChangedParameters changedParameters =
-        new ChangedParameters(left, right != null ? new ArrayList<>(right) : null, context);
-    if (null == left) left = new ArrayList<>();
-    if (null == right) right = new ArrayList<>();
+      final List<Parameter> left, final List<Parameter> right, final DiffContext context) {
+    final DeferredBuilder<Changed> builder = new DeferredBuilder<>();
+    final List<Parameter> wLeft = Optional.ofNullable(left).orElseGet(Collections::emptyList);
+    final List<Parameter> wRight = Optional.ofNullable(right).map(ArrayList::new).orElseGet(ArrayList::new);
 
-    for (Parameter leftPara : left) {
-      leftPara = refPointer.resolveRef(leftComponents, leftPara, leftPara.get$ref());
+    final ChangedParameters changedParameters = new ChangedParameters(wLeft, wRight, context);
 
-      Optional<Parameter> rightParam = contains(rightComponents, right, leftPara);
-      if (!rightParam.isPresent()) {
-        changedParameters.getMissing().add(leftPara);
+    for (Parameter leftParam : wLeft) {
+      leftParam = refPointer.resolveRef(leftComponents, leftParam, leftParam.get$ref());
+      Optional<Parameter> rightParamOpt = contains(rightComponents, wRight, leftParam);
+      if (!rightParamOpt.isPresent()) {
+        changedParameters.getMissing().add(leftParam);
       } else {
-        Parameter rightPara = rightParam.get();
-        right.remove(rightPara);
+        Parameter rightParam = rightParamOpt.get();
+        wRight.remove(rightParam);
         builder
-            .with(openApiDiff.getParameterDiff().diff(leftPara, rightPara, context))
+            .with(openApiDiff.getParameterDiff().diff(leftParam, rightParam, context))
             .ifPresent(changedParameters.getChanged()::add);
       }
     }
-    changedParameters.getIncreased().addAll(right);
+    changedParameters.getIncreased().addAll(wRight);
 
     return builder.buildIsChanged(changedParameters);
   }

core/src/test/java/org/openapitools/openapidiff/core/OpenApiDiffTest.java

@@ -3,6 +3,9 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.openapitools.openapidiff.core.TestUtils.assertOpenApiAreEquals;
 
+import io.swagger.parser.OpenAPIParser;
+import io.swagger.v3.oas.models.OpenAPI;
+import io.swagger.v3.parser.core.models.ParseOptions;
 import java.io.FileWriter;
 import java.io.IOException;
 import java.nio.file.Path;
@@ -11,6 +14,7 @@
 import org.junit.jupiter.api.io.TempDir;
 import org.openapitools.openapidiff.core.model.ChangedOpenApi;
 import org.openapitools.openapidiff.core.model.ChangedOperation;
+import org.openapitools.openapidiff.core.model.DiffResult;
 import org.openapitools.openapidiff.core.model.Endpoint;
 import org.openapitools.openapidiff.core.output.HtmlRender;
 import org.openapitools.openapidiff.core.output.JsonRender;
@@ -21,6 +25,9 @@ public class OpenApiDiffTest {
   private final String OPENAPI_DOC1 = "petstore_v2_1.yaml";
   private final String OPENAPI_DOC2 = "petstore_v2_2.yaml";
   private final String OPENAPI_EMPTY_DOC = "petstore_v2_empty.yaml";
+  private final String OPENAPI_DOC3 = "petstore_openapi3.yaml";
+
+  private static final OpenAPIParser PARSER = new OpenAPIParser();
 
   @Test
   public void testEqual() {
@@ -104,4 +111,41 @@ public void testDiffAndJson(@TempDir Path tempDir) throws IOException {
     }
     assertThat(path).isNotEmptyFile();
   }
+
+  /** Testing that repetitive specs comparisons has to produce consistent result. */
+  @Test
+  public void testComparisonConsistency() {
+    final OpenAPI oldSpec = loadSpecFromFile(OPENAPI_DOC3);
+    final OpenAPI newSpec = loadSpecFromFile(OPENAPI_DOC3);
+
+    final ChangedOpenApi diff1 = OpenApiCompare.fromSpecifications(oldSpec, newSpec);
+    assertThat(diff1.isChanged()).isEqualTo(DiffResult.NO_CHANGES);
+    assertThat(diff1.getNewEndpoints()).isEmpty();
+    assertThat(diff1.getMissingEndpoints()).isEmpty();
+    assertThat(diff1.getChangedOperations()).isEmpty();
+
+    final ChangedOpenApi diff2 = OpenApiCompare.fromSpecifications(oldSpec, newSpec);
+    assertThat(diff2.isChanged()).isEqualTo(DiffResult.NO_CHANGES);
+    assertThat(diff2.getNewEndpoints()).isEmpty();
+    assertThat(diff2.getMissingEndpoints()).isEmpty();
+    assertThat(diff2.getChangedOperations()).isEmpty();
+  }
+
+  @Test
+  public void testSpecObjectsAreNotChangesAfterComparison() {
+    final OpenAPI oldSpec = loadSpecFromFile(OPENAPI_DOC3);
+    final OpenAPI newSpec = loadSpecFromFile(OPENAPI_DOC3);
+
+    OpenApiCompare.fromSpecifications(oldSpec, newSpec);
+    OpenApiCompare.fromSpecifications(oldSpec, newSpec);
+
+    final OpenAPI expectedOldSpec = loadSpecFromFile(OPENAPI_DOC3);
+    final OpenAPI expectedNewSpec = loadSpecFromFile(OPENAPI_DOC3);
+    assertThat(oldSpec).isEqualTo(expectedOldSpec);
+    assertThat(newSpec).isEqualTo(expectedNewSpec);
+  }
+
+  private static OpenAPI loadSpecFromFile(String specFile) {
+    return PARSER.readLocation(specFile, null, new ParseOptions()).getOpenAPI();
+  }
 }