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



##########
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:
       I see, there is some literature abut double-check lock pattern, and some 
is not very pleasant with it, especially for Java implementations.
   
   What would be the disadvantage of simply removing the first `if` and leaving 
the whole logic in the `synchronized` block?

##########
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:
       I see, there is wide literature abut double-check lock pattern, and some 
is not very pleasant with it, especially for Java implementations.
   
   What would be the disadvantage of simply removing the first `if` and leaving 
the whole logic in the `synchronized` block?




-- 
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:
[email protected]


Reply via email to