From ae65212f1915bef951b0b7c22c2e678e46d70d6e Mon Sep 17 00:00:00 2001 From: Winston Li Date: Sun, 6 Aug 2017 23:15:55 +0100 Subject: [PATCH 1/5] Create http client facade --- .../io/http/ning/NingHttpClient.java | 84 +++++++++++++++++++ .../io/http/ning/NingHttpClientFacade.java | 23 +++++ 2 files changed, 107 insertions(+) create mode 100644 services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/io/http/ning/NingHttpClient.java create mode 100644 services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/io/http/ning/NingHttpClientFacade.java diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/io/http/ning/NingHttpClient.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/io/http/ning/NingHttpClient.java new file mode 100644 index 0000000000..2f5c579da8 --- /dev/null +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/io/http/ning/NingHttpClient.java @@ -0,0 +1,84 @@ +package uk.ac.ic.wlgitbridge.io.http.ning; + +import com.ning.http.client.*; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import uk.ac.ic.wlgitbridge.util.FunctionT; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.util.concurrent.ExecutionException; +import java.util.function.Function; + +public class NingHttpClient implements NingHttpClientFacade { + + private static final Logger log + = LoggerFactory.getLogger(NingHttpClient.class); + + private final AsyncHttpClient http; + + public NingHttpClient(AsyncHttpClient http) { + this.http = http; + } + + @Override + public byte[] get( + String url, + FunctionT handler + ) throws E { + try { + return http + .prepareGet(url) + .execute(new AsyncCompletionHandler() { + + ByteArrayOutputStream bytes = new ByteArrayOutputStream(); + + @Override + public STATE onHeadersReceived( + HttpResponseHeaders headers + ) throws E { + return handler.apply(headers) + ? STATE.CONTINUE : STATE.ABORT; + } + + @Override + public STATE onBodyPartReceived( + HttpResponseBodyPart content + ) throws IOException { + bytes.write(content.getBodyPartBytes()); + return STATE.CONTINUE; + } + + @Override + public byte[] onCompleted( + Response response + ) throws IOException { + byte[] ret = bytes.toByteArray(); + bytes.close(); + log.info( + response.getStatusCode() + + " " + + response.getStatusText() + + " (" + + ret.length + + "B) -> " + + url + ); + return ret; + } + + }).get(); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } catch (ExecutionException e) { + try { + /* No clean way to do this */ + //noinspection unchecked + throw (E) e.getCause(); + } catch (ClassCastException cce) { + throw new RuntimeException(cce); + } + } + } + +} diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/io/http/ning/NingHttpClientFacade.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/io/http/ning/NingHttpClientFacade.java new file mode 100644 index 0000000000..2d37bff367 --- /dev/null +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/io/http/ning/NingHttpClientFacade.java @@ -0,0 +1,23 @@ +package uk.ac.ic.wlgitbridge.io.http.ning; + +import com.ning.http.client.HttpResponseHeaders; +import uk.ac.ic.wlgitbridge.util.FunctionT; + +import java.util.concurrent.ExecutionException; +import java.util.function.Function; + +public interface NingHttpClientFacade { + + /** + * Performs a GET request + * @param url the target URL + * @param handler handler for the response headers. Returning false + * aborts the request. + * @return + */ + byte[] get( + String url, + FunctionT handler + ) throws E; + +} From 4e7636c94ad7ebc4a71cecbdd1ff71c1a735209b Mon Sep 17 00:00:00 2001 From: Winston Li Date: Mon, 7 Aug 2017 23:42:31 +0100 Subject: [PATCH 2/5] Decouple http client from UrlResourceCache --- .../bridge/resource/UrlResourceCache.java | 92 ++++++------------- .../io/http/ning/NingHttpClient.java | 11 +-- .../io/http/ning/NingHttpClientFacade.java | 3 +- 3 files changed, 28 insertions(+), 78 deletions(-) diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/resource/UrlResourceCache.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/resource/UrlResourceCache.java index bde1d1f4b9..9a2c72a548 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/resource/UrlResourceCache.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/resource/UrlResourceCache.java @@ -1,15 +1,15 @@ package uk.ac.ic.wlgitbridge.bridge.resource; -import com.ning.http.client.*; +import com.ning.http.client.AsyncHttpClient; import uk.ac.ic.wlgitbridge.bridge.db.DBStore; import uk.ac.ic.wlgitbridge.data.filestore.RawFile; import uk.ac.ic.wlgitbridge.data.filestore.RepositoryFile; import uk.ac.ic.wlgitbridge.git.exception.SizeLimitExceededException; -import uk.ac.ic.wlgitbridge.snapshot.base.Request; +import uk.ac.ic.wlgitbridge.io.http.ning.NingHttpClient; +import uk.ac.ic.wlgitbridge.io.http.ning.NingHttpClientFacade; import uk.ac.ic.wlgitbridge.snapshot.exception.FailedConnectionException; import uk.ac.ic.wlgitbridge.util.Log; -import java.io.ByteArrayOutputStream; import java.io.IOException; import java.util.List; import java.util.Map; @@ -23,8 +23,15 @@ public class UrlResourceCache implements ResourceCache { private final DBStore dbStore; - public UrlResourceCache(DBStore dbStore) { + private final NingHttpClientFacade http; + + UrlResourceCache(DBStore dbStore, NingHttpClientFacade http) { this.dbStore = dbStore; + this.http = http; + } + + public UrlResourceCache(DBStore dbStore) { + this(dbStore, new NingHttpClient(new AsyncHttpClient())); } @Override @@ -74,71 +81,24 @@ public class UrlResourceCache implements ResourceCache { byte[] contents; Log.info("GET -> " + url); try { - contents = Request.httpClient.prepareGet(url).execute( - new AsyncCompletionHandler() { - - ByteArrayOutputStream bytes = new ByteArrayOutputStream(); - - @Override - public STATE onHeadersReceived( - HttpResponseHeaders headers - ) throws SizeLimitExceededException { - List contentLengths - = headers.getHeaders().get("Content-Length"); - if (!maxFileSize.isPresent()) { - return STATE.CONTINUE; - } - if (contentLengths.isEmpty()) { - return STATE.CONTINUE; - } - long contentLength = Long.parseLong(contentLengths.get(0)); - long maxFileSize_ = maxFileSize.get(); - if (contentLength <= maxFileSize_) { - return STATE.CONTINUE; - } - throw new SizeLimitExceededException( - Optional.of(path), contentLength, maxFileSize_ - ); + contents = http.get(url, hs -> { + List contentLengths + = hs.getHeaders().get("Content-Length"); + if (!maxFileSize.isPresent()) { + return true; } - - @Override - public STATE onBodyPartReceived( - HttpResponseBodyPart bodyPart - ) throws Exception { - bytes.write(bodyPart.getBodyPartBytes()); - return STATE.CONTINUE; + if (contentLengths.isEmpty()) { + return true; } - - @Override - public byte[] onCompleted( - Response response - ) throws Exception { - byte[] data = bytes.toByteArray(); - bytes.close(); - Log.info( - response.getStatusCode() - + " " - + response.getStatusText() - + " (" - + data.length - + "B) -> " - + url - ); - return data; + long contentLength = Long.parseLong(contentLengths.get(0)); + long maxFileSize_ = maxFileSize.get(); + if (contentLength <= maxFileSize_) { + return true; } - - }).get(); - } catch (InterruptedException e) { - Log.warn( - "Interrupted when fetching project: " + - projectName + - ", url: " + - url + - ", path: " + - path, - e - ); - throw new FailedConnectionException(); + throw new SizeLimitExceededException( + Optional.of(path), contentLength, maxFileSize_ + ); + }); } catch (ExecutionException e) { Throwable cause = e.getCause(); if (cause instanceof SizeLimitExceededException) { diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/io/http/ning/NingHttpClient.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/io/http/ning/NingHttpClient.java index 2f5c579da8..b9f3d215ff 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/io/http/ning/NingHttpClient.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/io/http/ning/NingHttpClient.java @@ -8,7 +8,6 @@ import uk.ac.ic.wlgitbridge.util.FunctionT; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.util.concurrent.ExecutionException; -import java.util.function.Function; public class NingHttpClient implements NingHttpClientFacade { @@ -25,7 +24,7 @@ public class NingHttpClient implements NingHttpClientFacade { public byte[] get( String url, FunctionT handler - ) throws E { + ) throws ExecutionException { try { return http .prepareGet(url) @@ -70,14 +69,6 @@ public class NingHttpClient implements NingHttpClientFacade { }).get(); } catch (InterruptedException e) { throw new RuntimeException(e); - } catch (ExecutionException e) { - try { - /* No clean way to do this */ - //noinspection unchecked - throw (E) e.getCause(); - } catch (ClassCastException cce) { - throw new RuntimeException(cce); - } } } diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/io/http/ning/NingHttpClientFacade.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/io/http/ning/NingHttpClientFacade.java index 2d37bff367..f0bf4c3770 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/io/http/ning/NingHttpClientFacade.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/io/http/ning/NingHttpClientFacade.java @@ -4,7 +4,6 @@ import com.ning.http.client.HttpResponseHeaders; import uk.ac.ic.wlgitbridge.util.FunctionT; import java.util.concurrent.ExecutionException; -import java.util.function.Function; public interface NingHttpClientFacade { @@ -18,6 +17,6 @@ public interface NingHttpClientFacade { byte[] get( String url, FunctionT handler - ) throws E; + ) throws ExecutionException; } From 34e558ab63941b11bb67a44fe2724f435306b67f Mon Sep 17 00:00:00 2001 From: Winston Li Date: Tue, 8 Aug 2017 23:55:03 +0100 Subject: [PATCH 3/5] Create noop db store and test file --- .../bridge/db/noop/NoopDbStore.java | 66 +++++++++++++++++++ .../bridge/resource/UrlResourceCacheTest.java | 42 ++++++++++++ 2 files changed, 108 insertions(+) create mode 100644 services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/db/noop/NoopDbStore.java create mode 100644 services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/bridge/resource/UrlResourceCacheTest.java diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/db/noop/NoopDbStore.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/db/noop/NoopDbStore.java new file mode 100644 index 0000000000..9519bf0fe4 --- /dev/null +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/db/noop/NoopDbStore.java @@ -0,0 +1,66 @@ +package uk.ac.ic.wlgitbridge.bridge.db.noop; + +import uk.ac.ic.wlgitbridge.bridge.db.DBStore; +import uk.ac.ic.wlgitbridge.bridge.db.ProjectState; + +import java.sql.Timestamp; +import java.util.List; + +public class NoopDbStore implements DBStore { + + @Override + public int getNumProjects() { + return 0; + } + + @Override + public List getProjectNames() { + return null; + } + + @Override + public void setLatestVersionForProject(String project, int versionID) { + + } + + @Override + public int getLatestVersionForProject(String project) { + return 0; + } + + @Override + public void addURLIndexForProject(String projectName, String url, String path) { + + } + + @Override + public void deleteFilesForProject(String project, String... files) { + + } + + @Override + public String getPathForURLInProject(String projectName, String url) { + return null; + } + + @Override + public String getOldestUnswappedProject() { + return null; + } + + @Override + public int getNumUnswappedProjects() { + return 0; + } + + @Override + public ProjectState getProjectState(String projectName) { + return null; + } + + @Override + public void setLastAccessedTime(String projectName, Timestamp time) { + + } + +} diff --git a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/bridge/resource/UrlResourceCacheTest.java b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/bridge/resource/UrlResourceCacheTest.java new file mode 100644 index 0000000000..e180a07731 --- /dev/null +++ b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/bridge/resource/UrlResourceCacheTest.java @@ -0,0 +1,42 @@ +package uk.ac.ic.wlgitbridge.bridge.resource; + +import org.junit.Test; +import uk.ac.ic.wlgitbridge.bridge.db.noop.NoopDbStore; +import uk.ac.ic.wlgitbridge.io.http.ning.NingHttpClientFacade; + +import java.util.Collections; +import java.util.Optional; + +import static org.mockito.Mockito.mock; + +public class UrlResourceCacheTest { + + private static String PROJ = "proj"; + + private static String URL = "http://localhost/file.jpg"; + + private static String NEW_PATH = "file1.jpg"; + + private final NingHttpClientFacade http = mock(NingHttpClientFacade.class); + + private final UrlResourceCache cache + = new UrlResourceCache(new NoopDbStore(), http); + + @Test + public void getThrowsSizeLimitWhenContentLengthTooBig() throws Exception { + when(http.get(any(), any())).thenAnswer(invoc -> { + Object[] args = invoc.getArguments(); + //noinspection unchecked + ((FunctionT< + HttpResponseHeaders, Boolean, SizeLimitExceededException + >) args[1]).apply(withContentLength(2)); + return new byte[0]; + }); + + cache.get( + PROJ, URL, NEW_PATH, + Collections.emptyMap(), Collections.emptyMap(), + Optional.of(2L) + ); + } +} From 6c71c2cb9716b29d5feeb5c77bcf9082087c659d Mon Sep 17 00:00:00 2001 From: Winston Li Date: Sun, 13 Aug 2017 17:09:15 +0100 Subject: [PATCH 4/5] Add ning http headers class --- .../io/http/ning/NingHttpHeaders.java | 46 +++++++++++++++++++ .../bridge/resource/UrlResourceCacheTest.java | 17 ++++++- 2 files changed, 61 insertions(+), 2 deletions(-) create mode 100644 services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/io/http/ning/NingHttpHeaders.java diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/io/http/ning/NingHttpHeaders.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/io/http/ning/NingHttpHeaders.java new file mode 100644 index 0000000000..69093f37c4 --- /dev/null +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/io/http/ning/NingHttpHeaders.java @@ -0,0 +1,46 @@ +package uk.ac.ic.wlgitbridge.io.http.ning; + +import com.ning.http.client.FluentCaseInsensitiveStringsMap; +import com.ning.http.client.HttpResponseHeaders; + +import java.util.*; + +public class NingHttpHeaders extends HttpResponseHeaders { + + private final FluentCaseInsensitiveStringsMap map; + + private NingHttpHeaders(FluentCaseInsensitiveStringsMap map) { + this.map = map; + } + + public static NingHttpHeadersBuilder builder() { + return new NingHttpHeadersBuilder(); + } + + @Override + public FluentCaseInsensitiveStringsMap getHeaders() { + return map; + } + + public static class NingHttpHeadersBuilder { + + private final Map> map; + + private NingHttpHeadersBuilder() { + map = new HashMap<>(); + } + + public NingHttpHeadersBuilder addHeader(String key, String... values) { + map.computeIfAbsent(key, __ -> new ArrayList<>()) + .addAll(Arrays.asList(values)); + return this; + } + + public NingHttpHeaders build() { + return new NingHttpHeaders( + new FluentCaseInsensitiveStringsMap(map)); + } + + } + +} diff --git a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/bridge/resource/UrlResourceCacheTest.java b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/bridge/resource/UrlResourceCacheTest.java index e180a07731..77c38f8996 100644 --- a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/bridge/resource/UrlResourceCacheTest.java +++ b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/bridge/resource/UrlResourceCacheTest.java @@ -1,13 +1,19 @@ package uk.ac.ic.wlgitbridge.bridge.resource; +import com.ning.http.client.HttpResponseHeaders; import org.junit.Test; import uk.ac.ic.wlgitbridge.bridge.db.noop.NoopDbStore; +import uk.ac.ic.wlgitbridge.git.exception.SizeLimitExceededException; import uk.ac.ic.wlgitbridge.io.http.ning.NingHttpClientFacade; +import uk.ac.ic.wlgitbridge.io.http.ning.NingHttpHeaders; +import uk.ac.ic.wlgitbridge.util.FunctionT; -import java.util.Collections; +import java.util.HashMap; import java.util.Optional; +import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; public class UrlResourceCacheTest { @@ -22,6 +28,13 @@ public class UrlResourceCacheTest { private final UrlResourceCache cache = new UrlResourceCache(new NoopDbStore(), http); + private static HttpResponseHeaders withContentLength(long cl) { + return NingHttpHeaders + .builder() + .addHeader("Content-Length", String.valueOf(cl)) + .build(); + } + @Test public void getThrowsSizeLimitWhenContentLengthTooBig() throws Exception { when(http.get(any(), any())).thenAnswer(invoc -> { @@ -35,7 +48,7 @@ public class UrlResourceCacheTest { cache.get( PROJ, URL, NEW_PATH, - Collections.emptyMap(), Collections.emptyMap(), + new HashMap<>(), new HashMap<>(), Optional.of(2L) ); } From 90e3417aec271a1c0cd396fa20a6d525f01fcd30 Mon Sep 17 00:00:00 2001 From: Winston Li Date: Sun, 13 Aug 2017 17:23:45 +0100 Subject: [PATCH 5/5] Add tests for UrlResourceCache content lengths --- .../bridge/resource/UrlResourceCacheTest.java | 75 ++++++++++++++++--- 1 file changed, 65 insertions(+), 10 deletions(-) diff --git a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/bridge/resource/UrlResourceCacheTest.java b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/bridge/resource/UrlResourceCacheTest.java index 77c38f8996..c58165c3ea 100644 --- a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/bridge/resource/UrlResourceCacheTest.java +++ b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/bridge/resource/UrlResourceCacheTest.java @@ -3,13 +3,16 @@ package uk.ac.ic.wlgitbridge.bridge.resource; import com.ning.http.client.HttpResponseHeaders; import org.junit.Test; import uk.ac.ic.wlgitbridge.bridge.db.noop.NoopDbStore; +import uk.ac.ic.wlgitbridge.bridge.util.CastUtil; import uk.ac.ic.wlgitbridge.git.exception.SizeLimitExceededException; import uk.ac.ic.wlgitbridge.io.http.ning.NingHttpClientFacade; import uk.ac.ic.wlgitbridge.io.http.ning.NingHttpHeaders; import uk.ac.ic.wlgitbridge.util.FunctionT; +import java.io.IOException; import java.util.HashMap; import java.util.Optional; +import java.util.concurrent.ExecutionException; import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; @@ -35,21 +38,73 @@ public class UrlResourceCacheTest { .build(); } - @Test - public void getThrowsSizeLimitWhenContentLengthTooBig() throws Exception { + private void respondWithContentLength(long cl, long actual) + throws ExecutionException { when(http.get(any(), any())).thenAnswer(invoc -> { Object[] args = invoc.getArguments(); //noinspection unchecked ((FunctionT< HttpResponseHeaders, Boolean, SizeLimitExceededException - >) args[1]).apply(withContentLength(2)); - return new byte[0]; + >) args[1]).apply(withContentLength(cl)); + return new byte[CastUtil.assumeInt(actual)]; }); - - cache.get( - PROJ, URL, NEW_PATH, - new HashMap<>(), new HashMap<>(), - Optional.of(2L) - ); } + + private void respondWithContentLength(long cl) throws ExecutionException { + respondWithContentLength(cl, cl); + } + + private void getWithMaxLength(Optional max) + throws IOException, SizeLimitExceededException { + cache.get( + PROJ, URL, NEW_PATH, new HashMap<>(), new HashMap<>(), max); + } + + private void getWithMaxLength(long max) + throws IOException, SizeLimitExceededException { + getWithMaxLength(Optional.of(max)); + } + + private void getWithoutLimit() + throws IOException, SizeLimitExceededException { + getWithMaxLength(Optional.empty()); + } + + @Test + public void getDoesNotThrowWhenContentLengthLT() throws Exception { + respondWithContentLength(1); + getWithMaxLength(2); + } + + @Test + public void getDoesNotThrowWhenContentLengthEQ() throws Exception { + respondWithContentLength(2); + getWithMaxLength(2); + } + + @Test (expected = SizeLimitExceededException.class) + public void getThrowsSizeLimitExceededWhenContentLengthGT() + throws Exception { + respondWithContentLength(3); + getWithMaxLength(2); + } + + @Test + public void getWithEmptyContentIsValid() throws Exception { + respondWithContentLength(0); + getWithMaxLength(0); + } + + @Test + public void getWithoutLimitDoesNotThrow() throws Exception { + respondWithContentLength(Integer.MAX_VALUE, 0); + getWithoutLimit(); + } + + @Test (expected = SizeLimitExceededException.class) + public void getThrowsIfActualContentTooBig() throws Exception { + respondWithContentLength(0, 10); + getWithMaxLength(5); + } + }