#48 Extract cyclic dependency check into a handler, fix optional requ…

…est type

injector-extras/src/main/java/ch/jalu/injector/extras/handlers/AllInstancesAnnotationHandler.java

@@ -82,7 +82,7 @@ public Object instantiateWith(Object... values) {
         }
 
         @Override
-        public boolean isNewlyCreated() {
+        public boolean isInstantiation() {
             return true;
         }
     }

injector/src/main/java/ch/jalu/injector/InjectorBuilder.java

@@ -1,6 +1,7 @@
 package ch.jalu.injector;
 
 import ch.jalu.injector.handlers.Handler;
+import ch.jalu.injector.handlers.dependency.CyclicDependenciesDetector;
 import ch.jalu.injector.handlers.dependency.FactoryDependencyHandler;
 import ch.jalu.injector.handlers.dependency.SavedAnnotationsHandler;
 import ch.jalu.injector.handlers.dependency.SingletonStoreDependencyHandler;
@@ -45,6 +46,7 @@ public static List<Handler> createDefaultHandlers(String rootPackage) {
             new FactoryDependencyHandler(),
             new SingletonStoreDependencyHandler(),
             // Instantiation provider
+            new CyclicDependenciesDetector(),
             new DefaultInjectionProvider(rootPackage),
             // PostConstruct
             new PostConstructMethodInvoker()));

injector/src/main/java/ch/jalu/injector/InjectorImpl.java

@@ -15,15 +15,14 @@
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 
 import static ch.jalu.injector.context.StandardResolutionType.REQUEST_SCOPED;
 import static ch.jalu.injector.context.StandardResolutionType.REQUEST_SCOPED_IF_HAS_DEPENDENCIES;
 import static ch.jalu.injector.context.StandardResolutionType.SINGLETON;
 import static ch.jalu.injector.utils.InjectorUtils.checkNotNull;
+import static ch.jalu.injector.utils.InjectorUtils.containsNullValue;
 import static ch.jalu.injector.utils.InjectorUtils.firstNotNull;
 import static ch.jalu.injector.utils.InjectorUtils.rethrowException;
 
@@ -59,12 +58,12 @@ public <T> void register(Class<? super T> clazz, T object) {
     @Override
     public void provide(Class<? extends Annotation> clazz, Object object) {
         checkNotNull(clazz, "Provided annotation may not be null");
-        for (Handler handler : config.getHandlers()) {
-            try {
+        try {
+            for (Handler handler : config.getHandlers()) {
                 handler.onAnnotation(clazz, object);
-            } catch (Exception e) {
-                rethrowException(e);
             }
+        } catch (Exception e) {
+            rethrowException(e);
         }
     }
 
@@ -103,25 +102,25 @@ public <T> Collection<T> retrieveAllOfType(Class<T> clazz) {
     public <T> void registerProvider(Class<T> clazz, Provider<? extends T> provider) {
         checkNotNull(clazz, "Class may not be null");
         checkNotNull(provider, "Provider may not be null");
-        for (Handler handler : config.getHandlers()) {
-            try {
+        try {
+            for (Handler handler : config.getHandlers()) {
                 handler.onProvider(clazz, provider);
-            } catch (Exception e) {
-                rethrowException(e);
             }
+        } catch (Exception e) {
+            rethrowException(e);
         }
     }
 
     @Override
     public <T, P extends Provider<? extends T>> void registerProvider(Class<T> clazz, Class<P> providerClass) {
         checkNotNull(clazz, "Class may not be null");
         checkNotNull(providerClass, "Provider class may not be null");
-        for (Handler handler : config.getHandlers()) {
-            try {
+        try {
+            for (Handler handler : config.getHandlers()) {
                 handler.onProviderClass(clazz, providerClass);
-            } catch (Exception e) {
-                rethrowException(e);
             }
+        } catch (Exception e) {
+            rethrowException(e);
         }
     }
 
@@ -131,13 +130,18 @@ public InjectorConfig getConfig() {
 
     @SuppressWarnings("unchecked")
     private <T> T resolve(ResolutionType resolutionType, Class<?> clazz) {
-        return (T) resolveObject(
-            new ResolutionContext(this, new ObjectIdentifier(resolutionType, clazz)),
-            new HashSet<>());
+        return (T) resolveContext(
+            new ResolutionContext(this, new ObjectIdentifier(resolutionType, clazz)));
     }
 
+    /**
+     * Returns the object as defined by the given context.
+     *
+     * @param context the context to resolve the object for
+     * @return the resolved object, {@code null} if the context specifies it is optional and some criteria is not met
+     */
     @Nullable
-    private Object resolveObject(ResolutionContext context, Set<Class<?>> traversedClasses) {
+    protected Object resolveContext(ResolutionContext context) {
         // TODO #49: Convert singleton store to a Handler impl.
         if (context.getIdentifier().getResolutionType() == StandardResolutionType.SINGLETON) {
             Object knownSingleton = objects.get(context.getIdentifier().getTypeAsClass());
@@ -147,34 +151,77 @@ private Object resolveObject(ResolutionContext context, Set<Class<?>> traversedC
         }
 
         Resolution<?> resolution = findResolutionOrFail(context);
+        if (isContextChildOfOptionalRequest(context) && resolution.isInstantiation()) {
+            return null;
+        }
 
-        traversedClasses.add(context.getIdentifier().getTypeAsClass());
-        validateInjectionHasNoCircularDependencies(resolution, traversedClasses);
+        Object[] resolvedDependencies = resolveDependencies(context, resolution);
+        if (containsNullValue(resolvedDependencies)) {
+            throwForUnexpectedNullDependency(context);
+            return null;
+        }
 
-        for (ObjectIdentifier identifier : resolution.getDependencies()) {
-            if (traversedClasses.contains(identifier.getTypeAsClass())) {
-                throw new InjectorException("Found cyclic dependency - already traversed '"
-                    + identifier.getTypeAsClass() + "' (full traversal list: " + traversedClasses + ")");
-            }
+        Object object = runPostConstructHandlers(resolution.instantiateWith(resolvedDependencies), context, resolution);
+        if (resolution.isInstantiation() && context.getIdentifier().getResolutionType() == SINGLETON) {
+            register((Class) context.getOriginalIdentifier().getTypeAsClass(), object);
         }
+        return object;
+    }
 
-        List<ObjectIdentifier> dependencies = resolution.getDependencies();
-        Object[] resolvedDependencies = dependencies.stream()
-            .map(identifier -> new ResolutionContext(this, identifier))
-            .map(dependencyContext -> resolveObject(dependencyContext, new HashSet<>(traversedClasses)))
-            .toArray();
-        Object object = resolution.instantiateWith(resolvedDependencies);
-
-        if (resolution.isNewlyCreated()) {
-            object = runPostConstructHandlers(object, context, resolution);
-            if (context.getIdentifier().getResolutionType() == StandardResolutionType.SINGLETON) {
-                register((Class) context.getOriginalIdentifier().getTypeAsClass(), object);
+    /**
+     * Resolves the dependencies as defined by the given resolution.
+     * If a dependency is resolved to {@code null}, the process is aborted and the remaining dependencies
+     * are not resolved.
+     *
+     * @param context the resolution context
+     * @param resolution the resolution whose dependencies should be provided
+     * @return array with the dependencies, in the same order as given by the resolution
+     */
+    protected Object[] resolveDependencies(ResolutionContext context, Resolution<?> resolution) {
+        final int totalDependencies = resolution.getDependencies().size();
+        final Object[] resolvedDependencies = new Object[totalDependencies];
+
+        int index = 0;
+        for (ObjectIdentifier dependencyId : resolution.getDependencies()) {
+            Object dependency = resolveContext(context.createChildContext(dependencyId));
+            if (dependency == null) {
+                break;
             }
+            resolvedDependencies[index] = dependency;
+            ++index;
         }
-        return object;
+        return resolvedDependencies;
+    }
+
+    /**
+     * Called when a resolved dependency is null, this method may throw an exception in the cases when this
+     * should not happen. If this method does not throw an exception, null is returned from {@link #resolveContext}.
+     *
+     * @param context the resolution context
+     */
+    protected void throwForUnexpectedNullDependency(ResolutionContext context) {
+        if (context.getIdentifier().getResolutionType() == REQUEST_SCOPED_IF_HAS_DEPENDENCIES
+            || isContextChildOfOptionalRequest(context)) {
+            // Situation where null may occur, so throw no exception
+            return;
+        }
+        throw new InjectorException("Found null returned as dependency while resolving '"
+            + context.getIdentifier() + "'");
+    }
+
+    private static boolean isContextChildOfOptionalRequest(ResolutionContext context) {
+        return !context.getParents().isEmpty()
+            && context.getParents().get(0).getIdentifier().getResolutionType() == REQUEST_SCOPED_IF_HAS_DEPENDENCIES;
     }
 
-    private Resolution<?> findResolutionOrFail(ResolutionContext context) {
+    /**
+     * Calls the defined handlers and returns the first {@link Resolution} that is returned based on
+     * the provided resolution context. Throws an exception if no handler returned a resolution.
+     *
+     * @param context the context to find the resolution for
+     * @return the resolution
+     */
+    protected Resolution<?> findResolutionOrFail(ResolutionContext context) {
         try {
             for (Handler handler : config.getHandlers()) {
                 Resolution<?> resolution = handler.resolve(context);
@@ -203,32 +250,29 @@ private Resolution<?> findResolutionOrFail(ResolutionContext context) {
             + "require the default constructor");
     }
 
-    private <T> T runPostConstructHandlers(T instance, ResolutionContext context, Resolution<?> resolution) {
-        T object = instance;
-        for (Handler handler : config.getHandlers()) {
-            try {
-                object = firstNotNull(handler.postProcess(object, context, resolution), object);
-            } catch (Exception e) {
-                rethrowException(e);
-            }
-        }
-        return object;
-    }
-
     /**
-     * Validates that none of the dependencies' types are present in the given collection
-     * of traversed classes. This prevents circular dependencies.
+     * Invokes the handler's post construct method when appropriate. Returns the object as returned by the
+     * handlers, which may be different from the provided one.
      *
-     * @param resolution the resolution method to get the dependencies from
-     * @param traversedClasses the collection of traversed classes
+     * @param instance the object that was resolved
+     * @param context the resolution context
+     * @param resolution the resolution used to get the object
+     * @param <T> the object's type
+     * @return the object to use (as post construct methods may change it)
      */
-    private static void validateInjectionHasNoCircularDependencies(Resolution<?> resolution,
-                                                                   Set<Class<?>> traversedClasses) {
-        for (ObjectIdentifier identifier : resolution.getDependencies()) {
-            if (traversedClasses.contains(identifier.getTypeAsClass())) {
-                throw new InjectorException("Found cyclic dependency - already traversed '"
-                    + identifier.getTypeAsClass() + "' (full traversal list: " + traversedClasses + ")");
+    protected <T> T runPostConstructHandlers(T instance, ResolutionContext context, Resolution<?> resolution) {
+        if (!resolution.isInstantiation()) {
+            return instance;
+        }
+
+        T object = instance;
+        try {
+            for (Handler handler : config.getHandlers()) {
+                object = firstNotNull(handler.postProcess(object, context, resolution), object);
             }
+        } catch (Exception e) {
+            rethrowException(e);
         }
+        return object;
     }
 }

injector/src/main/java/ch/jalu/injector/context/ObjectIdentifier.java

@@ -23,6 +23,9 @@ public ObjectIdentifier(ResolutionType resolutionType, Type type, Annotation...
         this.annotations = Arrays.asList(annotations);
     }
 
+    /**
+     * @return the resolution type (scope) requested for the object
+     */
     public ResolutionType getResolutionType() {
         return resolutionType;
     }
@@ -69,4 +72,9 @@ public Class<?> getTypeAsClass() {
     public List<Annotation> getAnnotations() {
         return annotations;
     }
+
+    @Override
+    public String toString() {
+        return "ObjId[type=" + type + ", annotations=" + annotations + "]";
+    }
 }

injector/src/main/java/ch/jalu/injector/context/ResolutionContext.java

@@ -3,6 +3,9 @@
 import ch.jalu.injector.Injector;
 import ch.jalu.injector.exceptions.InjectorException;
 
+import java.util.ArrayList;
+import java.util.List;
+
 /**
  * Resolution context: contains data about the object that is requested, such as identifying
  * information about the object to retrieve or construct and the context in which it is being
@@ -13,7 +16,14 @@ public class ResolutionContext {
     private final Injector injector;
     private final ObjectIdentifier originalIdentifier;
     private ObjectIdentifier identifier;
+    private List<ResolutionContext> parents = new ArrayList<>();
 
+    /**
+     * Creates a new resolution context with no predecessors.
+     *
+     * @param injector the injector
+     * @param identifier the identifier of the object to create
+     */
     public ResolutionContext(Injector injector, ObjectIdentifier identifier) {
         this.injector = injector;
         this.originalIdentifier = identifier;
@@ -35,6 +45,10 @@ public ObjectIdentifier getIdentifier() {
         return identifier;
     }
 
+    public List<ResolutionContext> getParents() {
+        return parents;
+    }
+
     /**
      * Sets the class to instantiate an object of.
      *
@@ -48,4 +62,17 @@ public void setIdentifier(ObjectIdentifier identifier) {
                 + "' is not a child of original class '" + originalIdentifier.getTypeAsClass() + "'");
         }
     }
+
+    /**
+     * Creates a context for the given identifier with this context as parent.
+     *
+     * @param identifier the identifier to create a context for
+     * @return the child context
+     */
+    public ResolutionContext createChildContext(ObjectIdentifier identifier) {
+        ResolutionContext child = new ResolutionContext(injector, identifier);
+        child.parents.addAll(this.parents);
+        child.parents.add(this);
+        return child;
+    }
 }

injector/src/main/java/ch/jalu/injector/handlers/dependency/CyclicDependenciesDetector.java

@@ -0,0 +1,50 @@
+package ch.jalu.injector.handlers.dependency;
+
+import ch.jalu.injector.context.ObjectIdentifier;
+import ch.jalu.injector.context.ResolutionContext;
+import ch.jalu.injector.exceptions.InjectorException;
+import ch.jalu.injector.handlers.Handler;
+import ch.jalu.injector.handlers.instantiation.Resolution;
+
+import javax.annotation.Nullable;
+import java.lang.reflect.Type;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+/**
+ * Detects cycles in the dependencies based on the context's parents.
+ * This handler should come at the start of the chain so it can stop it with an appropriate error message.
+ * If not stopped by this handler, cyclic dependencies will cause a StackOverflowException.
+ */
+public class CyclicDependenciesDetector implements Handler {
+
+    @Override
+    public Resolution<?> resolve(ResolutionContext context) {
+        ObjectIdentifier duplicateIdentifier = findRepeatedIdentifier(context);
+        if (duplicateIdentifier != null) {
+            String traversalList = buildParentsList(context);
+            throw new InjectorException("Found cyclic dependency' - already traversed '" + duplicateIdentifier
+                + "' (full traversal list: " + traversalList + " -> " + context.getIdentifier() + ")");
+        }
+        return null;
+    }
+
+    @Nullable
+    private static ObjectIdentifier findRepeatedIdentifier(ResolutionContext context) {
+        Set<Type> types = new HashSet<>();
+        types.add(context.getIdentifier().getType());
+        for (ResolutionContext parent : context.getParents()) {
+            if (!types.add(parent.getIdentifier().getType())) {
+                return parent.getIdentifier();
+            }
+        }
+        return null;
+    }
+
+    private static String buildParentsList(ResolutionContext context) {
+        return context.getParents().stream()
+            .map(ctx -> ctx.getIdentifier().getType().getTypeName())
+            .collect(Collectors.joining(" -> "));
+    }
+}