From beaf95f51d78139cffd146d9e4e7d387c7407e2e Mon Sep 17 00:00:00 2001 From: Sean Gilligan Date: Thu, 5 Oct 2023 12:58:01 -0700 Subject: [PATCH] DefaultRpcClient: add constructor for configurabe Transport implementations * Add TransportFactory interface * Add constructor that takes a TransportFactory, make it the canonical constructor * Rename JavaNet test spec to DefaultRpcClientSpec and have it use the factory to construct an client using the JavaNet client. TODO: Testing could obviously be improved. See the TODOs in the code. --- .../bitcoin/rx/ChainTipPublisher.java | 5 ++ .../consensusj/jsonrpc/DefaultRpcClient.java | 36 ++++++++----- .../jsonrpc/DefaultRpcClientSpec.groovy | 51 +++++++++++++++++++ .../jsonrpc/JsonRpcClientJavaNetSpec.groovy | 47 ----------------- 4 files changed, 78 insertions(+), 61 deletions(-) create mode 100644 cj-btc-rx/src/main/java/org/consensusj/bitcoin/rx/ChainTipPublisher.java create mode 100644 consensusj-jsonrpc/src/test/groovy/org/consensusj/jsonrpc/DefaultRpcClientSpec.groovy delete mode 100644 consensusj-jsonrpc/src/test/groovy/org/consensusj/jsonrpc/JsonRpcClientJavaNetSpec.groovy diff --git a/cj-btc-rx/src/main/java/org/consensusj/bitcoin/rx/ChainTipPublisher.java b/cj-btc-rx/src/main/java/org/consensusj/bitcoin/rx/ChainTipPublisher.java new file mode 100644 index 000000000..5f3fe7703 --- /dev/null +++ b/cj-btc-rx/src/main/java/org/consensusj/bitcoin/rx/ChainTipPublisher.java @@ -0,0 +1,5 @@ +package org.consensusj.bitcoin.rx;/** + * + */ +public class ChainTipPublisher { +} diff --git a/consensusj-jsonrpc/src/main/java/org/consensusj/jsonrpc/DefaultRpcClient.java b/consensusj-jsonrpc/src/main/java/org/consensusj/jsonrpc/DefaultRpcClient.java index 65871f268..53daf9042 100644 --- a/consensusj-jsonrpc/src/main/java/org/consensusj/jsonrpc/DefaultRpcClient.java +++ b/consensusj-jsonrpc/src/main/java/org/consensusj/jsonrpc/DefaultRpcClient.java @@ -20,9 +20,6 @@ import java.util.function.Function; import java.util.function.Supplier; -// TODO: the constructor should take a JsonRpcTransport implementation object. -// We now have DefaultRpcClient which is currently hard-coded to use the JavaNet transport (1-line change + recompile to switch) - // We're overusing inheritance in this hierarchy. We are breaking Effective Java, Item 18: Favor composition over inheritance. // We're using inheritance to configure: // A. The set of JSON-RPC methods that are supported, e.g. `getblockcount()` (which creates potential conflicts with `send()` etc) @@ -54,34 +51,45 @@ */ public class DefaultRpcClient implements JsonRpcClient { private static final Logger log = LoggerFactory.getLogger(DefaultRpcClient.class); + private static final JsonRpcMessage.Version DEFAULT_JSON_RPC_VERSION = JsonRpcMessage.Version.V2; + /** - * The default JSON-RPC version in JsonRpcRequest is now '2.0', but since most - * requests are created inside {@code RpcClient} subclasses, we'll continue to default - * to '1.0' in this base class. + * Functional interface for creating JsonRpcTransport instances. This is used to prevent a circular + * dependency on {@link ObjectMapper}. */ - private static final JsonRpcMessage.Version DEFAULT_JSON_RPC_VERSION = JsonRpcMessage.Version.V1; - + @FunctionalInterface + public interface TransportFactory { + /** + * @param mapper mapper that is shared between {@code DefaultRpcClient} and the {@link JsonRpcTransport}. + * @return a transport instance + */ + JsonRpcTransport create(ObjectMapper mapper); + } protected final JsonRpcMessage.Version jsonRpcVersion; protected final ObjectMapper mapper; private final JavaType defaultType; private final JsonRpcTransport transport; + public DefaultRpcClient(URI server, final String rpcUser, final String rpcPassword) { + this(JsonRpcTransport.getDefaultSSLContext(), DEFAULT_JSON_RPC_VERSION, server, rpcUser, rpcPassword); + } + public DefaultRpcClient(JsonRpcMessage.Version jsonRpcVersion, URI server, final String rpcUser, final String rpcPassword) { this(JsonRpcTransport.getDefaultSSLContext(), jsonRpcVersion, server, rpcUser, rpcPassword); } public DefaultRpcClient(SSLContext sslContext, JsonRpcMessage.Version jsonRpcVersion, URI server, final String rpcUser, final String rpcPassword) { + this((m) -> new JsonRpcClientJavaNet(m, sslContext, server, rpcUser, rpcPassword), jsonRpcVersion); + } + + public DefaultRpcClient(TransportFactory transportFactory, JsonRpcMessage.Version jsonRpcVersion) { this.jsonRpcVersion = jsonRpcVersion; mapper = new ObjectMapper(); // TODO: Provide external API to configure FAIL_ON_UNKNOWN_PROPERTIES mapper.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false); defaultType = mapper.getTypeFactory().constructType(Object.class); - // TODO: Right now you can manually switch between JsonRpcClientJavaNet and JsonRpcClientHttpUrlConnection by - // commenting out one line or the other and recompiling. There needs to be a new constructor which will - // allow the default (JsonRpcClientJavaNet) to be overridden. - transport = new JsonRpcClientJavaNet(mapper, sslContext, server, rpcUser, rpcPassword); - //transport = new JsonRpcClientHttpUrlConnection(mapper, sslContext, server, rpcUser, rpcPassword); + transport = transportFactory.create(mapper); } - + @Override public JsonRpcMessage.Version getJsonRpcVersion() { return jsonRpcVersion; diff --git a/consensusj-jsonrpc/src/test/groovy/org/consensusj/jsonrpc/DefaultRpcClientSpec.groovy b/consensusj-jsonrpc/src/test/groovy/org/consensusj/jsonrpc/DefaultRpcClientSpec.groovy new file mode 100644 index 000000000..84b37af00 --- /dev/null +++ b/consensusj-jsonrpc/src/test/groovy/org/consensusj/jsonrpc/DefaultRpcClientSpec.groovy @@ -0,0 +1,51 @@ +package org.consensusj.jsonrpc + +import spock.lang.Ignore +import spock.lang.Specification + +import org.consensusj.bitcoin.jsonrpc.RpcURI + +/** + * RPCClient test specification + */ +@Ignore("Integration test") +class DefaultRpcClientSpec extends Specification { + private final int regTestPort = RpcURI.RPCPORT_REGTEST + private final URI testServerUri = "http://localhost:${regTestPort}".toURI(); + private final String user = "bitcoinrpc" + private final String pass = "pass" + // TODO: Refactor so tests automatically run against both factories. + private final DefaultRpcClient.TransportFactory transportFactory = (m) -> new JsonRpcClientJavaNet(m, JsonRpcTransport.getDefaultSSLContext(), testServerUri, user, pass) + //private final DefaultRpcClient.TransportFactory transportFactory = (m) -> new JsonRpcClientHttpUrlConnection(m, JsonRpcTransport.getDefaultSSLContext(), testServerUri, user, pass) + + def "constructor works correctly" () { + when: + def client = new DefaultRpcClient(transportFactory, JsonRpcMessage.Version.V2) + + then: + client.serverURI == "http://localhost:${regTestPort}".toURI() + client.getJsonRpcVersion() == JsonRpcMessage.Version.V2 + } + +// TODO: Create JsonRpcClientJavaNetSpec that tests the sendRequestForResponseString method + +// def "get block count as string" () { +// when: +// def client = new DefaultRpcClient(testServerUri, user, pass) +// JsonRpcRequest req = new JsonRpcRequest("getblockcount"); +// String blockcount = client.sendRequestForResponseString(req).join() +// +// then: +// blockcount instanceof String +// blockcount.length() >= 1 +// } + + def "get block count as JsonRpcResponse" () { + when: + def client = new DefaultRpcClient(transportFactory, JsonRpcMessage.Version.V2) + Integer blockcount = client.send("getblockcount", Integer.class) + + then: + blockcount >= 0 + } +} diff --git a/consensusj-jsonrpc/src/test/groovy/org/consensusj/jsonrpc/JsonRpcClientJavaNetSpec.groovy b/consensusj-jsonrpc/src/test/groovy/org/consensusj/jsonrpc/JsonRpcClientJavaNetSpec.groovy deleted file mode 100644 index 287bb2309..000000000 --- a/consensusj-jsonrpc/src/test/groovy/org/consensusj/jsonrpc/JsonRpcClientJavaNetSpec.groovy +++ /dev/null @@ -1,47 +0,0 @@ -package org.consensusj.jsonrpc - -import spock.lang.Ignore -import spock.lang.Specification - -import org.consensusj.bitcoin.jsonrpc.RpcURI - -/** - * RPCClient test specification - */ -@Ignore("Integration test") -class JsonRpcClientJavaNetSpec extends Specification { - private final int regTestPort = RpcURI.RPCPORT_REGTEST - private final URI testServerUri = "http://localhost:${regTestPort}".toURI(); - private final String user = "bitcoinrpc" - private final String pass = "pass" - - - def "constructor works correctly" () { - when: - def client = new JsonRpcClientJavaNet(testServerUri, user, pass) - - then: - client.serverURI == "http://localhost:${regTestPort}".toURI() - client.getJsonRpcVersion() == JsonRpcMessage.Version.V2 - } - - def "get block count as string" () { - when: - def client = new JsonRpcClientJavaNet(testServerUri, user, pass) - JsonRpcRequest req = new JsonRpcRequest("getblockcount"); - String blockcount = client.sendRequestForResponseString(req).join() - - then: - blockcount instanceof String - blockcount.length() >= 1 - } - - def "get block count as JsonRpcResponse" () { - when: - def client = new JsonRpcClientJavaNet(testServerUri, user, pass) - Integer blockcount = client.send("getblockcount", Integer.class) - - then: - blockcount >= 0 - } -}