From 6b5aea138ffb31551815e7ba4dad76552b3143ad Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Fri, 8 Jul 2016 08:54:23 -0700 Subject: [PATCH] Internal: Initialize Clock directly instead of with guice The Clock interface, which basically allows testing in watcher to "time warp" is currently constructed using guice. This change constructs it using a protected method on XPackPlugin which can be overriden in tests. This allows removing the ClockModule. For now, the Clock still needs to be bound in guice, but this at least removes one guice construction, and shows how other things can be overriden for tests. Original commit: elastic/x-pack-elasticsearch@7addaea0861c9bdecd4f15c3a1d56f4e5d5fe751 --- .../org/elasticsearch/xpack/XPackPlugin.java | 10 +++++-- .../xpack/support/clock/ClockModule.java | 20 ------------- .../xpack/TimeWarpedXPackPlugin.java | 28 ++----------------- .../xpack/support/clock/ClockMock.java | 2 +- .../AbstractWatcherIntegrationTestCase.java | 3 +- .../test/integration/BasicWatcherTests.java | 2 +- 6 files changed, 14 insertions(+), 51 deletions(-) delete mode 100644 elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/support/clock/ClockModule.java diff --git a/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/XPackPlugin.java b/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/XPackPlugin.java index 17be9e38b95..58394ca01a8 100644 --- a/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/XPackPlugin.java +++ b/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/XPackPlugin.java @@ -55,7 +55,8 @@ import org.elasticsearch.xpack.rest.action.RestXPackInfoAction; import org.elasticsearch.xpack.rest.action.RestXPackUsageAction; import org.elasticsearch.xpack.security.Security; import org.elasticsearch.xpack.security.authc.AuthenticationModule; -import org.elasticsearch.xpack.support.clock.ClockModule; +import org.elasticsearch.xpack.support.clock.Clock; +import org.elasticsearch.xpack.support.clock.SystemClock; import org.elasticsearch.xpack.watcher.Watcher; public class XPackPlugin extends Plugin implements ScriptPlugin, ActionPlugin { @@ -132,10 +133,15 @@ public class XPackPlugin extends Plugin implements ScriptPlugin, ActionPlugin { return Collections.emptyList(); } + // overridable by tests + protected Clock getClock() { + return SystemClock.INSTANCE; + } + @Override public Collection nodeModules() { ArrayList modules = new ArrayList<>(); - modules.add(new ClockModule()); + modules.add(b -> b.bind(Clock.class).toInstance(getClock())); modules.addAll(notification.nodeModules()); modules.addAll(licensing.nodeModules()); modules.addAll(security.nodeModules()); diff --git a/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/support/clock/ClockModule.java b/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/support/clock/ClockModule.java deleted file mode 100644 index b024dbf4dea..00000000000 --- a/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/support/clock/ClockModule.java +++ /dev/null @@ -1,20 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ -package org.elasticsearch.xpack.support.clock; - -import org.elasticsearch.common.inject.AbstractModule; - -/** - * - */ -public class ClockModule extends AbstractModule { - - @Override - protected void configure() { - bind(Clock.class).toInstance(SystemClock.INSTANCE); - } - -} diff --git a/elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/TimeWarpedXPackPlugin.java b/elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/TimeWarpedXPackPlugin.java index b2b69c4d284..9159b2a721f 100644 --- a/elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/TimeWarpedXPackPlugin.java +++ b/elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/TimeWarpedXPackPlugin.java @@ -5,43 +5,19 @@ */ package org.elasticsearch.xpack; -import org.elasticsearch.common.inject.Module; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.xpack.support.clock.Clock; import org.elasticsearch.xpack.support.clock.ClockMock; -import org.elasticsearch.xpack.support.clock.ClockModule; import org.elasticsearch.xpack.watcher.test.TimeWarpedWatcher; -import java.util.ArrayList; -import java.util.Collection; -import java.util.List; - public class TimeWarpedXPackPlugin extends XPackPlugin { - public TimeWarpedXPackPlugin(Settings settings) { super(settings); watcher = new TimeWarpedWatcher(settings); } @Override - public Collection nodeModules() { - List modules = new ArrayList<>(super.nodeModules()); - for (int i = 0; i < modules.size(); ++i) { - Module module = modules.get(i); - if (module instanceof ClockModule) { - // replacing the clock module so we'll be able - // to control time in tests - modules.set(i, new MockClockModule()); - } - } - return modules; - } - - public static class MockClockModule extends ClockModule { - @Override - protected void configure() { - bind(ClockMock.class).asEagerSingleton(); - bind(Clock.class).to(ClockMock.class); - } + protected Clock getClock() { + return new ClockMock(); } } diff --git a/elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/support/clock/ClockMock.java b/elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/support/clock/ClockMock.java index 25ea390ae66..318b01d4c60 100644 --- a/elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/support/clock/ClockMock.java +++ b/elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/support/clock/ClockMock.java @@ -12,7 +12,7 @@ import org.joda.time.DateTimeZone; import java.util.concurrent.TimeUnit; /** - * + * A clock that can be modified for testing. */ public class ClockMock implements Clock { diff --git a/elasticsearch/x-pack/watcher/src/test/java/org/elasticsearch/xpack/watcher/test/AbstractWatcherIntegrationTestCase.java b/elasticsearch/x-pack/watcher/src/test/java/org/elasticsearch/xpack/watcher/test/AbstractWatcherIntegrationTestCase.java index bc635db8019..b671984c582 100644 --- a/elasticsearch/x-pack/watcher/src/test/java/org/elasticsearch/xpack/watcher/test/AbstractWatcherIntegrationTestCase.java +++ b/elasticsearch/x-pack/watcher/src/test/java/org/elasticsearch/xpack/watcher/test/AbstractWatcherIntegrationTestCase.java @@ -39,6 +39,7 @@ import org.elasticsearch.test.TestCluster; import org.elasticsearch.test.store.MockFSIndexStore; import org.elasticsearch.test.transport.AssertingLocalTransport; import org.elasticsearch.test.transport.MockTransportService; +import org.elasticsearch.xpack.support.clock.Clock; import org.elasticsearch.xpack.watcher.WatcherLifeCycleService; import org.elasticsearch.xpack.watcher.WatcherService; import org.elasticsearch.xpack.watcher.WatcherState; @@ -270,7 +271,7 @@ public abstract class AbstractWatcherIntegrationTestCase extends ESIntegTestCase private void setupTimeWarp() throws Exception { if (timeWarped()) { - timeWarp = new TimeWarp(getInstanceFromMaster(ScheduleTriggerEngineMock.class), getInstanceFromMaster(ClockMock.class)); + timeWarp = new TimeWarp(getInstanceFromMaster(ScheduleTriggerEngineMock.class), (ClockMock)getInstanceFromMaster(Clock.class)); } } diff --git a/elasticsearch/x-pack/watcher/src/test/java/org/elasticsearch/xpack/watcher/test/integration/BasicWatcherTests.java b/elasticsearch/x-pack/watcher/src/test/java/org/elasticsearch/xpack/watcher/test/integration/BasicWatcherTests.java index 5a1cb2d465f..ac17bc31653 100644 --- a/elasticsearch/x-pack/watcher/src/test/java/org/elasticsearch/xpack/watcher/test/integration/BasicWatcherTests.java +++ b/elasticsearch/x-pack/watcher/src/test/java/org/elasticsearch/xpack/watcher/test/integration/BasicWatcherTests.java @@ -57,7 +57,7 @@ import static org.hamcrest.Matchers.notNullValue; /** */ -public class BasicWatcherTests extends AbstractWatcherIntegrationTestCase { +public class BasicWatcherTests extends AbstractWatcherIntegrationTestCase { @Override protected boolean timeWarped() {