henrib commented on code in PR #400:
URL: https://github.com/apache/commons-jexl/pull/400#discussion_r3143564873


##########
src/main/java/org/apache/commons/jexl3/introspection/JexlPermissions.java:
##########
@@ -69,71 +89,121 @@ public interface JexlPermissions {
     /**
      * A permission delegation that augments the RESTRICTED permission with an 
explicit
      * set of classes.
-     * <p>Typical use case is to deny access to a package - and thus all its 
classes - but allow
+     * <p>A typical use case is to deny access to a package - and thus all its 
classes - but allow
      * a few specific classes.</p>
      * <p>Note that the newer positive restriction syntax is preferable as in:
      * <code>RESTRICTED.compose("java.lang { +Class {} }")</code>.</p>
      */
-     final class ClassPermissions extends JexlPermissions.Delegate {
-
-        /** The set of explicitly allowed classes, overriding the delegate 
permissions. */
-        private final Set<String> allowedClasses;
-
-        /**
-         * Creates permissions based on the RESTRICTED set but allowing an 
explicit set.
-         *
-         * @param allow the set of allowed classes
-         */
-        public ClassPermissions(final Class<?>... allow) {
-            this(JexlPermissions.RESTRICTED,
-                    Arrays.stream(Objects.requireNonNull(allow))
-                        .map(Class::getCanonicalName)
-                        .collect(Collectors.toList()));
+    final class ClassPermissions extends JexlPermissions.Delegate {
+      /**
+       * The set of explicitly allowed classes, overriding the delegate 
permissions.
+       */
+      private final Set<String> allowedClasses;
+
+      /**
+       * Creates permissions based on the RESTRICTED set but allowing an 
explicit set.
+       *
+       * @param allow the set of allowed classes
+       */
+      public ClassPermissions(final Class<?>... allow) {
+        this(JexlPermissions.RESTRICTED, allow);
+      }
+
+      /**
+       * Creates permissions by augmenting an existing set with an explicit 
set of allowed classes.
+       * @param permissions the base permissions to augment
+       * @param allow the set of allowed classes
+       */
+      public ClassPermissions(final JexlPermissions permissions, final 
Class<?>... allow) {
+        this(permissions, 
Arrays.stream(Objects.requireNonNull(allow)).map(Class::getCanonicalName).collect(Collectors.toList()));
+      }
+
+      /**
+       * Required for compose().
+       *
+       * @param delegate the base to delegate to
+       * @param allow    the list of class canonical names
+       */
+      public ClassPermissions(final JexlPermissions delegate, final 
Collection<String> allow) {
+        super(Objects.requireNonNull(delegate));
+        allowedClasses = new HashSet<>(Objects.requireNonNull(allow));
+      }
+
+      @Override
+      public boolean allow(final Constructor<?> constructor) {

Review Comment:
   I'll respectfully disagree on the semantics: you grant and revoke 
permissions and permissions do allow or deny actions (as in: grant read 
permissions to allow select on a table). The builders could use grant/revoke 
but the instances allow/deny invoking/accessing methods/fields.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to