Rework of the permission API

src/main/java/net/minestom/server/command/ConsoleSender.java

@@ -3,16 +3,15 @@ package net.minestom.server.command;
 import net.minestom.server.permission.Permission;
 import org.jetbrains.annotations.NotNull;
 
-import java.util.Collection;
+import java.util.Set;
-import java.util.LinkedList;
+import java.util.concurrent.CopyOnWriteArraySet;
-import java.util.List;
 
 /**
  * Represents the console when sending a command to the server.
  */
 public class ConsoleSender implements CommandSender {
 
-    private final List<Permission> permissions = new LinkedList<>();
+    private final Set<Permission> permissions = new CopyOnWriteArraySet<>();
 
     @Override
     public void sendMessage(@NotNull String message) {
@@ -21,7 +20,7 @@ public class ConsoleSender implements CommandSender {
 
     @NotNull
     @Override
-    public Collection<Permission> getAllPermissions() {
+    public Set<Permission> getAllPermissions() {
         return permissions;
     }
 }

src/main/java/net/minestom/server/entity/Entity.java

@@ -96,7 +96,7 @@ public abstract class Entity implements Viewable, EventHandler, DataContainer, P
     private final int id;
     protected final Set<Player> viewers = new CopyOnWriteArraySet<>();
     private Data data;
-    private final List<Permission> permissions = new LinkedList<>();
+    private final Set<Permission> permissions = new CopyOnWriteArraySet<>();
 
     protected UUID uuid;
     private boolean isActive; // False if entity has only been instanced without being added somewhere
@@ -372,7 +372,7 @@ public abstract class Entity implements Viewable, EventHandler, DataContainer, P
 
     @NotNull
     @Override
-    public Collection<Permission> getAllPermissions() {
+    public Set<Permission> getAllPermissions() {
         return permissions;
     }
 

src/main/java/net/minestom/server/permission/BasicPermission.java

@@ -1,15 +0,0 @@
-package net.minestom.server.permission;
-
-import net.minestom.server.command.CommandSender;
-import org.jetbrains.annotations.NotNull;
-
-/**
- * Basic {@link Permission} implementation that only requires the permission to be given to the {@link CommandSender} to be considered applied
- * (eg. no arguments)
- */
-public class BasicPermission implements Permission<Object> {
-    @Override
-    public boolean isValidFor(@NotNull PermissionHandler permissionHandler, Object data) {
-        return true;
-    }
-}

src/main/java/net/minestom/server/permission/Permission.java

@@ -1,47 +1,70 @@
 package net.minestom.server.permission;
 
 import net.minestom.server.command.CommandSender;
-import net.minestom.server.data.Data;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
+import org.jglrxavpok.hephaistos.nbt.NBTCompound;
+
+import java.util.Objects;
 
 /**
  * Representation of a permission granted to a {@link CommandSender}.
- *
- * @param <T> the type of data that this permission can handle in {@link #isValidFor(PermissionHandler, Object)}.
- *            Used if you want to allow passing additional data to check if the permission is valid in a certain situation,
- *            you can default it to {@link Object} if you do not need it.
  */
-@FunctionalInterface
+public class Permission {
-public interface Permission<T> {
+
+    private String permissionName;
+    private NBTCompound data;
 
     /**
-     * Does the given {@link CommandSender} have the permission represented by this object?
+     * Creates a new permission object with optional data.
-     * <p>
-     * Called with {@link CommandSender#hasPermission(Permission)}, the {@link CommandSender} requires to both
-     * have this permission and validate the condition in this method.
      *
-     * @param permissionHandler the permission handler
+     * @param permissionName the name of the permission
-     * @param data              the optional data (eg the number of home possible, placing a block at X position)
+     * @param data           the optional data of the permission
-     * @return true if the commandSender possesses this permission
      */
-    boolean isValidFor(@NotNull PermissionHandler permissionHandler, @Nullable T data);
+    public Permission(@NotNull String permissionName, @Nullable NBTCompound data) {
-
+        this.permissionName = permissionName;
-    /**
+        this.data = data;
-     * Writes any required data for this permission inside the given destination.
-     *
-     * @param destination the {@link Data} to write to
-     */
-    default void write(@NotNull Data destination) {
     }
 
     /**
-     * Reads any required data for this permission from the given destination.
+     * Creates a new permission object without additional data
      *
-     * @param source the {@link Data} to read from
+     * @param permissionName the name of the permission
-     * @return this for chaining
      */
-    default Permission read(@Nullable Data source) {
+    public Permission(@NotNull String permissionName) {
-        return this;
+        this(permissionName, null);
+    }
+
+    /**
+     * Gets the name of the permission.
+     *
+     * @return the permission name
+     */
+    @NotNull
+    public String getPermissionName() {
+        return permissionName;
+    }
+
+    /**
+     * Gets the data associated to this permission.
+     *
+     * @return the nbt data of this permission, can be null if not any
+     */
+    @Nullable
+    public NBTCompound getNBTData() {
+        return data;
+    }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+        Permission that = (Permission) o;
+        return permissionName.equals(that.permissionName) && Objects.equals(data, that.data);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(permissionName, data);
     }
 }

src/main/java/net/minestom/server/permission/PermissionHandler.java

@@ -3,7 +3,7 @@ package net.minestom.server.permission;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
-import java.util.Collection;
+import java.util.Set;
 
 /**
  * Represents an object which can have permissions.
@@ -11,16 +11,16 @@ import java.util.Collection;
 public interface PermissionHandler {
 
     /**
-     * Returns all permissions associated to this command sender.
+     * Returns all permissions associated to this handler.
      * The returned collection should be modified only by subclasses.
      *
-     * @return the permissions of this command sender.
+     * @return the permissions of this handler.
      */
     @NotNull
-    Collection<Permission> getAllPermissions();
+    Set<Permission> getAllPermissions();
 
     /**
-     * Adds a {@link Permission} to this commandSender
+     * Adds a {@link Permission} to this handler.
      *
      * @param permission the permission to add
      */
@@ -29,7 +29,7 @@ public interface PermissionHandler {
     }
 
     /**
-     * Removes a {@link Permission} from this commandSender
+     * Removes a {@link Permission} from this handler.
      *
      * @param permission the permission to remove
      */
@@ -38,55 +38,49 @@ public interface PermissionHandler {
     }
 
     /**
-     * Checks if the given {@link Permission} is possessed by this command sender.
+     * Gets if this handler has the permission {@code permission}.
-     * Simple shortcut to <pre>getAllPermissions().contains(permission) &amp;&amp; permission.isValidFor(this)</pre> for readability.
+     * <p>
+     * Uses {@link Permission#equals(Object)} internally.
      *
-     * @param p permission to check against
+     * @param permission the permission to check
-     * @return true if the sender has the permission and validate {@link Permission#isValidFor(PermissionHandler, Object)}
+     * @return true if the handler has the permission
      */
-    default boolean hasPermission(@NotNull Permission p) {
+    default boolean hasPermission(@NotNull Permission permission) {
-        return hasPermission(p, null);
+        for (Permission permissionLoop : getAllPermissions()) {
+            if (permissionLoop.equals(permission)) {
+                return true;
             }
-
+        }
-    default <T> boolean hasPermission(@NotNull Permission<T> p, @Nullable T data) {
+        return false;
-        return getAllPermissions().contains(p) && p.isValidFor(this, data);
     }
 
     /**
-     * Checks if the given {@link Permission} is possessed by this command sender.
+     * Gets if this handler has the permission with the name {@code permissionName} and which verify the optional
-     * Will call {@link Permission#isValidFor(PermissionHandler, Object)} on all permissions that are an instance of {@code permissionClass}.
+     * {@link PermissionVerifier}.
-     * If no matching permission is found, this result returns false.
      *
-     * @param permissionClass the permission class to check
+     * @param permissionName     the permission name
-     * @return true if the sender has the permission and validate {@link Permission#isValidFor(PermissionHandler, Object)}
+     * @param permissionVerifier the optional verifier
-     * @see #getAllPermissions()
+     * @return true if the handler has the permission
      */
-    default boolean hasPermission(@NotNull Class<? extends Permission> permissionClass) {
+    default boolean hasPermission(@NotNull String permissionName, @Nullable PermissionVerifier permissionVerifier) {
-        boolean result = true;
+        for (Permission permission : getAllPermissions()) {
-        boolean foundPerm = false;
+            if (permission.getPermissionName().equals(permissionName)) {
-        for (Permission p : getAllPermissions()) {
+                return permissionVerifier != null ?
-            if (permissionClass.isInstance(p)) {
+                        permissionVerifier.isValid(permission.getNBTData()) :
-                foundPerm = true;
+                        true;
-                result &= p.isValidFor(this, null);
             }
         }
-        if (!foundPerm)
         return false;
-        return result;
     }
 
-    default <T> boolean hasPermission(@NotNull Class<? extends Permission<T>> permissionClass, @Nullable T data) {
+    /**
-        boolean result = true;
+     * Gets if this handler has the permission with the name {@code permissionName}.
-        boolean foundPerm = false;
+     *
-        for (Permission p : getAllPermissions()) {
+     * @param permissionName the permission name
-            if (permissionClass.isInstance(p)) {
+     * @return true if the handler has the permission
-                foundPerm = true;
+     */
-                result &= p.isValidFor(this, data);
+    default boolean hasPermission(@NotNull String permissionName) {
-            }
+        return hasPermission(permissionName, null);
-        }
-        if (!foundPerm)
-            return false;
-        return result;
     }
 
 }

src/main/java/net/minestom/server/permission/PermissionVerifier.java

@@ -0,0 +1,20 @@
+package net.minestom.server.permission;
+
+import org.jetbrains.annotations.Nullable;
+import org.jglrxavpok.hephaistos.nbt.NBTCompound;
+
+/**
+ * Interface used to check if the {@link NBTCompound nbt data} of a {@link Permission} is correct.
+ */
+@FunctionalInterface
+public interface PermissionVerifier {
+
+    /**
+     * Called when using {@link PermissionHandler#hasPermission(String, PermissionVerifier)}.
+     *
+     * @param nbtCompound the data of the permission, can be null if not any
+     * @return true if {@link PermissionHandler#hasPermission(String, PermissionVerifier)}
+     * should return true, false otherwise
+     */
+    boolean isValid(@Nullable NBTCompound nbtCompound);
+}

src/test/java/permissions/TestPermissions.java

@@ -3,8 +3,7 @@ package permissions;
 import net.minestom.server.MinecraftServer;
 import net.minestom.server.entity.Player;
 import net.minestom.server.permission.Permission;
-import net.minestom.server.permission.PermissionHandler;
+import org.jglrxavpok.hephaistos.nbt.NBTCompound;
-import org.jetbrains.annotations.NotNull;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -19,6 +18,8 @@ public class TestPermissions {
 
     private Player player;
 
+    private Permission permission1, permission2;
+
     @BeforeEach
     public void init() {
         MinecraftServer.init(); // for entity manager
@@ -32,67 +33,42 @@ public class TestPermissions {
                 return false;
             }
         };
+
+        permission1 = new Permission("perm.name",
+                new NBTCompound()
+                        .setString("name", "Minestom")
+                        .setInt("amount", 5));
+
+        permission2 = new Permission("perm.name2");
     }
 
     @Test
     public void noPermission() {
-        assertFalse(player.hasPermission(Permission.class));
+        assertFalse(player.hasPermission(""));
-    }
+        assertFalse(player.hasPermission("random.permission"));
-
-    class PermTest1 implements Permission<Object> {
-        @Override
-        public boolean isValidFor(@NotNull PermissionHandler permissionHandler, Object data) {
-            return true;
-        }
-    }
-
-    class PermTest2 implements Permission<Object> {
-        @Override
-        public boolean isValidFor(@NotNull PermissionHandler permissionHandler, Object data) {
-            return true;
-        }
     }
 
     @Test
     public void hasPermissionClass() {
-        assertFalse(player.hasPermission(Permission.class));
-        player.addPermission(new PermTest1());
-        assertTrue(player.hasPermission(PermTest1.class));
-        assertFalse(player.hasPermission(PermTest2.class));
-        assertTrue(player.hasPermission(Permission.class)); // allow superclasses
 
-        player.addPermission(new PermTest2());
+        assertFalse(player.hasPermission(permission1));
-        assertTrue(player.hasPermission(PermTest2.class));
+        player.addPermission(permission1);
-    }
+        assertTrue(player.hasPermission(permission1));
+        assertFalse(player.hasPermission(permission2));
 
-    class BooleanPerm implements Permission<Object> {
+        player.addPermission(permission2);
-        private final boolean value;
+        assertTrue(player.hasPermission(permission2));
-
-        BooleanPerm(boolean v) {
-            this.value = v;
-        }
-
-        @Override
-        public boolean isValidFor(@NotNull PermissionHandler permissionHandler, Object data) {
-            return value;
-        }
     }
 
     @Test
-    public void hasTwoPermissionsOfSameClassButContradictEachOther() {
+    public void hasPermissionNameNbt() {
-        player.addPermission(new BooleanPerm(true));
+        player.addPermission(permission1);
-        assertTrue(player.hasPermission(BooleanPerm.class));
+        assertTrue(player.hasPermission("perm.name"));
-        player.addPermission(new BooleanPerm(false));
+        assertTrue(player.hasPermission("perm.name",
-        assertFalse(player.hasPermission(BooleanPerm.class)); // all permissions must be valid
+                nbtCompound -> nbtCompound != null && nbtCompound.getString("name").equals("Minestom")));
-    }
 
-    @Test
+        player.addPermission(permission2);
-    public void singlePermission() {
+        assertFalse(player.hasPermission("perm.name2", nbtCompound -> nbtCompound != null));
-        Permission p = (commandSender, data) -> true;
-        player.addPermission(p);
-        assertTrue(p.isValidFor(player, null));
-        assertTrue(player.hasPermission(p));
-        assertTrue(player.hasPermission(Permission.class));
     }
 
     @AfterEach