From 9f0ff4e50448640266d896ad6ba4daa7b327d368 Mon Sep 17 00:00:00 2001
From: Jake Potrebic <15055071+Machine-Maker@users.noreply.github.com>
Date: Tue, 14 Sep 2021 17:43:45 -0700
Subject: [PATCH] core: fix And/OrPermission losing conditional tree (#296)

---
 CHANGELOG.md                                  |  1 +
 .../permission/AndPermission.java             |  6 +----
 .../permission/OrPermission.java              |  6 +----
 .../CommandPermissionTest.java                | 27 +++++++++++++++++++
 4 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7991b06f..ca63f894 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 - Bukkit: Permission checking and syntax string for Bukkit '/help' command
+- And/OrPermission factory method `of` did not preserve the conditional tree
 
 ## [1.5.0]
 
diff --git a/cloud-core/src/main/java/cloud/commandframework/permission/AndPermission.java b/cloud-core/src/main/java/cloud/commandframework/permission/AndPermission.java
index 4ea5105a..ad4e1c9d 100644
--- a/cloud-core/src/main/java/cloud/commandframework/permission/AndPermission.java
+++ b/cloud-core/src/main/java/cloud/commandframework/permission/AndPermission.java
@@ -50,11 +50,7 @@ public final class AndPermission implements CommandPermission {
      * @return Constructed permission
      */
     public static @NonNull CommandPermission of(final @NonNull Collection<CommandPermission> permissions) {
-        final Set<CommandPermission> permissionSet = new HashSet<>();
-        for (final CommandPermission permission : permissions) {
-            permissionSet.addAll(permission.getPermissions());
-        }
-        return new AndPermission(permissionSet);
+        return new AndPermission(new HashSet<>(permissions));
     }
 
     @Override
diff --git a/cloud-core/src/main/java/cloud/commandframework/permission/OrPermission.java b/cloud-core/src/main/java/cloud/commandframework/permission/OrPermission.java
index 68c78dbe..6ba599fc 100644
--- a/cloud-core/src/main/java/cloud/commandframework/permission/OrPermission.java
+++ b/cloud-core/src/main/java/cloud/commandframework/permission/OrPermission.java
@@ -50,11 +50,7 @@ public final class OrPermission implements CommandPermission {
      * @return Constructed permission
      */
     public static @NonNull CommandPermission of(final @NonNull Collection<CommandPermission> permissions) {
-        final Set<CommandPermission> permissionSet = new HashSet<>();
-        for (final CommandPermission permission : permissions) {
-            permissionSet.addAll(permission.getPermissions());
-        }
-        return new OrPermission(permissionSet);
+        return new OrPermission(new HashSet<>(permissions));
     }
 
     @Override
diff --git a/cloud-core/src/test/java/cloud/commandframework/CommandPermissionTest.java b/cloud-core/src/test/java/cloud/commandframework/CommandPermissionTest.java
index 9f8c4a89..e27dc991 100644
--- a/cloud-core/src/test/java/cloud/commandframework/CommandPermissionTest.java
+++ b/cloud-core/src/test/java/cloud/commandframework/CommandPermissionTest.java
@@ -28,13 +28,16 @@ import cloud.commandframework.execution.CommandExecutionCoordinator;
 import cloud.commandframework.keys.SimpleCloudKey;
 import cloud.commandframework.meta.CommandMeta;
 import cloud.commandframework.meta.SimpleCommandMeta;
+import cloud.commandframework.permission.AndPermission;
 import cloud.commandframework.permission.CommandPermission;
+import cloud.commandframework.permission.OrPermission;
 import cloud.commandframework.permission.Permission;
 import cloud.commandframework.permission.PredicatePermission;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 
+import java.util.Arrays;
 import java.util.concurrent.CompletionException;
 import java.util.concurrent.atomic.AtomicBoolean;
 
@@ -89,6 +92,30 @@ class CommandPermissionTest {
         assertTrue(manager.hasPermission(new TestCommandSender("one", "two"), test));
     }
 
+    @Test
+    void testComplexOrPermissions() {
+        final CommandPermission andOne = AndPermission.of(Arrays.asList(Permission.of("perm.one"),
+                Permission.of("perm.two")));
+        final CommandPermission andTwo = AndPermission.of(Arrays.asList(Permission.of("perm.three"),
+                (PredicatePermission<?>) (s) -> false));
+        final CommandPermission orPermission = OrPermission.of(Arrays.asList(andOne, andTwo));
+        assertFalse(manager.hasPermission(new TestCommandSender("does.have", "also.does.have"), orPermission));
+        assertFalse(manager.hasPermission(new TestCommandSender("perm.one", "perm.three"), orPermission));
+        assertTrue(manager.hasPermission(new TestCommandSender("perm.one", "perm.two"), orPermission));
+    }
+
+    @Test
+    void testComplexAndPermissions() {
+        final CommandPermission orOne = OrPermission.of(Arrays.asList(Permission.of("perm.one"),
+                (PredicatePermission<?>) (s) -> false));
+        final CommandPermission orTwo = OrPermission.of(Arrays.asList(Permission.of("perm.two"),
+                Permission.of("perm.three")));
+        final CommandPermission andPermission = AndPermission.of(Arrays.asList(orOne, orTwo));
+        assertFalse(manager.hasPermission(new TestCommandSender("perm.one"), andPermission));
+        assertTrue(manager.hasPermission(new TestCommandSender("perm.one", "perm.two"), andPermission));
+        assertTrue(manager.hasPermission(new TestCommandSender("perm.one", "perm.three"), andPermission));
+    }
+
     @Test
     void testPredicatePermissions() {
         final AtomicBoolean condition = new AtomicBoolean(true);