Repository: wicket Updated Branches: refs/heads/master 17be97395 -> 7d90522c4
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. (cherry picked from commit 9d37302580511e75d271a72f523ab97dc02ff261) Project: http://git-wip-us.apache.org/repos/asf/wicket/repo Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/6dae9283 Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/6dae9283 Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/6dae9283 Branch: refs/heads/master Commit: 6dae92835e9c70206b5e127be59064eb547337fe Parents: 17be973 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:34:32 2015 +0300 ---------------------------------------------------------------------- .../wicket/guice/GuiceFieldValueFactory.java | 35 +++++-- .../wicket/guice/GuiceProxyTargetLocator.java | 98 +++++++++++++++----- .../guice/JavaxInjectGuiceInjectorTest.java | 25 +++-- .../wicket/guice/JavaxInjectTestComponent.java | 13 --- 4 files changed, 123 insertions(+), 48 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/wicket/blob/6dae9283/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 1e63752..2726b2a 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/6dae9283/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 a7b63b4..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 { @@ -65,43 +95,67 @@ class GuiceProxyTargetLocator implements IProxyTargetLocator catch (Exception e) { throw new WicketRuntimeException("Error accessing member: " + fieldName + - " of class: " + className, e); + " of class: " + className, e); } // 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/6dae9283/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 4f2869c..1ea6598 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 @@ -25,6 +25,8 @@ import org.junit.Test; import com.google.inject.ConfigurationException; import com.google.inject.spi.Message; +import javax.inject.Inject; + /** */ public class JavaxInjectGuiceInjectorTest extends AbstractInjectorTest @@ -51,15 +53,10 @@ public class JavaxInjectGuiceInjectorTest extends AbstractInjectorTest @Test public void required() { - JavaxInjectTestComponent component = newTestComponent("id"); - - // get the lazy proxy - IAjaxCallListener nonExisting = component.getNonExisting(); - try { - // call any method on the lazy proxy - nonExisting.getAfterHandler(null); + JavaxInjectTestComponent component = new MyJavaxInjectWithNonExistingTestComponent(); + // Throws exception because component.getNonExisting() cannot be injected fail("Fields annotated with @javax.inject.Inject are required!"); } catch (ConfigurationException cx) @@ -68,4 +65,18 @@ public class JavaxInjectGuiceInjectorTest extends AbstractInjectorTest 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/6dae9283/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 9f5673e..ad1b1a1 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 @@ -24,7 +24,6 @@ import javax.inject.Named; import org.apache.wicket.Component; import com.google.inject.Provider; -import org.apache.wicket.ajax.attributes.IAjaxCallListener; /** */ @@ -57,13 +56,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 IAjaxCallListener nonExisting; - private final JavaxInjectTestNoComponent noComponent; @Inject @@ -149,11 +141,6 @@ public class JavaxInjectTestComponent extends Component implements TestComponent return injectedTypeLiteralField; } - public IAjaxCallListener getNonExisting() - { - return nonExisting; - } - @Override protected void onRender() {
