martin-g commented on code in PR #1369:
URL: https://github.com/apache/wicket/pull/1369#discussion_r2810907243
##########
wicket-cdi/src/main/java/org/apache/wicket/cdi/BeanManagerLookup.java:
##########
@@ -100,47 +83,22 @@ public BeanManager lookup()
return null;
}
}
- },
- FALLBACK {
- @Override
- public BeanManager lookup()
- {
- return
CdiConfiguration.get(Application.get()).getFallbackBeanManager();
- }
};
public abstract BeanManager lookup();
}
- private static BeanManagerLookupStrategy lastSuccessful =
BeanManagerLookupStrategy.CUSTOM;
-
- private BeanManagerLookup()
- {
- }
-
public static BeanManager lookup()
{
- BeanManager ret = lastSuccessful.lookup();
- if (ret != null)
- return ret;
-
for (BeanManagerLookupStrategy curStrategy :
BeanManagerLookupStrategy.values())
{
- ret = curStrategy.lookup();
+ BeanManager ret = curStrategy.lookup();
if (ret != null)
{
- lastSuccessful = curStrategy;
return ret;
}
}
-
- throw new IllegalStateException(
- "No BeanManager found via the CDI provider and no
fallback specified. Check your "
- + "CDI setup or specify a fallback BeanManager
in the CdiConfiguration.");
+ return null;
Review Comment:
What is the benefit here of returning `null` than throwing an exception ?
##########
wicket-cdi/src/main/java/org/apache/wicket/cdi/CdiConfiguration.java:
##########
@@ -110,6 +77,13 @@ public CdiConfiguration setFallbackBeanManager(BeanManager
fallbackBeanManager)
*/
public void configure(Application application)
{
+ if(beanManager == null)
Review Comment:
```suggestion
if (beanManager == null)
```
##########
wicket-cdi/src/main/java/org/apache/wicket/cdi/CdiConfiguration.java:
##########
@@ -61,46 +64,10 @@ public CdiConfiguration
setPropagation(IConversationPropagation propagation)
public BeanManager getBeanManager()
{
- return beanManager;
- }
-
- /**
- * Sets a BeanManager that should be used at first.
- *
- * @param beanManager
- * @return this instance
- */
- public CdiConfiguration setBeanManager(BeanManager beanManager)
- {
-
- if (Application.exists() &&
CdiConfiguration.get(Application.get()) != null)
+ if (beanManager == null)
throw new IllegalStateException(
- "A CdiConfiguration is already set for the
application.");
-
- this.beanManager = beanManager;
- return this;
- }
-
- public BeanManager getFallbackBeanManager()
- {
- return fallbackBeanManager;
- }
-
- /**
- * Sets a BeanManager that should be used if all strategies to lookup a
- * BeanManager fail. This can be used in scenarios where you do not have
- * JNDI available and do not want to bootstrap the CDI provider. It
should
- * be noted that the fallback BeanManager can only be used within the
- * context of a Wicket application (ie. Application.get() should return
the
- * application that was configured with this CdiConfiguration).
- *
- * @param fallbackBeanManager
- * @return this instance
- */
- public CdiConfiguration setFallbackBeanManager(BeanManager
fallbackBeanManager)
- {
- this.fallbackBeanManager = fallbackBeanManager;
- return this;
+ "app not configured or no BeanManager was
resolved during the configuration");
Review Comment:
The error message is vague and does not suggest actionable guidance for the
developers.
##########
wicket-cdi/src/main/java/org/apache/wicket/cdi/CdiConfiguration.java:
##########
@@ -39,15 +39,18 @@ public class CdiConfiguration
private BeanManager beanManager;
- private BeanManager fallbackBeanManager;
-
/**
* Constructor
*/
public CdiConfiguration()
{
}
+ public CdiConfiguration(BeanManager beanManager)
+ {
+ this.beanManager = beanManager;
Review Comment:
Maybe add a non-null check ?!
Otherwise, it behaves as the no-arg constructor if `null` is passed.
##########
wicket-cdi/src/main/java/org/apache/wicket/cdi/BeanManagerLookup.java:
##########
@@ -100,47 +83,22 @@ public BeanManager lookup()
return null;
}
}
- },
- FALLBACK {
- @Override
- public BeanManager lookup()
- {
- return
CdiConfiguration.get(Application.get()).getFallbackBeanManager();
- }
};
public abstract BeanManager lookup();
}
- private static BeanManagerLookupStrategy lastSuccessful =
BeanManagerLookupStrategy.CUSTOM;
-
- private BeanManagerLookup()
Review Comment:
Why is this constructor removed ?
This is a utility class with a single static method.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
