Address PR comments:

1.Remove unused import 2.Edit incorrect merge 3.Adress minor method and variable renaming
opensearch-project · Jul 23, 2021 · b10d4f3 · b10d4f3
1 parent 16c8ce6
commit b10d4f3
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 12 deletions.
diff --git a/...ain/java/org/opensearch/security/dlic/rest/validation/AbstractConfigurationValidator.java b/...ain/java/org/opensearch/security/dlic/rest/validation/AbstractConfigurationValidator.java
@@ -22,7 +22,6 @@
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Set;
-import java.util.Iterator;
 
 import org.opensearch.security.support.ConfigConstants;
 import org.opensearch.security.DefaultObjectMapper;
@@ -41,7 +40,6 @@
 import com.fasterxml.jackson.core.JsonParser;
 import com.fasterxml.jackson.core.JsonToken;
 import com.fasterxml.jackson.databind.JsonNode;
-import com.fasterxml.jackson.databind.node.ArrayNode;
 import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableList;
 
@@ -194,7 +192,7 @@ public boolean validate() {
         for (Entry<String, DataType> allowedKey : allowedKeys.entrySet()) {
             JsonNode value = contentAsNode.get(allowedKey.getKey());
             if (value != null) {
-                if (hasNullElement(value)) {
+                if (hasNullArrayElement(value)) {
                     this.errorType = ErrorType.NULL_ARRAY_ELEMENT;
                     return false;
                 }
@@ -315,13 +313,15 @@ protected final boolean hasParams() {
         return param != null && param.length > 0;
     }
 
-    private boolean hasNullElement(JsonNode node) {
-        for (JsonNode jsonNode: node) {
-            if(jsonNode.isNull() && node.isArray()) {
-                return true;
+    private boolean hasNullArrayElement(JsonNode node) {
+        for (JsonNode element: node) {
+            if(element.isNull()) {
+                if (node.isArray())
+                    return true;
             }
-            else {
-                if (hasNullElement(jsonNode)) return true;
+            else if (element.isContainerNode()) {
+                if (hasNullArrayElement(element))
+                    return true;
             }
         }
         return false;

diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/RolesApiTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/RolesApiTest.java
@@ -15,7 +15,6 @@
 
 package org.opensearch.security.dlic.rest.api;
 
-import java.util.Arrays;
 import java.util.List;
 
 import org.junit.runner.RunWith;
@@ -489,7 +488,7 @@ public void testRolesApiForNonSuperAdmin() throws Exception {
         Assert.assertEquals(HttpStatus.SC_FORBIDDEN, response.getStatusCode());
 
         // Put read only roles
-        response = rh.executePutRequest("/_opendistro/_security/api/roles/opendistro_security_transport_client",
+        response = rh.executePutRequest( ENDPOINT + "/roles/opendistro_security_transport_client",
                                         FileHelper.loadFile("restapi/roles_captains.json"), new Header[0]);
         Assert.assertEquals(HttpStatus.SC_FORBIDDEN, response.getStatusCode());
 
@@ -511,7 +510,7 @@ public void testRolesApiForNonSuperAdmin() throws Exception {
 
         // put hidden role
         String body = FileHelper.loadFile("restapi/roles_captains.json");
-        response = rh.executePutRequest("/_opendistro/_security/api/roles/opendistro_security_internal", body, new Header[0]);
+        response = rh.executePutRequest( ENDPOINT+ "/roles/opendistro_security_internal", body, new Header[0]);
         Assert.assertEquals(org.apache.http.HttpStatus.SC_NOT_FOUND, response.getStatusCode());
 
         // Patch single hidden roles