Juan Hernandez has uploaded a new change for review.

Change subject: core: Loadable auth providers
......................................................................

core: Loadable auth providers

Currently the different types of authentication providers is hard coded
in the LdapFactory class. This patch uses the service provider mechanism
to make that configurable, so that different authentication brokers can
be dropped in the classpath (in the lib directory of the .ear, for
example) and detected automatically when the engine is restarted.

In order to do this the .jar file containing the authentication provider
has to contain an implementation of the LdapBroker interface and a
META-INF/service/org.ovirt.engine.core.bll.adbroker.LdapBroker file
containing fully qualified name of the implementation class.

This patch also changes the current LDAP and internal authentication
providers so that they use this mechanism.

Change-Id: Iae6b71c76a1fcd90e6eb306478a1621f5834e3ef
Signed-off-by: Juan Hernandez <juan.hernan...@redhat.com>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InternalBrokerImpl.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/LdapBroker.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/LdapBrokerBase.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/LdapBrokerImpl.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/LdapBrokerUtils.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/LdapFactory.java
A 
backend/manager/modules/bll/src/main/resources/META-INF/services/org.ovirt.engine.core.bll.adbroker.LdapBroker
7 files changed, 130 insertions(+), 39 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/14/9414/1

diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InternalBrokerImpl.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InternalBrokerImpl.java
index 009b0fe..bccbde7 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InternalBrokerImpl.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InternalBrokerImpl.java
@@ -1,11 +1,25 @@
 package org.ovirt.engine.core.bll;
 
+import java.util.Collections;
+import java.util.Set;
+
 import org.ovirt.engine.core.bll.adbroker.LdapBrokerBase;
 
 public class InternalBrokerImpl extends LdapBrokerBase {
+    // The type of this broker:
+    private static final String TYPE = "Internal";
+
+    // The set of domains managed by this broker:
+    private static final Set<String> DOMAINS = 
Collections.singleton("internal");
+
     @Override
-    protected String getBrokerType() {
-        return "Internal";
+    public Set<String> getDomains() {
+        return DOMAINS;
+    }
+
+    @Override
+    protected String getBrokerType () {
+        return TYPE;
     }
 }
 
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/LdapBroker.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/LdapBroker.java
index c11ca97..46fdb1f 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/LdapBroker.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/LdapBroker.java
@@ -1,5 +1,12 @@
 package org.ovirt.engine.core.bll.adbroker;
 
+import java.util.Set;
+
 public interface LdapBroker {
+    /**
+     * Returns the set of domains managed by this broker.
+     */
+    Set<String> getDomains();
+
     LdapReturnValueBase RunAdAction(AdActionType actionType, 
LdapBrokerBaseParameters parameters);
 }
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/LdapBrokerBase.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/LdapBrokerBase.java
index d6becaf..dc85932 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/LdapBrokerBase.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/LdapBrokerBase.java
@@ -1,40 +1,71 @@
 package org.ovirt.engine.core.bll.adbroker;
 
-import org.ovirt.engine.core.utils.log.Log;
-import org.ovirt.engine.core.utils.log.LogFactory;
+import java.lang.reflect.Constructor;
+
+import org.apache.log4j.Logger;
 import org.ovirt.engine.core.utils.ReflectionUtils;
 
 public abstract class LdapBrokerBase implements LdapBroker {
-    private static final String CommandsContainerAssemblyName = 
LdapBrokerBase.class.getPackage().getName();
-    private static final String CommandPrefix = "Command";
+    // The log:
+    private static final Logger log = Logger.getLogger(LdapBrokerBase.class);
 
-    private static Log log = LogFactory.getLog(LdapBrokerBase.class);
+    // The full qualified name of the package where all command classes are
+    // expected to live:
+    private static final String PACKAGE_NAME = 
LdapBrokerBase.class.getPackage().getName();
 
+    // All the command classes are expected to have a name ending in this
+    // suffix:
+    private static final String CLASS_SUFFIX = "Command";
+
+    /**
+     * This method should be implemented by subclasses. They should return an
+     * string that identifies the type of the broker and that will be used as a
+     * prefix to build the names of the classes implementing the commands. For
+     * example, if a subclass returns <code>Ldap</code> then the command 
classes
+     * will be expected to have names like <code>LdapWhateverCommand</code>.
+     */
     protected abstract String getBrokerType();
 
-    public LdapReturnValueBase RunAdAction(AdActionType actionType, 
LdapBrokerBaseParameters parameters) {
-        log.debug("RunAdAction Entry, actionType=" + actionType.toString());
-        BrokerCommandBase command = CreateCommand(actionType, parameters);
-        return command.execute();
+    /**
+     * Calculates the fully qualified name of the command class corresponding 
to
+     * the given action.
+     *
+     * @param action the type of action to be performed
+     * @return the name of the command class that performs that action
+     */
+    private String getCommandClassName(AdActionType action) {
+        return PACKAGE_NAME + "." + getBrokerType() + action + CLASS_SUFFIX;
     }
 
-    private BrokerCommandBase CreateCommand(AdActionType action, 
LdapBrokerBaseParameters parameters) {
+    private BrokerCommandBase createCommand(AdActionType action, 
LdapBrokerBaseParameters parameters) {
+        // Compute the name of the class:
+        final String commandClassName = getCommandClassName(action);
+
+        // Try to load the class:
+        Class<?> commandClass = null;
         try {
-            java.lang.Class type = 
java.lang.Class.forName(GetCommandTypeName(action));
-            java.lang.reflect.Constructor info = 
ReflectionUtils.findConstructor(type, parameters.getClass());
-            Object tempVar = info.newInstance(parameters);
-            return (BrokerCommandBase) ((tempVar instanceof BrokerCommandBase) 
? tempVar : null);
+            commandClass = Class.forName(commandClassName);
+        }
+        catch (ClassNotFoundException exception) {
+            log.error("Can't load command class \"" + commandClassName + 
"\".", exception);
+            return null;
         }
 
-        catch (java.lang.Exception e) {
-            log.errorFormat("LdapBrokerCommandBase: Failed to get type 
information using reflection for Action: {0}",
-                    action);
+        // Create an instance of the class:
+        try {
+            Constructor<?> commandConstructor = 
ReflectionUtils.findConstructor(commandClass, parameters.getClass());
+            Object instance = commandConstructor.newInstance(parameters);
+            return (BrokerCommandBase) ((instance instanceof 
BrokerCommandBase) ? instance : null);
+        }
+        catch (Exception exception) {
+            log.error("Can't create an instance of class \"" + 
commandClassName + "\".", exception);
             return null;
         }
     }
 
-    private String GetCommandTypeName(AdActionType action) {
-        return String
-                .format("%1$s.%2$s%3$s%4$s", CommandsContainerAssemblyName, 
getBrokerType(), action, CommandPrefix);
+    public LdapReturnValueBase RunAdAction(AdActionType actionType, 
LdapBrokerBaseParameters parameters) {
+        log.debug("Running action \"" + actionType + "\".");
+        BrokerCommandBase command = createCommand(actionType, parameters);
+        return command.execute();
     }
 }
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/LdapBrokerImpl.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/LdapBrokerImpl.java
index 57ea4a4..5ae05f4 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/LdapBrokerImpl.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/LdapBrokerImpl.java
@@ -1,8 +1,31 @@
 package org.ovirt.engine.core.bll.adbroker;
 
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
 public class LdapBrokerImpl extends LdapBrokerBase {
+    // The type of this broker:
+    private static final String TYPE = "Ldap";
+
+    // The set of domains managed by this broker:
+    private static final Set<String> DOMAINS = new HashSet<String>();
+
+    static {
+        // We get the list of domains once and cache it for the life of the
+        // engine, this is not a problem as the only way to add a domain is to
+        // run the engine-manage-domains tool and then restart the engine:
+        final List<String> list = LdapBrokerUtils.getDomainsList(true);
+        DOMAINS.addAll(list);
+    }
+
     @Override
-    protected String getBrokerType() {
-        return "Ldap";
+    public Set<String> getDomains () {
+        return DOMAINS;
+    }
+
+    @Override
+    protected String getBrokerType () {
+        return TYPE;
     }
 }
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/LdapBrokerUtils.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/LdapBrokerUtils.java
index f8586b3..2cc6bd0 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/LdapBrokerUtils.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/LdapBrokerUtils.java
@@ -2,7 +2,6 @@
 
 import java.net.SocketTimeoutException;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Hashtable;
@@ -44,7 +43,6 @@
      */
     public static List<String> getDomainsList(boolean filterInternalDomain) {
         String[] domains = Config.<String> 
GetValue(ConfigValues.DomainName).split("[,]", -1);
-        List<String> domainsList = Arrays.asList(domains);
         List<String> results = new ArrayList<String>();
         for (String domain : domains) {
             String trimmedDomain = domain.trim();
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/LdapFactory.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/LdapFactory.java
index 88a3027..8842d0e 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/LdapFactory.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/LdapFactory.java
@@ -1,25 +1,41 @@
 package org.ovirt.engine.core.bll.adbroker;
 
-import org.ovirt.engine.core.bll.InternalBrokerImpl;
-import org.ovirt.engine.core.common.config.Config;
-import org.ovirt.engine.core.common.config.ConfigValues;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.ServiceLoader;
+import java.util.Set;
+
+import org.apache.log4j.Logger;
 
 public final class LdapFactory {
+    // The log:
+    private static final Logger log = Logger.getLogger(LdapFactory.class);
 
-    private static LdapBroker internalInstance;
-    private static LdapBroker ldapInstance;
-    private static String internalDomain = Config.<String> 
GetValue(ConfigValues.AdminDomain).trim();
+    // The list of broker implementations loaded using the service provider
+    // mechanism (the initial size is 2 because usually we will have only two
+    // brokers, the internal one and the LDAP one):
+    private static List<LdapBroker> brokers = new ArrayList<LdapBroker>(2);
 
     static {
-        internalInstance = new InternalBrokerImpl();
-        ldapInstance = new LdapBrokerImpl();
+        // Load the providers using the service provider interface mechanism:
+        final ServiceLoader<LdapBroker> loader = 
ServiceLoader.load(LdapBroker.class);
+        for (LdapBroker broker : loader) {
+            brokers.add(broker);
+            log.info("Loaded broker of class \"" + broker.getClass() + "\".");
+        }
     }
 
-    public static LdapBroker getInstance(String domain) {
-        if (domain.equalsIgnoreCase(internalDomain)) {
-            return internalInstance;
-        } else {
-            return ldapInstance;
+    public static LdapBroker getInstance(final String domain) {
+        // Try to find a broker that can handle the given domain:
+        for (LdapBroker broker : brokers) {
+            final Set<String> domains = broker.getDomains();
+            if (domains.contains(domain)) {
+                return broker;
+            }
         }
+
+        // If we are here we haven't been able to find a broker of the 
requested type:
+        log.error("Can't find any broker for domain \"" + domain + "\".");
+        return null;
     }
 }
diff --git 
a/backend/manager/modules/bll/src/main/resources/META-INF/services/org.ovirt.engine.core.bll.adbroker.LdapBroker
 
b/backend/manager/modules/bll/src/main/resources/META-INF/services/org.ovirt.engine.core.bll.adbroker.LdapBroker
new file mode 100644
index 0000000..0205fe6
--- /dev/null
+++ 
b/backend/manager/modules/bll/src/main/resources/META-INF/services/org.ovirt.engine.core.bll.adbroker.LdapBroker
@@ -0,0 +1,2 @@
+org.ovirt.engine.core.bll.InternalBrokerImpl
+org.ovirt.engine.core.bll.adbroker.LdapBrokerImpl


--
To view, visit http://gerrit.ovirt.org/9414
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Iae6b71c76a1fcd90e6eb306478a1621f5834e3ef
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Juan Hernandez <juan.hernan...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to