From 1c71dbbac3731d484e19d9b64b3e498a03295dba Mon Sep 17 00:00:00 2001 From: Mikko Karjalainen Date: Fri, 11 Oct 2019 09:10:16 +0100 Subject: [PATCH 1/2] Remove hamcrest dependency from styx common production code. --- .../EpollClientEventLoopGroupFactory.java | 9 ++- .../styx/client/OriginsInventoryTest.java | 64 +++++++---------- .../hotels/styx/common/MorePreconditions.java | 68 ++----------------- .../testing/matcher/TransformingMatcher.java | 62 ----------------- .../styx/common/MorePreconditionsTest.java | 26 ++----- .../EpollServerEventLoopGroupFactory.java | 9 ++- 6 files changed, 48 insertions(+), 190 deletions(-) delete mode 100644 components/common/src/main/java/com/hotels/styx/common/testing/matcher/TransformingMatcher.java diff --git a/components/client/src/main/java/com/hotels/styx/client/netty/eventloop/epoll/EpollClientEventLoopGroupFactory.java b/components/client/src/main/java/com/hotels/styx/client/netty/eventloop/epoll/EpollClientEventLoopGroupFactory.java index f4b4578a02..2c0ca773aa 100644 --- a/components/client/src/main/java/com/hotels/styx/client/netty/eventloop/epoll/EpollClientEventLoopGroupFactory.java +++ b/components/client/src/main/java/com/hotels/styx/client/netty/eventloop/epoll/EpollClientEventLoopGroupFactory.java @@ -1,5 +1,5 @@ /* - Copyright (C) 2013-2018 Expedia Inc. + Copyright (C) 2013-2019 Expedia Inc. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -24,6 +24,7 @@ import static com.hotels.styx.common.MorePreconditions.checkArgument; import static com.hotels.styx.common.MorePreconditions.checkNotEmpty; +import static java.lang.String.format; /** * A factory for creating Epoll-based client event loops. @@ -39,8 +40,10 @@ public class EpollClientEventLoopGroupFactory implements ClientEventLoopFactory * @param clientWorkerThreadsCount number of threads */ public EpollClientEventLoopGroupFactory(String name, int clientWorkerThreadsCount) { - this.name = checkNotEmpty(name); - this.clientWorkerThreadsCount = checkArgument(clientWorkerThreadsCount, clientWorkerThreadsCount > -1); + this.name = checkNotEmpty(name, "Event loop group name is null or empty"); + this.clientWorkerThreadsCount = checkArgument(clientWorkerThreadsCount, + clientWorkerThreadsCount > -1, + format("Invalid client worker thread count (%d). It must be > -1.", clientWorkerThreadsCount)); } @Override diff --git a/components/client/src/test/unit/java/com/hotels/styx/client/OriginsInventoryTest.java b/components/client/src/test/unit/java/com/hotels/styx/client/OriginsInventoryTest.java index 4b213a4049..2a628b7853 100644 --- a/components/client/src/test/unit/java/com/hotels/styx/client/OriginsInventoryTest.java +++ b/components/client/src/test/unit/java/com/hotels/styx/client/OriginsInventoryTest.java @@ -30,7 +30,6 @@ import com.hotels.styx.client.origincommands.DisableOrigin; import com.hotels.styx.client.origincommands.EnableOrigin; import com.hotels.styx.support.matchers.LoggingTestSupport; -import org.hamcrest.Matcher; import org.testng.annotations.AfterMethod; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; @@ -44,7 +43,6 @@ import static com.hotels.styx.api.extension.service.ConnectionPoolSettings.defaultConnectionPoolSettings; import static com.hotels.styx.client.OriginsInventory.OriginState.ACTIVE; import static com.hotels.styx.client.OriginsInventory.OriginState.DISABLED; -import static com.hotels.styx.common.testing.matcher.TransformingMatcher.hasDerivedValue; import static com.hotels.styx.support.matchers.ContainsExactlyOneMatcher.containsExactlyOne; import static com.hotels.styx.support.matchers.IsOptional.isAbsent; import static com.hotels.styx.support.matchers.IsOptional.isValue; @@ -93,7 +91,7 @@ public void stop() { public void startsMonitoringNewOrigins() { inventory.setOrigins(ORIGIN_1, ORIGIN_2); - assertThat(inventory, hasActiveOrigins(2)); + assertThat(inventory.originCount(ACTIVE), is(2)); verify(monitor).monitor(singleton(ORIGIN_1)); verify(monitor).monitor(singleton(ORIGIN_2)); @@ -110,14 +108,14 @@ public void updatesOriginPortNumber() throws Exception { inventory.setOrigins(originV1); - assertThat(inventory, hasActiveOrigins(1)); + assertThat(inventory.originCount(ACTIVE),is(1)); verify(monitor).monitor(singleton(originV1)); assertThat(gaugeValue("origins.generic-app.acme-01.status"), isValue(1)); verify(eventBus).post(any(OriginsSnapshot.class)); inventory.setOrigins(originV2); - assertThat(inventory, hasActiveOrigins(1)); + assertThat(inventory.originCount(ACTIVE), is(1)); verify(monitor).stopMonitoring(singleton(originV1)); verify(monitor).monitor(singleton(originV2)); assertThat(gaugeValue("origins.generic-app.acme-01.status"), isValue(1)); @@ -131,14 +129,14 @@ public void updatesOriginHostName() throws Exception { inventory.setOrigins(originV1); - assertThat(inventory, hasActiveOrigins(1)); + assertThat(inventory.originCount(ACTIVE), is(1)); verify(monitor).monitor(singleton(originV1)); assertThat(gaugeValue("origins.generic-app.acme-01.status"), isValue(1)); verify(eventBus).post(any(OriginsSnapshot.class)); inventory.setOrigins(originV2); - assertThat(inventory, hasActiveOrigins(1)); + assertThat(inventory.originCount(ACTIVE), is(1)); verify(monitor).stopMonitoring(singleton(originV1)); verify(monitor).monitor(singleton(originV2)); assertThat(gaugeValue("origins.generic-app.acme-01.status"), isValue(1)); @@ -187,7 +185,7 @@ public void shutsConnectionPoolForModifiedOrigin() { public void ignoresUnchangedOrigins() throws Exception { inventory.setOrigins(ORIGIN_1, ORIGIN_2); - assertThat(inventory, hasActiveOrigins(2)); + assertThat(inventory.originCount(ACTIVE), is(2)); verify(monitor).monitor(singleton(ORIGIN_1)); verify(monitor).monitor(singleton(ORIGIN_2)); assertThat(gaugeValue("origins.generic-app.app-01.status"), isValue(1)); @@ -196,7 +194,7 @@ public void ignoresUnchangedOrigins() throws Exception { inventory.setOrigins(ORIGIN_1, ORIGIN_2); - assertThat(inventory, hasActiveOrigins(2)); + assertThat(inventory.originCount(ACTIVE), is(2)); verify(monitor, times(1)).monitor(singleton(ORIGIN_1)); verify(monitor, times(1)).monitor(singleton(ORIGIN_2)); verify(eventBus).post(any(OriginsSnapshot.class)); @@ -206,7 +204,7 @@ public void ignoresUnchangedOrigins() throws Exception { public void removesOrigin() throws Exception { inventory.setOrigins(ORIGIN_1, ORIGIN_2); - assertThat(inventory, hasActiveOrigins(2)); + assertThat(inventory.originCount(ACTIVE), is(2)); verify(monitor).monitor(singleton(ORIGIN_1)); verify(monitor).monitor(singleton(ORIGIN_2)); assertThat(gaugeValue("origins.generic-app.app-01.status"), isValue(1)); @@ -215,7 +213,7 @@ public void removesOrigin() throws Exception { inventory.setOrigins(ORIGIN_2); - assertThat(inventory, hasActiveOrigins(1)); + assertThat(inventory.originCount(ACTIVE), is(1)); verify(monitor).stopMonitoring(singleton(ORIGIN_1)); assertThat(gaugeValue("origins.generic-app.app-01.status"), isAbsent()); assertThat(gaugeValue("origins.generic-app.app-02.status"), isValue(1)); @@ -268,7 +266,7 @@ public void willNotDisableOriginsNotBelongingToTheApp() { inventory.onCommand(new DisableOrigin(id("some-other-app"), ORIGIN_1.id())); - assertThat(inventory, hasActiveOrigins(1)); + assertThat(inventory.originCount(ACTIVE), is(1)); verify(eventBus).post(any(OriginsSnapshot.class)); } @@ -279,7 +277,7 @@ public void willNotEnableOriginsNotBelongingToTheApp() { inventory.onCommand(new DisableOrigin(ORIGIN_1.applicationId(), ORIGIN_1.id())); inventory.onCommand(new EnableOrigin(id("some-other-app"), ORIGIN_1.id())); - assertThat(inventory, hasNoActiveOrigins()); + assertThat(inventory.originCount(ACTIVE), is(0)); verify(eventBus, times(2)).post(any(OriginsSnapshot.class)); } @@ -289,8 +287,8 @@ public void disablingAnOriginRemovesItFromActiveSetAndStopsHealthCheckMonitoring inventory.onCommand(new DisableOrigin(ORIGIN_1.applicationId(), ORIGIN_1.id())); - assertThat(inventory, hasNoActiveOrigins()); - assertThat(inventory, hasDisabledOrigins(1)); + assertThat(inventory.originCount(ACTIVE), is(0)); + assertThat(inventory.originCount(DISABLED), is(1)); verify(monitor).stopMonitoring(singleton(ORIGIN_1)); assertThat(gaugeValue("origins.generic-app.app-01.status"), isValue(-1)); @@ -304,8 +302,8 @@ public void disablingAnOriginRemovesItFromInactiveSetAndStopsHealthCheckMonitori inventory.originUnhealthy(ORIGIN_1); inventory.onCommand(new DisableOrigin(ORIGIN_1.applicationId(), ORIGIN_1.id())); - assertThat(inventory, hasNoActiveOrigins()); - assertThat(inventory, hasDisabledOrigins(1)); + assertThat(inventory.originCount(ACTIVE), is(0)); + assertThat(inventory.originCount(DISABLED), is(1)); verify(monitor).stopMonitoring(singleton(ORIGIN_1)); assertThat(gaugeValue("origins.generic-app.app-01.status"), isValue(-1)); @@ -327,11 +325,11 @@ public void enablingAnOriginWillReInitiateHealthCheckMonitoring() { @Test public void removesUnhealthyOriginsFromActiveSet() { inventory.setOrigins(ORIGIN_1); - assertThat(inventory, hasActiveOrigins(1)); + assertThat(inventory.originCount(ACTIVE), is(1)); inventory.originUnhealthy(ORIGIN_1); - assertThat(inventory, hasNoActiveOrigins()); + assertThat(inventory.originCount(ACTIVE), is(0)); assertThat(gaugeValue("origins.generic-app.app-01.status"), isValue(0)); verify(eventBus, times(2)).post(any(OriginsSnapshot.class)); } @@ -339,12 +337,12 @@ public void removesUnhealthyOriginsFromActiveSet() { @Test public void putsHealthyOriginsBackIntoActiveSet() { inventory.setOrigins(ORIGIN_1); - assertThat(inventory, hasActiveOrigins(1)); + assertThat(inventory.originCount(ACTIVE), is(1)); inventory.originUnhealthy(ORIGIN_1); inventory.originHealthy(ORIGIN_1); - assertThat(inventory, hasActiveOrigins(1)); + assertThat(inventory.originCount(ACTIVE), is(1)); assertThat(gaugeValue("origins.generic-app.app-01.status"), isValue(1)); verify(eventBus, times(3)).post(any(OriginsSnapshot.class)); } @@ -352,26 +350,26 @@ public void putsHealthyOriginsBackIntoActiveSet() { @Test public void reportingUpRepeatedlyDoesNotAffectCurrentActiveOrigins() { inventory.setOrigins(ORIGIN_1); - assertThat(inventory, hasActiveOrigins(1)); + assertThat(inventory.originCount(ACTIVE), is(1)); inventory.originHealthy(ORIGIN_1); inventory.originHealthy(ORIGIN_1); inventory.originHealthy(ORIGIN_1); - assertThat(inventory, hasActiveOrigins(1)); + assertThat(inventory.originCount(ACTIVE), is(1)); verify(eventBus, times(1)).post(any(OriginsSnapshot.class)); } @Test public void reportingDownRepeatedlyDoesNotAffectCurrentActiveOrigins() { inventory.setOrigins(ORIGIN_1); - assertThat(inventory, hasActiveOrigins(1)); + assertThat(inventory.originCount(ACTIVE), is(1)); inventory.originUnhealthy(ORIGIN_1); inventory.originUnhealthy(ORIGIN_1); inventory.originUnhealthy(ORIGIN_1); - assertThat(inventory, hasActiveOrigins(0)); + assertThat(inventory.originCount(ACTIVE), is(0)); verify(eventBus, times(2)).post(any(OriginsSnapshot.class)); } @@ -475,22 +473,6 @@ public void stopsMonitoringAndUnregistersWhenClosed() { verify(eventBus).unregister(eq(inventory)); } - private static Matcher hasNoActiveOrigins() { - return hasActiveOrigins(0); - } - - private static Matcher hasActiveOrigins(int numberOfActiveOrigins) { - return hasOrigins(numberOfActiveOrigins, ACTIVE); - } - - private static Matcher hasDisabledOrigins(int numberOfDisabledOrigins) { - return hasOrigins(numberOfDisabledOrigins, DISABLED); - } - - private static Matcher hasOrigins(int numberOfOrigins, OriginsInventory.OriginState state) { - return hasDerivedValue(inventory -> inventory.originCount(state), is(numberOfOrigins)); - } - private Optional gaugeValue(String name) { return gauge(name) .map(Gauge::getValue) diff --git a/components/common/src/main/java/com/hotels/styx/common/MorePreconditions.java b/components/common/src/main/java/com/hotels/styx/common/MorePreconditions.java index 04ae4960db..97e0388851 100644 --- a/components/common/src/main/java/com/hotels/styx/common/MorePreconditions.java +++ b/components/common/src/main/java/com/hotels/styx/common/MorePreconditions.java @@ -1,5 +1,5 @@ /* - Copyright (C) 2013-2018 Expedia Inc. + Copyright (C) 2013-2019 Expedia Inc. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -15,15 +15,7 @@ */ package com.hotels.styx.common; -import com.google.common.base.Preconditions; -import org.hamcrest.Description; -import org.hamcrest.Matcher; -import org.hamcrest.StringDescription; - import static com.google.common.base.Strings.isNullOrEmpty; -import static org.hamcrest.Matchers.allOf; -import static org.hamcrest.Matchers.greaterThanOrEqualTo; -import static org.hamcrest.Matchers.lessThanOrEqualTo; /** * Static convenience methods that help a method or constructor check whether it was invoked @@ -37,8 +29,8 @@ public final class MorePreconditions { * @return the same string if non-empty * @throws NullPointerException if {@code value} is null or empty */ - public static String checkNotEmpty(String value) { - Preconditions.checkArgument(!isNullOrEmpty(value)); + public static String checkNotEmpty(String value, String message) { + checkArgument(value, !isNullOrEmpty(value), message); return value; } @@ -51,61 +43,13 @@ public static String checkNotEmpty(String value) { * @return the same reference passed in (if {@code expression} is true) * @throws IllegalArgumentException if {@code expression} is false */ - public static T checkArgument(T reference, boolean expression) { - Preconditions.checkArgument(expression); - return reference; - } - - - /** - * Ensures that an object reference passed as a parameter to the calling method satisfies - * the condition specified by the matcher. - * - * @param reference an object reference - * @param condition the condition to be satisfied - * @param reference type - * @return the same reference passed in - * @throws IllegalArgumentException if {@code condition} is false - */ - public static T checkArgument(T reference, Matcher condition) { - return checkArgument(reference, condition, "argument"); - } - - /** - * Ensures that an object reference passed as a parameter to the calling method satisfies - * the condition specified by the matcher. - * - * @param reference an object reference - * @param condition the condition to be satisfied - * @param referenceName the name of the argument to use in the error description - * @param reference type - * @return the same reference passed in - * @throws IllegalArgumentException if {@code condition} is false - */ - public static T checkArgument(T reference, Matcher condition, String referenceName) { - if (!condition.matches(reference)) { - Description description = new StringDescription(); - description.appendText(referenceName) - .appendText("\nExpected: ") - .appendDescriptionOf(condition) - .appendText("\n but: "); - condition.describeMismatch(reference, description); - throw new IllegalArgumentException(description.toString()); + public static T checkArgument(T reference, boolean expression, String message) { + if (!expression) { + throw new IllegalArgumentException(message); } return reference; } - /** - * A matcher that checks whether an integer is between a given minimum and maximum (inclusive). - * - * @param minimum minimum value (inclusive) - * @param maximum maximum value (inclusive) - * @return the matcher - */ - public static Matcher inRange(int minimum, int maximum) { - return allOf(greaterThanOrEqualTo(minimum), lessThanOrEqualTo(maximum)); - } - private MorePreconditions() { } } diff --git a/components/common/src/main/java/com/hotels/styx/common/testing/matcher/TransformingMatcher.java b/components/common/src/main/java/com/hotels/styx/common/testing/matcher/TransformingMatcher.java deleted file mode 100644 index 65d41bbf0c..0000000000 --- a/components/common/src/main/java/com/hotels/styx/common/testing/matcher/TransformingMatcher.java +++ /dev/null @@ -1,62 +0,0 @@ -/* - Copyright (C) 2013-2018 Expedia Inc. - - 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.hotels.styx.common.testing.matcher; - -import org.hamcrest.Description; -import org.hamcrest.Matcher; -import org.hamcrest.TypeSafeMatcher; - -import java.util.function.Function; - -import static java.util.Objects.requireNonNull; - -/** - * A matcher that transforms a value and then applies another matcher to it when used. - *

- * The advantage of this over transforming the value inside the test code is that it can be re-used while maintaining - * the familiar syntax of hamcrest assertThat. - * - * @param type to match against - * @param post-transformation type of delegate matcher - */ -public class TransformingMatcher extends TypeSafeMatcher { - private final Function transformation; - private final Matcher matcher; - - protected TransformingMatcher(Function transformation, Matcher matcher) { - this.transformation = requireNonNull(transformation); - this.matcher = requireNonNull(matcher); - } - - public static TransformingMatcher hasDerivedValue(Function transformation, Matcher matcher) { - return new TransformingMatcher<>(transformation, matcher); - } - - @Override - protected boolean matchesSafely(E e) { - return matcher.matches(transformation.apply(e)); - } - - @Override - public void describeTo(Description description) { - matcher.describeTo(description); - } - - @Override - protected void describeMismatchSafely(E item, Description mismatchDescription) { - matcher.describeMismatch(transformation.apply(item), mismatchDescription); - } -} diff --git a/components/common/src/test/java/com/hotels/styx/common/MorePreconditionsTest.java b/components/common/src/test/java/com/hotels/styx/common/MorePreconditionsTest.java index fb18a43f5a..b394869c61 100644 --- a/components/common/src/test/java/com/hotels/styx/common/MorePreconditionsTest.java +++ b/components/common/src/test/java/com/hotels/styx/common/MorePreconditionsTest.java @@ -1,5 +1,5 @@ /* - Copyright (C) 2013-2018 Expedia Inc. + Copyright (C) 2013-2019 Expedia Inc. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -18,11 +18,8 @@ import org.testng.annotations.Test; import static com.hotels.styx.common.MorePreconditions.checkArgument; -import static com.hotels.styx.common.MorePreconditions.inRange; import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.is; -import static org.hamcrest.core.StringContains.containsString; import static org.testng.Assert.fail; public class MorePreconditionsTest { @@ -30,35 +27,26 @@ public class MorePreconditionsTest { @Test public void checkArgumentSimpleMessageFailure() { try { - checkArgument(0, greaterThan(0)); + checkArgument(0, false, "Value should be greater than zero"); fail("no exception thrown"); } catch (IllegalArgumentException expected) { - assertThat(expected.getMessage(), containsString("argument\n" + - "Expected: a value greater than <0>\n" + - " but: <0> was equal to <0>")); + assertThat(expected.getMessage(), is("Value should be greater than zero")); } } @Test public void checkArgumentFullMessageFailure() { try { - checkArgument(0, greaterThan(0), "healthy count threshold"); + checkArgument(0, false, "Healthy count threshold"); fail("no exception thrown"); } catch (IllegalArgumentException expected) { - assertThat(expected.getMessage(), containsString("healthy count threshold\n" + - "Expected: a value greater than <0>\n" + - " but: <0> was equal to <0>")); + assertThat(expected.getMessage(), is("Healthy count threshold")); } } @Test - public void checkArgumentInRange() { - assertThat(checkArgument(0, inRange(-3, 3)), is(0)); - } - - @Test(expectedExceptions = IllegalArgumentException.class) - public void checkArgumentInRangeFailure() { - checkArgument(0, inRange(3, 4)); + public void returnsTheValueUnchanged() { + assertThat(checkArgument(0, true, ""), is(0)); } } diff --git a/components/server/src/main/java/com/hotels/styx/server/netty/eventloop/epoll/EpollServerEventLoopGroupFactory.java b/components/server/src/main/java/com/hotels/styx/server/netty/eventloop/epoll/EpollServerEventLoopGroupFactory.java index 014b268940..3e1f62ce6c 100644 --- a/components/server/src/main/java/com/hotels/styx/server/netty/eventloop/epoll/EpollServerEventLoopGroupFactory.java +++ b/components/server/src/main/java/com/hotels/styx/server/netty/eventloop/epoll/EpollServerEventLoopGroupFactory.java @@ -1,5 +1,5 @@ /* - Copyright (C) 2013-2018 Expedia Inc. + Copyright (C) 2013-2019 Expedia Inc. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -22,6 +22,7 @@ import static com.hotels.styx.common.MorePreconditions.checkArgument; import static com.hotels.styx.server.netty.eventloop.epoll.EpollEventLoopGroups.newEventLoopGroup; +import static java.lang.String.format; import static java.util.Objects.requireNonNull; /** @@ -34,8 +35,10 @@ public class EpollServerEventLoopGroupFactory implements ServerEventLoopFactory public EpollServerEventLoopGroupFactory(String name, int bossThreadsCount, int workerThreadsCount) { this.name = requireNonNull(name); - this.bossThreadsCount = checkArgument(bossThreadsCount, bossThreadsCount > -1); - this.workerThreadsCount = checkArgument(workerThreadsCount, workerThreadsCount > -1); + this.bossThreadsCount = checkArgument(bossThreadsCount, bossThreadsCount > -1, + format("Invalid boss thread count (%d). It should be > -1.", bossThreadsCount)); + this.workerThreadsCount = checkArgument(workerThreadsCount, workerThreadsCount > -1, + format("Invalid worker thread count (%d). It should be > -1.", workerThreadsCount)); } @Override From b75612bd1ab238124c0847ab38b0ad17ff4f3519 Mon Sep 17 00:00:00 2001 From: Mikko Karjalainen Date: Fri, 11 Oct 2019 09:15:59 +0100 Subject: [PATCH 2/2] Update jackson/databind module in plugin/examples to 2.9.10. --- plugin-examples/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin-examples/pom.xml b/plugin-examples/pom.xml index cb6883b850..9030e58d8a 100644 --- a/plugin-examples/pom.xml +++ b/plugin-examples/pom.xml @@ -45,7 +45,7 @@ com.fasterxml.jackson.core jackson-databind - 2.9.9.3 + ${jackson.version}