garydgregory commented on code in PR #400:
URL: https://github.com/apache/commons-jexl/pull/400#discussion_r3143470119
##########
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:
Too bad we're stuck with "allow", it should be "grant". You "grant" and
"deny" permissions.
##########
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;
+
+ /**
Review Comment:
How about using a builder instead of a bunch of constructors?
##########
src/main/java/org/apache/commons/jexl3/internal/introspection/Permissions.java:
##########
@@ -20,58 +20,73 @@
import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
import java.lang.reflect.Proxy;
import java.util.Collections;
+import java.util.HashMap;
import java.util.HashSet;
-import java.util.LinkedHashSet;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
-import java.util.concurrent.ConcurrentHashMap;
import java.util.function.BiPredicate;
import org.apache.commons.jexl3.annotations.NoJexl;
import org.apache.commons.jexl3.introspection.JexlPermissions;
/**
- * Checks whether an element (ctor, field or method) is visible by JEXL
introspection.
- * <p>Default implementation does this by checking if element has been
annotated with NoJexl.</p>
+ * Checks whether an element (ctor, field, or method) is visible by JEXL
introspection.
+ * <p>The default implementation does this by checking if an element has been
annotated with NoJexl.</p>
*
- * <p>The NoJexl annotation allows a fine grain permissions on executable
objects (methods, fields, constructors).
+ * <p>The NoJexl annotation allows a fine grain permission on executable
objects (methods, fields, constructors).
* </p>
* <ul>
- * <li>NoJexl of a package implies all classes (including derived classes) and
all interfaces
+ * <li>NoJexl of a package implies all classes (including derived classes),
and all interfaces
* of that package are invisible to JEXL.</li>
- * <li>NoJexl on a class implies this class and all its derived classes are
invisible to JEXL.</li>
+ * <li>NoJexl on a class implies this class, and all its derived classes are
invisible to JEXL.</li>
* <li>NoJexl on a (public) field makes it not visible as a property to
JEXL.</li>
* <li>NoJexl on a constructor prevents that constructor to be used to
instantiate through 'new'.</li>
* <li>NoJexl on a method prevents that method and any of its overrides to be
visible to JEXL.</li>
* <li>NoJexl on an interface prevents all methods of that interface and their
overrides to be visible to JEXL.</li>
* </ul>
- * <p> It is possible to further refine permissions on classes used through
libraries where source code form can
- * not be altered using an instance of permissions using {@link
JexlPermissions#parse(String...)}.</p>
+ * <p>It is possible to define permissions on external library classes used
for which the source code
+ * cannot be altered using an instance of permissions using {@link
JexlPermissions#parse(String...)}.</p>
*/
public class Permissions implements JexlPermissions {
+ /**
+ * Represents the ability to create a copy of an object.
+ * Any class implementing this interface must provide a concrete
+ * implementation for the {@code copy()} method, which returns
+ * a new instance of the object that is a logical copy of the original.
+ *
+ * @param <T> the type of object that can be copied
+ */
+ interface Copyable<T> {
Review Comment:
Can't we reuse `java.lang.Cloneable`?
--
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]
