From cd462c71879750fc560a8b80d9e7195e81fef66a Mon Sep 17 00:00:00 2001 From: David Hagar Date: Fri, 21 Jun 2013 13:25:52 -0700 Subject: [PATCH 1/2] Flow configuration loader classes --- .../collector/ConfigLoadingException.java | 9 +++ .../event/collector/FlowConfiguration.java | 25 +++++++ .../collector/FlowConfigurationLoader.java | 9 +++ .../HttpFlowConfigurationLoader.java | 53 ++++++++++++++ .../collector/TestFlowConfiguration.java | 17 +++++ .../TestHttpFlowConfigurationLoader.java | 71 +++++++++++++++++++ 6 files changed, 184 insertions(+) create mode 100644 src/main/java/com/proofpoint/event/collector/ConfigLoadingException.java create mode 100644 src/main/java/com/proofpoint/event/collector/FlowConfiguration.java create mode 100644 src/main/java/com/proofpoint/event/collector/FlowConfigurationLoader.java create mode 100644 src/main/java/com/proofpoint/event/collector/HttpFlowConfigurationLoader.java create mode 100644 src/test/java/com/proofpoint/event/collector/TestFlowConfiguration.java create mode 100644 src/test/java/com/proofpoint/event/collector/TestHttpFlowConfigurationLoader.java diff --git a/src/main/java/com/proofpoint/event/collector/ConfigLoadingException.java b/src/main/java/com/proofpoint/event/collector/ConfigLoadingException.java new file mode 100644 index 0000000..8a27246 --- /dev/null +++ b/src/main/java/com/proofpoint/event/collector/ConfigLoadingException.java @@ -0,0 +1,9 @@ +package com.proofpoint.event.collector; + +public class ConfigLoadingException extends Exception +{ + public ConfigLoadingException(String message, Exception ex) + { + super(message, ex); + } +} diff --git a/src/main/java/com/proofpoint/event/collector/FlowConfiguration.java b/src/main/java/com/proofpoint/event/collector/FlowConfiguration.java new file mode 100644 index 0000000..278bf1a --- /dev/null +++ b/src/main/java/com/proofpoint/event/collector/FlowConfiguration.java @@ -0,0 +1,25 @@ +package com.proofpoint.event.collector; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; + +import javax.validation.constraints.NotNull; +import java.util.Set; + +public class FlowConfiguration +{ + private Set propertiesToSerialize; + + @JsonCreator + public FlowConfiguration(@JsonProperty Set propertiesToSerialize) + { + this.propertiesToSerialize = propertiesToSerialize; + } + + @NotNull(message = "is missing") + @JsonProperty + public Set getPropertiesToSerialize() + { + return propertiesToSerialize; + } +} diff --git a/src/main/java/com/proofpoint/event/collector/FlowConfigurationLoader.java b/src/main/java/com/proofpoint/event/collector/FlowConfigurationLoader.java new file mode 100644 index 0000000..04a84f5 --- /dev/null +++ b/src/main/java/com/proofpoint/event/collector/FlowConfigurationLoader.java @@ -0,0 +1,9 @@ +package com.proofpoint.event.collector; + +import java.net.URI; + +public interface FlowConfigurationLoader +{ + public FlowConfiguration loadConfiguration(URI configurationLocation) + throws ConfigLoadingException; +} diff --git a/src/main/java/com/proofpoint/event/collector/HttpFlowConfigurationLoader.java b/src/main/java/com/proofpoint/event/collector/HttpFlowConfigurationLoader.java new file mode 100644 index 0000000..dcca767 --- /dev/null +++ b/src/main/java/com/proofpoint/event/collector/HttpFlowConfigurationLoader.java @@ -0,0 +1,53 @@ +package com.proofpoint.event.collector; + +import com.google.common.annotations.VisibleForTesting; +import com.proofpoint.http.client.HttpClient; +import com.proofpoint.http.client.Request; +import com.proofpoint.json.JsonCodec; + +import javax.inject.Inject; +import javax.ws.rs.core.HttpHeaders; +import javax.ws.rs.core.MediaType; +import java.net.URI; + +import static com.google.common.base.Preconditions.checkNotNull; +import static com.proofpoint.http.client.FullJsonResponseHandler.createFullJsonResponseHandler; +import static java.lang.String.format; + + +public class HttpFlowConfigurationLoader + implements FlowConfigurationLoader +{ + @VisibleForTesting + static final String LOADING_EXCEPTION_MESSAGE = "Config could not be loaded from location %s"; + private static final JsonCodec flowConfigurationCodec = JsonCodec.jsonCodec(FlowConfiguration.class); + private final HttpClient httpClient; + private final FlowConfiguration defaultConfig; + + @Inject + public HttpFlowConfigurationLoader(HttpClient httpClient, FlowConfiguration defaultConfig) + { + this.httpClient = checkNotNull(httpClient, "httpClient is null"); + this.defaultConfig = defaultConfig; + } + + @Override + public FlowConfiguration loadConfiguration(URI configurationLocation) + throws ConfigLoadingException + { + if (configurationLocation == null) { + return defaultConfig; + } + + try { + Request.Builder requestBuilder = Request.builder().prepareGet() + .setUri(configurationLocation) + .setHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON); + + return httpClient.execute(requestBuilder.build(), createFullJsonResponseHandler(flowConfigurationCodec)).getValue(); + } + catch (Exception ex) { + throw new ConfigLoadingException(format(LOADING_EXCEPTION_MESSAGE, configurationLocation), ex); + } + } +} diff --git a/src/test/java/com/proofpoint/event/collector/TestFlowConfiguration.java b/src/test/java/com/proofpoint/event/collector/TestFlowConfiguration.java new file mode 100644 index 0000000..61af5c3 --- /dev/null +++ b/src/test/java/com/proofpoint/event/collector/TestFlowConfiguration.java @@ -0,0 +1,17 @@ +package com.proofpoint.event.collector; + +import org.testng.annotations.Test; + +import javax.validation.constraints.NotNull; + +import static com.proofpoint.testing.ValidationAssertions.assertFailsValidation; + +public class TestFlowConfiguration +{ + @Test() + public void testNullPropertiesToSerialize() + { + FlowConfiguration flowConfig = new FlowConfiguration(null); + assertFailsValidation(flowConfig, "propertiesToSerialize", "is missing", NotNull.class); + } +} diff --git a/src/test/java/com/proofpoint/event/collector/TestHttpFlowConfigurationLoader.java b/src/test/java/com/proofpoint/event/collector/TestHttpFlowConfigurationLoader.java new file mode 100644 index 0000000..9200ed0 --- /dev/null +++ b/src/test/java/com/proofpoint/event/collector/TestHttpFlowConfigurationLoader.java @@ -0,0 +1,71 @@ +package com.proofpoint.event.collector; + +import com.google.common.collect.ImmutableSet; +import com.proofpoint.http.client.FullJsonResponseHandler.JsonResponse; +import com.proofpoint.http.client.HttpClient; +import com.proofpoint.http.client.Request; +import com.proofpoint.http.client.ResponseHandler; +import org.testng.annotations.Test; + +import java.net.URI; + +import static java.lang.String.format; +import static org.mockito.Matchers.anyObject; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.testng.Assert.assertEquals; + +public class TestHttpFlowConfigurationLoader +{ + FlowConfiguration defaultConfig = new FlowConfiguration(ImmutableSet.of("default")); + FlowConfiguration remoteConfig = new FlowConfiguration(ImmutableSet.of("")); + + @Test + public void testBasicRemoteConfigurationLoad() + throws Exception + { + HttpClient client = mock(HttpClient.class); + JsonResponse jsonResponse = mock(JsonResponse.class); + when(client.execute((Request) anyObject(), (ResponseHandler) anyObject())).thenReturn(jsonResponse); + when(jsonResponse.getValue()).thenReturn(remoteConfig); + + FlowConfigurationLoader loader = new HttpFlowConfigurationLoader(client, defaultConfig); + assertEquals(loader.loadConfiguration(URI.create("http://www.remoteconfiglocation.com/")), remoteConfig); + } + + @Test + public void testNoRemoteConfigurationLocationLoad() + throws Exception + { + FlowConfigurationLoader loader = new HttpFlowConfigurationLoader(mock(HttpClient.class), defaultConfig); + assertEquals(loader.loadConfiguration(null), defaultConfig); + } + + @Test(expectedExceptions = ConfigLoadingException.class) + public void testClientExceptionThrowsConfigLoadingException() + throws Exception + { + HttpClient client = mock(HttpClient.class); + when(client.execute((Request) anyObject(), (ResponseHandler) anyObject())).thenThrow(new Exception("Bad!")); + + FlowConfigurationLoader loader = new HttpFlowConfigurationLoader(client, defaultConfig); + loader.loadConfiguration(URI.create("http://www.remoteconfiglocation.com/")); + } + + @Test() + public void testConfigLoadingExceptionMessage() + throws Exception + { + String configLocation = "http://www.remoteconfiglocation.com/foo.json"; + HttpClient client = mock(HttpClient.class); + when(client.execute((Request) anyObject(), (ResponseHandler) anyObject())).thenThrow(new Exception("Bad!")); + + FlowConfigurationLoader loader = new HttpFlowConfigurationLoader(client, defaultConfig); + try { + loader.loadConfiguration(URI.create(configLocation)); + } + catch (ConfigLoadingException ex) { + assertEquals(ex.getMessage(), format(HttpFlowConfigurationLoader.LOADING_EXCEPTION_MESSAGE, configLocation)); + } + } +} From 6870e188f256631481b3dff01d55be49987a4ef0 Mon Sep 17 00:00:00 2001 From: David Hagar Date: Mon, 24 Jun 2013 14:26:11 -0700 Subject: [PATCH 2/2] Code review feedback --- .../event/collector/FlowConfiguration.java | 2 +- .../collector/ForFlowConfigurationLoader.java | 5 ++ .../HttpFlowConfigurationLoader.java | 24 +++------ .../collector/TestFlowConfiguration.java | 49 ++++++++++++++++++- .../TestHttpFlowConfigurationLoader.java | 44 ++++++++--------- 5 files changed, 81 insertions(+), 43 deletions(-) create mode 100644 src/main/java/com/proofpoint/event/collector/ForFlowConfigurationLoader.java diff --git a/src/main/java/com/proofpoint/event/collector/FlowConfiguration.java b/src/main/java/com/proofpoint/event/collector/FlowConfiguration.java index 278bf1a..3723386 100644 --- a/src/main/java/com/proofpoint/event/collector/FlowConfiguration.java +++ b/src/main/java/com/proofpoint/event/collector/FlowConfiguration.java @@ -11,7 +11,7 @@ public class FlowConfiguration private Set propertiesToSerialize; @JsonCreator - public FlowConfiguration(@JsonProperty Set propertiesToSerialize) + public FlowConfiguration(@JsonProperty("propertiesToSerialize") Set propertiesToSerialize) { this.propertiesToSerialize = propertiesToSerialize; } diff --git a/src/main/java/com/proofpoint/event/collector/ForFlowConfigurationLoader.java b/src/main/java/com/proofpoint/event/collector/ForFlowConfigurationLoader.java new file mode 100644 index 0000000..f82299a --- /dev/null +++ b/src/main/java/com/proofpoint/event/collector/ForFlowConfigurationLoader.java @@ -0,0 +1,5 @@ +package com.proofpoint.event.collector; + +public @interface ForFlowConfigurationLoader +{ +} diff --git a/src/main/java/com/proofpoint/event/collector/HttpFlowConfigurationLoader.java b/src/main/java/com/proofpoint/event/collector/HttpFlowConfigurationLoader.java index dcca767..59e2799 100644 --- a/src/main/java/com/proofpoint/event/collector/HttpFlowConfigurationLoader.java +++ b/src/main/java/com/proofpoint/event/collector/HttpFlowConfigurationLoader.java @@ -1,6 +1,5 @@ package com.proofpoint.event.collector; -import com.google.common.annotations.VisibleForTesting; import com.proofpoint.http.client.HttpClient; import com.proofpoint.http.client.Request; import com.proofpoint.json.JsonCodec; @@ -11,40 +10,31 @@ import java.net.URI; import static com.google.common.base.Preconditions.checkNotNull; -import static com.proofpoint.http.client.FullJsonResponseHandler.createFullJsonResponseHandler; +import static com.proofpoint.http.client.JsonResponseHandler.createJsonResponseHandler; import static java.lang.String.format; public class HttpFlowConfigurationLoader implements FlowConfigurationLoader { - @VisibleForTesting - static final String LOADING_EXCEPTION_MESSAGE = "Config could not be loaded from location %s"; - private static final JsonCodec flowConfigurationCodec = JsonCodec.jsonCodec(FlowConfiguration.class); + private static final String LOADING_EXCEPTION_MESSAGE = "Config could not be loaded from location %s"; + private static final JsonCodec FLOW_CONFIGURATION_CODEC = JsonCodec.jsonCodec(FlowConfiguration.class); private final HttpClient httpClient; - private final FlowConfiguration defaultConfig; @Inject - public HttpFlowConfigurationLoader(HttpClient httpClient, FlowConfiguration defaultConfig) + public HttpFlowConfigurationLoader(@ForFlowConfigurationLoader HttpClient httpClient) { this.httpClient = checkNotNull(httpClient, "httpClient is null"); - this.defaultConfig = defaultConfig; } @Override public FlowConfiguration loadConfiguration(URI configurationLocation) throws ConfigLoadingException { - if (configurationLocation == null) { - return defaultConfig; - } - + checkNotNull(configurationLocation, "configurationLocation is null"); try { - Request.Builder requestBuilder = Request.builder().prepareGet() - .setUri(configurationLocation) - .setHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON); - - return httpClient.execute(requestBuilder.build(), createFullJsonResponseHandler(flowConfigurationCodec)).getValue(); + Request.Builder requestBuilder = Request.builder().prepareGet().setUri(configurationLocation).setHeader(HttpHeaders.ACCEPT, MediaType.APPLICATION_JSON); + return httpClient.execute(requestBuilder.build(), createJsonResponseHandler(FLOW_CONFIGURATION_CODEC)); } catch (Exception ex) { throw new ConfigLoadingException(format(LOADING_EXCEPTION_MESSAGE, configurationLocation), ex); diff --git a/src/test/java/com/proofpoint/event/collector/TestFlowConfiguration.java b/src/test/java/com/proofpoint/event/collector/TestFlowConfiguration.java index 61af5c3..fdcb7c0 100644 --- a/src/test/java/com/proofpoint/event/collector/TestFlowConfiguration.java +++ b/src/test/java/com/proofpoint/event/collector/TestFlowConfiguration.java @@ -1,17 +1,64 @@ package com.proofpoint.event.collector; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Maps; +import com.proofpoint.json.JsonCodec; +import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; import javax.validation.constraints.NotNull; +import java.util.Map; +import static com.proofpoint.json.JsonCodec.jsonCodec; +import static com.proofpoint.json.testing.JsonTester.assertJsonEncode; +import static com.proofpoint.json.testing.JsonTester.decodeJson; import static com.proofpoint.testing.ValidationAssertions.assertFailsValidation; +import static com.proofpoint.testing.ValidationAssertions.assertValidates; + public class TestFlowConfiguration { - @Test() + private final JsonCodec codec = jsonCodec(FlowConfiguration.class); + private Map map; + + @BeforeMethod + public void setup() { + map = Maps.newHashMap(ImmutableMap.of("propertiesToSerialize", ImmutableList.of("property1"))); + } + + @Test + public void testNoPropertiesToSerializeInJsonDecode() + { + map.remove("propertiesToSerialize"); + assertFailsValidation(decodeJson(codec, map), "propertiesToSerialize", "is missing", NotNull.class); + } + + @Test + public void testJsonEncode() + { + assertJsonEncode(assertValidates(new FlowConfiguration(ImmutableSet.of("property1"))), map); + } + + @Test public void testNullPropertiesToSerialize() { FlowConfiguration flowConfig = new FlowConfiguration(null); assertFailsValidation(flowConfig, "propertiesToSerialize", "is missing", NotNull.class); } + + @Test + public void testPassesValidation() + { + FlowConfiguration flowConfig = new FlowConfiguration(ImmutableSet.of("foo")); + assertValidates(flowConfig); + } + + @Test + public void testEmptyPropertiesPassesValidation() + { + FlowConfiguration flowConfig = new FlowConfiguration(ImmutableSet.of()); + assertValidates(flowConfig); + } } diff --git a/src/test/java/com/proofpoint/event/collector/TestHttpFlowConfigurationLoader.java b/src/test/java/com/proofpoint/event/collector/TestHttpFlowConfigurationLoader.java index 9200ed0..4889682 100644 --- a/src/test/java/com/proofpoint/event/collector/TestHttpFlowConfigurationLoader.java +++ b/src/test/java/com/proofpoint/event/collector/TestHttpFlowConfigurationLoader.java @@ -1,23 +1,26 @@ package com.proofpoint.event.collector; import com.google.common.collect.ImmutableSet; -import com.proofpoint.http.client.FullJsonResponseHandler.JsonResponse; import com.proofpoint.http.client.HttpClient; import com.proofpoint.http.client.Request; import com.proofpoint.http.client.ResponseHandler; import org.testng.annotations.Test; +import javax.ws.rs.core.HttpHeaders; +import javax.ws.rs.core.MediaType; import java.net.URI; import static java.lang.String.format; import static org.mockito.Matchers.anyObject; +import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertSame; +import static org.testng.Assert.fail; public class TestHttpFlowConfigurationLoader { - FlowConfiguration defaultConfig = new FlowConfiguration(ImmutableSet.of("default")); FlowConfiguration remoteConfig = new FlowConfiguration(ImmutableSet.of("")); @Test @@ -25,31 +28,21 @@ public void testBasicRemoteConfigurationLoad() throws Exception { HttpClient client = mock(HttpClient.class); - JsonResponse jsonResponse = mock(JsonResponse.class); - when(client.execute((Request) anyObject(), (ResponseHandler) anyObject())).thenReturn(jsonResponse); - when(jsonResponse.getValue()).thenReturn(remoteConfig); + URI configLocation = URI.create("http://www.remoteconfiglocation.com/"); + Request.Builder requestBuilder = Request.builder().prepareGet().setUri(configLocation).setHeader(HttpHeaders.ACCEPT, MediaType.APPLICATION_JSON); + when(client.execute(eq(requestBuilder.build()), (ResponseHandler) anyObject())).thenReturn(remoteConfig); - FlowConfigurationLoader loader = new HttpFlowConfigurationLoader(client, defaultConfig); - assertEquals(loader.loadConfiguration(URI.create("http://www.remoteconfiglocation.com/")), remoteConfig); + FlowConfigurationLoader loader = new HttpFlowConfigurationLoader(client); + assertEquals(loader.loadConfiguration(configLocation), remoteConfig); } - @Test + @Test(expectedExceptions = NullPointerException.class) public void testNoRemoteConfigurationLocationLoad() throws Exception { - FlowConfigurationLoader loader = new HttpFlowConfigurationLoader(mock(HttpClient.class), defaultConfig); - assertEquals(loader.loadConfiguration(null), defaultConfig); - } - - @Test(expectedExceptions = ConfigLoadingException.class) - public void testClientExceptionThrowsConfigLoadingException() - throws Exception - { - HttpClient client = mock(HttpClient.class); - when(client.execute((Request) anyObject(), (ResponseHandler) anyObject())).thenThrow(new Exception("Bad!")); - - FlowConfigurationLoader loader = new HttpFlowConfigurationLoader(client, defaultConfig); - loader.loadConfiguration(URI.create("http://www.remoteconfiglocation.com/")); + FlowConfigurationLoader loader = new HttpFlowConfigurationLoader(mock(HttpClient.class)); + loader.loadConfiguration(null); + fail("Should have thrown", new NullPointerException("Config location was null")); } @Test() @@ -58,14 +51,17 @@ public void testConfigLoadingExceptionMessage() { String configLocation = "http://www.remoteconfiglocation.com/foo.json"; HttpClient client = mock(HttpClient.class); - when(client.execute((Request) anyObject(), (ResponseHandler) anyObject())).thenThrow(new Exception("Bad!")); + Exception thrownException = new Exception("Bad!"); + when(client.execute((Request) anyObject(), (ResponseHandler) anyObject())).thenThrow(thrownException); - FlowConfigurationLoader loader = new HttpFlowConfigurationLoader(client, defaultConfig); + FlowConfigurationLoader loader = new HttpFlowConfigurationLoader(client); try { loader.loadConfiguration(URI.create(configLocation)); + fail("expected ConfigLoadingException"); } catch (ConfigLoadingException ex) { - assertEquals(ex.getMessage(), format(HttpFlowConfigurationLoader.LOADING_EXCEPTION_MESSAGE, configLocation)); + assertEquals(ex.getMessage(), format("Config could not be loaded from location http://www.remoteconfiglocation.com/foo.json", configLocation)); + assertSame(ex.getCause(), thrownException); } } }