Issue #49: Fixed bug where FindParameter does not consider repeat par…

…ameters (#50)

* Fixed comparator bug
* Fixed optimization bug
* Added a second test for out_of_order_parameters

src/Criteo.OpenApi.Comparator.UTest/Resource/issue_49_false_1015_equal_schemas/diff.json

@@ -0,0 +1,13 @@
+[
+  {
+    "Severity": "Info",
+    "Message": "The versions have not changed.",
+    "OldJsonRef": "#/info/version",
+    "NewJsonRef": "#/info/version",
+    "OldJsonPath": "#/info/version",
+    "NewJsonPath": "#/info/version",
+    "Id": 1001,
+    "Code": "NoVersionChange",
+    "Mode": "Update"
+  }
+]

src/Criteo.OpenApi.Comparator.UTest/Resource/issue_49_false_1015_equal_schemas/new.yaml

@@ -0,0 +1,27 @@
+openapi: "3.0.2"
+info:
+  title: "Api"
+  version: "1.0.1"
+paths:
+  /example:
+    get:
+      parameters:
+        - name: "access_token"
+          in: "query"
+          required: false
+          schema:
+            anyOf:
+              - type: "string"
+              - type: "null"
+            title: "Access Token"
+        - name: "access_token"
+          in: "header"
+          required: false
+          schema:
+            anyOf:
+              - type: "string"
+              - type: "null"
+            title: "Access Token"
+      responses:
+        "200":
+          description: "Successful Response"

src/Criteo.OpenApi.Comparator.UTest/Resource/issue_49_false_1015_equal_schemas/old.yaml

@@ -0,0 +1,27 @@
+openapi: "3.0.2"
+info:
+  title: "Api"
+  version: "1.0.0"
+paths:
+  /example:
+    get:
+      parameters:
+        - name: "access_token"
+          in: "query"
+          required: false
+          schema:
+            anyOf:
+              - type: "string"
+              - type: "null"
+            title: "Access Token"
+        - name: "access_token"
+          in: "header"
+          required: false
+          schema:
+            anyOf:
+              - type: "string"
+              - type: "null"
+            title: "Access Token"
+      responses:
+        "200":
+          description: "Successful Response"

src/Criteo.OpenApi.Comparator.UTest/Resource/issue_49_false_1015_out_of_order_parameters/diff.json

@@ -0,0 +1,35 @@
+[
+  {
+    "Severity": "Info",
+    "Message": "The versions have not changed.",
+    "OldJsonRef": "#/info/version",
+    "NewJsonRef": "#/info/version",
+    "OldJsonPath": "#/info/version",
+    "NewJsonPath": "#/info/version",
+    "Id": 1001,
+    "Code": "NoVersionChange",
+    "Mode": "Update"
+  },
+  {
+    "Severity": "Error",
+    "Message": "The order of parameter 'access_token' was changed. ",
+    "OldJsonRef": "#/paths/~1example/get/parameters",
+    "NewJsonRef": "#/paths/~1example/get/parameters",
+    "OldJsonPath": "#/paths/~1example/get/parameters",
+    "NewJsonPath": "#/paths/~1example/get/parameters",
+    "Id": 1042,
+    "Code": "ChangedParameterOrder",
+    "Mode": "Update"
+  },
+  {
+    "Severity": "Error",
+    "Message": "The order of parameter 'access_token' was changed. ",
+    "OldJsonRef": "#/paths/~1example/get/parameters",
+    "NewJsonRef": "#/paths/~1example/get/parameters",
+    "OldJsonPath": "#/paths/~1example/get/parameters",
+    "NewJsonPath": "#/paths/~1example/get/parameters",
+    "Id": 1042,
+    "Code": "ChangedParameterOrder",
+    "Mode": "Update"
+  }
+]

src/Criteo.OpenApi.Comparator.UTest/Resource/issue_49_false_1015_out_of_order_parameters/new.yaml

@@ -0,0 +1,27 @@
+openapi: "3.0.2"
+info:
+  title: "Api"
+  version: "1.0.1"
+paths:
+  /example:
+    get:
+      parameters:
+        - name: "access_token"
+          in: "query"
+          required: false
+          schema:
+            anyOf:
+              - type: "string"
+              - type: "null"
+            title: "Access Token"
+        - name: "access_token"
+          in: "header"
+          required: false
+          schema:
+            anyOf:
+              - type: "string"
+              - type: "null"
+            title: "Access Token"
+      responses:
+        "200":
+          description: "Successful Response"

src/Criteo.OpenApi.Comparator.UTest/Resource/issue_49_false_1015_out_of_order_parameters/old.yaml

@@ -0,0 +1,27 @@
+openapi: "3.0.2"
+info:
+  title: "Api"
+  version: "1.0.0"
+paths:
+  /example:
+    get:
+      parameters:
+        - name: "access_token"
+          in: "header"
+          required: false
+          schema:
+            anyOf:
+              - type: "string"
+              - type: "null"
+            title: "Access Token"
+        - name: "access_token"
+          in: "query"
+          required: false
+          schema:
+            anyOf:
+              - type: "string"
+              - type: "null"
+            title: "Access Token"
+      responses:
+        "200":
+          description: "Successful Response"

src/Criteo.OpenApi.Comparator/Comparators/OperationComparator.cs

@@ -127,7 +127,7 @@ private void CheckRequiredParametersRemoval(ComparisonContext context,
         {
             foreach (var oldParameter in oldParameters)
             {
-                var newParameter = FindParameter(oldParameter.Name, newParameters, context.NewOpenApiDocument.Components?.Parameters);
+                var newParameter = FindParameter(oldParameter, newParameters, context.NewOpenApiDocument.Components?.Parameters);
 
                 // we should use PushItemByName instead of PushProperty because Swagger `parameters` is
                 // an array of parameters.
@@ -163,7 +163,7 @@ private static void CheckRequiredParametersAddition(ComparisonContext context,
             foreach (var newParameter in newParameters)
             {
                 OpenApiParameter oldParameter = FindParameter(
-                    newParameter.Name,
+                    newParameter,
                     oldParameters,
                     context.OldOpenApiDocument.Components?.Parameters
                 );
@@ -257,30 +257,75 @@ private static int FindParameterIndex(OpenApiParameter parameter, IList<OpenApiP
         /// <summary>
         /// Finds parameter name in the list of operation parameters or global parameters
         /// </summary>
-        /// <param name="name">name of the parameter to search</param>
+        /// <param name="key">The parameter used as a key. It is expected that this parameter comes from the opposite
+        /// file. The parameter that has the same name and is most similar to this parameter will be returned</param>
         /// <param name="operationParameters">list of operation parameters to search</param>
         /// <param name="documentParameters">Dictionary of global parameters to search</param>
         /// <returns>Swagger Parameter if found; otherwise null</returns>
         private static OpenApiParameter FindParameter(
-            string name,
+            OpenApiParameter key,
             IEnumerable<OpenApiParameter> operationParameters,
             IDictionary<string, OpenApiParameter> documentParameters)
         {
+            string name = key.Name;
             if (name == null || operationParameters == null)
                 return null;
 
+            var candidateParameters = new List<OpenApiParameter>();
             foreach (var parameter in operationParameters)
             {
                 if (name.Equals(parameter.Name))
-                    return parameter;
+                    candidateParameters.Add(parameter);
+                else
+                {
+                    var referencedParameter = parameter.Reference.Resolve(documentParameters);
 
-                var referencedParameter = parameter.Reference.Resolve(documentParameters);
+                    if (referencedParameter != null && name.Equals(referencedParameter.Name))
+                        candidateParameters.Add(referencedParameter);
+                }
+            }
 
-                if (referencedParameter != null && name.Equals(referencedParameter.Name))
-                    return referencedParameter;
+            if (!candidateParameters.Any())
+                return null;
+
+            if(candidateParameters.Count == 1)
+                return candidateParameters[0];
+
+            return GetClosestFunctionally(key, candidateParameters);
+        }
+
+        /// <summary>
+        /// Determines the most similar parameter to the key parameter based on functional similarity.
+        /// </summary>
+        /// <param name="key">The parameter to score against for functional similarity</param>
+        /// <param name="candidateParameters">The candidates. One of these will be selected as the most similar</param>
+        /// <returns>The most similar parameter functionally to the key parameter</returns>
+        private static OpenApiParameter GetClosestFunctionally(OpenApiParameter key, List<OpenApiParameter> candidateParameters)
+        {
+            OpenApiParameter bestMatch = null;
+            var bestMatchScore = 0;
+
+            foreach (var candidate in candidateParameters)
+            {
+                var score = 0;
+
+                score += key.Reference == candidate.Reference ? 1 : 0;
+                score += key.In == candidate.In ? 1 : 0;
+                score += key.Required == candidate.Required ? 1 : 0;
+                score += key.Deprecated == candidate.Deprecated ? 1 : 0;
+                score += key.AllowEmptyValue == candidate.AllowEmptyValue ? 1 : 0;
+                score += key.Style == candidate.Style ? 1 : 0;
+                score += key.Explode == candidate.Explode ? 1 : 0;
+                score += key.AllowReserved == candidate.AllowReserved ? 1 : 0;
+
+                if (score <= bestMatchScore)
+                    continue;
+
+                bestMatch = candidate;
+                bestMatchScore = score;
             }
 
-            return null;
+            return bestMatch;
         }
     }
 }