From 86a663144f7aa32a62d2b7aefc62e1ad3119e129 Mon Sep 17 00:00:00 2001 From: Mikko Karjalainen Date: Wed, 3 Oct 2018 09:05:39 +0100 Subject: [PATCH] Lazily create expensive Optional default values. --- .../OriginRestrictionLoadBalancingStrategy.java | 2 +- .../styx/client/StyxBackendServiceClient.java | 2 +- .../StickySessionLoadBalancingStrategy.java | 2 +- ...ginRestrictionLoadBalancingStrategyTest.java | 17 +++++++++++++++++ .../StickySessionLoadBalancingStrategyTest.java | 15 +++++++++++++++ .../LoadBalancingStrategyFactoryProvider.java | 2 +- .../file/FileBackedBackendServicesRegistry.java | 2 +- .../servers/WiremockStyxRequestAdapter.java | 4 ++-- 8 files changed, 39 insertions(+), 7 deletions(-) diff --git a/components/client/src/main/java/com/hotels/styx/client/OriginRestrictionLoadBalancingStrategy.java b/components/client/src/main/java/com/hotels/styx/client/OriginRestrictionLoadBalancingStrategy.java index 095d543a29..c384c7c3da 100644 --- a/components/client/src/main/java/com/hotels/styx/client/OriginRestrictionLoadBalancingStrategy.java +++ b/components/client/src/main/java/com/hotels/styx/client/OriginRestrictionLoadBalancingStrategy.java @@ -72,7 +72,7 @@ public Optional choose(LoadBalancer.Preferences context) { } } ) - .orElse(delegate.choose(context)); + .orElseGet(() -> delegate.choose(context)); } private Predicate originIsAllowed(String cookieValue) { diff --git a/components/client/src/main/java/com/hotels/styx/client/StyxBackendServiceClient.java b/components/client/src/main/java/com/hotels/styx/client/StyxBackendServiceClient.java index 0c9674bbe2..0a11e4c1a9 100644 --- a/components/client/src/main/java/com/hotels/styx/client/StyxBackendServiceClient.java +++ b/components/client/src/main/java/com/hotels/styx/client/StyxBackendServiceClient.java @@ -287,7 +287,7 @@ public Optional preferredOrigins() { return rewrittenRequest.cookie(originsRestrictionCookieName) .map(RequestCookie::value) .map(Optional::of) - .orElse(rewrittenRequest.cookie("styx_origin_" + id).map(RequestCookie::value)); + .orElseGet(() -> rewrittenRequest.cookie("styx_origin_" + id).map(RequestCookie::value)); } else { return rewrittenRequest.cookie("styx_origin_" + id).map(RequestCookie::value); } diff --git a/components/client/src/main/java/com/hotels/styx/client/stickysession/StickySessionLoadBalancingStrategy.java b/components/client/src/main/java/com/hotels/styx/client/stickysession/StickySessionLoadBalancingStrategy.java index 7c7ab7b491..5199eabbd5 100644 --- a/components/client/src/main/java/com/hotels/styx/client/stickysession/StickySessionLoadBalancingStrategy.java +++ b/components/client/src/main/java/com/hotels/styx/client/stickysession/StickySessionLoadBalancingStrategy.java @@ -40,7 +40,7 @@ public Optional choose(LoadBalancer.Preferences context) { return context.preferredOrigins() .flatMap(preferredHost -> originById(activeOrigins.snapshot(), preferredHost)) .map(Optional::of) - .orElse(delegate.choose(context)); + .orElseGet(() -> delegate.choose(context)); } private Optional originById(Iterable origins, String id) { diff --git a/components/client/src/test/unit/java/com/hotels/styx/client/OriginRestrictionLoadBalancingStrategyTest.java b/components/client/src/test/unit/java/com/hotels/styx/client/OriginRestrictionLoadBalancingStrategyTest.java index c58013b495..dac3a6d0c1 100644 --- a/components/client/src/test/unit/java/com/hotels/styx/client/OriginRestrictionLoadBalancingStrategyTest.java +++ b/components/client/src/test/unit/java/com/hotels/styx/client/OriginRestrictionLoadBalancingStrategyTest.java @@ -113,6 +113,23 @@ public void usesSingleOriginMatchingRegularExpression() { assertThat(chosenOne.get(), is(origins.get(1))); } + @Test + public void dontDelegateWhenOriginRestrictionCookieMatches() { + Random mockRandom = mock(Random.class); + when(mockRandom.nextInt(eq(1))).thenReturn(0); + + strategy = new OriginRestrictionLoadBalancingStrategy(() -> origins, delegate, mockRandom); + + strategy.choose(lbPreference(Optional.of("origin-1"))); + strategy.choose(lbPreference(Optional.of("origin-1"))); + strategy.choose(lbPreference(Optional.of("origin-1"))); + strategy.choose(lbPreference(Optional.of("origin-1"))); + strategy.choose(lbPreference(Optional.of("origin-1"))); + strategy.choose(lbPreference(Optional.of("origin-1"))); + + verify(delegate, never()).choose(any(LoadBalancer.Preferences.class)); + } + @Test public void usesMultipleOriginsMatchingRegularExpression() { Random mockRandom = mock(Random.class); diff --git a/components/client/src/test/unit/java/com/hotels/styx/client/stickysession/StickySessionLoadBalancingStrategyTest.java b/components/client/src/test/unit/java/com/hotels/styx/client/stickysession/StickySessionLoadBalancingStrategyTest.java index 4a9aea342a..aaab0c3777 100644 --- a/components/client/src/test/unit/java/com/hotels/styx/client/stickysession/StickySessionLoadBalancingStrategyTest.java +++ b/components/client/src/test/unit/java/com/hotels/styx/client/stickysession/StickySessionLoadBalancingStrategyTest.java @@ -34,7 +34,10 @@ import static java.util.Collections.EMPTY_LIST; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.core.Is.is; +import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -63,6 +66,18 @@ public void selectsThePreferredOrigin() { assertThat(chosenOne, is(Optional.of(ORIGIN_1))); } + @Test + public void dontDelegateWhenPreferredOriginIsFound() { + LoadBalancer delegate = mock(LoadBalancer.class); + StickySessionLoadBalancingStrategy aStrategy = new StickySessionLoadBalancingStrategy(activeOrigins, delegate); + + when(activeOrigins.snapshot()).thenReturn(asList(ORIGIN_0, ORIGIN_1, ORIGIN_2)); + + aStrategy.choose(lbPreferences(Optional.of(ORIGIN_1.id()))); + + verify(delegate, never()).choose(any(LoadBalancer.Preferences.class)); + } + @Test public void failsOverToNextAvailableOriginSelectedByTheLoadBalancingStrategyIfRequestedOriginIsNotRegistered() { when(activeOrigins.snapshot()).thenReturn(asList(ORIGIN_0, ORIGIN_2)); diff --git a/components/proxy/src/main/java/com/hotels/styx/proxy/LoadBalancingStrategyFactoryProvider.java b/components/proxy/src/main/java/com/hotels/styx/proxy/LoadBalancingStrategyFactoryProvider.java index 3a58331293..a9d0fce2b8 100644 --- a/components/proxy/src/main/java/com/hotels/styx/proxy/LoadBalancingStrategyFactoryProvider.java +++ b/components/proxy/src/main/java/com/hotels/styx/proxy/LoadBalancingStrategyFactoryProvider.java @@ -50,7 +50,7 @@ public static LoadBalancingStrategyFactoryProvider newProvider(Configuration con public LoadBalancerFactory get() { return configurations.get(LOAD_BALANCING_STRATEGY_KEY) .map(this::newFactoryInstance) - .orElse(busyConnectionBalancer()); + .orElseGet(this::busyConnectionBalancer); } private LoadBalancerFactory busyConnectionBalancer() { diff --git a/components/proxy/src/main/java/com/hotels/styx/proxy/backends/file/FileBackedBackendServicesRegistry.java b/components/proxy/src/main/java/com/hotels/styx/proxy/backends/file/FileBackedBackendServicesRegistry.java index 99229d69e3..46b3cb6657 100644 --- a/components/proxy/src/main/java/com/hotels/styx/proxy/backends/file/FileBackedBackendServicesRegistry.java +++ b/components/proxy/src/main/java/com/hotels/styx/proxy/backends/file/FileBackedBackendServicesRegistry.java @@ -160,7 +160,7 @@ public Registry create(Environment environment, Configuration re "missing [services.registry.factory.config.originsFile] config value for factory class FileBackedBackendServicesRegistry.Factory")); FileMonitorSettings monitorSettings = registryConfiguration.get("monitor", FileMonitorSettings.class) - .orElse(new FileMonitorSettings()); + .orElseGet(FileMonitorSettings::new); return registry(originsFile, monitorSettings); } diff --git a/system-tests/e2e-testsupport/src/main/java/com/hotels/styx/servers/WiremockStyxRequestAdapter.java b/system-tests/e2e-testsupport/src/main/java/com/hotels/styx/servers/WiremockStyxRequestAdapter.java index 2e25467521..c09ce2f671 100644 --- a/system-tests/e2e-testsupport/src/main/java/com/hotels/styx/servers/WiremockStyxRequestAdapter.java +++ b/system-tests/e2e-testsupport/src/main/java/com/hotels/styx/servers/WiremockStyxRequestAdapter.java @@ -76,7 +76,7 @@ public HttpHeader header(String key) { public ContentTypeHeader contentTypeHeader() { return styxRequest.header(CONTENT_TYPE) .map(ContentTypeHeader::new) - .orElse(ContentTypeHeader.absent()); + .orElseGet(ContentTypeHeader::absent); } @Override @@ -102,7 +102,7 @@ public Set getAllHeaderKeys() { public QueryParameter queryParameter(String key) { return styxRequest.queryParam(key) .map(value -> new QueryParameter(key, ImmutableList.of(value))) - .orElse(QueryParameter.absent(key)); + .orElseGet(() -> QueryParameter.absent(key)); } @Override