diff --git a/build.gradle b/build.gradle index 69bd4b127..82fb4f8a2 100644 --- a/build.gradle +++ b/build.gradle @@ -69,6 +69,7 @@ allprojects { testImplementation("junit:junit:${junitVersion}") testImplementation("org.apache.commons:commons-collections4:${commonsCollections4Version}") testImplementation("org.mockito:mockito-core:${mockitoVersion}") + testImplementation("org.objenesis:objenesis:3.2") } clean.doLast { diff --git a/src/main/java/org/fisco/bcos/sdk/v3/client/ClientImpl.java b/src/main/java/org/fisco/bcos/sdk/v3/client/ClientImpl.java index c6f67060f..e03f35f41 100644 --- a/src/main/java/org/fisco/bcos/sdk/v3/client/ClientImpl.java +++ b/src/main/java/org/fisco/bcos/sdk/v3/client/ClientImpl.java @@ -107,6 +107,9 @@ public class ClientImpl implements Client { private GroupNodeIniConfig groupNodeIniConfig; private CryptoSuite cryptoSuite; private RpcJniObj rpcJniObj; + private boolean started; + private boolean stopped; + private boolean destroyed; protected final ObjectMapper objectMapper = getObjectMapper(); @@ -1594,21 +1597,27 @@ public void getFilterLogsAsync(LogFilterResponse filter, RespCallback> ResponseCallback createResponseCallback( diff --git a/src/main/java/org/fisco/bcos/sdk/v3/eventsub/EventSubscribe.java b/src/main/java/org/fisco/bcos/sdk/v3/eventsub/EventSubscribe.java index b6f6653f8..66c973d97 100644 --- a/src/main/java/org/fisco/bcos/sdk/v3/eventsub/EventSubscribe.java +++ b/src/main/java/org/fisco/bcos/sdk/v3/eventsub/EventSubscribe.java @@ -38,7 +38,7 @@ public interface EventSubscribe { */ static EventSubscribe build(String group, ConfigOption configOption) throws JniException { Client client = Client.build(group, configOption); - return new EventSubscribeImp(client, configOption); + return new EventSubscribeImp(client, configOption, true); } /** diff --git a/src/main/java/org/fisco/bcos/sdk/v3/eventsub/EventSubscribeImp.java b/src/main/java/org/fisco/bcos/sdk/v3/eventsub/EventSubscribeImp.java index 66558d39e..4c14c6c64 100644 --- a/src/main/java/org/fisco/bcos/sdk/v3/eventsub/EventSubscribeImp.java +++ b/src/main/java/org/fisco/bcos/sdk/v3/eventsub/EventSubscribeImp.java @@ -18,9 +18,10 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import java.math.BigInteger; +import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.Set; -import org.fisco.bcos.sdk.jni.BcosSDKJniObj; import org.fisco.bcos.sdk.jni.common.JniException; import org.fisco.bcos.sdk.jni.event.EventSubJniObj; import org.fisco.bcos.sdk.v3.client.Client; @@ -37,14 +38,25 @@ public class EventSubscribeImp implements EventSubscribe { private String groupId; private ConfigOption configOption; private CryptoSuite cryptoSuite; + private final Client ownerClient; + private final boolean ownsClient; private EventSubJniObj eventSubJniObj; + private boolean stopped; + private boolean destroyed; private final ObjectMapper objectMapper = ObjectMapperFactory.getObjectMapper(); public EventSubscribeImp(Client client, ConfigOption configOption) throws JniException { + this(client, configOption, false); + } + + EventSubscribeImp(Client client, ConfigOption configOption, boolean ownsClient) + throws JniException { this.groupId = client.getGroup(); this.configOption = configOption; this.cryptoSuite = client.getCryptoSuite(); + this.ownerClient = client; + this.ownsClient = ownsClient; this.eventSubJniObj = EventSubJniObj.build(client.getNativePointer()); this.configOption = client.getConfigOption(); @@ -186,30 +198,63 @@ public String subscribeEvent(EventSubParams params, EventSubCallback callback) { @Override public void unsubscribeEvent(String eventId) { - eventSubJniObj.unsubscribeEvent(eventId); + if (eventSubJniObj != null) { + eventSubJniObj.unsubscribeEvent(eventId); + } } @Override public Set getAllSubscribedEvents() { - // TODO: impl - return null; + if (eventSubJniObj == null) { + return Collections.emptySet(); + } + Set subscribedEvents = eventSubJniObj.getAllSubscribedEvents(); + return subscribedEvents == null ? Collections.emptySet() : subscribedEvents; } @Override - public void start() { - eventSubJniObj.start(); + public synchronized void start() { + if (destroyed) { + return; + } + ownerClient.start(); + stopped = false; } @Override - public void stop() { - eventSubJniObj.stop(); + public synchronized void stop() { + if (destroyed || stopped) { + return; + } + unsubscribeAllEvents(); + if (ownsClient) { + ownerClient.stop(); + } + stopped = true; } @Override - public void destroy() { + public synchronized void destroy() { + if (destroyed) { + return; + } + stop(); if (eventSubJniObj != null) { - BcosSDKJniObj.destroy(eventSubJniObj.getNativePointer()); eventSubJniObj = null; } + if (ownsClient) { + ownerClient.destroy(); + } + destroyed = true; + } + + private void unsubscribeAllEvents() { + Set subscribedEvents = getAllSubscribedEvents(); + if (subscribedEvents.isEmpty()) { + return; + } + for (String eventId : new HashSet<>(subscribedEvents)) { + unsubscribeEvent(eventId); + } } } diff --git a/src/test/java/org/fisco/bcos/sdk/v3/client/ClientImplTest.java b/src/test/java/org/fisco/bcos/sdk/v3/client/ClientImplTest.java new file mode 100644 index 000000000..7b54955fa --- /dev/null +++ b/src/test/java/org/fisco/bcos/sdk/v3/client/ClientImplTest.java @@ -0,0 +1,58 @@ +package org.fisco.bcos.sdk.v3.client; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +import java.lang.reflect.Field; +import org.fisco.bcos.sdk.jni.rpc.RpcJniObj; +import org.fisco.bcos.sdk.v3.crypto.CryptoSuite; +import org.junit.Test; +import org.objenesis.ObjenesisStd; + +public class ClientImplTest { + + private static ClientImpl allocateClientImpl() { + return new ObjenesisStd().newInstance(ClientImpl.class); + } + + @Test + public void testStopIsIdempotent() throws Exception { + ClientImpl client = allocateClientImpl(); + RpcJniObj rpcJniObj = mock(RpcJniObj.class); + setField(client, "rpcJniObj", rpcJniObj); + setBooleanField(client, "started", true); + + client.stop(); + client.stop(); + + verify(rpcJniObj, times(1)).stop(); + } + + @Test + public void testDestroyIsIdempotent() throws Exception { + ClientImpl client = allocateClientImpl(); + CryptoSuite cryptoSuite = mock(CryptoSuite.class); + // rpcJniObj is left null so BcosSDKJniObj.destroy() static native call is skipped + setField(client, "cryptoSuite", cryptoSuite); + setBooleanField(client, "started", true); + + client.destroy(); + client.destroy(); + + verify(cryptoSuite, times(1)).destroy(); + } + + private static void setField(Object target, String fieldName, Object value) throws Exception { + Field field = target.getClass().getDeclaredField(fieldName); + field.setAccessible(true); + field.set(target, value); + } + + private static void setBooleanField(Object target, String fieldName, boolean value) + throws Exception { + Field field = target.getClass().getDeclaredField(fieldName); + field.setAccessible(true); + field.setBoolean(target, value); + } +} diff --git a/src/test/java/org/fisco/bcos/sdk/v3/eventsub/EventSubscribeImpTest.java b/src/test/java/org/fisco/bcos/sdk/v3/eventsub/EventSubscribeImpTest.java new file mode 100644 index 000000000..8f22cae02 --- /dev/null +++ b/src/test/java/org/fisco/bcos/sdk/v3/eventsub/EventSubscribeImpTest.java @@ -0,0 +1,101 @@ +package org.fisco.bcos.sdk.v3.eventsub; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.lang.reflect.Field; +import java.util.Arrays; +import java.util.HashSet; +import org.fisco.bcos.sdk.jni.event.EventSubJniObj; +import org.fisco.bcos.sdk.v3.client.Client; +import org.junit.Test; +import org.objenesis.ObjenesisStd; + +public class EventSubscribeImpTest { + + /** + * Allocates an {@link EventSubscribeImp} without running its constructor (which would trigger a + * JNI call to {@code EventSubJniObj.build}), then injects the supplied fields via reflection. + */ + private static EventSubscribeImp allocateEventSubscribeImp( + Client client, EventSubJniObj eventSubJniObj, boolean ownsClient) throws Exception { + EventSubscribeImp instance = new ObjenesisStd().newInstance(EventSubscribeImp.class); + setField(instance, "ownerClient", client); + setBooleanField(instance, "ownsClient", ownsClient); + setField(instance, "eventSubJniObj", eventSubJniObj); + setField(instance, "groupId", "group0"); + return instance; + } + + @Test + public void testStartActivatesEventChannelForBothCases() throws Exception { + Client client = mock(Client.class); + + // Borrowed client: start() should still delegate to ownerClient.start() + EventSubscribeImp borrowed = + allocateEventSubscribeImp(client, mock(EventSubJniObj.class), false); + borrowed.start(); + verify(client, times(1)).start(); + + // Owned client: start() should also delegate to ownerClient.start() + EventSubscribeImp owned = + allocateEventSubscribeImp(client, mock(EventSubJniObj.class), true); + owned.start(); + verify(client, times(2)).start(); + } + + @Test + public void testStopOnSharedClientOnlyUnsubscribesOnce() throws Exception { + Client client = mock(Client.class); + EventSubJniObj eventSubJniObj = mock(EventSubJniObj.class); + when(eventSubJniObj.getAllSubscribedEvents()) + .thenReturn(new HashSet<>(Arrays.asList("event-a", "event-b"))); + + EventSubscribeImp eventSubscribe = + allocateEventSubscribeImp(client, eventSubJniObj, false); + + eventSubscribe.stop(); + eventSubscribe.stop(); + + verify(eventSubJniObj, times(1)).getAllSubscribedEvents(); + verify(eventSubJniObj, times(1)).unsubscribeEvent("event-a"); + verify(eventSubJniObj, times(1)).unsubscribeEvent("event-b"); + verify(eventSubJniObj, never()).stop(); + verify(client, never()).stop(); + } + + @Test + public void testDestroyOnOwnedClientDelegatesLifecycleOnce() throws Exception { + Client client = mock(Client.class); + EventSubJniObj eventSubJniObj = mock(EventSubJniObj.class); + when(eventSubJniObj.getAllSubscribedEvents()) + .thenReturn(new HashSet<>(Arrays.asList("event-a"))); + + EventSubscribeImp eventSubscribe = + allocateEventSubscribeImp(client, eventSubJniObj, true); + + eventSubscribe.destroy(); + eventSubscribe.destroy(); + + verify(eventSubJniObj, times(1)).unsubscribeEvent("event-a"); + verify(eventSubJniObj, never()).stop(); + verify(client, times(1)).stop(); + verify(client, times(1)).destroy(); + } + + private static void setField(Object target, String fieldName, Object value) throws Exception { + Field field = target.getClass().getDeclaredField(fieldName); + field.setAccessible(true); + field.set(target, value); + } + + private static void setBooleanField(Object target, String fieldName, boolean value) + throws Exception { + Field field = target.getClass().getDeclaredField(fieldName); + field.setAccessible(true); + field.setBoolean(target, value); + } +}