#92 - Fix/support circular dependency binding when using Provider interface

rbygrave · rbygrave · commit 28fe259a6e96 · 2021-05-02T12:53:17.000+12:00
diff --git a/inject-generator/src/main/java/io/avaje/inject/generator/MetaData.java b/inject-generator/src/main/java/io/avaje/inject/generator/MetaData.java
@@ -169,7 +169,7 @@ String buildMethod(MetaDataOrdering ordering) {
     }
 
     for (String depend : dependsOn) {
-      if (GenericType.isGeneric(depend)) {
+      if (GenericType.isGeneric(depend) && !Util.isProvider(depend)) {
         // provide implementation of generic interface as a parameter to the build method
         final MetaData providerMeta = findProviderOf(depend, ordering);
         String prov = (providerMeta == null) ? "UnknownProvider" : Util.shortName(providerMeta.type);
diff --git a/inject-generator/src/main/java/io/avaje/inject/generator/MetaDataOrdering.java b/inject-generator/src/main/java/io/avaje/inject/generator/MetaDataOrdering.java
@@ -165,15 +165,17 @@ private int processQueueRound() {
   }
 
   private boolean allDependenciesWired(MetaData queuedMeta) {
-
     for (String dependency : queuedMeta.getDependsOn()) {
-      ProviderList providerList = providers.get(dependency);
-      if (providerList == null) {
-        // missing dependencies - leave to end
-        return false;
-      } else {
-        if (!providerList.isAllWired()) {
+      if (!Util.isProvider(dependency)) {
+        // check non-provider dependency is satisfied
+        ProviderList providerList = providers.get(dependency);
+        if (providerList == null) {
+          // dependency not yet satisfied
           return false;
+        } else {
+          if (!providerList.isAllWired()) {
+            return false;
+          }
         }
       }
     }
diff --git a/inject-generator/src/main/java/io/avaje/inject/generator/MethodReader.java b/inject-generator/src/main/java/io/avaje/inject/generator/MethodReader.java
@@ -281,14 +281,16 @@ static class MethodParam {
 
     String builderGetDependency(String builderName) {
       StringBuilder sb = new StringBuilder();
-      if (genericType != null) {
+      if (isGenericParam()) {
         // passed as provider to build method
         sb.append("prov").append(providerIndex).append(".get(");
       } else {
         sb.append(builderName).append(".").append(utilType.getMethod());
       }
       if (genericType == null) {
         sb.append(Util.shortName(paramType)).append(".class");
+      } else if (isProvider()) {
+        sb.append(providerParam()).append(".class");
       }
       if (named != null) {
         sb.append(",\"").append(named).append("\"");
@@ -297,10 +299,22 @@ String builderGetDependency(String builderName) {
       return sb.toString();
     }
 
+    private String providerParam() {
+      return Util.shortName(Util.unwrapProvider(paramType));
+    }
+
+    boolean isProvider() {
+      return Util.isProvider(paramType);
+    }
+
     boolean isGenericType() {
       return genericType != null;
     }
 
+    boolean isGenericParam() {
+      return isGenericType() && !isProvider();
+    }
+
     String getDependsOn() {
       return paramType;
     }
diff --git a/inject-generator/src/main/java/io/avaje/inject/generator/SimpleBeanWriter.java b/inject-generator/src/main/java/io/avaje/inject/generator/SimpleBeanWriter.java
@@ -100,13 +100,13 @@ private void writeStaticFactoryMethod() {
     writer.append(CODE_COMMENT_BUILD, shortName).eol();
     writer.append("  public static void build(Builder builder");
     for (MethodReader.MethodParam param : constructor.getParams()) {
-      if (param.isGenericType()) {
+      if (param.isGenericParam()) {
         param.addProviderParam(writer, providerIndex++);
       }
     }
     for (MethodReader methodReader : beanReader.getInjectMethods()) {
       for (MethodReader.MethodParam param : methodReader.getParams()) {
-        if (param.isGenericType()) {
+        if (param.isGenericParam()) {
           param.addProviderParam(writer, providerIndex++);
         }
       }
@@ -143,7 +143,7 @@ private void writeStaticFactoryMethod() {
         List<MethodReader.MethodParam> methodParams = methodReader.getParams();
         for (int i = 0; i < methodParams.size(); i++) {
           if (i > 0) {
-            writer.append(",");
+            writer.append(" ,");
           }
           writer.append(methodParams.get(i).builderGetDependency("b"));
         }
diff --git a/inject-generator/src/main/java/io/avaje/inject/generator/Util.java b/inject-generator/src/main/java/io/avaje/inject/generator/Util.java
@@ -82,7 +82,7 @@ static UtilType determineType(TypeMirror rawType) {
     return UtilType.of(rawType.toString());
   }
 
-  private static boolean isProvider(String rawType) {
+  static boolean isProvider(String rawType) {
     return rawType.startsWith(PROVIDER_PREFIX);
   }
 
diff --git a/inject-generator/src/main/java/io/avaje/inject/generator/UtilType.java b/inject-generator/src/main/java/io/avaje/inject/generator/UtilType.java
@@ -6,6 +6,7 @@ private enum Type {
     LIST,
     SET,
     OPTIONAL,
+    PROVIDER,
     OTHER
   }
 
@@ -24,6 +25,8 @@ static UtilType of(String rawType) {
       return new UtilType(Type.SET, rawType);
     } else if (rawType.startsWith("java.util.Optional<")) {
       return new UtilType(Type.OPTIONAL, rawType);
+    } else if (Util.isProvider(rawType)) {
+      return new UtilType(Type.PROVIDER, rawType);
     } else {
       return new UtilType(Type.OTHER, rawType);
     }
@@ -37,8 +40,9 @@ String rawType() {
         return Util.extractList(rawType);
       case OPTIONAL:
         return Util.extractOptionalType(rawType);
+      default:
+        return rawType;
     }
-    return rawType;
   }
 
   String getMethod() {
@@ -49,6 +53,8 @@ String getMethod() {
         return "getList(";
       case OPTIONAL:
         return "getOptional(";
+      case PROVIDER:
+        return "getProvider(";
     }
     return "get(";
   }
diff --git a/inject-test/src/test/java/org/example/circular/Cupholder.java b/inject-test/src/test/java/org/example/circular/Cupholder.java
@@ -0,0 +1,23 @@
+package org.example.circular;
+
+import javax.inject.Inject;
+import javax.inject.Provider;
+import javax.inject.Singleton;
+
+@Singleton
+public class Cupholder {
+
+  public final Provider<Seat> seatProvider;
+
+  /**
+   * Resolve circular dependency via Provider interface rather than field injection.
+   */
+  @Inject
+  public Cupholder(Provider<Seat> seatProvider) {
+    this.seatProvider = seatProvider;
+  }
+
+  public String hello() {
+    return "CupHello" + seatProvider.get().hello();
+  }
+}
diff --git a/inject-test/src/test/java/org/example/circular/CupholderTest.java b/inject-test/src/test/java/org/example/circular/CupholderTest.java
@@ -0,0 +1,24 @@
+package org.example.circular;
+
+import io.avaje.inject.SystemContext;
+import org.junit.jupiter.api.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class CupholderTest {
+
+  @Test
+  void circularDependency_via_providerInterface() {
+
+    Cupholder cupholder = SystemContext.getBean(Cupholder.class);
+    String hello = cupholder.hello();
+
+    assertThat(hello).isEqualTo("CupHelloSeatHello");
+
+    // check circular binding
+    Seat seat = SystemContext.getBean(Seat.class);
+    Cupholder seatCupholder = seat.getCupholder();
+
+    assertThat(seatCupholder).isSameAs(cupholder);
+  }
+}
diff --git a/inject-test/src/test/java/org/example/circular/Seat.java b/inject-test/src/test/java/org/example/circular/Seat.java
@@ -0,0 +1,23 @@
+package org.example.circular;
+
+import javax.inject.Inject;
+import javax.inject.Singleton;
+
+@Singleton
+public class Seat {
+
+  private final Cupholder cupholder;
+
+  @Inject
+  Seat(Cupholder cupholder) {
+    this.cupholder = cupholder;
+  }
+
+  public Cupholder getCupholder() {
+    return cupholder;
+  }
+
+  public String hello() {
+    return "SeatHello";
+  }
+}
diff --git a/inject/src/main/java/io/avaje/inject/spi/Builder.java b/inject/src/main/java/io/avaje/inject/spi/Builder.java
@@ -3,6 +3,7 @@
 import io.avaje.inject.BeanContext;
 import io.avaje.inject.BeanEntry;
 
+import javax.inject.Provider;
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
@@ -127,6 +128,16 @@ static Builder newBuilder(String name, String[] provides, String[] dependsOn) {
    */
   <T> Optional<T> getOptional(Class<T> cls, String name);
 
+  /**
+   * Return Provider of T given the type.
+   */
+  <T> Provider<T> getProvider(Class<T> cls);
+
+  /**
+   * Return Provider of T given the type and name.
+   */
+  <T> Provider<T> getProvider(Class<T> cls, String name);
+
   /**
    * Get a dependency.
    */
diff --git a/inject/src/main/java/io/avaje/inject/spi/DBuilder.java b/inject/src/main/java/io/avaje/inject/spi/DBuilder.java
@@ -5,6 +5,7 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import javax.inject.Provider;
 import java.util.ArrayList;
 import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
@@ -216,6 +217,18 @@ public <T> Optional<T> getOptional(Class<T> cls, String name) {
     return Optional.ofNullable(bean);
   }
 
+  @Override
+  public <T> Provider<T> getProvider(Class<T> cls) {
+    return getProvider(cls, null);
+  }
+
+  @Override
+  public <T> Provider<T> getProvider(Class<T> cls, String name) {
+    ProviderPromise<T> promise = new ProviderPromise<>(cls, name);
+    injectors.add(promise);
+    return promise;
+  }
+
   @Override
   public <T> T get(Class<T> cls) {
     return get(cls, null);
diff --git a/inject/src/main/java/io/avaje/inject/spi/ProviderPromise.java b/inject/src/main/java/io/avaje/inject/spi/ProviderPromise.java
@@ -0,0 +1,30 @@
+package io.avaje.inject.spi;
+
+import javax.inject.Provider;
+import java.util.function.Consumer;
+
+/**
+ * Provides late binding of Provider (like field/setter injection).
+ */
+class ProviderPromise<T> implements Provider<T>, Consumer<Builder> {
+
+  private final Class<T> type;
+  private final String name;
+  private T bean;
+
+  ProviderPromise(Class<T> type, String name) {
+    this.type = type;
+    this.name = name;
+  }
+
+  @Override
+  public void accept(Builder builder) {
+    this.bean = builder.get(type, name);
+  }
+
+  @Override
+  public T get() {
+    return bean;
+  }
+
+}

Original file line number	Diff line number	Diff line change
`@@ -169,7 +169,7 @@ String buildMethod(MetaDataOrdering ordering) {`
`169`	`169`	`}`
`170`	`170`
`171`	`171`	`for (String depend : dependsOn) {`
`172`		`- if (GenericType.isGeneric(depend)) {`
	`172`	`+ if (GenericType.isGeneric(depend) && !Util.isProvider(depend)) {`
`173`	`173`	`// provide implementation of generic interface as a parameter to the build method`
`174`	`174`	`final MetaData providerMeta = findProviderOf(depend, ordering);`
`175`	`175`	`String prov = (providerMeta == null) ? "UnknownProvider" : Util.shortName(providerMeta.type);`
Original file line number	Diff line number	Diff line change
`@@ -165,15 +165,17 @@ private int processQueueRound() {`
`165`	`165`	`}`
`166`	`166`
`167`	`167`	`private boolean allDependenciesWired(MetaData queuedMeta) {`
`168`		`-`
`169`	`168`	`for (String dependency : queuedMeta.getDependsOn()) {`
`170`		`- ProviderList providerList = providers.get(dependency);`
`171`		`- if (providerList == null) {`
`172`		`- // missing dependencies - leave to end`
`173`		`- return false;`
`174`		`- } else {`
`175`		`- if (!providerList.isAllWired()) {`
	`169`	`+ if (!Util.isProvider(dependency)) {`
	`170`	`+ // check non-provider dependency is satisfied`
	`171`	`+ ProviderList providerList = providers.get(dependency);`
	`172`	`+ if (providerList == null) {`
	`173`	`+ // dependency not yet satisfied`
`176`	`174`	`return false;`
	`175`	`+ } else {`
	`176`	`+ if (!providerList.isAllWired()) {`
	`177`	`+ return false;`
	`178`	`+ }`
`177`	`179`	`}`
`178`	`180`	`}`
`179`	`181`	`}`
Original file line number	Diff line number	Diff line change
`@@ -100,13 +100,13 @@ private void writeStaticFactoryMethod() {`
`100`	`100`	`writer.append(CODE_COMMENT_BUILD, shortName).eol();`
`101`	`101`	`writer.append(" public static void build(Builder builder");`
`102`	`102`	`for (MethodReader.MethodParam param : constructor.getParams()) {`
`103`		`- if (param.isGenericType()) {`
	`103`	`+ if (param.isGenericParam()) {`
`104`	`104`	`param.addProviderParam(writer, providerIndex++);`
`105`	`105`	`}`
`106`	`106`	`}`
`107`	`107`	`for (MethodReader methodReader : beanReader.getInjectMethods()) {`
`108`	`108`	`for (MethodReader.MethodParam param : methodReader.getParams()) {`
`109`		`- if (param.isGenericType()) {`
	`109`	`+ if (param.isGenericParam()) {`
`110`	`110`	`param.addProviderParam(writer, providerIndex++);`
`111`	`111`	`}`
`112`	`112`	`}`
`@@ -143,7 +143,7 @@ private void writeStaticFactoryMethod() {`
`143`	`143`	`List<MethodReader.MethodParam> methodParams = methodReader.getParams();`
`144`	`144`	`for (int i = 0; i < methodParams.size(); i++) {`
`145`	`145`	`if (i > 0) {`
`146`		`- writer.append(",");`
	`146`	`+ writer.append(" ,");`
`147`	`147`	`}`
`148`	`148`	`writer.append(methodParams.get(i).builderGetDependency("b"));`
`149`	`149`	`}`
Original file line number	Diff line number	Diff line change
`@@ -82,7 +82,7 @@ static UtilType determineType(TypeMirror rawType) {`
`82`	`82`	`return UtilType.of(rawType.toString());`
`83`	`83`	`}`
`84`	`84`
`85`		`- private static boolean isProvider(String rawType) {`
	`85`	`+ static boolean isProvider(String rawType) {`
`86`	`86`	`return rawType.startsWith(PROVIDER_PREFIX);`
`87`	`87`	`}`
`88`	`88`
Original file line number	Diff line number	Diff line change
`@@ -6,6 +6,7 @@ private enum Type {`
`6`	`6`	`LIST,`
`7`	`7`	`SET,`
`8`	`8`	`OPTIONAL,`
	`9`	`+ PROVIDER,`
`9`	`10`	`OTHER`
`10`	`11`	`}`
`11`	`12`
`@@ -24,6 +25,8 @@ static UtilType of(String rawType) {`
`24`	`25`	`return new UtilType(Type.SET, rawType);`
`25`	`26`	`} else if (rawType.startsWith("java.util.Optional<")) {`
`26`	`27`	`return new UtilType(Type.OPTIONAL, rawType);`
	`28`	`+ } else if (Util.isProvider(rawType)) {`
	`29`	`+ return new UtilType(Type.PROVIDER, rawType);`
`27`	`30`	`} else {`
`28`	`31`	`return new UtilType(Type.OTHER, rawType);`
`29`	`32`	`}`
`@@ -37,8 +40,9 @@ String rawType() {`
`37`	`40`	`return Util.extractList(rawType);`
`38`	`41`	`case OPTIONAL:`
`39`	`42`	`return Util.extractOptionalType(rawType);`
	`43`	`+ default:`
	`44`	`+ return rawType;`
`40`	`45`	`}`
`41`		`- return rawType;`
`42`	`46`	`}`
`43`	`47`
`44`	`48`	`String getMethod() {`
`@@ -49,6 +53,8 @@ String getMethod() {`
`49`	`53`	`return "getList(";`
`50`	`54`	`case OPTIONAL:`
`51`	`55`	`return "getOptional(";`
	`56`	`+ case PROVIDER:`
	`57`	`+ return "getProvider(";`
`52`	`58`	`}`
`53`	`59`	`return "get(";`
`54`	`60`	`}`