Add tests

pom.xml

@@ -108,6 +108,7 @@
         <slf4j.version>1.7.12</slf4j.version>
         <!-- If compiling with different hadoop version also modify default hadoop coordinates in TaskConfig.java -->
         <hadoop.compile.version>2.8.5</hadoop.compile.version>
+        <mockito.version>3.2.4</mockito.version>
         <powermock.version>2.0.2</powermock.version>
         <aws.sdk.version>1.11.199</aws.sdk.version>
         <caffeine.version>2.8.0</caffeine.version>
@@ -1040,6 +1041,12 @@
                 <version>4.12</version>
                 <scope>test</scope>
             </dependency>
+            <dependency>
+              <groupId>org.mockito</groupId>
+              <artifactId>mockito-core</artifactId>
+              <version>${mockito.version}</version>
+              <scope>test</scope>
+            </dependency>
             <dependency>
                 <groupId>org.powermock</groupId>
                 <artifactId>powermock-core</artifactId>

server/pom.xml

@@ -318,6 +318,11 @@
             <artifactId>junit</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.mockito</groupId>
+            <artifactId>mockito-core</artifactId>
+            <scope>test</scope>
+        </dependency>
         <dependency>
             <groupId>org.hamcrest</groupId>
             <artifactId>hamcrest-all</artifactId>

server/src/main/java/org/apache/druid/guice/StorageNodeModule.java

@@ -19,6 +19,7 @@
 
 package org.apache.druid.guice;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.inject.Binder;
 import com.google.inject.Module;
 import com.google.inject.Provides;
@@ -43,7 +44,8 @@ import javax.annotation.Nullable;
 public class StorageNodeModule implements Module
 {
   private static final EmittingLogger log = new EmittingLogger(StorageNodeModule.class);
-  private static final String IS_SEGMENT_CACHE_CONFIGURED = "IS_SEGMENT_CACHE_CONFIGURED";
+  @VisibleForTesting
+  static final String IS_SEGMENT_CACHE_CONFIGURED = "IS_SEGMENT_CACHE_CONFIGURED";
 
   @Override
   public void configure(Binder binder)
@@ -87,7 +89,7 @@ public class StorageNodeModule implements Module
   )
   {
     if (serverTypeConfig == null) {
-      throw new ProvisionException("Must override the binding for ServerTypeConfig if you want a DruidServerMetadata.");
+      throw new ProvisionException("Must override the binding for ServerTypeConfig if you want a DataNodeService.");
     }
     if (!isSegmentCacheConfigured) {
       log.info("Segment cache not configured on ServerType [%s]", serverTypeConfig.getServerType());

server/src/test/java/org/apache/druid/guice/StorageNodeModuleTest.java

@@ -0,0 +1,196 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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 org.apache.druid.guice;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.inject.Guice;
+import com.google.inject.Injector;
+import com.google.inject.Key;
+import com.google.inject.ProvisionException;
+import com.google.inject.Scopes;
+import com.google.inject.name.Names;
+import com.google.inject.util.Modules;
+import org.apache.druid.discovery.DataNodeService;
+import org.apache.druid.guice.annotations.Self;
+import org.apache.druid.query.DruidProcessingConfig;
+import org.apache.druid.segment.loading.SegmentLoaderConfig;
+import org.apache.druid.segment.loading.StorageLocationConfig;
+import org.apache.druid.server.DruidNode;
+import org.apache.druid.server.coordination.DruidServerMetadata;
+import org.apache.druid.server.coordination.ServerType;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.junit.runner.RunWith;
+import org.mockito.Answers;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.junit.MockitoJUnitRunner;
+
+import javax.validation.Validation;
+import javax.validation.Validator;
+import java.util.Collections;
+
+@RunWith(MockitoJUnitRunner.class)
+public class StorageNodeModuleTest
+{
+  private static final boolean INJECT_SERVER_TYPE_CONFIG = true;
+
+  @Rule
+  public ExpectedException exceptionRule = ExpectedException.none();
+
+  @Mock(answer = Answers.RETURNS_MOCKS)
+  private ObjectMapper mapper;
+  @Mock(answer = Answers.RETURNS_MOCKS)
+  private DruidNode self;
+  @Mock(answer = Answers.RETURNS_MOCKS)
+  private ServerTypeConfig serverTypeConfig;
+  @Mock
+  private DruidProcessingConfig druidProcessingConfig;
+  @Mock
+  private SegmentLoaderConfig segmentLoaderConfig;
+  @Mock
+  private StorageLocationConfig storageLocation;
+
+  private Injector injector;
+  private StorageNodeModule target;
+
+  @Before
+  public void setUp()
+  {
+    Mockito.when(segmentLoaderConfig.getLocations()).thenReturn(Collections.singletonList(storageLocation));
+
+    target = new StorageNodeModule();
+    injector = makeInjector(INJECT_SERVER_TYPE_CONFIG);
+  }
+
+  @Test
+  public void testIsSegmentCacheConfiguredIsInjected()
+  {
+    Boolean isSegmentCacheConfigured = injector.getInstance(
+        Key.get(Boolean.class, Names.named(StorageNodeModule.IS_SEGMENT_CACHE_CONFIGURED))
+    );
+    Assert.assertNotNull(isSegmentCacheConfigured);
+    Assert.assertTrue(isSegmentCacheConfigured);
+  }
+
+  @Test
+  public void testIsSegmentCacheConfiguredWithNoLocationsConfiguredIsInjected()
+  {
+    mockSegmentCacheNotConfigured();
+    Boolean isSegmentCacheConfigured = injector.getInstance(
+        Key.get(Boolean.class, Names.named(StorageNodeModule.IS_SEGMENT_CACHE_CONFIGURED))
+    );
+    Assert.assertNotNull(isSegmentCacheConfigured);
+    Assert.assertFalse(isSegmentCacheConfigured);
+  }
+
+  @Test
+  public void getDataNodeServiceWithNoServerTypeConfigShouldThrowProvisionException()
+  {
+    exceptionRule.expect(ProvisionException.class);
+    exceptionRule.expectMessage("Must override the binding for ServerTypeConfig if you want a DataNodeService.");
+    injector = makeInjector(!INJECT_SERVER_TYPE_CONFIG);
+    injector.getInstance(DataNodeService.class);
+  }
+
+  @Test
+  public void getDataNodeServiceWithNoSegmentCacheConfiguredThrowProvisionException()
+  {
+    exceptionRule.expect(ProvisionException.class);
+    exceptionRule.expectMessage("Segment cache locations must be set on historicals.");
+    Mockito.doReturn(ServerType.HISTORICAL).when(serverTypeConfig).getServerType();
+    mockSegmentCacheNotConfigured();
+    injector.getInstance(DataNodeService.class);
+  }
+
+  @Test
+  public void getDataNodeServiceIsInjectedAsSingleton()
+  {
+    DataNodeService dataNodeService = injector.getInstance(DataNodeService.class);
+    Assert.assertNotNull(dataNodeService);
+    DataNodeService other = injector.getInstance(DataNodeService.class);
+    Assert.assertSame(dataNodeService, other);
+  }
+
+  @Test
+  public void getDataNodeServiceIsInjectedAndDiscoverable()
+  {
+    DataNodeService dataNodeService = injector.getInstance(DataNodeService.class);
+    Assert.assertNotNull(dataNodeService);
+    Assert.assertTrue(dataNodeService.isDiscoverable());
+  }
+
+  @Test
+  public void getDataNodeServiceWithSegmentCacheNotConfiguredIsInjectedAndDiscoverable()
+  {
+    mockSegmentCacheNotConfigured();
+    DataNodeService dataNodeService = injector.getInstance(DataNodeService.class);
+    Assert.assertNotNull(dataNodeService);
+    Assert.assertFalse(dataNodeService.isDiscoverable());
+  }
+
+  @Test
+  public void testDruidServerMetadataIsInjectedAsSingleton()
+  {
+    DruidServerMetadata druidServerMetadata = injector.getInstance(DruidServerMetadata.class);
+    Assert.assertNotNull(druidServerMetadata);
+    DruidServerMetadata other = injector.getInstance(DruidServerMetadata.class);
+    Assert.assertSame(druidServerMetadata, other);
+  }
+
+  @Test
+  public void testDruidServerMetadataWithNoServerTypeConfigShouldThrowProvisionException()
+  {
+    exceptionRule.expect(ProvisionException.class);
+    exceptionRule.expectMessage("Must override the binding for ServerTypeConfig if you want a DruidServerMetadata.");
+    injector = makeInjector(!INJECT_SERVER_TYPE_CONFIG);
+    injector.getInstance(DruidServerMetadata.class);
+  }
+
+  private Injector makeInjector(boolean withServerTypeConfig)
+  {
+    return Guice.createInjector(
+        Modules.override(
+            (binder) -> {
+              binder.bind(DruidNode.class).annotatedWith(Self.class).toInstance(self);
+              binder.bind(Validator.class).toInstance(Validation.buildDefaultValidatorFactory().getValidator());
+              binder.bind(DruidProcessingConfig.class).toInstance(druidProcessingConfig);
+              binder.bindScope(LazySingleton.class, Scopes.SINGLETON);
+            },
+            target
+        ).with(
+            (binder) -> {
+              binder.bind(SegmentLoaderConfig.class).toInstance(segmentLoaderConfig);
+              if (withServerTypeConfig) {
+                binder.bind(ServerTypeConfig.class).toInstance(serverTypeConfig);
+              }
+            }
+        )
+    );
+  }
+
+  private void mockSegmentCacheNotConfigured()
+  {
+    Mockito.doReturn(Collections.emptyList()).when(segmentLoaderConfig).getLocations();
+  }
+}

server/src/test/java/org/apache/druid/server/coordination/SegmentLoadDropHandlerTest.java

@@ -25,7 +25,6 @@ import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
 import com.google.common.util.concurrent.ListenableFuture;
 import org.apache.druid.guice.ServerTypeConfig;
-import org.apache.druid.java.util.common.IAE;
 import org.apache.druid.java.util.common.Intervals;
 import org.apache.druid.java.util.common.concurrent.Execs;
 import org.apache.druid.java.util.common.concurrent.ScheduledExecutorFactory;
@@ -276,40 +275,6 @@ public class SegmentLoadDropHandlerTest
     segmentLoadDropHandler.stop();
   }
 
-  @Test
-  public void testSegmentLoading1BrokerWithNoLocations() throws Exception
-  {
-    SegmentLoadDropHandler segmentLoadDropHandlerBrokerWithNoLocations = new SegmentLoadDropHandler(
-        jsonMapper,
-        segmentLoaderConfigNoLocations,
-        announcer,
-        EasyMock.createNiceMock(DataSegmentServerAnnouncer.class),
-        segmentManager,
-        scheduledExecutorFactory.create(5, "SegmentLoadDropHandlerTest-brokerNoLocations-[%d]"),
-        new ServerTypeConfig(ServerType.BROKER)
-    );
-
-    segmentLoadDropHandlerBrokerWithNoLocations.start();
-    segmentLoadDropHandler.stop();
-  }
-
-  @Test
-  public void testSegmentLoading1HistoricalWithNoLocations()
-  {
-    expectedException.expect(IAE.class);
-    expectedException.expectMessage("Segment cache locations must be set on historicals.");
-
-    new SegmentLoadDropHandler(
-        jsonMapper,
-        segmentLoaderConfigNoLocations,
-        announcer,
-        EasyMock.createNiceMock(DataSegmentServerAnnouncer.class),
-        segmentManager,
-        scheduledExecutorFactory.create(5, "SegmentLoadDropHandlerTest-[%d]"),
-        new ServerTypeConfig(ServerType.HISTORICAL)
-    );
-  }
-
   /**
    * Steps:
    * 1. addSegment() succesfully loads the segment and annouces it

services/pom.xml

@@ -166,6 +166,11 @@
             <artifactId>junit</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.mockito</groupId>
+            <artifactId>mockito-core</artifactId>
+            <scope>test</scope>
+        </dependency>
         <dependency>
             <groupId>org.hamcrest</groupId>
             <artifactId>hamcrest-core</artifactId>

services/src/main/java/org/apache/druid/cli/ServerRunnable.java

@@ -19,6 +19,7 @@
 
 package org.apache.druid.cli;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.inject.Binder;
@@ -201,6 +202,28 @@ public abstract class ServerRunnable extends GuiceRunnable
       this.useLegacyAnnouncer = useLegacyAnnouncer;
     }
 
+    @VisibleForTesting
+    DiscoverySideEffectsProvider(
+        final NodeRole nodeRole,
+        final List<Class<? extends DruidService>> serviceClasses,
+        final boolean useLegacyAnnouncer,
+        final DruidNode druidNode,
+        final DruidNodeAnnouncer announcer,
+        final ServiceAnnouncer legacyAnnouncer,
+        final Lifecycle lifecycle,
+        final Injector injector
+    )
+    {
+      this.nodeRole = nodeRole;
+      this.serviceClasses = serviceClasses;
+      this.useLegacyAnnouncer = useLegacyAnnouncer;
+      this.druidNode = druidNode;
+      this.announcer = announcer;
+      this.legacyAnnouncer = legacyAnnouncer;
+      this.lifecycle = lifecycle;
+      this.injector = injector;
+    }
+
     @Override
     public Child get()
     {

services/src/test/java/org/apache/druid/cli/DiscoverySideEffectsProviderTest.java

@@ -0,0 +1,138 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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 org.apache.druid.cli;
+
+import com.google.common.collect.ImmutableList;
+import com.google.inject.Injector;
+import org.apache.druid.curator.discovery.ServiceAnnouncer;
+import org.apache.druid.discovery.DiscoveryDruidNode;
+import org.apache.druid.discovery.DruidNodeAnnouncer;
+import org.apache.druid.discovery.DruidService;
+import org.apache.druid.discovery.NodeRole;
+import org.apache.druid.java.util.common.lifecycle.Lifecycle;
+import org.apache.druid.server.DruidNode;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.ArgumentMatchers;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.junit.MockitoJUnitRunner;
+
+import java.util.ArrayList;
+import java.util.List;
+
+@RunWith(MockitoJUnitRunner.class)
+public class DiscoverySideEffectsProviderTest
+{
+  private static final boolean USE_LEGACY_ANNOUNCER = true;
+
+  private NodeRole nodeRole;
+  @Mock
+  private DruidNode druidNode;
+  @Mock
+  private DruidNodeAnnouncer announcer;
+  @Mock
+  private ServiceAnnouncer legacyAnnouncer;
+  @Mock
+  private Lifecycle lifecycle;
+  @Mock
+  private Injector injector;
+  private List<Lifecycle.Handler> lifecycleHandlers;
+
+  private ServerRunnable.DiscoverySideEffectsProvider target;
+
+  @Before
+  public void setUp()
+  {
+    nodeRole = NodeRole.HISTORICAL;
+    lifecycleHandlers = new ArrayList<>();
+    Mockito.when(injector.getInstance(DiscoverableDruidService.class)).thenReturn(new DiscoverableDruidService());
+    Mockito.when(injector.getInstance(UnDiscoverableDruidService.class)).thenReturn(new UnDiscoverableDruidService());
+    Mockito.doAnswer((invocation) -> {
+      DiscoveryDruidNode discoveryDruidNode = invocation.getArgument(0);
+      boolean isAllServicesDiscoverable =
+          discoveryDruidNode.getServices().values().stream().allMatch(DruidService::isDiscoverable);
+      Assert.assertTrue(isAllServicesDiscoverable);
+      return null;
+    }).when(announcer).announce(ArgumentMatchers.any(DiscoveryDruidNode.class));
+    Mockito.doAnswer((invocation) -> lifecycleHandlers.add(invocation.getArgument(0)))
+           .when(lifecycle).addHandler(
+        ArgumentMatchers.any(Lifecycle.Handler.class),
+        ArgumentMatchers.eq(Lifecycle.Stage.ANNOUNCEMENTS)
+      );
+    target = new ServerRunnable.DiscoverySideEffectsProvider(
+        nodeRole,
+        ImmutableList.of(DiscoverableDruidService.class, UnDiscoverableDruidService.class),
+        USE_LEGACY_ANNOUNCER,
+        druidNode,
+        announcer,
+        legacyAnnouncer,
+        lifecycle,
+        injector
+    );
+  }
+
+  @Test
+  public void testGetShouldAddAnnouncementsForDiscoverableServices() throws Exception
+  {
+    ServerRunnable.DiscoverySideEffectsProvider.Child child = target.get();
+    Assert.assertNotNull(child);
+    Assert.assertEquals(1, lifecycleHandlers.size());
+    lifecycleHandlers.get(0).start();
+  }
+
+  /**
+   * Dummy service which is discoverable.
+   */
+  private static class DiscoverableDruidService extends DruidService
+  {
+    @Override
+    public String getName()
+    {
+      return "DiscoverableDruidService";
+    }
+
+    @Override
+    public boolean isDiscoverable()
+    {
+      return true;
+    }
+  }
+
+  /**
+   * Dummy service which is not discoverable.
+   */
+  private static class UnDiscoverableDruidService extends DruidService
+  {
+    @Override
+    public String getName()
+    {
+      return "UnDiscoverableDruidService";
+    }
+
+    @Override
+    public boolean isDiscoverable()
+    {
+      return false;
+    }
+  }
+}