Manage lifecycle of installed modules (#357)

* Manage lifecycle of installed modules

When an ApolloModule installs another ApolloModule, it should start managing lifecycles for all the keys from the installed ApolloModule.

Apollo will not manage the installed modules on the service shutdown if we don't do this.

apollo-core/src/main/java/com/spotify/apollo/module/AbstractApolloModule.java

@@ -22,6 +22,7 @@
 import com.google.common.base.MoreObjects;
 import com.google.common.collect.ImmutableSet;
 import com.google.inject.AbstractModule;
+import com.google.inject.Module;
 import com.google.inject.Key;
 
 import java.util.Set;
@@ -45,6 +46,16 @@ protected void manageLifecycle(Class<?> cls) {
     lifecycleManagedBuilder.add(Key.get(cls));
   }
 
+  @Override
+  protected void install(final Module module) {
+    super.install(module);
+    if (module instanceof ApolloModule) {
+      for (Key<?> key : ((ApolloModule) module).getLifecycleManaged()) {
+        manageLifecycle(key);
+      }
+    }
+  }
+
   @Override
   public double getPriority() {
     return 0.0;

apollo-core/src/test/java/com/spotify/apollo/core/ChildModuleWithLifecycledKeys.java

@@ -0,0 +1,72 @@
+/*
+ * -\-\-
+ * Spotify Apollo Service Core (aka Leto)
+ * --
+ * Copyright (C) 2013 - 2020 Spotify AB
+ * --
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * 
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * -/-/-
+ */
+package com.spotify.apollo.core;
+
+import com.google.inject.Inject;
+import com.google.inject.name.Named;
+import com.google.inject.name.Names;
+import com.spotify.apollo.module.AbstractApolloModule;
+import java.io.Closeable;
+import java.io.IOException;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+class ChildModuleWithLifecycledKeys extends AbstractApolloModule {
+
+  private final AtomicBoolean created;
+  private final AtomicBoolean closed;
+
+  public ChildModuleWithLifecycledKeys(AtomicBoolean created, AtomicBoolean closed) {
+    this.created = created;
+    this.closed = closed;
+  }
+
+  @Override
+  protected void configure() {
+    bind(AtomicBoolean.class).annotatedWith(Names.named("childCreated")).toInstance(created);
+    bind(AtomicBoolean.class).annotatedWith(Names.named("childClosed")).toInstance(closed);
+
+    bind(Foo.class).to(FooImpl.class);
+    manageLifecycle(Foo.class);
+  }
+
+  @Override
+  public String getId() {
+    return "child-lifecycle-module";
+  }
+
+  interface Foo {}
+
+  static class FooImpl implements Foo, Closeable {
+
+    final AtomicBoolean closed;
+
+    @Inject
+    FooImpl(
+        @Named("childCreated") AtomicBoolean created, @Named("childClosed") AtomicBoolean closed) {
+      this.closed = closed;
+      created.set(true);
+    }
+
+    @Override
+    public void close() throws IOException {
+      closed.set(true);
+    }
+  }
+}

apollo-core/src/test/java/com/spotify/apollo/core/ModuleWithLifecycledKeys.java

@@ -22,9 +22,7 @@
 import com.google.inject.Inject;
 import com.google.inject.name.Named;
 import com.google.inject.name.Names;
-
 import com.spotify.apollo.module.AbstractApolloModule;
-
 import java.io.Closeable;
 import java.io.IOException;
 import java.util.concurrent.atomic.AtomicBoolean;
@@ -33,17 +31,30 @@ class ModuleWithLifecycledKeys extends AbstractApolloModule {
 
   private final AtomicBoolean created;
   private final AtomicBoolean closed;
+  private final AtomicBoolean childCreated;
+  private final AtomicBoolean childClosed;
 
-  public ModuleWithLifecycledKeys(AtomicBoolean created, AtomicBoolean closed) {
+  public ModuleWithLifecycledKeys(
+      AtomicBoolean created,
+      AtomicBoolean closed,
+      AtomicBoolean childCreated,
+      AtomicBoolean childClosed) {
     this.created = created;
     this.closed = closed;
+    this.childClosed = childClosed;
+    this.childCreated = childCreated;
+  }
+
+  public ModuleWithLifecycledKeys(AtomicBoolean created, AtomicBoolean closed) {
+    this(created, closed, new AtomicBoolean(false), new AtomicBoolean(false));
   }
 
   @Override
   protected void configure() {
     bind(AtomicBoolean.class).annotatedWith(Names.named("created")).toInstance(created);
     bind(AtomicBoolean.class).annotatedWith(Names.named("closed")).toInstance(closed);
 
+    install(new ChildModuleWithLifecycledKeys(childCreated, childClosed));
     bind(Foo.class).to(FooImpl.class);
     manageLifecycle(Foo.class);
   }
@@ -53,16 +64,14 @@ public String getId() {
     return "lifecycle-module";
   }
 
-  interface Foo {
-  }
+  interface Foo {}
 
   static class FooImpl implements Foo, Closeable {
 
     final AtomicBoolean closed;
 
     @Inject
-    FooImpl(@Named("created") AtomicBoolean created,
-            @Named("closed") AtomicBoolean closed) {
+    FooImpl(@Named("created") AtomicBoolean created, @Named("closed") AtomicBoolean closed) {
       this.closed = closed;
       created.set(true);
     }

apollo-core/src/test/java/com/spotify/apollo/core/ServiceImplTest.java

@@ -446,6 +446,27 @@ public void testModuleClassesAreLifecycleManaged() throws Exception {
     assertThat(closed.get(), is(true));
   }
 
+  @Test
+  public void testChildModuleClassesAreLifecycleManaged() throws Exception {
+    AtomicBoolean created = new AtomicBoolean(false);
+    AtomicBoolean closed = new AtomicBoolean(false);
+    AtomicBoolean childCreated = new AtomicBoolean(false);
+    AtomicBoolean childClosed = new AtomicBoolean(false);
+    ModuleWithLifecycledKeys lifecycleModule =
+        new ModuleWithLifecycledKeys(created, closed, childCreated, childClosed);
+    Service service = ServiceImpl.builder("test")
+        .withModule(lifecycleModule)
+        .build();
+
+    try (Service.Instance instance = service.start()) {
+      instance.getSignaller().signalShutdown();
+      instance.waitForShutdown();
+    }
+
+    assertThat(childCreated.get(), is(true));
+    assertThat(childClosed.get(), is(true));
+  }
+
   @Test
   public void testResolve() throws Exception {
     AtomicBoolean created = new AtomicBoolean(false);