Merge pull request #516 from BCSol/master

Fix optional injection bugs

guice-bridge/src/test/java/org/jvnet/hk2/guice/bridge/test/GuiceBridgeTest.java

@@ -16,16 +16,21 @@
 
 package org.jvnet.hk2.guice.bridge.test;
 
+import java.util.Optional;
 
 import org.glassfish.hk2.api.ServiceLocator;
+import org.junit.After;
 import org.junit.Assert;
+import org.junit.Before;
 import org.junit.Test;
 import org.jvnet.hk2.guice.bridge.api.GuiceIntoHK2Bridge;
 import org.jvnet.hk2.guice.bridge.api.HK2IntoGuiceBridge;
 import org.jvnet.hk2.guice.bridge.test.utilities.Utilities;
 
+import com.google.inject.AbstractModule;
 import com.google.inject.Guice;
 import com.google.inject.Injector;
+import com.google.inject.TypeLiteral;
 
 /**
  * Tests for Guice bridge
@@ -36,7 +41,18 @@ public class GuiceBridgeTest {
     /* package */ static final String ALICE = "Alice";
     /* package */ static final String HATTER = "Hatter";
 
-    private static final ServiceLocator testLocator = Utilities.createLocator("GuiceBridgeTest", new GuiceBridgeTestModule());
+  private ServiceLocator testLocator;
+
+  @Before
+  public void setup() {
+    testLocator = Utilities.createLocator("GuiceBridgeTest", new GuiceBridgeTestModule());
+  }
+
+  @After
+  public void cleanup() {
+    testLocator.shutdown();
+    testLocator = null;
+  }
 
     /**
      * Tests a service from Guice being injected into an HK2 service
@@ -67,19 +83,77 @@ public void testGuiceServiceInHk2Service() {
         hk2Service4.verifyGuiceService();
     }
 
-    /**
-     * Tests a service from hk2 being injected into a Guice service
-     */
-    @Test
-    public void testHk2ServiceInGuiceService() {
-        Injector injector = Guice.createInjector(
-                new HK2IntoGuiceBridge(testLocator),
-                new HK2BridgeModule());
-        Assert.assertNotNull(injector);
-
-        GuiceService2 guiceService2 = injector.getInstance(GuiceService2.class);
-        Assert.assertNotNull(guiceService2);
-
-        guiceService2.verifyHK2Service();
-    }
+  /** Tests GuiceService into HK2Service as {@link Optional} */
+  @Test
+  public void testGuiceServiceInHk2ServiceAsOptionalWithPrecendence() {
+    Injector injector =
+        Guice.createInjector(
+            new AbstractModule() {
+              @Override
+              protected void configure() {
+                bind(GuiceService5.class).toInstance(() -> "atomic");
+                bind(new TypeLiteral<Optional<GuiceService5>>() {})
+                    .toInstance(Optional.<GuiceService5>of(() -> "specific"));
+              }
+            });
+    GuiceIntoHK2Bridge guiceBridge = testLocator.getService(GuiceIntoHK2Bridge.class);
+    guiceBridge.bridgeGuiceInjector(injector);
+
+    HK2Service5 service = testLocator.getService(HK2Service5.class);
+    Assert.assertNotNull(service);
+    Assert.assertNotNull(service.optionalGuiceService);
+    Assert.assertTrue(service.optionalGuiceService.isPresent());
+    Assert.assertEquals("specific", service.optionalGuiceService.get().getName());
+  }
+
+  /** Tests GuiceService into HK2Service as {@link Optional} */
+  @Test
+  public void testGuiceServiceInHk2ServiceAsOptionalAtomic() {
+    Injector injector =
+        Guice.createInjector(
+            new AbstractModule() {
+              @Override
+              protected void configure() {
+                bind(GuiceService5.class).toInstance(() -> "atomic");
+              }
+            });
+    GuiceIntoHK2Bridge guiceBridge = testLocator.getService(GuiceIntoHK2Bridge.class);
+    guiceBridge.bridgeGuiceInjector(injector);
+
+    HK2Service5 service = testLocator.getService(HK2Service5.class);
+    Assert.assertNotNull(service);
+    Assert.assertNotNull(service.optionalGuiceService);
+    Assert.assertTrue(service.optionalGuiceService.isPresent());
+    Assert.assertEquals("atomic", service.optionalGuiceService.get().getName());
+  }
+
+  /** Tests GuiceService into HK2Service as {@link Optional} */
+  @Test
+  public void testGuiceServiceInHk2ServiceAsOptionalNotBound() {
+    Injector injector =
+        Guice.createInjector(
+            new AbstractModule() {
+              @Override
+              protected void configure() {}
+            });
+    GuiceIntoHK2Bridge guiceBridge = testLocator.getService(GuiceIntoHK2Bridge.class);
+    guiceBridge.bridgeGuiceInjector(injector);
+
+    HK2Service5 service = testLocator.getService(HK2Service5.class);
+    Assert.assertNotNull(service);
+    Assert.assertNotNull(service.optionalGuiceService);
+  }
+
+  /** Tests a service from hk2 being injected into a Guice service */
+  @Test
+  public void testHk2ServiceInGuiceService() {
+    Injector injector =
+        Guice.createInjector(new HK2IntoGuiceBridge(testLocator), new HK2BridgeModule());
+    Assert.assertNotNull(injector);
+
+    GuiceService2 guiceService2 = injector.getInstance(GuiceService2.class);
+    Assert.assertNotNull(guiceService2);
+
+    guiceService2.verifyHK2Service();
+  }
 }

guice-bridge/src/test/java/org/jvnet/hk2/guice/bridge/test/GuiceBridgeTestModule.java

@@ -34,6 +34,7 @@ public void configure(DynamicConfiguration config) {
         config.addActiveDescriptor(HK2Service2Impl.class);
         config.addActiveDescriptor(HK2Service3.class);
         config.addActiveDescriptor(HK2Service4.class);
+        config.addActiveDescriptor(HK2Service5.class);
     }
 
 }

guice-bridge/src/test/java/org/jvnet/hk2/guice/bridge/test/GuiceService5.java

@@ -0,0 +1,15 @@
+/********************************************************************************
+ * Copyright (c) 2020 Contributors to the Eclipse Foundation.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0.
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ ********************************************************************************/
+package org.jvnet.hk2.guice.bridge.test;
+
+@FunctionalInterface
+public interface GuiceService5 {
+  public String getName();
+}

guice-bridge/src/test/java/org/jvnet/hk2/guice/bridge/test/HK2Service5.java

@@ -0,0 +1,19 @@
+/********************************************************************************
+ * Copyright (c) 2020 Contributors to the Eclipse Foundation.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0.
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ ********************************************************************************/
+package org.jvnet.hk2.guice.bridge.test;
+
+import java.util.Optional;
+
+import jakarta.inject.Inject;
+
+/** @author Balthasar Schüss */
+public class HK2Service5 {
+  @Inject Optional<GuiceService5> optionalGuiceService;
+}

hk2-locator/src/main/java/org/jvnet/hk2/internal/OptionalActiveDescriptor.java

@@ -15,93 +15,78 @@
  */
 package org.jvnet.hk2.internal;
 
-import java.lang.annotation.Annotation;
 import java.lang.reflect.Type;
 import java.util.HashSet;
 import java.util.Optional;
-import java.util.Set;
-import jakarta.inject.Provider;
-import org.glassfish.hk2.api.ActiveDescriptor;
+
 import org.glassfish.hk2.api.DescriptorType;
 import org.glassfish.hk2.api.DescriptorVisibility;
 import org.glassfish.hk2.api.Injectee;
-import org.glassfish.hk2.api.IterableProvider;
 import org.glassfish.hk2.api.PerLookup;
 import org.glassfish.hk2.api.ServiceHandle;
-import org.glassfish.hk2.api.messaging.Topic;
 import org.glassfish.hk2.utilities.AbstractActiveDescriptor;
 import org.glassfish.hk2.utilities.reflection.ReflectionHelper;
 
 /**
- * Descriptor for {@link Optional} to allow for injection of a service if it exists or else
- * {@link Optional#EMPTY}.  It will also allow for injection of an {@link Optional} directly as well.
+ * Descriptor for {@link Optional} to allow for injection of a service if it exists or else {@link
+ * Optional#EMPTY}. It will also allow for injection of an {@link Optional} directly as well.
+ *
  * @author jonathan coustick
  */
-public class OptionalActiveDescriptor<T> extends AbstractActiveDescriptor<Optional> {
-
-    private Injectee injectee;
-    private Type requiredType;
-    private ServiceLocatorImpl locator;
-
-    /**
-     * For serialization
-     */
-    public OptionalActiveDescriptor() {
-        super();
-    }
-
-    /*package-private*/ OptionalActiveDescriptor(Injectee injectee, ServiceLocatorImpl locator, Type requiredType) {
-        super(new HashSet<Type>(),
-                PerLookup.class,
-                null,
-                new HashSet<Annotation>(),
-                DescriptorType.CLASS,
-                DescriptorVisibility.NORMAL,
-                0,
-                null,
-                null,
-                locator.getPerLocatorUtilities().getAutoAnalyzerName(injectee.getInjecteeClass()),
-                null);
-
-        this.requiredType = requiredType;
-        this.injectee = injectee;
-        this.locator = locator;
-    }
+public class OptionalActiveDescriptor<T> extends AbstractActiveDescriptor<Optional<T>> {
+
+  private Injectee injectee;
+  private ServiceLocatorImpl locator;
+
+  /** For serialization */
+  public OptionalActiveDescriptor() {
+    super();
+  }
 
-    @Override
-    public Class<?> getImplementationClass() {
-        return Optional.class;
-    }
+  /*package-private*/ OptionalActiveDescriptor(
+      Injectee injectee,
+      ServiceLocatorImpl locator) {
+    super(
+        new HashSet<Type>(),
+        PerLookup.class,
+        ReflectionHelper.getNameFromAllQualifiers(
+            injectee.getRequiredQualifiers(), injectee.getParent()),
+        injectee.getRequiredQualifiers(),
+        DescriptorType.CLASS,
+        DescriptorVisibility.NORMAL,
+        0,
+        null,
+        null,
+        locator.getPerLocatorUtilities().getAutoAnalyzerName(injectee.getInjecteeClass()),
+        null);
+    this.injectee = injectee;
+    this.locator = locator;
+  }
 
-    @Override
-    public Type getImplementationType() {
-        return Optional.class;
-    }
+  @Override
+  public Class<?> getImplementationClass() {
+    return Optional.class;
+  }
 
-    @Override
-    public Optional<T> create(ServiceHandle<?> root) {
-        Set<Annotation> qualifierAnnotations = getQualifierAnnotations();
-        Annotation[] annotationsArray = qualifierAnnotations.toArray(new Annotation[qualifierAnnotations.size()]);
+  @Override
+  public Type getImplementationType() {
+    return Optional.class;
+  }
 
-        ServiceHandle<T> handle = locator.getServiceHandle(requiredType, annotationsArray);
-        if (handle == null) {
-            Class<?> rawType = ReflectionHelper.getRawClass(requiredType);
-            if (Provider.class.equals(rawType) || Iterable.class.equals(rawType) || IterableProvider.class.equals(rawType) || Topic.class.equals(rawType)
-                    || Optional.class.equals(rawType)) {
-                SystemInjecteeImpl copy = new SystemInjecteeImpl(rawType, injectee.getRequiredQualifiers(), injectee.getPosition(),
-                        injectee.getParent(), true, injectee.isSelf(), injectee.getUnqualified(), this);
-                ActiveDescriptor descriptor = locator.getInjecteeDescriptor(copy);
-                return Optional.of((T) descriptor.create(root));
-            }
-            ServiceHandle<Optional<T>> optionalHandle = locator.getServiceHandle(injectee.getRequiredType(), annotationsArray);
-            if (optionalHandle == null) {
-                return Optional.empty();
-            } else {
-                return optionalHandle.getService();
-            }
-        }
-        T service = handle.getService();
-        return Optional.ofNullable(service);
-    }
-
+  @SuppressWarnings("unchecked")
+  @Override
+  public Optional<T> create(ServiceHandle<?> root) {
+    Injectee unwrapped =
+        new SystemInjecteeImpl(
+            ReflectionHelper.getFirstTypeArgument(injectee.getRequiredType()),
+            injectee.getRequiredQualifiers(),
+            injectee.getPosition(),
+            injectee.getParent(),
+            true,
+            injectee.isSelf(),
+            injectee.getUnqualified(),
+            this);
+    return Optional.ofNullable(locator.getInjecteeDescriptor(unwrapped))
+        .flatMap(d -> Optional.ofNullable((T) d.create(root)));
+  }
 }