Improve performance of Permission or/and logic

Signed-off-by: Irmo van den Berge <[email protected]>
Incendo · Dec 28, 2022 · ef429fd · ef429fd
1 parent b1e55c5
commit ef429fd
Show file tree

Hide file tree

Showing 4 changed files with 91 additions and 16 deletions.
diff --git a/cloud-core/src/main/java/cloud/commandframework/CommandTree.java b/cloud-core/src/main/java/cloud/commandframework/CommandTree.java
@@ -44,7 +44,6 @@
 import io.leangen.geantyref.GenericTypeReflector;
 import io.leangen.geantyref.TypeToken;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
@@ -878,7 +877,7 @@ public void verifyAndRegister() {
 
                 CommandPermission permission;
                 if (existingPermission != null) {
-                    permission = OrPermission.of(Arrays.asList(commandPermission, existingPermission));
+                    permission = commandPermission.or(existingPermission);
                 } else {
                     permission = commandPermission;
                 }
@@ -893,7 +892,7 @@ public void verifyAndRegister() {
                             .getSetting(CommandManager.ManagerSettings.ENFORCE_INTERMEDIARY_PERMISSIONS)) {
                         permission = command.getCommandPermission();
                     } else {
-                        permission = OrPermission.of(Arrays.asList(permission, command.getCommandPermission()));
+                        permission = permission.or(command.getCommandPermission());
                     }
                 }
 

diff --git a/cloud-core/src/main/java/cloud/commandframework/permission/AndPermission.java b/cloud-core/src/main/java/cloud/commandframework/permission/AndPermission.java
@@ -67,6 +67,32 @@ public final class AndPermission implements CommandPermission {
         return this.permissions;
     }
 
+    @Override
+    public @NonNull CommandPermission and(final @NonNull CommandPermission other) {
+        if (this.permissions.contains(other)) {
+            return this;
+        } else {
+            final Set<CommandPermission> objects = new HashSet<>(this.permissions);
+            addToSet(objects, other);
+            return new AndPermission(objects);
+        }
+    }
+
+    @Override
+    public @NonNull CommandPermission and(final @NonNull CommandPermission @NonNull... other) {
+        if (other.length == 0) {
+            return this;
+        } else if (other.length == 1) {
+            return this.and(other[0]);
+        } else {
+            final Set<CommandPermission> objects = new HashSet<>(this.permissions);
+            for (final CommandPermission permission : other) {
+                addToSet(objects, permission);
+            }
+            return new AndPermission(objects);
+        }
+    }
+
     @Override
     public String toString() {
         final StringBuilder stringBuilder = new StringBuilder();
@@ -97,4 +123,15 @@ public boolean equals(final Object o) {
     public int hashCode() {
         return Objects.hash(this.getPermissions());
     }
+
+    private static void addToSet(
+        @NonNull final Set<CommandPermission> objects,
+        @NonNull final CommandPermission permission
+    ) {
+        if (permission instanceof AndPermission) {
+            objects.addAll(permission.getPermissions());
+        } else {
+            objects.add(permission);
+        }
+    }
 }
diff --git a/cloud-core/src/main/java/cloud/commandframework/permission/CommandPermission.java b/cloud-core/src/main/java/cloud/commandframework/permission/CommandPermission.java
@@ -63,10 +63,13 @@ public interface CommandPermission {
     @API(status = API.Status.STABLE, since = "1.4.0")
     default @NonNull CommandPermission or(final @NonNull CommandPermission other) {
         requireNonNull(other, "other");
-        final Set<CommandPermission> permission = new HashSet<>(2);
-        permission.add(this);
-        permission.add(other);
-        return OrPermission.of(permission);
+
+        // Performance optimization
+        if (other instanceof OrPermission) {
+            return other.or(this);
+        }
+
+        return OrPermission.of(Arrays.asList(this, other));
     }
 
     /**
@@ -95,10 +98,13 @@ public interface CommandPermission {
     @API(status = API.Status.STABLE, since = "1.4.0")
     default @NonNull CommandPermission and(final @NonNull CommandPermission other) {
         requireNonNull(other, "other");
-        final Set<CommandPermission> permission = new HashSet<>(2);
-        permission.add(this);
-        permission.add(other);
-        return AndPermission.of(permission);
+
+        // Performance optimization
+        if (other instanceof AndPermission) {
+            return other.and(this);
+        }
+
+        return AndPermission.of(Arrays.asList(this, other));
     }
 
     /**

diff --git a/cloud-core/src/main/java/cloud/commandframework/permission/OrPermission.java b/cloud-core/src/main/java/cloud/commandframework/permission/OrPermission.java
@@ -53,11 +53,7 @@ public final class OrPermission implements CommandPermission {
     public static @NonNull CommandPermission of(final @NonNull Collection<CommandPermission> permissions) {
         final Set<CommandPermission> objects = new HashSet<>();
         for (final CommandPermission permission : permissions) {
-            if (permission instanceof OrPermission) {
-                objects.addAll(permission.getPermissions());
-            } else {
-                objects.add(permission);
-            }
+            addToSet(objects, permission);
         }
         return new OrPermission(objects);
     }
@@ -67,6 +63,32 @@ public final class OrPermission implements CommandPermission {
         return this.permissions;
     }
 
+    @Override
+    public @NonNull CommandPermission or(final @NonNull CommandPermission other) {
+        if (this.permissions.contains(other)) {
+            return this;
+        } else {
+            final Set<CommandPermission> objects = new HashSet<>(this.permissions);
+            addToSet(objects, other);
+            return new OrPermission(objects);
+        }
+    }
+
+    @Override
+    public @NonNull CommandPermission or(final @NonNull CommandPermission @NonNull... other) {
+        if (other.length == 0) {
+            return this;
+        } else if (other.length == 1) {
+            return this.or(other[0]);
+        } else {
+            final Set<CommandPermission> objects = new HashSet<>(this.permissions);
+            for (final CommandPermission permission : other) {
+                addToSet(objects, permission);
+            }
+            return new OrPermission(objects);
+        }
+    }
+
     @Override
     public String toString() {
         final StringBuilder stringBuilder = new StringBuilder();
@@ -97,4 +119,15 @@ public boolean equals(final Object o) {
     public int hashCode() {
         return Objects.hash(this.getPermissions());
     }
+
+    private static void addToSet(
+        @NonNull final Set<CommandPermission> objects,
+        @NonNull final CommandPermission permission
+    ) {
+        if (permission instanceof OrPermission) {
+            objects.addAll(permission.getPermissions());
+        } else {
+            objects.add(permission);
+        }
+    }
 }