You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@isis.apache.org by ah...@apache.org on 2021/05/25 14:58:59 UTC

[isis] branch master updated: ISIS-2690: cleaning up ApplicationFeatureId

This is an automated email from the ASF dual-hosted git repository.

ahuber pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/isis.git


The following commit(s) were added to refs/heads/master by this push:
     new 0f985fa  ISIS-2690: cleaning up ApplicationFeatureId
0f985fa is described below

commit 0f985fab1f1a36205ba3b36a6f98192b13eb0c43
Author: Andi Huber <ah...@apache.org>
AuthorDate: Tue May 25 16:58:42 2021 +0200

    ISIS-2690: cleaning up ApplicationFeatureId
---
 .../services/appfeat/ApplicationFeatureId.java     | 63 ++++++++++++----------
 .../secman/adoc/modules/secman/pages/about.adoc    |  2 +-
 .../dom/ApplicationPermissionValueSet.java         |  9 +---
 .../isis/testdomain/shiro/ShiroSecmanLdapTest.java |  2 +-
 4 files changed, 40 insertions(+), 36 deletions(-)

diff --git a/api/applib/src/main/java/org/apache/isis/applib/services/appfeat/ApplicationFeatureId.java b/api/applib/src/main/java/org/apache/isis/applib/services/appfeat/ApplicationFeatureId.java
index b3fe053..c546df6 100644
--- a/api/applib/src/main/java/org/apache/isis/applib/services/appfeat/ApplicationFeatureId.java
+++ b/api/applib/src/main/java/org/apache/isis/applib/services/appfeat/ApplicationFeatureId.java
@@ -53,6 +53,14 @@ import lombok.val;
  * This value is {@link Comparable}, the implementation of which considers
  * {@link #getSort() (feature) sort}, {@link #getNamespace() namespace},
  * {@link #getTypeSimpleName() type simple name} and {@link #getMemberName() member name}.
+ * <p>
+ * If the represented member is an <i>action</i>, then {@link #getMemberName() member name}
+ * must <b>not</b> include any parameter list or parentheses.
+ * Consequently method overloading is not supported.
+ * <p>
+ * If there is a member name clash involving an <i>action</i> and an <i>association</i>,
+ * then consequently any permissions defined automatically apply to both and one cannot separate
+ * these.
  *
  * @since 1.x revised for 2.0 {@index}
  */
@@ -77,11 +85,7 @@ implements
         if(identifier.getType().isClass()) {
             return newType(logicalTypeName);
         }
-        if(identifier.getType().isPropertyOrCollection()) {
-            return newMember(logicalTypeName, identifier.getMemberName());
-        }
-        // its an action
-        return newMember(logicalTypeName, identifier.getMemberNameAndParameterClassNamesIdentityString());
+        return newMember(logicalTypeName, identifier.getMemberName());
     }
 
     public static ApplicationFeatureId newFeature(
@@ -131,7 +135,7 @@ implements
     public static ApplicationFeatureId newMember(final String logicalTypeName, final String memberName) {
         val featureId = new ApplicationFeatureId(ApplicationFeatureSort.MEMBER);
         initType(featureId, logicalTypeName);
-        featureId.memberName = memberName;
+        initMember(featureId, memberName);
         return featureId;
     }
 
@@ -144,7 +148,7 @@ implements
         val logicalTypeName = fullyQualifiedName.substring(0, i);
         val memberName = fullyQualifiedName.substring(i + 1);
         initType(featureId, logicalTypeName);
-        featureId.memberName = memberName;
+        initMember(featureId, memberName);
         return featureId;
     }
 
@@ -169,6 +173,21 @@ implements
         featureId.memberName = null;
     }
 
+    private static void initMember(final ApplicationFeatureId featureId, final @Nullable String memberName) {
+        featureId.memberName = memberNameForSignature(memberName); // just in case
+    }
+
+    // strips off the parameter types
+    private static String memberNameForSignature(final @Nullable String memberSiganture) {
+        if(_Strings.isEmpty(memberSiganture)) {
+            return memberSiganture;
+        }
+        final int paramListStartIndex = memberSiganture.indexOf('(');
+        return paramListStartIndex>-1
+                ? memberSiganture.substring(0, paramListStartIndex)
+                : memberSiganture;
+    }
+
     // -- CONSTRUCTOR
 
     private ApplicationFeatureId(final ApplicationFeatureSort sort) {
@@ -208,6 +227,16 @@ implements
     @Programmatic
     @Getter private String typeSimpleName;
 
+    /**
+     * Logical (simple) name of the member (in case of actions not including the parameter list).
+     * Consequently method overloading is not supported.
+     * <p>
+     * If there is a member name clash involving an <i>action</i> and an <i>association</i>,
+     * then consequently any permissions defined automatically apply to both and one cannot separate
+     * these.
+     * <p>
+     * {@code null} if not a member
+     */
     @Programmatic
     @Getter private String memberName;
 
@@ -396,24 +425,4 @@ implements
         return newFeature(namespace, this.getTypeSimpleName(), this.getMemberName());
     }
 
-
-    /**
-     * For action members, returns a variant of the featureId that strips off the parameter types.
-     *
-     * <p>
-     *     This is for use cases where distinguishing between overloaded versions of an action (which is itself strongly discouraged)
-     *     is unimportant.  One example is the permissions management of SecMan.
-     * </p>
-     * @return
-     */
-    public ApplicationFeatureId asNonOverloaded() {
-        if (getSort() == ApplicationFeatureSort.MEMBER) {
-            val memberName = this.getMemberName();
-            val paramAt = memberName.indexOf('(');
-            if(paramAt != -1) {
-                return newFeature(this.getNamespace(), this.getTypeSimpleName(), memberName.substring(0, paramAt));
-            }
-        }
-        return this;
-    }
 }
diff --git a/extensions/security/secman/adoc/modules/secman/pages/about.adoc b/extensions/security/secman/adoc/modules/secman/pages/about.adoc
index ce79e64..405a89c 100644
--- a/extensions/security/secman/adoc/modules/secman/pages/about.adoc
+++ b/extensions/security/secman/adoc/modules/secman/pages/about.adoc
@@ -5,7 +5,7 @@
 
 SecMan provides an implementation of the xref:refguide:core:index/security/authorization/Authorizor.adoc[Authorizor] SPI, to determine whether the currently logged-in user has access to a given object member
 
-This authorization is implemented using domain entities, which means that authorizations/permissions can they be administered from within your Apache Isis application.
+This authorization is implemented using domain entities, which means that authorizations/permissions can then be administered from within your Apache Isis application.
 Moreover, the set of permissions (to features) is derived completely from your application's metamodel; in essence the permissions are "type-safe".
 The domain model is explained in more detail <<domain-model,below>>.
 
diff --git a/extensions/security/secman/api/src/main/java/org/apache/isis/extensions/secman/api/permission/dom/ApplicationPermissionValueSet.java b/extensions/security/secman/api/src/main/java/org/apache/isis/extensions/secman/api/permission/dom/ApplicationPermissionValueSet.java
index 9622ae2..0f48aad 100644
--- a/extensions/security/secman/api/src/main/java/org/apache/isis/extensions/secman/api/permission/dom/ApplicationPermissionValueSet.java
+++ b/extensions/security/secman/api/src/main/java/org/apache/isis/extensions/secman/api/permission/dom/ApplicationPermissionValueSet.java
@@ -19,7 +19,6 @@
 package org.apache.isis.extensions.secman.api.permission.dom;
 
 import java.io.Serializable;
-import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 
@@ -146,10 +145,9 @@ public class ApplicationPermissionValueSet implements Serializable {
             final ApplicationFeatureId featureId,
             final ApplicationPermissionMode mode) {
 
-        val featureIdNonOverloaded = featureId.asNonOverloaded();
-        for (val pathId : featureIdNonOverloaded.getPathIds()) {
+        for (val pathId : featureId.getPathIds()) {
             val permissionValues = permissionsByFeature.get(pathId);
-            val evaluation = permissionsEvaluationService.evaluate(featureIdNonOverloaded, mode, permissionValues);
+            val evaluation = permissionsEvaluationService.evaluate(featureId, mode, permissionValues);
             if(evaluation != null) {
                 return evaluation;
             }
@@ -183,7 +181,4 @@ public class ApplicationPermissionValueSet implements Serializable {
                 '}';
     }
 
-
-
-
 }
diff --git a/regressiontests/incubating/src/test/java/org/apache/isis/testdomain/shiro/ShiroSecmanLdapTest.java b/regressiontests/incubating/src/test/java/org/apache/isis/testdomain/shiro/ShiroSecmanLdapTest.java
index 214dc65..c8c9ab7 100644
--- a/regressiontests/incubating/src/test/java/org/apache/isis/testdomain/shiro/ShiroSecmanLdapTest.java
+++ b/regressiontests/incubating/src/test/java/org/apache/isis/testdomain/shiro/ShiroSecmanLdapTest.java
@@ -147,7 +147,7 @@ class ShiroSecmanLdapTest extends AbstractShiroTest {
         val olafUser = applicationUserRepository.findByUsername(username).orElse(null);
         assertNotNull(olafUser);
         assertNotNull(olafUser.getStatus());
-        assertFalse(olafUser.getStatus().isEnabled());
+        assertFalse(olafUser.getStatus().isUnlocked());
     }
 
     @Test