[HLRC] Fix issue in equals for GlobalOperationPrivileges

This commit fixes an issue in the equals implementation for
GlobalOperationPrivileges and adds few tests.

Author: Yogesh Gaikwad

client/rest-high-level/src/main/java/org/elasticsearch/client/security/user/privileges/GlobalOperationPrivilege.java

@@ -19,6 +19,7 @@
 
 package org.elasticsearch.client.security.user.privileges;
 
+import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.xcontent.XContentParser;
 
 import java.io.IOException;
@@ -52,10 +53,16 @@ public class GlobalOperationPrivilege {
      *            The privilege definition.
      */
     public GlobalOperationPrivilege(String category, String operation, Map<String, Object> privilege) {
-        this.category = Objects.requireNonNull(category);
-        this.operation = Objects.requireNonNull(operation);
+        if (Strings.hasText(category) == false) {
+            throw new IllegalArgumentException("category is required");
+        }
+        this.category = category;
+        if (Strings.hasText(operation) == false) {
+            throw new IllegalArgumentException("operation is required");
+        }
+        this.operation = operation;
         if (privilege == null || privilege.isEmpty()) {
-            throw new IllegalArgumentException("Privileges cannot be empty or null");
+            throw new IllegalArgumentException("privileges cannot be empty or null");
         }
         this.privilege = Collections.unmodifiableMap(privilege);
     }
@@ -84,7 +91,7 @@ public boolean equals(Object o) {
         if (this == o) {
             return true;
         }
-        if (o == null || (false == this instanceof GlobalOperationPrivilege)) {
+        if (o == null || (this.getClass() != o.getClass())) {
             return false;
         }
         final GlobalOperationPrivilege that = (GlobalOperationPrivilege) o;

client/rest-high-level/src/main/java/org/elasticsearch/client/security/user/privileges/GlobalPrivileges.java

@@ -80,7 +80,7 @@ public final class GlobalPrivileges implements ToXContentObject {
      */
     public GlobalPrivileges(Collection<? extends GlobalOperationPrivilege> privileges) {
         if (privileges == null || privileges.isEmpty()) {
-            throw new IllegalArgumentException("Privileges cannot be empty or null");
+            throw new IllegalArgumentException("privileges cannot be empty or null");
         }
         // duplicates are just ignored
         this.privileges = Collections.unmodifiableSet(new HashSet<>(Objects.requireNonNull(privileges)));
@@ -91,7 +91,7 @@ public GlobalPrivileges(Collection<? extends GlobalOperationPrivilege> privilege
             final Set<String> allOperations = privilegesByCategory.getValue().stream().map(p -> p.getOperation())
                     .collect(Collectors.toSet());
             if (allOperations.size() != privilegesByCategory.getValue().size()) {
-                throw new IllegalArgumentException("Different privileges for the same category and operation are not permitted");
+                throw new IllegalArgumentException("different privileges for the same category and operation are not permitted");
             }
         }
     }

client/rest-high-level/src/test/java/org/elasticsearch/client/security/user/privileges/GlobalOperationPrivilegeTests.java

@@ -0,0 +1,84 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.client.security.user.privileges;
+
+import org.elasticsearch.common.Strings;
+import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.test.EqualsHashCodeTestUtils;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Map;
+
+import static org.hamcrest.Matchers.equalTo;
+
+public class GlobalOperationPrivilegeTests extends ESTestCase {
+
+    public void testConstructor() {
+        final String category = randomFrom(Arrays.asList("", "   ", null, randomAlphaOfLength(5)));
+        final String operation = randomFrom(Arrays.asList("", "   ", null, randomAlphaOfLength(5)));
+        final Map<String, Object> privilege = randomFrom(Arrays.asList(null, Collections.emptyMap(), Collections.singletonMap("k1", "v1")));
+
+        if (Strings.hasText(category) && Strings.hasText(operation) && privilege != null && privilege.isEmpty() == false) {
+            GlobalOperationPrivilege globalOperationPrivilege = new GlobalOperationPrivilege(category, operation, privilege);
+            assertThat(globalOperationPrivilege.getCategory(), equalTo(category));
+            assertThat(globalOperationPrivilege.getOperation(), equalTo(operation));
+            assertThat(globalOperationPrivilege.getRaw(), equalTo(privilege));
+        } else {
+            final IllegalArgumentException ile = expectThrows(IllegalArgumentException.class,
+                    () -> new GlobalOperationPrivilege(category, operation, privilege));
+            if (Strings.hasText(category) == false) {
+                assertThat(ile.getMessage(), equalTo("category is required"));
+            } else if (Strings.hasText(operation) == false) {
+                assertThat(ile.getMessage(), equalTo("operation is required"));
+            } else {
+                assertThat(ile.getMessage(), equalTo("privileges cannot be empty or null"));
+            }
+        }
+    }
+
+    public void testEqualsHashCode() {
+        final String category = randomAlphaOfLength(5);
+        final String operation = randomAlphaOfLength(5);
+        final Map<String, Object> privilege = Collections.singletonMap(randomAlphaOfLength(4), randomAlphaOfLength(5));
+        GlobalOperationPrivilege globalOperationPrivilege = new GlobalOperationPrivilege(category, operation, privilege);
+
+        EqualsHashCodeTestUtils.checkEqualsAndHashCode(globalOperationPrivilege, (original) -> {
+            return new GlobalOperationPrivilege(original.getCategory(), original.getOperation(), original.getRaw());
+        });
+        EqualsHashCodeTestUtils.checkEqualsAndHashCode(globalOperationPrivilege, (original) -> {
+            return new GlobalOperationPrivilege(original.getCategory(), original.getOperation(), original.getRaw());
+        }, GlobalOperationPrivilegeTests::mutateTestItem);
+    }
+
+    private static GlobalOperationPrivilege mutateTestItem(GlobalOperationPrivilege original) {
+        switch (randomIntBetween(0, 2)) {
+        case 0:
+            return new GlobalOperationPrivilege(randomAlphaOfLength(5), original.getOperation(), original.getRaw());
+        case 1:
+            return new GlobalOperationPrivilege(original.getCategory(), randomAlphaOfLength(5), original.getRaw());
+        case 2:
+            return new GlobalOperationPrivilege(original.getCategory(), original.getOperation(),
+                    Collections.singletonMap(randomAlphaOfLength(4), randomAlphaOfLength(4)));
+        default:
+            return new GlobalOperationPrivilege(randomAlphaOfLength(5), original.getOperation(), original.getRaw());
+        }
+    }
+}

client/rest-high-level/src/test/java/org/elasticsearch/client/security/user/privileges/GlobalPrivilegesTests.java

@@ -21,6 +21,7 @@
 
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.test.AbstractXContentTestCase;
+import org.elasticsearch.test.EqualsHashCodeTestUtils;
 
 import java.io.IOException;
 import java.util.Arrays;
@@ -56,13 +57,13 @@ public void testEmptyOrNullGlobalOperationPrivilege() {
         final Map<String, Object> privilege = randomBoolean() ? null : Collections.emptyMap();
         final IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
                 () -> new GlobalOperationPrivilege(randomAlphaOfLength(2), randomAlphaOfLength(2), privilege));
-        assertThat(e.getMessage(), is("Privileges cannot be empty or null"));
+        assertThat(e.getMessage(), is("privileges cannot be empty or null"));
     }
 
     public void testEmptyOrNullGlobalPrivileges() {
         final List<GlobalOperationPrivilege> privileges = randomBoolean() ? null : Collections.emptyList();
         final IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> new GlobalPrivileges(privileges));
-        assertThat(e.getMessage(), is("Privileges cannot be empty or null"));
+        assertThat(e.getMessage(), is("privileges cannot be empty or null"));
     }
 
     public void testDuplicateGlobalOperationPrivilege() {
@@ -81,7 +82,7 @@ public void testSameScopeGlobalOperationPrivilege() {
                 privilege.getOperation(), buildRandomGlobalScopedPrivilege().getRaw());
         IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
                 () -> new GlobalPrivileges(Arrays.asList(privilege, sameOperationPrivilege)));
-        assertThat(e.getMessage(), is("Different privileges for the same category and operation are not permitted"));
+        assertThat(e.getMessage(), is("different privileges for the same category and operation are not permitted"));
     }
 
     private static GlobalOperationPrivilege buildRandomGlobalScopedPrivilege() {
@@ -91,4 +92,21 @@ private static GlobalOperationPrivilege buildRandomGlobalScopedPrivilege() {
         }
         return new GlobalOperationPrivilege("application", randomAlphaOfLength(2) + idCounter++, privilege);
     }
+
+    public void testEqualsHashCode() {
+        final List<GlobalOperationPrivilege> privilegeList = Arrays
+                .asList(randomArray(1, 4, size -> new GlobalOperationPrivilege[size], () -> buildRandomGlobalScopedPrivilege()));
+        GlobalPrivileges globalPrivileges = new GlobalPrivileges(privilegeList);
+
+        EqualsHashCodeTestUtils.checkEqualsAndHashCode(globalPrivileges, (original) -> {
+            return new GlobalPrivileges(original.getPrivileges());
+        });
+        EqualsHashCodeTestUtils.checkEqualsAndHashCode(globalPrivileges, (original) -> {
+            return new GlobalPrivileges(original.getPrivileges());
+        }, (original) -> {
+            final List<GlobalOperationPrivilege> newList = Arrays
+                    .asList(randomArray(1, 4, size -> new GlobalOperationPrivilege[size], () -> buildRandomGlobalScopedPrivilege()));
+            return new GlobalPrivileges(newList);
+        });
+    }
 }