Refactor share permission (#1154)

* Refactored Sorting * Refactored Pagination * Refactored Pagination * Pagination refactor builds and tests pass * Codacy fixes * Refactored SharePermission to NonTransferable * Fixed build * Fixed startup bug * Fixed codacy and inspection comments * Update elide-core/src/main/java/com/yahoo/elide/core/EntityDictionary.java Co-Authored-By: Jon Kilroy <[email protected]> * Inspection rework Co-authored-by: Jon Kilroy <[email protected]>
yahoo · Apr 23, 2020 · 4095dfe · 4095dfe
1 parent 65600f6
commit 4095dfe
Show file tree

Hide file tree

Showing 58 changed files with 215 additions and 378 deletions.
diff --git a/elide-annotations/src/main/java/com/yahoo/elide/annotation/NonTransferable.java b/elide-annotations/src/main/java/com/yahoo/elide/annotation/NonTransferable.java
@@ -0,0 +1,30 @@
+/*
+ * Copyright 2020, Yahoo Inc.
+ * Licensed under the Apache License, Version 2.0
+ * See LICENSE file in project root for terms.
+ */
+package com.yahoo.elide.annotation;
+
+import static java.lang.annotation.ElementType.PACKAGE;
+import static java.lang.annotation.ElementType.TYPE;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
+import java.lang.annotation.Inherited;
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+
+/**
+ * Marks that the given entity cannot be added to another collection after creation of the entity.
+ */
+@Target({TYPE, PACKAGE})
+@Retention(RUNTIME)
+@Inherited
+public @interface NonTransferable {
+
+    /**
+     * If NonTransferable is used at the package level, it can be disabled for individual entities by setting
+     * this flag to false.
+     * @return true if enabled.
+     */
+    boolean enabled() default true;
+}
diff --git a/elide-annotations/src/main/java/com/yahoo/elide/annotation/SharePermission.java b/elide-annotations/src/main/java/com/yahoo/elide/annotation/SharePermission.java
diff --git a/elide-core/src/main/java/com/yahoo/elide/core/EntityDictionary.java b/elide-core/src/main/java/com/yahoo/elide/core/EntityDictionary.java
@@ -14,8 +14,8 @@
 import com.yahoo.elide.annotation.Exclude;
 import com.yahoo.elide.annotation.Include;
 import com.yahoo.elide.annotation.MappedInterface;
+import com.yahoo.elide.annotation.NonTransferable;
 import com.yahoo.elide.annotation.SecurityCheck;
-import com.yahoo.elide.annotation.SharePermission;
 import com.yahoo.elide.core.exceptions.HttpStatusException;
 import com.yahoo.elide.core.exceptions.InternalServerErrorException;
 import com.yahoo.elide.core.exceptions.InvalidAttributeException;
@@ -814,9 +814,10 @@ public <T> void initializeEntity(T entity) {
      * @param entityClass the entity type to check for the shareable permissions
      * @return true if entityClass is shareable.  False otherwise.
      */
-    public boolean isShareable(Class<?> entityClass) {
-        return getAnnotation(entityClass, SharePermission.class) != null
-                && getAnnotation(entityClass, SharePermission.class).sharable();
+    public boolean isTransferable(Class<?> entityClass) {
+        NonTransferable nonTransferable = getAnnotation(entityClass, NonTransferable.class);
+
+        return (nonTransferable == null || !nonTransferable.enabled());
     }
 
     /**

diff --git a/elide-core/src/main/java/com/yahoo/elide/core/EntityPermissions.java b/elide-core/src/main/java/com/yahoo/elide/core/EntityPermissions.java
@@ -7,8 +7,8 @@
 
 import com.yahoo.elide.annotation.CreatePermission;
 import com.yahoo.elide.annotation.DeletePermission;
+import com.yahoo.elide.annotation.NonTransferable;
 import com.yahoo.elide.annotation.ReadPermission;
-import com.yahoo.elide.annotation.SharePermission;
 import com.yahoo.elide.annotation.UpdatePermission;
 import com.yahoo.elide.generated.parsers.ExpressionLexer;
 import com.yahoo.elide.generated.parsers.ExpressionParser;
@@ -43,7 +43,7 @@ public class EntityPermissions implements CheckInstantiator {
             ReadPermission.class,
             CreatePermission.class,
             DeletePermission.class,
-            SharePermission.class,
+            NonTransferable.class,
             UpdatePermission.class
     );
 
@@ -79,7 +79,7 @@ public EntityPermissions(EntityDictionary dictionary,
             final Map<String, ParseTree> fieldPermissions = new HashMap<>();
             fieldOrMethodList.stream()
                     .forEach(member -> bindMemberPermissions(fieldPermissions, member, annotationClass));
-            if (annotationClass != SharePermission.class) {
+            if (annotationClass != NonTransferable.class) {
                 ParseTree classPermission = bindClassPermissions(cls, annotationClass);
                 if (classPermission != null || !fieldPermissions.isEmpty()) {
                     bindings.put(annotationClass, new AnnotationBinding(classPermission, fieldPermissions));

diff --git a/elide-core/src/main/java/com/yahoo/elide/core/PersistentResource.java b/elide-core/src/main/java/com/yahoo/elide/core/PersistentResource.java
@@ -8,8 +8,8 @@
 import com.yahoo.elide.annotation.Audit;
 import com.yahoo.elide.annotation.CreatePermission;
 import com.yahoo.elide.annotation.DeletePermission;
+import com.yahoo.elide.annotation.NonTransferable;
 import com.yahoo.elide.annotation.ReadPermission;
-import com.yahoo.elide.annotation.SharePermission;
 import com.yahoo.elide.annotation.UpdatePermission;
 import com.yahoo.elide.audit.InvalidSyntaxException;
 import com.yahoo.elide.audit.LogMessage;
@@ -139,7 +139,7 @@ public static <T> PersistentResource<T> createObject(
         //hashcode and equals are only based on the ID/UUID & type.
         assignId(newResource, id);
 
-        // Keep track of new resources for non shareable resources
+        // Keep track of new resources for non-transferable resources
         requestScope.getNewPersistentResources().add(newResource);
         checkPermission(CreatePermission.class, newResource);
 
@@ -477,7 +477,7 @@ protected boolean updateToManyRelation(String fieldName,
 
         added = Sets.difference(updated, deleted);
 
-        checkSharePermission(added);
+        checkTransferablePermission(added);
 
         Collection collection = (Collection) this.getValueUnchecked(fieldName);
 
@@ -539,11 +539,11 @@ protected boolean updateToOneRelation(String fieldName,
             if (newValue == null) {
                 return false;
             }
-            checkSharePermission(resourceIdentifiers);
+            checkTransferablePermission(resourceIdentifiers);
         } else if (oldResource.getObject().equals(newValue)) {
             return false;
         } else {
-            checkSharePermission(resourceIdentifiers);
+            checkTransferablePermission(resourceIdentifiers);
             if (hasInverseRelation(fieldName)) {
                 deleteInverseRelation(fieldName, oldResource.getObject());
                 oldResource.markDirty();
@@ -713,7 +713,7 @@ public void addRelation(String fieldName, PersistentResource newRelation) {
         if (!newRelation.isNewlyCreated() && relationshipAlreadyExists(fieldName, newRelation)) {
             return;
         }
-        checkSharePermission(Collections.singleton(newRelation));
+        checkTransferablePermission(Collections.singleton(newRelation));
         Object relation = this.getValueUnchecked(fieldName);
 
         if (relation instanceof Collection) {
@@ -737,7 +737,7 @@ public void addRelation(String fieldName, PersistentResource newRelation) {
      *
      * @param resourceIdentifiers The persistent resources that are being added
      */
-    protected void checkSharePermission(Set<PersistentResource> resourceIdentifiers) {
+    protected void checkTransferablePermission(Set<PersistentResource> resourceIdentifiers) {
         if (resourceIdentifiers == null) {
             return;
         }
@@ -747,7 +747,7 @@ protected void checkSharePermission(Set<PersistentResource> resourceIdentifiers)
         for (PersistentResource persistentResource : resourceIdentifiers) {
             if (!newResources.contains(persistentResource)
                     && !lineage.getRecord(persistentResource.getType()).contains(persistentResource)) {
-                checkPermission(SharePermission.class, persistentResource);
+                checkPermission(NonTransferable.class, persistentResource);
             }
         }
     }

diff --git a/elide-core/src/main/java/com/yahoo/elide/request/EntityProjection.java b/elide-core/src/main/java/com/yahoo/elide/request/EntityProjection.java
@@ -7,7 +7,6 @@
 package com.yahoo.elide.request;
 
 import com.yahoo.elide.core.filter.expression.FilterExpression;
-
 import com.google.common.collect.Sets;
 import lombok.AllArgsConstructor;
 import lombok.Builder;

diff --git a/elide-core/src/main/java/com/yahoo/elide/request/Pagination.java b/elide-core/src/main/java/com/yahoo/elide/request/Pagination.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2019, Yahoo Inc.
+ * Copyright 2020, Yahoo Inc.
  * Licensed under the Apache License, Version 2.0
  * See LICENSE file in project root for terms.
  */

diff --git a/elide-core/src/main/java/com/yahoo/elide/security/executors/ActivePermissionExecutor.java b/elide-core/src/main/java/com/yahoo/elide/security/executors/ActivePermissionExecutor.java
@@ -11,8 +11,8 @@
 
 import com.yahoo.elide.annotation.CreatePermission;
 import com.yahoo.elide.annotation.DeletePermission;
+import com.yahoo.elide.annotation.NonTransferable;
 import com.yahoo.elide.annotation.ReadPermission;
-import com.yahoo.elide.annotation.SharePermission;
 import com.yahoo.elide.annotation.UpdatePermission;
 import com.yahoo.elide.core.EntityDictionary;
 import com.yahoo.elide.core.RequestScope;
@@ -98,7 +98,7 @@ public <A extends Annotation> ExpressionResult checkPermission(
     }
 
     /**
-     * Check permission on class. Checking on SharePermission falls to check ReadPermission.
+     * Check permission on class. Checking on Transferable falls to check ReadPermission.
      *
      * @param annotationClass annotation class
      * @param resource resource
@@ -114,8 +114,8 @@ public <A extends Annotation> ExpressionResult checkPermission(Class<A> annotati
                                                                    PersistentResource resource,
                                                                    ChangeSpec changeSpec) {
         Supplier<Expression> expressionSupplier = () -> {
-            if (SharePermission.class == annotationClass) {
-                if (requestScope.getDictionary().isShareable(resource.getResourceClass())) {
+            if (NonTransferable.class == annotationClass) {
+                if (requestScope.getDictionary().isTransferable(resource.getResourceClass())) {
                     return expressionBuilder.buildAnyFieldExpressions(resource, ReadPermission.class, changeSpec);
                 }
                 return PermissionExpressionBuilder.FAIL_EXPRESSION;

diff --git a/elide-core/src/main/java/com/yahoo/elide/security/permissions/PermissionCondition.java b/elide-core/src/main/java/com/yahoo/elide/security/permissions/PermissionCondition.java
@@ -8,8 +8,8 @@
 
 import com.yahoo.elide.annotation.CreatePermission;
 import com.yahoo.elide.annotation.DeletePermission;
+import com.yahoo.elide.annotation.NonTransferable;
 import com.yahoo.elide.annotation.ReadPermission;
-import com.yahoo.elide.annotation.SharePermission;
 import com.yahoo.elide.annotation.UpdatePermission;
 import com.yahoo.elide.security.ChangeSpec;
 import com.yahoo.elide.security.PersistentResource;
@@ -37,7 +37,7 @@ public class PermissionCondition {
                     .put(UpdatePermission.class, "UPDATE")
                     .put(DeletePermission.class, "DELETE")
                     .put(CreatePermission.class, "CREATE")
-                    .put(SharePermission.class, "SHARE")
+                    .put(NonTransferable.class, "NO TRANSFER")
                     .build();
 
     /**

diff --git a/elide-core/src/test/java/com/yahoo/elide/core/EntityDictionaryTest.java b/elide-core/src/test/java/com/yahoo/elide/core/EntityDictionaryTest.java
@@ -426,12 +426,12 @@ public void testGetIdAnnotationsSubClass() throws Exception {
 
     @Test
     public void testIsSharableTrue() throws Exception {
-        assertTrue(isShareable(Right.class));
+        assertTrue(isTransferable(Right.class));
     }
 
     @Test
     public void testIsSharableFalse() throws Exception {
-        assertFalse(isShareable(Left.class));
+        assertFalse(isTransferable(Left.class));
     }
 
     @Test

diff --git a/elide-core/src/test/java/com/yahoo/elide/core/PersistenceResourceTestSetup.java b/elide-core/src/test/java/com/yahoo/elide/core/PersistenceResourceTestSetup.java
@@ -43,13 +43,13 @@
 import example.Publisher;
 import example.Right;
 import example.UpdateAndCreate;
-import example.packageshareable.ContainerWithPackageShare;
-import example.packageshareable.ShareableWithPackageShare;
-import example.packageshareable.UnshareableWithEntityUnshare;
+import example.nontransferable.ContainerWithPackageShare;
+import example.nontransferable.ShareableWithPackageShare;
+import example.nontransferable.Untransferable;
+import nocreate.NoCreateEntity;
 
 import lombok.AllArgsConstructor;
 import lombok.EqualsAndHashCode;
-import nocreate.NoCreateEntity;
 
 import java.util.Collection;
 import java.util.HashSet;
@@ -97,7 +97,7 @@ protected static EntityDictionary initDictionary() {
         dictionary.bindEntity(ComputedBean.class);
         dictionary.bindEntity(ContainerWithPackageShare.class);
         dictionary.bindEntity(ShareableWithPackageShare.class);
-        dictionary.bindEntity(UnshareableWithEntityUnshare.class);
+        dictionary.bindEntity(Untransferable.class);
         return dictionary;
     }