From ca8b129986fc9b684ef1fb71d56955315233bd53 Mon Sep 17 00:00:00 2001 From: andreoss Date: Sun, 23 Aug 2020 12:25:02 -0400 Subject: [PATCH 1/5] (#179) Check if the arguments match when selecting ctor --- src/main/java/com/jcabi/http/request/BaseRequest.java | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/jcabi/http/request/BaseRequest.java b/src/main/java/com/jcabi/http/request/BaseRequest.java index eb6f87d5..d88a418b 100644 --- a/src/main/java/com/jcabi/http/request/BaseRequest.java +++ b/src/main/java/com/jcabi/http/request/BaseRequest.java @@ -284,8 +284,15 @@ public Request through(final Class type, Constructor ctor = null; for (final Constructor opt : type.getDeclaredConstructors()) { if (opt.getParameterTypes().length == args.length + 1) { - ctor = opt; - break; + final Class[] types = opt.getParameterTypes(); + boolean allmatch = true; + for (int i = 1; i < types.length && allmatch; i++) { + allmatch &= types[i].isAssignableFrom(args[i - 1].getClass()); + } + if (allmatch) { + ctor = opt; + break; + } } } if (ctor == null) { From f9be7b8b66cf108ac6709e9b6cecf79b93140719 Mon Sep 17 00:00:00 2001 From: andreoss Date: Sun, 23 Aug 2020 12:46:19 -0400 Subject: [PATCH 2/5] (#179) Handle primitives correctly --- .../com/jcabi/http/request/BaseRequest.java | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/jcabi/http/request/BaseRequest.java b/src/main/java/com/jcabi/http/request/BaseRequest.java index d88a418b..6273f429 100644 --- a/src/main/java/com/jcabi/http/request/BaseRequest.java +++ b/src/main/java/com/jcabi/http/request/BaseRequest.java @@ -29,6 +29,7 @@ */ package com.jcabi.http.request; +import com.fasterxml.jackson.databind.util.ClassUtil; import com.google.common.base.Joiner; import com.jcabi.aspects.Immutable; import com.jcabi.aspects.Loggable; @@ -283,13 +284,17 @@ public Request through(final Class type, final Object... args) { Constructor ctor = null; for (final Constructor opt : type.getDeclaredConstructors()) { - if (opt.getParameterTypes().length == args.length + 1) { - final Class[] types = opt.getParameterTypes(); - boolean allmatch = true; - for (int i = 1; i < types.length && allmatch; i++) { - allmatch &= types[i].isAssignableFrom(args[i - 1].getClass()); + final Class[] types = opt.getParameterTypes(); + if (types.length == args.length + 1) { + boolean match = true; + for (int i = 1; i < types.length && match; i++) { + Class arg = types[i]; + if (types[i].isPrimitive()) { + arg = ClassUtil.wrapperType(arg); + } + match = arg.isAssignableFrom(args[i - 1].getClass()); } - if (allmatch) { + if (match) { ctor = opt; break; } From 02ea05eaf6524fba5efee8b1a68c26c9b632d91b Mon Sep 17 00:00:00 2001 From: andreoss Date: Wed, 2 Sep 2020 16:30:45 -0400 Subject: [PATCH 3/5] (#179) Handle multiple constructors correctly --- .../com/jcabi/http/request/BaseRequest.java | 120 ++++++++++++------ 1 file changed, 81 insertions(+), 39 deletions(-) diff --git a/src/main/java/com/jcabi/http/request/BaseRequest.java b/src/main/java/com/jcabi/http/request/BaseRequest.java index 6273f429..dc488a39 100644 --- a/src/main/java/com/jcabi/http/request/BaseRequest.java +++ b/src/main/java/com/jcabi/http/request/BaseRequest.java @@ -280,45 +280,11 @@ public Response fetch(final InputStream stream) throws IOException { } @Override - public Request through(final Class type, - final Object... args) { - Constructor ctor = null; - for (final Constructor opt : type.getDeclaredConstructors()) { - final Class[] types = opt.getParameterTypes(); - if (types.length == args.length + 1) { - boolean match = true; - for (int i = 1; i < types.length && match; i++) { - Class arg = types[i]; - if (types[i].isPrimitive()) { - arg = ClassUtil.wrapperType(arg); - } - match = arg.isAssignableFrom(args[i - 1].getClass()); - } - if (match) { - ctor = opt; - break; - } - } - } - if (ctor == null) { - throw new IllegalArgumentException( - String.format( - "class %s doesn't have a ctor with %d argument(s)", - type.getName(), args.length - ) - ); - } - final Object[] params = new Object[args.length + 1]; - params[0] = this.wire; - System.arraycopy(args, 0, params, 1, args.length); - final Wire decorated; - try { - decorated = Wire.class.cast(ctor.newInstance(params)); - } catch (final InstantiationException - | IllegalAccessException | InvocationTargetException ex) { - throw new IllegalStateException(ex); - } - return this.through(decorated); + public Request through( + final Class type, + final Object... args + ) { + return this.through(this.mkWire(type, args)); } @Override @@ -358,6 +324,32 @@ public String toString() { .toString(); } + /** + * Create an instance of Wire. + * + * @param type Type of Wire. + * @param args Ctor arguments. + * @param Type of Wire. + * @return An instance of Wire + */ + private Wire mkWire( + final Class type, + final Object... args + ) { + final Constructor ctor = BaseRequest.findCtor(type, args); + final Object[] params = new Object[args.length + 1]; + params[0] = this.wire; + System.arraycopy(args, 0, params, 1, args.length); + final Wire decorated; + try { + decorated = Wire.class.cast(ctor.newInstance(params)); + } catch (final InstantiationException + | IllegalAccessException | InvocationTargetException ex) { + throw new IllegalStateException(ex); + } + return decorated; + } + /** * Fetch response from server. * @param stream The content to send. @@ -403,6 +395,56 @@ private static URI createUri(final String uri) { return addr; } + /** + * Find a ctor which match arguments. + * @param type A type. + * @param args Ctor arguments. + * @param Type of object + * @return A proper ctor for args. + */ + private static Constructor findCtor( + final Class type, final Object... args + ) { + Constructor ctor = null; + for (final Constructor opt : type.getDeclaredConstructors()) { + final Class[] types = opt.getParameterTypes(); + if (types.length == args.length + 1) { + boolean match = true; + for (int inx = 1; inx < types.length && match; ++inx) { + match = BaseRequest + .wrappedIfNeeded(types[inx]) + .isAssignableFrom(args[inx - 1].getClass()); + } + if (match) { + ctor = opt; + break; + } + } + } + if (ctor == null) { + throw new IllegalArgumentException( + String.format( + "class %s doesn't have a ctor with %d argument(s)", + type.getName(), args.length + ) + ); + } + return ctor; + } + + /** + * Wrap primitive types. + * @param type A type which could be primitive + * @return Wrapped type if it was a primitive + */ + private static Class wrappedIfNeeded(final Class type) { + Class arg = type; + if (arg.isPrimitive()) { + arg = ClassUtil.wrapperType(arg); + } + return arg; + } + /** * Base URI. * From cb1fef9223e6244788646911848807e61a10b6bb Mon Sep 17 00:00:00 2001 From: andreoss Date: Wed, 2 Sep 2020 16:31:23 -0400 Subject: [PATCH 4/5] (#179) Expose cache via contructor --- .../java/com/jcabi/http/wire/CachingWire.java | 96 +++++++++++++------ .../com/jcabi/http/wire/CachingWireTest.java | 40 +++++++- 2 files changed, 107 insertions(+), 29 deletions(-) diff --git a/src/main/java/com/jcabi/http/wire/CachingWire.java b/src/main/java/com/jcabi/http/wire/CachingWire.java index 311951e4..f8a9f00c 100644 --- a/src/main/java/com/jcabi/http/wire/CachingWire.java +++ b/src/main/java/com/jcabi/http/wire/CachingWire.java @@ -42,6 +42,7 @@ import java.net.URI; import java.util.Collection; import java.util.Map; +import java.util.concurrent.Callable; import java.util.concurrent.ExecutionException; import lombok.EqualsAndHashCode; import lombok.ToString; @@ -77,23 +78,25 @@ @Immutable @ToString @EqualsAndHashCode(of = {"origin", "regex"}) +@SuppressWarnings("PMD.OnlyOneConstructorShouldDoInitialization") public final class CachingWire implements Wire { /** * Loader. */ private static final CacheLoader> LOADER = - new CacheLoader>() { + LoadingCache, Response>> LOADER = + new CacheLoader, Response>>() { @Override - public LoadingCache load( - final Wire key) { + public LoadingCache, Response> load( + final Wire key + ) { return CacheBuilder.newBuilder().build( - new CacheLoader() { + new CacheLoader, Response>() { @Override - public Response load(final CachingWire.Query query) - throws IOException { - return query.fetch(); + public Response load(final Callable query) + throws Exception { + return query.call(); } } ); @@ -101,12 +104,17 @@ public Response load(final CachingWire.Query query) }; /** - * Cache. + * Default cache. */ private static final LoadingCache> CACHE = + LoadingCache, Response>> CACHE = CacheBuilder.newBuilder().build(CachingWire.LOADER); + /** + * Default flushing regex. + */ + private static final String NEVER = "$never"; + /** * Original wire. */ @@ -117,12 +125,17 @@ public Response load(final CachingWire.Query query) */ private final transient String regex; + /** + * Cache. + */ + private final LoadingCache, Response> cache; + /** * Public ctor. * @param wire Original wire */ public CachingWire(final Wire wire) { - this(wire, "$never"); + this(wire, CachingWire.NEVER); } /** @@ -134,16 +147,49 @@ public CachingWire(final Wire wire) { public CachingWire(final Wire wire, final String flsh) { this.origin = wire; this.regex = flsh; + this.cache = CACHE.getUnchecked(this); + } + + /** + * Public ctor. + * @param wire Original wire + * @param storage Cache + * @since 1.17.4 + */ + public CachingWire( + final Wire wire, + final LoadingCache, Response> storage + ) { + this(wire, CachingWire.NEVER, storage); + } + + /** + * Public ctor. + * @param wire Original wire + * @param flsh Flushing regular expression + * @param storage Cache + * @since 1.17.4 + */ + public CachingWire( + final Wire wire, + final String flsh, + final LoadingCache, Response> storage + ) { + this.origin = wire; + this.regex = flsh; + this.cache = storage; } // @checkstyle ParameterNumber (5 lines) @Override - public Response send(final Request req, final String home, + public Response send( + final Request req, final String home, final String method, final Collection> headers, final InputStream content, final int connect, - final int read) throws IOException { + final int read + ) throws IOException { final URI uri = req.uri().get(); final StringBuilder label = new StringBuilder(Tv.HUNDRED) .append(method).append(' ').append(uri.getPath()); @@ -151,16 +197,12 @@ public Response send(final Request req, final String home, label.append('?').append(uri.getQuery()); } if (label.toString().matches(this.regex)) { - try { - CachingWire.CACHE.get(this).invalidateAll(); - } catch (final ExecutionException ex) { - throw new IllegalStateException(ex); - } + this.cache.invalidateAll(); } final Response rsp; if (method.equals(Request.GET)) { try { - rsp = CachingWire.CACHE.get(this).get( + rsp = this.cache.get( new CachingWire.Query( this.origin, req, home, headers, content, connect, read @@ -194,7 +236,7 @@ public static void invalidate() { */ @ToString @EqualsAndHashCode(of = {"origin", "request", "uri", "headers"}) - private static final class Query { + private static final class Query implements Callable { /** * Origin wire. */ @@ -241,10 +283,12 @@ private static final class Query { * @param rdd Read timeout * @checkstyle ParameterNumberCheck (5 lines) */ - Query(final Wire wire, final Request req, final String home, + Query( + final Wire wire, final Request req, final String home, final Collection> hdrs, final InputStream input, final int cnct, - final int rdd) { + final int rdd + ) { this.origin = wire; this.request = req; this.uri = home; @@ -254,12 +298,8 @@ private static final class Query { this.read = rdd; } - /** - * Fetch. - * @return Response - * @throws IOException If fails - */ - public Response fetch() throws IOException { + @Override + public Response call() throws IOException { return this.origin.send( this.request, this.uri, Request.GET, this.headers, this.body, this.connect, this.read diff --git a/src/test/java/com/jcabi/http/wire/CachingWireTest.java b/src/test/java/com/jcabi/http/wire/CachingWireTest.java index 2475f617..05e85003 100644 --- a/src/test/java/com/jcabi/http/wire/CachingWireTest.java +++ b/src/test/java/com/jcabi/http/wire/CachingWireTest.java @@ -29,14 +29,19 @@ */ package com.jcabi.http.wire; +import com.google.common.cache.CacheBuilder; +import com.google.common.cache.CacheLoader; +import com.google.common.cache.LoadingCache; import com.jcabi.aspects.Tv; import com.jcabi.http.Request; +import com.jcabi.http.Response; import com.jcabi.http.mock.MkAnswer; import com.jcabi.http.mock.MkContainer; import com.jcabi.http.mock.MkGrizzlyContainer; import com.jcabi.http.request.JdkRequest; import com.jcabi.http.response.RestResponse; import java.net.HttpURLConnection; +import java.util.concurrent.Callable; import org.hamcrest.MatcherAssert; import org.hamcrest.Matchers; import org.junit.jupiter.api.Test; @@ -45,7 +50,7 @@ * Test case for {@link CachingWire}. * @since 1.0 */ -public final class CachingWireTest { +final class CachingWireTest { /** * CachingWire can cache GET requests. @@ -117,4 +122,37 @@ void flushesOnRegularExpressionMatch() throws Exception { ); } + /** + * CachingWire can use custom cache. + * @throws Exception If something goes wrong inside + */ + @Test + void cachesGetRequestWithCustomCache() throws Exception { + final MkContainer container = new MkGrizzlyContainer().next( + new MkAnswer.Simple("") + ).next( + new MkAnswer.Simple(HttpURLConnection.HTTP_BAD_GATEWAY) + ).start(); + final LoadingCache, Response> cache = + CacheBuilder + .newBuilder() + .build( + new CacheLoader, Response>() { + @Override + public Response load(final Callable query) + throws Exception { + return query.call(); + } + } + ); + final Request req = new JdkRequest(container.home()) + .through(CachingWire.class, cache); + for (int idx = 0; idx < Tv.TEN; ++idx) { + req.fetch().as(RestResponse.class) + .assertStatus(HttpURLConnection.HTTP_OK); + } + container.stop(); + MatcherAssert.assertThat(container.queries(), Matchers.equalTo(1)); + } + } From 8c58144ac003daec3906388ce88e01cce73b0e6c Mon Sep 17 00:00:00 2001 From: andreoss Date: Wed, 2 Sep 2020 17:02:56 -0400 Subject: [PATCH 5/5] (#179) Add puzzle --- .../java/com/jcabi/http/wire/CachingWire.java | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/jcabi/http/wire/CachingWire.java b/src/main/java/com/jcabi/http/wire/CachingWire.java index f8a9f00c..1797610a 100644 --- a/src/main/java/com/jcabi/http/wire/CachingWire.java +++ b/src/main/java/com/jcabi/http/wire/CachingWire.java @@ -67,12 +67,25 @@ * .uri().path("/save/123").back() * .fetch(); * + *

Since 1.17.3, you can pass a {@see LoadingCache} alongside the wire. + * + * 

{@code
+ * final LoadingCache, Response> cache = ...;
+ * new JdkRequest(uri)
+ *   .through(CachingWire.class, cache)
+ *   .uri().path("/save/123").back()
+ *   .fetch();
+ *  }
+ * *

The regular expression provided will be used against a string * constructed as an HTTP method, space, path of the URI together with * query part. * *

The class is immutable and thread-safe. - * + * @todo #179:30m This implementation depends on Guava. Investigate for a + * possible shared interface between this class and other implementations for + * caching. If this shared interface is possible replace this task with a task + * for implementing it. * @since 1.0 */ @Immutable