Modify testValidateParamValuesAreEqual() and test

Signed-off-by: Tianli Feng <[email protected]>
opensearch-project · Mar 16, 2022 · 02e68b5 · 02e68b5
1 parent 7195d42
commit 02e68b5
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 24 deletions.
diff --git a/server/src/main/java/org/opensearch/rest/RestRequest.java b/server/src/main/java/org/opensearch/rest/RestRequest.java
@@ -582,26 +582,27 @@ public static XContentType parseContentType(List<String> header) {
     /**
      * The method is only used to validate whether the values of the 2 request parameters "master_timeout" and "cluster_manager_timeout" is the same value or not.
      * If the 2 values are not the same, throw an {@link OpenSearchParseException}.
-     * @param keys Keys of the request parameters.
+     * @param keys Names of the request parameters.
      * @deprecated The method will be removed along with the request parameter "master_timeout".
      */
     @Deprecated
     public void validateParamValuesAreEqual(String... keys) {
-        // Filter the non-empty values of the parameters with the key, and get the distinct values.
-        Set<String> set = new HashSet<>();
+        // Track the last seen value and ensure that every subsequent value matches it.
+        // The value to be tracked is the non-empty values of the parameters with the key.
+        String lastSeenValue = null;
         for (String key : keys) {
             String value = param(key);
             if (!Strings.isNullOrEmpty(value)) {
-                set.add(value);
+                if (lastSeenValue == null || value.equals(lastSeenValue)) {
+                    lastSeenValue = value;
+                } else {
+                    throw new OpenSearchParseException(
+                        "The values of the request parameters: {} are required to be equal, otherwise please only assign value to one of the request parameters.",
+                        Arrays.toString(keys)
+                    );
+                }
             }
         }
-        // If there are more than 1 distinct value of the request parameters, throw an exception.
-        if (set.size() > 1) {
-            throw new OpenSearchParseException(
-                "The values of the request parameters: {} are required to be equal, otherwise please only assign value to one of the request parameters.",
-                Arrays.toString(keys)
-            );
-        }
     }
 
     public static class ContentTypeHeaderException extends RuntimeException {

diff --git a/server/src/test/java/org/opensearch/rest/RestRequestTests.java b/server/src/test/java/org/opensearch/rest/RestRequestTests.java
@@ -286,30 +286,38 @@ public void testRequiredContent() {
      * The test is added in 2.0 when the request parameter "cluster_manager_timeout" is introduced.
      * Remove the test along with the removal of the non-inclusive terminology "master_timeout".
      */
-    public void testValidateParamValuesAreEqual() {
+    public void testValidateParamValuesAreEqualWhenTheyAreEqual() {
         FakeRestRequest request = new FakeRestRequest();
-        List<String> valueList = new ArrayList<>(Arrays.asList(null, "", "value1", "value2"));
+        List<String> valueList = new ArrayList<>(Arrays.asList(null, "", "value1"));
         String valueForKey1 = randomFrom(valueList);
-        String valueForKey2 = randomFrom(valueList);
+        String valueForKey2 = "value1";
         request.params().put("key1", valueForKey1);
         request.params().put("key2", valueForKey2);
+        request.validateParamValuesAreEqual("key1", "key2");
+        assertTrue(
+            "Values of the 2 keys should be equal, or having a least 1 null value or empty String. Value of key1: "
+                + valueForKey1
+                + ". Value of key2: "
+                + valueForKey2,
+            Strings.isNullOrEmpty(valueForKey1) || valueForKey1.equals(valueForKey2)
+        );
+    }
+
+    /*
+     * The test is added in 2.0 when the request parameter "cluster_manager_timeout" is introduced.
+     * Remove the test along with the removal of the non-inclusive terminology "master_timeout".
+     */
+    public void testValidateParamValuesAreEqualWhenTheyAreNotEqual() {
+        FakeRestRequest request = new FakeRestRequest();
+        request.params().put("key1", "value1");
+        request.params().put("key2", "value2");
         try {
             request.validateParamValuesAreEqual("key1", "key2");
-            assertTrue(
-                "Values of the 2 keys should be equal, or having a least 1 null value or empty String. Value of key1: "
-                    + valueForKey1
-                    + ". Value of key2: "
-                    + valueForKey2,
-                Strings.isNullOrEmpty(valueForKey1) || Strings.isNullOrEmpty(valueForKey2) || valueForKey1.equals(valueForKey2)
-            );
         } catch (OpenSearchParseException e) {
             assertEquals(
                 "The values of the request parameters: [key1, key2] are required to be equal, otherwise please only assign value to one of the request parameters.",
                 e.getMessage()
             );
-            assertNotEquals(valueForKey1, valueForKey2);
-            assertFalse(Strings.isNullOrEmpty(valueForKey1));
-            assertFalse(Strings.isNullOrEmpty(valueForKey2));
         }
     }