Fix conditional factories (#785)

* Fix conditional factories

resolves #784

* Update MetaData.java

* Update MetaData.java

Author: SentryMan

blackbox-test-inject/src/main/java/org/example/myapp/conditional/BirdFactory.java

@@ -17,19 +17,19 @@ public class BirdFactory {
   @Bean
   @NoKiwi
   @RequiresProperty(missing = "finch-time")
-  public BlueJay jay() {
+  public Bird jay() {
     return new BlueJay();
   }
 
   @Bean
   @Secondary
-  public Cassowary dinosaur() {
+  public Bird dinosaur() {
     return new Cassowary();
   }
 
   @Bean
   @Finches
-  public StrawberryFinch finch() {
+  public Bird finch() {
     return new StrawberryFinch();
   }
 }

blackbox-test-inject/src/test/java/org/example/myapp/conditional/ConditionalTests.java

@@ -1,16 +1,11 @@
 package org.example.myapp.conditional;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
-import java.io.IOException;
-
-import org.example.myapp.conditional.Bird;
 import org.example.myapp.conditional.Bird.BlueJay;
 import org.example.myapp.conditional.Bird.StrawberryFinch;
-import org.example.myapp.conditional.BirdFactory;
-import org.example.myapp.conditional.BirdWatcher;
-import org.example.myapp.conditional.QualifiedBirdWatcher;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
@@ -44,7 +39,7 @@ void jay() {
     final BeanScope beanScope = BeanScope.builder().build();
 
     assertTrue(beanScope.getOptional(BirdFactory.class).isPresent());
-    assertTrue(beanScope.getOptional(BlueJay.class).isPresent());
+    assertThat(beanScope.get(Bird.class)).isInstanceOf(BlueJay.class);
     assertTrue(beanScope.getOptional(BirdWatcher.class).isEmpty());
   }
 

inject-generator/src/main/java/io/avaje/inject/generator/MetaData.java

@@ -1,9 +1,15 @@
 package io.avaje.inject.generator;
 
 import static io.avaje.inject.generator.APContext.typeElement;
-import java.util.*;
-import java.util.stream.Collectors;
 
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
 
 /**
  * Holds the data as per <code>@DependencyMeta</code>
@@ -15,13 +21,17 @@ final class MetaData implements Comparable<MetaData> {
       .thenComparing(MetaData::name, Comparator.nullsFirst(Comparator.naturalOrder()))
       .thenComparing(MetaData::compareProvides);
 
+  private static final Map<String, Integer> FACTORY_FREQUENCY = new HashMap<>();
+
   private static final String INDENT = "      ";
   private static final String NEWLINE = "\n";
 
   private final String type;
   private final String shortType;
   private final String name;
-  private String method;
+  private final String buildName;
+  private final String method;
+  private final String key;
   private boolean wired;
   private String providesAspect;
 
@@ -55,19 +65,28 @@ final class MetaData implements Comparable<MetaData> {
     this.provides = Util.addQualifierSuffix(meta.provides(), name);
     this.autoProvides = Util.addQualifierSuffix(meta.autoProvides(), name);
     this.importedComponent = meta.importedComponent();
+    this.key = createKey();
+    this.buildName = createBuildName();
   }
 
   MetaData(String type, String name) {
+    this(type, name, null);
+  }
+
+  MetaData(String type, String name, String method) {
     this.type = type;
     this.name = trimName(name);
     this.shortType = Util.shortName(type);
     this.provides = new ArrayList<>();
     this.dependsOn = new ArrayList<>();
+    this.method = method;
+    this.key = createKey();
+    this.buildName = createBuildName();
   }
 
   @Override
   public String toString() {
-    return (name == null) ? type : type + ":" + name;
+    return name == null ? type : type + ":" + name;
   }
 
   boolean importedComponent() {
@@ -90,18 +109,21 @@ private String trimName(String name) {
   }
 
   String buildName() {
+    return buildName;
+  }
+
+  private String createBuildName() {
     if (Util.isVoid(type)) {
       return "void_" + Util.trimMethod(method);
     } else {
       final String trimType = Util.trimMethod(Util.unwrapProvider(type));
+
       if (name != null) {
         return trimType + "_" + name.replaceAll("[^a-zA-Z0-9_$]+", "_");
-      } else {
-        if (buildNameIncludeMethod()) {
-          return trimType + "__" + Util.trimMethod(method);
-        }
-        return trimType;
+      } else if (buildNameIncludeMethod() || hasMethod() && FACTORY_FREQUENCY.get(type) > 0) {
+        return trimType + "__" + Util.trimMethod(method);
       }
+      return trimType;
     }
   }
 
@@ -110,15 +132,20 @@ private boolean buildNameIncludeMethod() {
     return type.contains("<") && hasMethod();
   }
 
-  public String key() {
+  String key() {
+    return key;
+  }
+
+  private String createKey() {
     if (Util.isVoid(type)) {
       return "method:" + method;
     }
-    if (name != null) {
-      return type + ":" + name;
-    } else {
-      return type;
+    String keyString = name != null ? type + ":" + name : type;
+    if (hasMethod() && FACTORY_FREQUENCY.compute(type, (k, v) -> v == null ? 0 : ++v) > 0) {
+      // allow multiple factory methods that produce the same types, typically conditional
+      keyString += FACTORY_FREQUENCY.get(type);
     }
+    return keyString;
   }
 
   boolean noDepends() {
@@ -294,10 +321,6 @@ void setDependsOn(List<String> dependsOn) {
     this.dependsOn = dependsOn.stream().map(Dependency::new).collect(Collectors.toList());
   }
 
-  void setMethod(String method) {
-    this.method = method;
-  }
-
   void setAutoProvides(List<String> autoProvides) {
     this.autoProvides = autoProvides;
   }

inject-generator/src/main/java/io/avaje/inject/generator/MethodReader.java

@@ -84,10 +84,10 @@ final class MethodReader {
     this.factoryType = beanType.getQualifiedName().toString();
     this.factoryShortName = Util.shortName(factoryType);
     this.isVoid = Util.isVoid(mainType);
-    String initMethod = (bean == null) ? null : bean.initMethod();
-    String destroyMethod = (bean == null) ? null : bean.destroyMethod();
-    this.destroyPriority = (bean == null) ? null : bean.destroyPriority();
-    this.beanCloseable = (bean != null) && bean.autoCloseable();
+    String initMethod = bean == null ? null : bean.initMethod();
+    String destroyMethod = bean == null ? null : bean.destroyMethod();
+    this.destroyPriority = bean == null ? null : bean.destroyPriority();
+    this.beanCloseable = bean != null && bean.autoCloseable();
     // for multiRegister we desire a qualifier name such that builder.isBeanAbsent() uses it and allows
     // other beans of the same type to also register, otherwise it defaults to slightly confusing behaviour
     this.name = multiRegister && qualifierName == null ? "multi" : qualifierName;
@@ -151,8 +151,7 @@ List<MethodParam> params() {
   }
 
   MetaData createMeta() {
-    MetaData metaData = new MetaData(returnTypeRaw, name);
-    metaData.setMethod(fullBuildMethod());
+    MetaData metaData = new MetaData(returnTypeRaw, name, fullBuildMethod());
 
     List<String> dependsOn = new ArrayList<>(params.size() + 1);
     dependsOn.add(factoryType);
@@ -374,6 +373,10 @@ Set<UType> genericTypes() {
     return typeReader == null ? Collections.emptySet() : typeReader.genericTypes();
   }
 
+  boolean hasConditions() {
+    return !conditions.isEmpty();
+  }
+
   void buildConditional(Append writer) {
     new ConditionalWriter(writer, conditions).write();
   }

inject-generator/src/main/java/io/avaje/inject/generator/TypeExtendsInjection.java

@@ -262,7 +262,7 @@ void validate() {
   void validateFactoryMethodDuplicates() {
     var map = new HashMap<String, MethodReader>();
     for (MethodReader method : factoryMethods) {
-      if (!method.isVoid()) {
+      if (!method.isVoid() && !method.hasConditions()) {
         var clashMethod = map.put(method.qualifiedKey(), method);
         if (clashMethod != null) {
           var msg = String.format("@Bean method %s() returns the same type as with method %s() without a unique name qualifier." +

inject-generator/src/test/java/io/avaje/inject/generator/models/valid/conditions/MyService.java

@@ -0,0 +1,3 @@
+package io.avaje.inject.generator.models.valid.conditions;
+
+public interface MyService {}

inject-generator/src/test/java/io/avaje/inject/generator/models/valid/conditions/MyServiceFactory.java

@@ -0,0 +1,21 @@
+package io.avaje.inject.generator.models.valid.conditions;
+
+import io.avaje.inject.Bean;
+import io.avaje.inject.Factory;
+import io.avaje.inject.RequiresProperty;
+
+@Factory
+class MyServiceFactory {
+
+  @Bean
+  @RequiresProperty(value = "service.type", equalTo = "one")
+  MyService myServiceOne() {
+        return new MyServiceOne();
+    }
+
+  @Bean
+  @RequiresProperty(value = "service.type", equalTo = "two")
+  MyService myServiceTwo() {
+        return new MyServiceTwo();
+    }
+}

inject-generator/src/test/java/io/avaje/inject/generator/models/valid/conditions/MyServiceOne.java

@@ -0,0 +1,3 @@
+package io.avaje.inject.generator.models.valid.conditions;
+
+public class MyServiceOne implements MyService {}

inject-generator/src/test/java/io/avaje/inject/generator/models/valid/conditions/MyServiceTwo.java

@@ -0,0 +1,3 @@
+package io.avaje.inject.generator.models.valid.conditions;
+
+public class MyServiceTwo implements MyService {}