IGNITE-28944 Refactor SecurityBasicPermissionSet

modules/core/src/main/java/org/apache/ignite/internal/processors/security/SecurityUtils.java

@@ -32,8 +32,10 @@
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Set;
 import java.util.UUID;
 import java.util.concurrent.Callable;
 import org.apache.ignite.IgniteCheckedException;
@@ -131,13 +133,13 @@ public static void restoreDefaultSerializeVersion() {
     /**
      * @return Allow all service permissions.
      */
-    public static Map<String, Collection<SecurityPermission>> compatibleServicePermissions() {
-        Map<String, Collection<SecurityPermission>> srvcPerms = new HashMap<>();
+    public static Map<String, Set<SecurityPermission>> compatibleServicePermissions() {
+        Map<String, Set<SecurityPermission>> srvcPerms = new HashMap<>();
 
-        srvcPerms.put("*", Arrays.asList(
+        srvcPerms.put("*", new HashSet<>(Arrays.asList(
             SecurityPermission.SERVICE_CANCEL,
             SecurityPermission.SERVICE_DEPLOY,
-            SecurityPermission.SERVICE_INVOKE));
+            SecurityPermission.SERVICE_INVOKE)));
 
         return srvcPerms;
     }
@@ -366,7 +368,8 @@ public static void authorizeAll(IgniteSecurity security, SecurityPermissionSet p
     }
 
     /** */
-    private static void authorizeAll(IgniteSecurity security, Map<String, Collection<SecurityPermission>> permissions) {
+    private static void authorizeAll(IgniteSecurity security,
+        Map<String, ? extends Collection<SecurityPermission>> permissions) {
         if (F.isEmpty(permissions))
             return;
 

modules/core/src/main/java/org/apache/ignite/plugin/security/SecurityBasicPermissionSet.java

@@ -23,8 +23,11 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Set;
+import java.util.stream.Collectors;
 import org.apache.ignite.internal.Order;
 import org.apache.ignite.internal.util.tostring.GridToStringInclude;
 import org.apache.ignite.internal.util.typedef.internal.A;
@@ -47,24 +50,24 @@ public class SecurityBasicPermissionSet implements SecurityPermissionSet {
     /** Cache permissions. */
     @GridToStringInclude
     @Order(0)
-    Map<String, Collection<SecurityPermission>> cachePermissions = new HashMap<>();
+    Map<String, Set<SecurityPermission>> cachePermissions = new HashMap<>();
 
     /** Task permissions. */
     @GridToStringInclude
     @Order(1)
-    Map<String, Collection<SecurityPermission>> taskPermissions = new HashMap<>();
+    Map<String, Set<SecurityPermission>> taskPermissions = new HashMap<>();
 
     /** Service permissions. */
     @GridToStringInclude
     @Order(2)
-    transient Map<String, Collection<SecurityPermission>> srvcPermissions = isSecurityCompatibilityMode()
+    transient Map<String, Set<SecurityPermission>> srvcPermissions = isSecurityCompatibilityMode()
             ? compatibleServicePermissions()
             : new HashMap<>();
 
     /** System permissions. */
     @GridToStringInclude
     @Order(3)
-    Collection<SecurityPermission> sysPermissions;
+    Set<SecurityPermission> sysPermissions;
 
     /** Default allow all. */
     @Order(4)
@@ -78,7 +81,7 @@ public class SecurityBasicPermissionSet implements SecurityPermissionSet {
     public void setCachePermissions(Map<String, Collection<SecurityPermission>> cachePermissions) {
         A.notNull(cachePermissions, "cachePermissions");
 
-        this.cachePermissions = cachePermissions;
+        this.cachePermissions = toHashSetMap(cachePermissions);
     }
 
     /**
@@ -89,7 +92,7 @@ public void setCachePermissions(Map<String, Collection<SecurityPermission>> cach
     public void setTaskPermissions(Map<String, Collection<SecurityPermission>> taskPermissions) {
         A.notNull(taskPermissions, "taskPermissions");
 
-        this.taskPermissions = taskPermissions;
+        this.taskPermissions = toHashSetMap(taskPermissions);
     }
 
     /**
@@ -100,7 +103,23 @@ public void setTaskPermissions(Map<String, Collection<SecurityPermission>> taskP
     public void setServicePermissions(Map<String, Collection<SecurityPermission>> srvcPermissions) {
         A.notNull(taskPermissions, "servicePermissions");
 
-        this.srvcPermissions = srvcPermissions;
+        this.srvcPermissions = toHashSetMap(srvcPermissions);
+    }
+
+    /**
+     * Copies content to a form with indemponent `hashCode` and `equals` results.
+     *
+     * @param cachePermissions Cache permissions.
+     * @return Map with hash set of security permissions.
+     */
+    private Map<String, Set<SecurityPermission>> toHashSetMap(Map<String, Collection<SecurityPermission>> cachePermissions) {
+        return cachePermissions.entrySet().stream()
+            .collect(Collectors.toMap(Map.Entry::getKey, e -> toHashSet(e.getValue())));
+    }
+
+    /** @return Hash set with permissions. */
+    private Set<SecurityPermission> toHashSet(Collection<SecurityPermission> col) {
+        return col instanceof HashSet<SecurityPermission> ? (HashSet<SecurityPermission>)col : new HashSet<>(col);
     }
 
     /**
@@ -109,7 +128,7 @@ public void setServicePermissions(Map<String, Collection<SecurityPermission>> sr
      * @param sysPermissions System permissions.
      */
     public void setSystemPermissions(Collection<SecurityPermission> sysPermissions) {
-        this.sysPermissions = sysPermissions;
+        this.sysPermissions = new HashSet<>(sysPermissions);
     }
 
     /**
@@ -122,17 +141,17 @@ public void setDefaultAllowAll(boolean dfltAllowAll) {
     }
 
     /** {@inheritDoc} */
-    @Override public Map<String, Collection<SecurityPermission>> cachePermissions() {
+    @Override public Map<String, ? extends Collection<SecurityPermission>> cachePermissions() {
         return cachePermissions;
     }
 
     /** {@inheritDoc} */
-    @Override public Map<String, Collection<SecurityPermission>> taskPermissions() {
+    @Override public Map<String, ? extends Collection<SecurityPermission>> taskPermissions() {
         return taskPermissions;
     }
 
     /** {@inheritDoc} */
-    @Override public Map<String, Collection<SecurityPermission>> servicePermissions() {
+    @Override public Map<String, ? extends Collection<SecurityPermission>> servicePermissions() {
         return srvcPermissions;
     }
 

modules/core/src/main/java/org/apache/ignite/plugin/security/SecurityPermissionSet.java

@@ -48,23 +48,23 @@ public interface SecurityPermissionSet extends Serializable, Message {
      *
      * @return Map of task names to task permissions.
      */
-    public Map<String, Collection<SecurityPermission>> taskPermissions();
+    public Map<String, ? extends Collection<SecurityPermission>> taskPermissions();
 
     /**
      * Map of cache names to cache permissions. Wildcards are allowed at the
      * end of cache names.
      *
      * @return Map of cache names to cache permissions.
      */
-    public Map<String, Collection<SecurityPermission>> cachePermissions();
+    public Map<String, ? extends Collection<SecurityPermission>> cachePermissions();
 
     /**
      * Map of service names to service permissions. Wildcards are allowed at the
      * end of service names.
      *
      * @return Map of service names to service permissions.
      */
-    public Map<String, Collection<SecurityPermission>> servicePermissions();
+    public Map<String, ? extends Collection<SecurityPermission>> servicePermissions();
 
     /**
      * Collection of system-wide permissions (events enable/disable, Visor task execution).

modules/core/src/test/java/org/apache/ignite/internal/processors/security/impl/TestSecurityProcessor.java

@@ -261,7 +261,7 @@ public static boolean contains(SecurityPermissionSet userPerms, String name, Sec
 
     /** */
     private static boolean contains(
-        Map<String, Collection<SecurityPermission>> userPerms,
+        Map<String, ? extends Collection<SecurityPermission>> userPerms,
         boolean dfltAllowAll,
         String name,
         SecurityPermission perm

modules/core/src/test/java/org/apache/ignite/plugin/security/SecurityBasicPermissionSetSerializationTest.java

@@ -18,7 +18,6 @@
 package org.apache.ignite.plugin.security;
 
 import java.nio.ByteBuffer;
-import java.util.Collection;
 import java.util.Map;
 import java.util.Set;
 import org.apache.ignite.IgniteCheckedException;
@@ -34,7 +33,6 @@
 import org.apache.ignite.testframework.GridTestUtils;
 import org.apache.ignite.testframework.junits.GridTestKernalContext;
 import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
-import org.jetbrains.annotations.Nullable;
 import org.junit.Test;
 
 import static org.apache.ignite.internal.util.CommonUtils.makeMessageType;
@@ -65,10 +63,11 @@ public void testWriteAndReadBack() throws Exception {
         src.setServicePermissions(Map.of("service", Set.of(SERVICE_INVOKE, SERVICE_CANCEL)));
         src.setCachePermissions(Map.of("cache", Set.of(CACHE_CREATE, CACHE_PUT)));
 
-        src.setCachePermissions(Map.of("cache", Set.of(CACHE_CREATE, CACHE_PUT)));
         SecurityBasicPermissionSet res = writeAndReadBack(src);
 
-        assertTrue("Permission sets are not equal [src=" + src + ", res=" + res + "]", deepEquals(src, res));
+        assertEquals("Permission sets are not equal", src, res);
+        assertEquals("Hashes of permission sets are not equal [src=" + src + ", res=" + res + "]",
+            src.hashCode(), res.hashCode());
     }
 
     /**
@@ -100,50 +99,4 @@ private <T extends Message> T writeAndReadBack(T msg) throws IgniteCheckedExcept
 
         return res;
     }
-
-    /**
-     * Perfroms deep equals of permission sets.
-     *
-     * @param lhs First permissions set for equality check.
-     * @param rhs Second permissions set for equality check.
-     * @return Whether specified permission sets are equal.
-     */
-    public static boolean deepEquals(SecurityPermissionSet lhs, SecurityPermissionSet rhs) {
-        if (lhs == rhs)
-            return true;
-
-        return lhs != null
-            && rhs != null
-            && lhs.defaultAllowAll() == rhs.defaultAllowAll()
-            && (F.isEmpty(rhs.systemPermissions()) && F.isEmpty(rhs.systemPermissions())
-            || F.eqNotOrdered(rhs.systemPermissions(), lhs.systemPermissions()))
-            && eqNotOrdered(rhs.taskPermissions(), lhs.taskPermissions())
-            && eqNotOrdered(rhs.servicePermissions(), lhs.servicePermissions())
-            && eqNotOrdered(rhs.cachePermissions(), lhs.cachePermissions());
-    }
-
-    /**
-     * @param m1 First map to check.
-     * @param m2 Second map to check
-     * @return {@code True} is maps are equal, {@code False} otherwise.
-     */
-    public static boolean eqNotOrdered(
-        @Nullable Map<String, Collection<SecurityPermission>> m1,
-        @Nullable Map<String, Collection<SecurityPermission>> m2) {
-        if (m1 == m2)
-            return true;
-
-        if (m1 == null || m2 == null)
-            return false;
-
-        if (m1.size() != m2.size())
-            return false;
-
-        for (Map.Entry<String, Collection<SecurityPermission>> e : m1.entrySet()) {
-            if (!F.eqNotOrdered(e.getValue(), m2.get(e.getKey())))
-                return false;
-        }
-
-        return true;
-    }
 }

modules/core/src/test/java/org/apache/ignite/testframework/junits/spi/GridSpiAbstractTest.java

@@ -727,7 +727,7 @@ private static class SecurityPermissionSetImpl implements SecurityPermissionSet
         }
 
         /** {@inheritDoc} */
-        @Override public Map<String, Collection<SecurityPermission>> taskPermissions() {
+        @Override public Map<String, ? extends Collection<SecurityPermission>> taskPermissions() {
             return Collections.emptyMap();
         }
 
@@ -737,7 +737,7 @@ private static class SecurityPermissionSetImpl implements SecurityPermissionSet
         }
 
         /** {@inheritDoc} */
-        @Override public Map<String, Collection<SecurityPermission>> servicePermissions() {
+        @Override public Map<String, ? extends Collection<SecurityPermission>> servicePermissions() {
             return Collections.emptyMap();
         }