Repository: wicket Updated Branches: refs/heads/wicket-6.x 899d72777 -> 27ccea351
WICKET-5935 IoC Guice: cache proxies and fail on creation when binding is missing - cache proxies in the same way the spring ioc caches them - If a bean has no binding the exception is now throw on construction instead of when that bean/proxy is used. Project: http://git-wip-us.apache.org/repos/asf/wicket/repo Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/27ccea35 Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/27ccea35 Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/27ccea35 Branch: refs/heads/wicket-6.x Commit: 27ccea351631c8d28bf7a954697f97bce599f7e2 Parents: 899d727 Author: Thomas Matthijs <[email protected]> Authored: Wed Jun 24 15:26:45 2015 +0200 Committer: Martin Tzvetanov Grigorov <[email protected]> Committed: Thu Jun 25 11:44:13 2015 +0300 ---------------------------------------------------------------------- .../wicket/guice/GuiceFieldValueFactory.java | 35 +++++-- .../wicket/guice/GuiceProxyTargetLocator.java | 96 +++++++++++++++----- .../guice/JavaxInjectGuiceInjectorTest.java | 29 ++++-- .../wicket/guice/JavaxInjectTestComponent.java | 13 --- 4 files changed, 124 insertions(+), 49 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/wicket/blob/27ccea35/wicket-guice/src/main/java/org/apache/wicket/guice/GuiceFieldValueFactory.java ---------------------------------------------------------------------- diff --git a/wicket-guice/src/main/java/org/apache/wicket/guice/GuiceFieldValueFactory.java b/wicket-guice/src/main/java/org/apache/wicket/guice/GuiceFieldValueFactory.java index d1543e1..91085c2 100644 --- a/wicket-guice/src/main/java/org/apache/wicket/guice/GuiceFieldValueFactory.java +++ b/wicket-guice/src/main/java/org/apache/wicket/guice/GuiceFieldValueFactory.java @@ -19,21 +19,25 @@ package org.apache.wicket.guice; import java.lang.annotation.Annotation; import java.lang.reflect.Field; import java.lang.reflect.Modifier; +import java.util.concurrent.ConcurrentMap; import javax.inject.Qualifier; import org.apache.wicket.injection.IFieldValueFactory; -import org.apache.wicket.proxy.IProxyTargetLocator; import org.apache.wicket.proxy.LazyInitProxyFactory; import com.google.inject.BindingAnnotation; import com.google.inject.Inject; +import org.apache.wicket.util.lang.Generics; /** * */ public class GuiceFieldValueFactory implements IFieldValueFactory { + private final ConcurrentMap<GuiceProxyTargetLocator, Object> cache = Generics.newConcurrentHashMap(); + private static final Object NULL_SENTINEL = new Object(); + private final boolean wrapInProxies; /** @@ -64,15 +68,34 @@ public class GuiceFieldValueFactory implements IFieldValueFactory { boolean optional = injectAnnotation != null && injectAnnotation.optional(); Annotation bindingAnnotation = findBindingAnnotation(field.getAnnotations()); - final IProxyTargetLocator locator = new GuiceProxyTargetLocator(field, bindingAnnotation, optional); + final GuiceProxyTargetLocator locator = new GuiceProxyTargetLocator(field, bindingAnnotation, optional); + + Object cachedValue = cache.get(locator); + if (cachedValue != null) + { + return cachedValue; + } - if (wrapInProxies) + target = locator.locateProxyTarget(); + if (target == null) { - target = LazyInitProxyFactory.createProxy(field.getType(), locator); + // Optional without a binding, return null } else { - target = locator.locateProxyTarget(); + if (wrapInProxies) + { + target = LazyInitProxyFactory.createProxy(field.getType(), locator); + } + } + + if (locator.isSingletonScope()) + { + Object tmpTarget = cache.putIfAbsent(locator, target == null ? NULL_SENTINEL : target); + if (tmpTarget != null) + { + target = tmpTarget; + } } if (!field.isAccessible()) @@ -89,7 +112,7 @@ public class GuiceFieldValueFactory implements IFieldValueFactory } } - return target; + return target == NULL_SENTINEL ? null : target; } /** http://git-wip-us.apache.org/repos/asf/wicket/blob/27ccea35/wicket-guice/src/main/java/org/apache/wicket/guice/GuiceProxyTargetLocator.java ---------------------------------------------------------------------- diff --git a/wicket-guice/src/main/java/org/apache/wicket/guice/GuiceProxyTargetLocator.java b/wicket-guice/src/main/java/org/apache/wicket/guice/GuiceProxyTargetLocator.java index 67c5c7b..bc2be96 100644 --- a/wicket-guice/src/main/java/org/apache/wicket/guice/GuiceProxyTargetLocator.java +++ b/wicket-guice/src/main/java/org/apache/wicket/guice/GuiceProxyTargetLocator.java @@ -20,13 +20,16 @@ import java.lang.annotation.Annotation; import java.lang.reflect.Field; import java.lang.reflect.Type; +import com.google.inject.ConfigurationException; import com.google.inject.Injector; import com.google.inject.Key; +import com.google.inject.Scopes; import com.google.inject.TypeLiteral; import org.apache.wicket.Application; import org.apache.wicket.WicketRuntimeException; import org.apache.wicket.proxy.IProxyTargetLocator; import org.apache.wicket.core.util.lang.WicketObjects; +import org.apache.wicket.util.lang.Objects; class GuiceProxyTargetLocator implements IProxyTargetLocator { @@ -40,8 +43,10 @@ class GuiceProxyTargetLocator implements IProxyTargetLocator private final String fieldName; + private Boolean isSingletonCache = null; + public GuiceProxyTargetLocator(final Field field, final Annotation bindingAnnotation, - final boolean optional) + final boolean optional) { this.bindingAnnotation = bindingAnnotation; this.optional = optional; @@ -52,9 +57,34 @@ class GuiceProxyTargetLocator implements IProxyTargetLocator @Override public Object locateProxyTarget() { - final GuiceInjectorHolder holder = Application.get().getMetaData( - GuiceInjectorHolder.INJECTOR_KEY); + Injector injector = getInjector(); + + final Key<?> key = newGuiceKey(); + // if the Inject annotation is marked optional and no binding is found + // then skip this injection (WICKET-2241) + if (optional) + { + // Guice 2.0 throws a ConfigurationException if no binding is find while 1.0 simply + // returns null. + try + { + if (injector.getBinding(key) == null) + { + return null; + } + } + catch (RuntimeException e) + { + return null; + } + } + + return injector.getInstance(key); + } + + private Key<?> newGuiceKey() + { final Type type; try { @@ -70,38 +100,62 @@ class GuiceProxyTargetLocator implements IProxyTargetLocator // using TypeLiteral to retrieve the key gives us automatic support for // Providers and other injectable TypeLiterals - final Key<?> key; - if (bindingAnnotation == null) { - key = Key.get(TypeLiteral.get(type)); + return Key.get(TypeLiteral.get(type)); } else { - key = Key.get(TypeLiteral.get(type), bindingAnnotation); + return Key.get(TypeLiteral.get(type), bindingAnnotation); } + } - Injector injector = holder.getInjector(); - - // if the Inject annotation is marked optional and no binding is found - // then skip this injection (WICKET-2241) - if (optional) + public boolean isSingletonScope() + { + if (isSingletonCache == null) { - // Guice 2.0 throws a ConfigurationException if no binding is find while 1.0 simply - // returns null. try { - if (injector.getBinding(key) == null) - { - return null; - } + isSingletonCache = Scopes.isSingleton(getInjector().getBinding(newGuiceKey())); } - catch (RuntimeException e) + catch (ConfigurationException ex) { - return null; + // No binding, if optional can pretend this is null singleton + if (optional) + isSingletonCache = true; + else + throw ex; } } + return isSingletonCache; + } - return injector.getInstance(key); + private Injector getInjector() + { + final GuiceInjectorHolder holder = Application.get().getMetaData( + GuiceInjectorHolder.INJECTOR_KEY); + + return holder.getInjector(); + } + + @Override + public boolean equals(Object o) + { + if (this == o) + return true; + if (!(o instanceof GuiceProxyTargetLocator)) + return false; + GuiceProxyTargetLocator that = (GuiceProxyTargetLocator) o; + return Objects.equal(optional, that.optional) && + Objects.equal(bindingAnnotation, that.bindingAnnotation) && + Objects.equal(className, that.className) && + Objects.equal(fieldName, that.fieldName); } + + @Override + public int hashCode() + { + return Objects.hashCode(bindingAnnotation, optional, className, fieldName); + } + } http://git-wip-us.apache.org/repos/asf/wicket/blob/27ccea35/wicket-guice/src/test/java/org/apache/wicket/guice/JavaxInjectGuiceInjectorTest.java ---------------------------------------------------------------------- diff --git a/wicket-guice/src/test/java/org/apache/wicket/guice/JavaxInjectGuiceInjectorTest.java b/wicket-guice/src/test/java/org/apache/wicket/guice/JavaxInjectGuiceInjectorTest.java index 7935606..cebf410 100644 --- a/wicket-guice/src/test/java/org/apache/wicket/guice/JavaxInjectGuiceInjectorTest.java +++ b/wicket-guice/src/test/java/org/apache/wicket/guice/JavaxInjectGuiceInjectorTest.java @@ -21,9 +21,11 @@ import static org.hamcrest.CoreMatchers.is; import com.google.inject.ConfigurationException; import com.google.inject.spi.Message; -import org.apache.wicket.settings.IResourceSettings; +import org.apache.wicket.ajax.attributes.IAjaxCallListener; import org.junit.Test; +import javax.inject.Inject; + /** */ public class JavaxInjectGuiceInjectorTest extends AbstractInjectorTest @@ -50,21 +52,30 @@ public class JavaxInjectGuiceInjectorTest extends AbstractInjectorTest @Test public void required() { - JavaxInjectTestComponent component = newTestComponent("id"); - - // get the lazy proxy - IResourceSettings nonExisting = component.getNonExisting(); - try { - // call any method on the lazy proxy - nonExisting.getCachingStrategy(); + JavaxInjectTestComponent component = new MyJavaxInjectWithNonExistingTestComponent(); + // Throws exception because component.getNonExisting() cannot be injected fail("Fields annotated with @javax.inject.Inject are required!"); } catch (ConfigurationException cx) { Message message = cx.getErrorMessages().iterator().next(); - assertThat(message.getMessage(), is(equalTo("No implementation for org.apache.wicket.settings.IResourceSettings was bound."))); + assertThat(message.getMessage(), is(equalTo("No implementation for org.apache.wicket.ajax.attributes.IAjaxCallListener was bound."))); + } + } + + private static class MyJavaxInjectWithNonExistingTestComponent extends JavaxInjectTestComponent { + @Inject + private IAjaxCallListener nonExisting; + + public MyJavaxInjectWithNonExistingTestComponent() { + super("id"); + } + + + public IAjaxCallListener getNonExisting() { + return nonExisting; } } } http://git-wip-us.apache.org/repos/asf/wicket/blob/27ccea35/wicket-guice/src/test/java/org/apache/wicket/guice/JavaxInjectTestComponent.java ---------------------------------------------------------------------- diff --git a/wicket-guice/src/test/java/org/apache/wicket/guice/JavaxInjectTestComponent.java b/wicket-guice/src/test/java/org/apache/wicket/guice/JavaxInjectTestComponent.java index 6f462a6..5792be5 100644 --- a/wicket-guice/src/test/java/org/apache/wicket/guice/JavaxInjectTestComponent.java +++ b/wicket-guice/src/test/java/org/apache/wicket/guice/JavaxInjectTestComponent.java @@ -23,7 +23,6 @@ import javax.inject.Named; import com.google.inject.Provider; import org.apache.wicket.Component; -import org.apache.wicket.settings.IResourceSettings; /** */ @@ -56,13 +55,6 @@ public class JavaxInjectTestComponent extends Component implements TestComponent @Named("named2") private String named2; - /** - * A non-existing bean. - * IResourceSettings is chosen randomly. Any non-primitive type would suffice - */ - @Inject - private IResourceSettings nonExisting; - private final JavaxInjectTestNoComponent noComponent; /** @@ -145,11 +137,6 @@ public class JavaxInjectTestComponent extends Component implements TestComponent return injectedTypeLiteralField; } - public IResourceSettings getNonExisting() - { - return nonExisting; - } - @Override protected void onRender() {
