Merge pull request #3 from qudini/codeborne-master

Update from codebourne master

README

@@ -1,5 +1,28 @@
 ## Changelog
 
+### 1.10
+
+Move creation of guice injector from `onConfigurationRead()` to `onApplicationStart()`.
+We found that method `onConfigurationRead()` can be called many times during application lifetime (on every application.conf change).
+It causes creating multiple instances of guice injector, thus causing memory leaks.
+
+### 1.9
+
+  * Added guava 23.0 dependency (Guice actually doesn't work without it)
+  * upgrade to play 1.5.1 (which adds methods Injector.setBeanSource() and automatically injects controllers' instances)
+
+### 1.8
+
+  * upgrade to play 1.5
+
+### 1.7
+
+  * create request.controllerInstance even if action method is static
+
+### 1.6
+
+  * upgrade guice to 4.1.0 and include multibindings
+
 ### 1.5
 
   * support injecting of non-static dependencies into controllers

conf/dependencies.yml

@@ -1,10 +1,11 @@
-self: play-guice -> guice 1.8
+self: play-guice -> guice 1.10
 
 require:
     - play 1.5.+
     - com.google.inject -> guice 4.1.0:
-        transitive: true
+        transitive: false
     - com.google.inject.extensions -> guice-multibindings 4.1.0:
         transitive: false
     - aopalliance 1.0:
-        transitive: true
+        transitive: false
+    - com.google.guava -> guava 23.0

src/play/modules/guice/GuicePlugin.java

@@ -36,15 +36,12 @@ public class GuicePlugin extends PlayPlugin implements BeanSource {
 
   @Override
   public void onApplicationStart() {
-    logger.debug("Starting Guice modules scanning");
+    logger.debug("Starting Guice modules injecting");
     loadInjector();
+    play.inject.Injector.setBeanSource(this);
     play.inject.Injector.inject(this);
     injectAnnotated();
-  }
-
-  @Override public void beforeActionInvocation(Method actionMethod) {
-    Http.Request request = Http.Request.current();
-    if (request.controllerInstance == null) request.controllerInstance = getBeanOfType(request.controllerClass);
+    injectPlayPlugins();
   }
 
   private void loadInjector() {
@@ -94,7 +91,7 @@ private String moduleList() {
     return moduleList.toString();
   }
 
-  @Override 
+  @Override
   public <T> T getBeanOfType(Class<T> clazz) {
     if (injector == null) {
       return null;
@@ -130,6 +127,12 @@ private void injectAnnotated() {
     }
   }
 
+  private void injectPlayPlugins() {
+    for (PlayPlugin plugin : Play.pluginCollection.getAllPlugins()) {
+      injector.injectMembers(plugin);
+    }
+  }
+
   private boolean isInjectable(Field field) {
     return Modifier.isStatic(field.getModifiers()) && 
         (field.isAnnotationPresent(javax.inject.Inject.class) || field.isAnnotationPresent(com.google.inject.Inject.class));

src/play/modules/guice/InjectSupport.java

@@ -7,5 +7,9 @@
 
 @Retention(RetentionPolicy.RUNTIME)
 @Target(ElementType.TYPE)
+@Deprecated
+/**
+ * @deprecated Don't use this feature. Static fields are evil.
+ */
 public @interface InjectSupport {
 }

test/play/modules/guice/GuicePluginTest.java

@@ -1,29 +1,15 @@
 package play.modules.guice;
 
 import org.junit.Test;
-import play.mvc.Controller;
-import play.mvc.Http;
 
-import static org.hamcrest.core.Is.is;
-import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertNull;
 
 public class GuicePluginTest {
   @Test
-  public void createNonStaticControllerInstance() throws Exception {
-    Http.Request.current.set(new Http.Request());
-    NonStaticController controllerBean = new NonStaticController();
+  public void getBeanOfType_returnsNull_ifInjectorIsNull() {
+    GuicePlugin plugin = new GuicePlugin();
+    plugin.injector = null;
 
-    GuicePlugin plugin = new GuicePlugin() {
-      @Override public <T> T getBeanOfType(Class<T> clazz) {
-        return (T) controllerBean;
-      }
-    };
-    plugin.beforeActionInvocation(NonStaticController.class.getMethod("hello"));
-
-    assertThat(Http.Request.current().controllerInstance, is(controllerBean));
-  }
-
-  static class NonStaticController extends Controller {
-    public void hello() {}
+    assertNull(plugin.getBeanOfType(String.class));
   }
 }