svn commit: r1531577 - in /ofbiz/trunk/framework/security: config/security.properties config/security.xml src/org/ofbiz/security/Security.java src/org/ofbiz/security/SecurityFactory.java

[adrianc]

Author: adrianc
Date: Sat Oct 12 18:54:59 2013
New Revision: 1531577

URL: http://svn.apache.org/r1531577
Log:
Refactored SecurityFactory implementation - use Java's ServiceLoader to discover Security implementations, instead of using configuration files.

Removed:
    ofbiz/trunk/framework/security/config/security.xml
Modified:
    ofbiz/trunk/framework/security/config/security.properties
    ofbiz/trunk/framework/security/src/org/ofbiz/security/Security.java
    ofbiz/trunk/framework/security/src/org/ofbiz/security/SecurityFactory.java

Modified: ofbiz/trunk/framework/security/config/security.properties
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/security/config/security.properties?rev=1531577&r1=1531576&r2=1531577&view=diff
==============================================================================
--- ofbiz/trunk/framework/security/config/security.properties (original)
+++ ofbiz/trunk/framework/security/config/security.properties Sat Oct 12 18:54:59 2013
@@ -20,9 +20,6 @@
 # OFBiz Security Settings
 ####
 
-# -- define the default security context to be user
-security.context=default
-
 # -- define the password restrictions --
 password.length.min=5
 

Modified: ofbiz/trunk/framework/security/src/org/ofbiz/security/Security.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/security/src/org/ofbiz/security/Security.java?rev=1531577&r1=1531576&r2=1531577&view=diff
==============================================================================
--- ofbiz/trunk/framework/security/src/org/ofbiz/security/Security.java (original)
+++ ofbiz/trunk/framework/security/src/org/ofbiz/security/Security.java Sat Oct 12 18:54:59 2013
@@ -28,6 +28,11 @@ import org.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>
  */
 public interface Security {
 

Modified: ofbiz/trunk/framework/security/src/org/ofbiz/security/SecurityFactory.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/security/src/org/ofbiz/security/SecurityFactory.java?rev=1531577&r1=1531576&r2=1531577&view=diff
==============================================================================
--- ofbiz/trunk/framework/security/src/org/ofbiz/security/SecurityFactory.java (original)
+++ ofbiz/trunk/framework/security/src/org/ofbiz/security/SecurityFactory.java Sat Oct 12 18:54:59 2013
@@ -18,111 +18,53 @@
  *******************************************************************************/
 package org.ofbiz.security;
 
-import java.net.URL;
-import java.util.concurrent.atomic.AtomicReference;
+import java.util.Iterator;
+import java.util.ServiceLoader;
 
+import org.ofbiz.base.util.Assert;
 import org.ofbiz.base.util.Debug;
-import org.ofbiz.base.util.UtilProperties;
-import org.ofbiz.base.util.UtilURL;
-import org.ofbiz.base.util.UtilXml;
+import org.ofbiz.base.util.cache.UtilCache;
 import org.ofbiz.entity.Delegator;
-import org.w3c.dom.Document;
-import org.w3c.dom.Element;
 
 /**
- * <code>SecurityFactory</code>
- *
- * This Factory class returns an instance of a security implementation.
- *
- * Setting the security implementation className is done in security.xml.
- * If no customized security name is given, the default implementation will be used (OFBizSecurity)
+ * A <code>Security</code> factory.
  */
 public final class SecurityFactory {
 
     public static final String module = SecurityFactory.class.getName();
-    private static final String DEFAULT_SECURITY_CLASS_NAME = "org.ofbiz.security.OFBizSecurity";
-    private static final String SECURITY_CONFIG_XML_FILENAME = "security.xml";
-    private static final AtomicReference<SecurityInfo> configRef = new AtomicReference<SecurityInfo>(null);
+    // 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.
+    private static final UtilCache<String, Security> authorizationCache = UtilCache.createUtilCache("security.AuthorizationCache");
 
     /**
-     * Returns an instance of a Security implementation as defined in the security.xml by defined name
-     * in security.properties.
+     * 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.
      *
-     * @param delegator the generic delegator
-     * @return instance of security implementation (default: OFBizSecurity)
+     * @param delegator The delegator
+     * @return A <code>Security</code> instance
      */
     @SuppressWarnings("deprecation")
     public static Security getInstance(Delegator delegator) throws SecurityConfigurationException {
-        Security security = null;
-        String securityClassName = DEFAULT_SECURITY_CLASS_NAME;
-        try {
-            SecurityInfo securityInfo = getSecurityInfo();
-            securityClassName = securityInfo.className;
-        } catch (SecurityConfigurationException e) {
-            Debug.logError(e, "Exception thrown while getting security configuration, using default security class " + DEFAULT_SECURITY_CLASS_NAME, module);
-        }
-        try {
-            ClassLoader loader = Thread.currentThread().getContextClassLoader();
-            Class<?> c = loader.loadClass(securityClassName);
-            security = (Security) c.newInstance();
-            security.setDelegator(delegator);
-        } catch (ClassNotFoundException cnf) {
-            throw new SecurityConfigurationException("Cannot load security implementation class", cnf);
-        } catch (InstantiationException ie) {
-            throw new SecurityConfigurationException("Cannot get instance of the security implementation", ie);
-        } catch (IllegalAccessException iae) {
-            throw new SecurityConfigurationException(iae.getMessage(), iae);
-        }
-        if (Debug.verboseOn()) {
-            Debug.logVerbose("Security implementation created for delegator " + delegator.getDelegatorName(), module);
-        }
-        return security;
-    }
-
-    private static SecurityInfo getSecurityInfo() throws SecurityConfigurationException {
-        SecurityInfo instance = configRef.get();
-        if (instance == null) {
-            URL confUrl = UtilURL.fromResource(SECURITY_CONFIG_XML_FILENAME);
-            if (confUrl == null) {
-                throw new SecurityConfigurationException("Could not find the " + SECURITY_CONFIG_XML_FILENAME + " file");
-            }
-            String securityName = UtilProperties.getPropertyValue("security.properties", "security.context");
-            if (Debug.verboseOn()) {
-                Debug.logVerbose("Security implementation context name from security.properties: " + securityName, module);
-            }
-            Document document = null;
-            try {
-                document = UtilXml.readXmlDocument(confUrl, true, true);
-            } catch (Exception e) {
-                throw new SecurityConfigurationException("Exception thrown while reading " + SECURITY_CONFIG_XML_FILENAME + ": ", e);
-            }
-            Element securityElement = UtilXml.firstChildElement(document.getDocumentElement(), "security", "name", securityName);
-            if (securityElement == null) {
-                throw new SecurityConfigurationException("Could not find the <security> element in the " + SECURITY_CONFIG_XML_FILENAME + " file");
-            }
-            instance = new SecurityInfo(securityElement);
-            if (configRef.compareAndSet(null, instance)) {
-                if (Debug.verboseOn()) {
-                    Debug.logVerbose("Security configuration read from " + SECURITY_CONFIG_XML_FILENAME + ", using class " + instance.className, module);
-                }
+        Assert.notNull("delegator", delegator);
+        Security security = authorizationCache.get(delegator.getDelegatorName());
+        if (security == null) {
+            Iterator<Security> iterator = ServiceLoader.load(Security.class).iterator();
+            if (iterator.hasNext()) {
+                security = iterator.next();
             } else {
-                instance = configRef.get();
+                security = new OFBizSecurity();
             }
-        }
-        return instance;
-    }
-
-    private static final class SecurityInfo {
-        private final String name;
-        private final String className;
-
-        private SecurityInfo(Element element) throws SecurityConfigurationException {
-            this.name = element.getAttribute("name");
-            this.className = element.getAttribute("class");
-            if (this.className.isEmpty()) {
-                throw new SecurityConfigurationException("<security> element class attribute is empty in the " + SECURITY_CONFIG_XML_FILENAME + " file");
+            security.setDelegator(delegator);
+            security = authorizationCache.putIfAbsentAndGet(delegator.getDelegatorName(), security);
+            Debug.logInfo("Security implementation " + security.getClass().getName() + " created for delegator " + delegator.getDelegatorName(), module);
+            if (Debug.verboseOn()) {
+                Debug.logVerbose("Security implementation " + security.getClass().getName() + " created for delegator " + delegator.getDelegatorName(), module);
             }
         }
+        return security;
     }
 
     private SecurityFactory() {}