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

mridulpathak pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git


The following commit(s) were added to refs/heads/trunk by this push:
     new 77a9d92e64 Replace Security.setDelegator with a SecurityProvider SPI
77a9d92e64 is described below

commit 77a9d92e644c4c10e030680439aa78f64803fd32
Author: Mridul Pathak <[email protected]>
AuthorDate: Wed Oct 28 12:17:33 2026 +0530

    Replace Security.setDelegator with a SecurityProvider SPI
    
    A Security instance used to be created through its no-argument constructor 
and then given its delegator through the deprecated setDelegator method, so 
SecurityFactory needed @SuppressWarnings("deprecation") and every 
implementation carried a mutable delegator field and a getDelegator method. 
SecurityFactory now loads a SecurityProvider through ServiceLoader and asks it 
to create the Security instance for a delegator, and both implementations take 
the delegator in their constructor and [...]
    
    Implementations register a SecurityProvider in 
META-INF/services/org.apache.ofbiz.security.SecurityProvider instead of the 
Security class itself, as securityext now does with 
OFBizApplicationSecurityProvider. The deprecated getDelegator, setDelegator, 
findUserLoginSecurityGroupByUserLoginId and securityGroupPermissionExists 
methods are removed from the Security interface; the last two are now private 
helpers of the implementations, which were their only callers.
---
 .../security/OFBizApplicationSecurity.java         | 33 ++++----------
 .../security/OFBizApplicationSecurityProvider.java | 35 +++++++++++++++
 ... => org.apache.ofbiz.security.SecurityProvider} |  2 +-
 .../java/org/apache/ofbiz/security/Security.java   | 33 +-------------
 .../org/apache/ofbiz/security/SecurityFactory.java | 51 +++++++---------------
 .../apache/ofbiz/security/SecurityProvider.java    | 40 +++++++++++++++++
 6 files changed, 101 insertions(+), 93 deletions(-)

diff --git 
a/applications/securityext/src/main/java/org/apache/ofbiz/securityext/security/OFBizApplicationSecurity.java
 
b/applications/securityext/src/main/java/org/apache/ofbiz/securityext/security/OFBizApplicationSecurity.java
index a6e06f4305..891c6a571a 100644
--- 
a/applications/securityext/src/main/java/org/apache/ofbiz/securityext/security/OFBizApplicationSecurity.java
+++ 
b/applications/securityext/src/main/java/org/apache/ofbiz/securityext/security/OFBizApplicationSecurity.java
@@ -26,7 +26,6 @@ import java.util.Map;
 
 import jakarta.servlet.http.HttpSession;
 
-import org.apache.ofbiz.base.util.Assert;
 import org.apache.ofbiz.base.util.Debug;
 import org.apache.ofbiz.base.util.UtilMisc;
 import org.apache.ofbiz.entity.Delegator;
@@ -44,14 +43,16 @@ public class OFBizApplicationSecurity implements Security {
 
     private static final String MODULE = 
OFBizApplicationSecurity.class.getName();
 
-    private Delegator delegator = null;
+    private final Delegator delegator;
 
     private static final Map<String, Map<String, String>> SIMPLE_ROLE_ENT = 
UtilMisc.toMap(
             "ORDERMGR", UtilMisc.<String, String>toMap("name", "OrderRole", 
"pkey", "orderId"),
             "FACILITY", UtilMisc.<String, String>toMap("name", 
"FacilityParty", "pkey", "facilityId"),
             "MARKETING", UtilMisc.<String, String>toMap("name", 
"MarketingCampaignRole", "pkey", "marketingCampaignId"));
 
-    public OFBizApplicationSecurity() { }
+    public OFBizApplicationSecurity(Delegator delegator) {
+        this.delegator = delegator;
+    }
 
     @Override
     public void clearUserData(GenericValue userLogin) {
@@ -61,9 +62,8 @@ public class OFBizApplicationSecurity implements Security {
         }
     }
 
-    @Override
-    @Deprecated
-    public Iterator<GenericValue> 
findUserLoginSecurityGroupByUserLoginId(String userLoginId) {
+    // Uses the UserLoginSecurityGroup cache to find the userLogin's security 
groups.
+    private Iterator<GenericValue> 
findUserLoginSecurityGroupByUserLoginId(String userLoginId) {
         try {
             List<GenericValue> collection = 
EntityUtil.filterByDate(EntityQuery.use(delegator).from("UserLoginSecurityGroup")
                     .where("userLoginId", 
userLoginId).cache(true).queryList());
@@ -74,12 +74,6 @@ public class OFBizApplicationSecurity implements Security {
         }
     }
 
-    @Override
-    @Deprecated
-    public Delegator getDelegator() {
-        return this.delegator;
-    }
-
     @Override
     public boolean hasEntityPermission(String entity, String action, 
GenericValue userLogin) {
         if (userLogin == null || entity == null || action == null) return 
false;
@@ -224,9 +218,8 @@ public class OFBizApplicationSecurity implements Security {
         return hasRolePermission(application, action, primaryKey, role, 
userLogin);
     }
 
-    @Override
-    @Deprecated
-    public boolean securityGroupPermissionExists(String groupId, String 
permission) {
+    // Caches the groupId, permission pair instead of the userLoginId, 
permission pair to keep the cache small.
+    private boolean securityGroupPermissionExists(String groupId, String 
permission) {
         try {
             return EntityQuery.use(delegator).from("SecurityGroupPermission")
                     .where("groupId", groupId, "permissionId", 
permission).cache(true).filterByDate().queryFirst() != null;
@@ -235,14 +228,4 @@ public class OFBizApplicationSecurity implements Security {
             return false;
         }
     }
-
-    @Override
-    @Deprecated
-    public void setDelegator(Delegator delegator) {
-        if (this.delegator != null) {
-            throw new IllegalStateException("This object has been initialized 
already.");
-        }
-        Assert.notNull("delegator", delegator);
-        this.delegator = delegator;
-    }
 }
diff --git 
a/applications/securityext/src/main/java/org/apache/ofbiz/securityext/security/OFBizApplicationSecurityProvider.java
 
b/applications/securityext/src/main/java/org/apache/ofbiz/securityext/security/OFBizApplicationSecurityProvider.java
new file mode 100644
index 0000000000..22c9f7f24f
--- /dev/null
+++ 
b/applications/securityext/src/main/java/org/apache/ofbiz/securityext/security/OFBizApplicationSecurityProvider.java
@@ -0,0 +1,35 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.ofbiz.securityext.security;
+
+import org.apache.ofbiz.entity.Delegator;
+import org.apache.ofbiz.security.Security;
+import org.apache.ofbiz.security.SecurityProvider;
+
+/**
+ * Creates {@link OFBizApplicationSecurity} instances. Registered in
+ * {@code META-INF/services/org.apache.ofbiz.security.SecurityProvider}.
+ */
+public final class OFBizApplicationSecurityProvider implements 
SecurityProvider {
+
+    @Override
+    public Security create(Delegator delegator) {
+        return new OFBizApplicationSecurity(delegator);
+    }
+}
diff --git 
a/applications/securityext/src/main/resources/META-INF/services/org.apache.ofbiz.security.Security
 
b/applications/securityext/src/main/resources/META-INF/services/org.apache.ofbiz.security.SecurityProvider
similarity index 91%
rename from 
applications/securityext/src/main/resources/META-INF/services/org.apache.ofbiz.security.Security
rename to 
applications/securityext/src/main/resources/META-INF/services/org.apache.ofbiz.security.SecurityProvider
index b412a28997..06ad12597c 100644
--- 
a/applications/securityext/src/main/resources/META-INF/services/org.apache.ofbiz.security.Security
+++ 
b/applications/securityext/src/main/resources/META-INF/services/org.apache.ofbiz.security.SecurityProvider
@@ -15,4 +15,4 @@
 # specific language governing permissions and limitations
 # under the License.
 
-org.apache.ofbiz.securityext.security.OFBizApplicationSecurity
+org.apache.ofbiz.securityext.security.OFBizApplicationSecurityProvider
diff --git 
a/framework/security/src/main/java/org/apache/ofbiz/security/Security.java 
b/framework/security/src/main/java/org/apache/ofbiz/security/Security.java
index d82ac21326..bb0963e28f 100644
--- a/framework/security/src/main/java/org/apache/ofbiz/security/Security.java
+++ b/framework/security/src/main/java/org/apache/ofbiz/security/Security.java
@@ -18,50 +18,21 @@
  
*******************************************************************************/
 package org.apache.ofbiz.security;
 
-import java.util.Iterator;
 import java.util.List;
 
 import jakarta.servlet.http.HttpSession;
 
-import org.apache.ofbiz.entity.Delegator;
 import org.apache.ofbiz.entity.GenericValue;
 
 /**
  * Security interface. This interface defines authorization-related methods.
  * <p>Implementations must be thread-safe because the instance
  * will be shared among multiple threads.</p>
- * <p>Note that the API is changing. New versions might not reference a
- * <code>Delegator</code>. Therefore, implementations requiring a 
<code>Delegator</code>
- * reference should get one from the <code>GenericValue</code> method 
argument.</p>
+ * <p>Instances are created by a {@link SecurityProvider} and are bound to the
+ * <code>Delegator</code> they were created for.</p>
  */
 public interface Security {
 
-    @Deprecated
-    Delegator getDelegator();
-
-    @Deprecated
-    void setDelegator(Delegator delegator);
-
-    /**
-     * Uses userLoginSecurityGroupByUserLoginId cache to speed up the finding 
of the userLogin's security group list.
-     * @param userLoginId The userLoginId to find security groups by
-     * @return An iterator made from the Collection either cached or retrieved 
from the database through the
-     *            UserLoginSecurityGroup Delegator.
-     */
-    @Deprecated
-    Iterator<GenericValue> findUserLoginSecurityGroupByUserLoginId(String 
userLoginId);
-
-    /**
-     * Finds whether or not a SecurityGroupPermission row exists given a 
groupId and permission.
-     * The groupId, permission pair is cached instead of the userLoginId, 
permission pair to keep the cache small and to
-     * make it more changeable.
-     * @param groupId The ID of the group
-     * @param permission The name of the permission
-     * @return boolean specifying whether or not a SecurityGroupPermission row 
exists
-     */
-    @Deprecated
-    boolean securityGroupPermissionExists(String groupId, String permission);
-
     /**
      * Checks to see if the currently logged in userLogin has the passed 
permission.
      * @param permission Name of the permission to check.
diff --git 
a/framework/security/src/main/java/org/apache/ofbiz/security/SecurityFactory.java
 
b/framework/security/src/main/java/org/apache/ofbiz/security/SecurityFactory.java
index af560418d0..5f1125bf9f 100644
--- 
a/framework/security/src/main/java/org/apache/ofbiz/security/SecurityFactory.java
+++ 
b/framework/security/src/main/java/org/apache/ofbiz/security/SecurityFactory.java
@@ -43,31 +43,26 @@ import org.apache.ofbiz.entity.util.EntityUtil;
 public final class SecurityFactory {
 
     private static final String MODULE = SecurityFactory.class.getName();
-    // The default implementation stores a Delegator reference, so we will 
cache by delegator name.
-    // The goal is to remove Delegator references in the Security interface, 
then we can use a singleton
-    // and eliminate the cache.
+    // Security instances are bound to a delegator, so they are cached by 
delegator name.
     private static final UtilCache<String, Security> AUTHORIZATION_CACHE = 
UtilCache.createUtilCache("security.AuthorizationCache");
 
     /**
-     * Returns a <code>Security</code> instance. The method uses Java's
-     * <a 
href="http://docs.oracle.com/javase/6/docs/api/java/util/ServiceLoader.html";><code>ServiceLoader</code></a>
-     * to get a <code>Security</code> instance. If no instance is found, a 
default implementation is used.
-     * The default implementation is based on/backed by the OFBiz entity 
engine.
+     * Returns the <code>Security</code> instance bound to 
<code>delegator</code>. The instance is created by
+     * the first {@link SecurityProvider} that Java's {@link ServiceLoader} 
finds. If no provider is found, a
+     * default implementation is used. The default implementation is based 
on/backed by the OFBiz entity engine.
      * @param delegator The delegator
      * @return A <code>Security</code> instance
      */
-    @SuppressWarnings("deprecation")
     public static Security getInstance(Delegator delegator) throws 
SecurityConfigurationException {
         Assert.notNull("delegator", delegator);
         Security security = 
AUTHORIZATION_CACHE.get(delegator.getDelegatorName());
         if (security == null) {
-            Iterator<Security> iterator = 
ServiceLoader.load(Security.class).iterator();
+            Iterator<SecurityProvider> iterator = 
ServiceLoader.load(SecurityProvider.class).iterator();
             if (iterator.hasNext()) {
-                security = iterator.next();
+                security = iterator.next().create(delegator);
             } else {
-                security = new OFBizSecurity();
+                security = new OFBizSecurity(delegator);
             }
-            security.setDelegator(delegator);
             security = 
AUTHORIZATION_CACHE.putIfAbsentAndGet(delegator.getDelegatorName(), security);
             if (Debug.verboseOn()) {
                 Debug.logVerbose("Security implementation " + 
security.getClass().getName() + " created for delegator "
@@ -81,9 +76,11 @@ public final class SecurityFactory {
 
     private static final class OFBizSecurity implements Security {
 
-        private Delegator delegator = null;
+        private final Delegator delegator;
 
-        private OFBizSecurity() { }
+        private OFBizSecurity(Delegator delegator) {
+            this.delegator = delegator;
+        }
 
         @Override
         public void clearUserData(GenericValue userLogin) {
@@ -93,9 +90,8 @@ public final class SecurityFactory {
             }
         }
 
-        @Override
-        @Deprecated
-        public Iterator<GenericValue> 
findUserLoginSecurityGroupByUserLoginId(String userLoginId) {
+        // Uses the UserLoginSecurityGroup cache to find the userLogin's 
security groups.
+        private Iterator<GenericValue> 
findUserLoginSecurityGroupByUserLoginId(String userLoginId) {
             try {
                 List<GenericValue> collection = 
EntityUtil.filterByDate(EntityQuery.use(delegator).from("UserLoginSecurityGroup")
                         .where("userLoginId", 
userLoginId).cache(true).queryList());
@@ -106,12 +102,6 @@ public final class SecurityFactory {
             }
         }
 
-        @Override
-        @Deprecated
-        public Delegator getDelegator() {
-            return this.delegator;
-        }
-
         @Override
         public boolean hasEntityPermission(String entity, String action, 
GenericValue userLogin) {
             if (userLogin == null || entity == null || action == null) return 
false;
@@ -238,9 +228,8 @@ public final class SecurityFactory {
             return hasRolePermission(application, action, primaryKey, role, 
userLogin);
         }
 
-        @Override
-        @Deprecated
-        public boolean securityGroupPermissionExists(String groupId, String 
permission) {
+        // Caches the groupId, permission pair instead of the userLoginId, 
permission pair to keep the cache small.
+        private boolean securityGroupPermissionExists(String groupId, String 
permission) {
             try {
                 return 
EntityQuery.use(delegator).from("SecurityGroupPermission")
                         .where("groupId", groupId, "permissionId", 
permission).cache(true).filterByDate().queryFirst() != null;
@@ -249,15 +238,5 @@ public final class SecurityFactory {
                 return false;
             }
         }
-
-        @Override
-        @Deprecated
-        public void setDelegator(Delegator delegator) {
-            if (this.delegator != null) {
-                throw new IllegalStateException("This object has been 
initialized already.");
-            }
-            Assert.notNull("delegator", delegator);
-            this.delegator = delegator;
-        }
     }
 }
diff --git 
a/framework/security/src/main/java/org/apache/ofbiz/security/SecurityProvider.java
 
b/framework/security/src/main/java/org/apache/ofbiz/security/SecurityProvider.java
new file mode 100644
index 0000000000..f7cf64fdf9
--- /dev/null
+++ 
b/framework/security/src/main/java/org/apache/ofbiz/security/SecurityProvider.java
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.ofbiz.security;
+
+import org.apache.ofbiz.entity.Delegator;
+
+/**
+ * Service provider interface that creates {@link Security} instances.
+ * <p>An implementation is registered in a
+ * {@code META-INF/services/org.apache.ofbiz.security.SecurityProvider} file 
and
+ * discovered by {@link SecurityFactory} through {@link 
java.util.ServiceLoader},
+ * so it must have a public no-argument constructor.</p>
+ */
+public interface SecurityProvider {
+
+    /**
+     * Creates a <code>Security</code> instance bound to the given delegator.
+     * <p>{@link SecurityFactory} caches the returned instance per delegator 
and
+     * shares it among threads.</p>
+     * @param delegator The delegator the instance reads security data from, 
never <code>null</code>
+     * @return A new <code>Security</code> instance
+     */
+    Security create(Delegator delegator);
+}

Reply via email to