[GitHub] [syncope] ilgrosso commented on a change in pull request #264: [SYNCOPE-1635] Use singleton bean for each domain for each rule with conf

[GitBox <gi...@apache.org>]

ilgrosso commented on a change in pull request #264:
URL: https://github.com/apache/syncope/pull/264#discussion_r636028258



##########
File path: core/spring/src/main/java/org/apache/syncope/core/spring/ImplementationManager.java
##########
@@ -96,7 +97,7 @@
         }
     }
 
-    public static Optional<PasswordRule> buildPasswordRule(final Implementation impl)
+    public static Optional<PasswordRule> buildPasswordRule(final String domain, final Implementation impl)

Review comment:
       same as above

##########
File path: core/spring/src/main/java/org/apache/syncope/core/spring/ImplementationManager.java
##########
@@ -48,7 +49,7 @@
 
     private static final Map<String, Class<?>> CLASS_CACHE = Collections.synchronizedMap(new HashMap<>());
 
-    public static Optional<Reportlet> buildReportlet(final Implementation impl)
+    public static Optional<Reportlet> buildReportlet(final String domain, final Implementation impl)

Review comment:
       Why adding a `domain` argument and not letting the method body getting it via `AuthContextUtils.getDomain()` ?

##########
File path: core/spring/src/main/java/org/apache/syncope/core/spring/ImplementationManager.java
##########
@@ -120,7 +121,7 @@
         }
     }
 
-    public static Optional<PullCorrelationRule> buildPullCorrelationRule(final Implementation impl)
+    public static Optional<PullCorrelationRule> buildPullCorrelationRule(final String domain, final Implementation impl)

Review comment:
       same as above

##########
File path: core/spring/src/main/java/org/apache/syncope/core/spring/ImplementationManager.java
##########
@@ -216,28 +217,27 @@
     }
 
     @SuppressWarnings("unchecked")
-    private static <T> T buildJavaWithConf(final Class<T> clazz) {
-        T bean = null;
-
+    private static <T> T buildJavaWithConf(final String domain, final Class<T> clazz) {
         if (clazz != null) {
-            if (ApplicationContextProvider.getBeanFactory().containsSingleton(clazz.getName())) {
-                bean = (T) ApplicationContextProvider.getBeanFactory().getSingleton(clazz.getName());
-            } else {
-                try {
-                    bean = (T) ApplicationContextProvider.getBeanFactory().
-                            createBean(clazz, AbstractBeanDefinition.AUTOWIRE_BY_TYPE, false);
-                    ApplicationContextProvider.getBeanFactory().registerSingleton(clazz.getName(), bean);
-                } catch (IllegalStateException e) {
-                    LOG.debug("While attempting to register {}", clazz.getName(), e);
+            String domainableBeanNameWithConf = domain + clazz.getName();
+            DefaultListableBeanFactory beanFactory = ApplicationContextProvider.getBeanFactory();
+
+            if (beanFactory.containsSingleton(domainableBeanNameWithConf)) {

Review comment:
       The same exact `if` with statement is also performed in the `synchronized` block: why?

##########
File path: core/spring/src/main/java/org/apache/syncope/core/spring/ImplementationManager.java
##########
@@ -72,7 +73,7 @@
         }
     }
 
-    public static Optional<AccountRule> buildAccountRule(final Implementation impl)
+    public static Optional<AccountRule> buildAccountRule(final String domain, final Implementation impl)

Review comment:
       same as above

##########
File path: core/spring/src/main/java/org/apache/syncope/core/spring/ImplementationManager.java
##########
@@ -145,7 +146,7 @@
         }
     }
 
-    public static Optional<PushCorrelationRule> buildPushCorrelationRule(final Implementation impl)
+    public static Optional<PushCorrelationRule> buildPushCorrelationRule(final String domain, final Implementation impl)

Review comment:
       same as above




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org