Move RequestHandle test cases to separate class

Add more tests. The old implementation was unregistering wrong request handlers due to a bad UUID mapping Now, when a new request handler registration comes in the old one is automatically removed before registering the new one.
electrode-io · Jan 6, 2020 · 773fefd · 773fefd
1 parent f0b6d06
commit 773fefd
Show file tree

Hide file tree

Showing 6 changed files with 110 additions and 15 deletions.
diff --git a/...src/androidTest/java/com/walmartlabs/electrode/reactnative/bridge/RequestHandleTests.java b/...src/androidTest/java/com/walmartlabs/electrode/reactnative/bridge/RequestHandleTests.java
@@ -105,4 +105,84 @@ public void onFailure(@NonNull FailureMessage failureMessage) {
         });
         waitForCountDownToFinishOrFail(countDownLatch);
     }
+
+    @Test
+    public void testMultipleRequestHandlers() {
+        final CountDownLatch countDownLatch = new CountDownLatch(4);
+        final int RESULT_AGE_FIRST = 10;
+        final int RESULT_AGE_SECOND = 20;
+        final int RESULT_AGE_THIRD = 30;
+
+        RequestHandlerHandle handlerHandle = PersonApi.requests().registerGetAgeRequestHandler(new ElectrodeBridgeRequestHandler<String, Integer>() {
+            @Override
+            public void onRequest(@Nullable String payload, @NonNull ElectrodeBridgeResponseListener<Integer> responseListener) {
+                responseListener.onSuccess(RESULT_AGE_FIRST);
+                countDownLatch.countDown();
+            }
+        });
+
+        RequestHandlerHandle handlerHandle1 = PersonApi.requests().registerGetAgeRequestHandler(new ElectrodeBridgeRequestHandler<String, Integer>() {
+            @Override
+            public void onRequest(@Nullable String payload, @NonNull ElectrodeBridgeResponseListener<Integer> responseListener) {
+                responseListener.onSuccess(RESULT_AGE_SECOND);
+                countDownLatch.countDown();
+            }
+        });
+
+        RequestHandlerHandle handlerHandle2 = PersonApi.requests().registerGetAgeRequestHandler(new ElectrodeBridgeRequestHandler<String, Integer>() {
+            @Override
+            public void onRequest(@Nullable String payload, @NonNull ElectrodeBridgeResponseListener<Integer> responseListener) {
+                responseListener.onSuccess(RESULT_AGE_THIRD);
+                countDownLatch.countDown();
+            }
+        });
+
+        PersonApi.requests().getAge("user", new ElectrodeBridgeResponseListener<Integer>() {
+            @Override
+            public void onSuccess(@Nullable Integer responseData) {
+                assertNotNull(responseData);
+                assertEquals(RESULT_AGE_THIRD, responseData.intValue());
+                countDownLatch.countDown();
+            }
+
+            @Override
+            public void onFailure(@NonNull FailureMessage failureMessage) {
+                fail();
+            }
+        });
+
+        handlerHandle.unregister();
+        handlerHandle1.unregister();
+
+        PersonApi.requests().getAge("user", new ElectrodeBridgeResponseListener<Integer>() {
+            @Override
+            public void onSuccess(@Nullable Integer responseData) {
+                assertNotNull(responseData);
+                assertEquals(RESULT_AGE_THIRD, responseData.intValue());
+                countDownLatch.countDown();
+            }
+
+            @Override
+            public void onFailure(@NonNull FailureMessage failureMessage) {
+               fail();
+            }
+        });
+
+        handlerHandle2.unregister();
+
+        PersonApi.requests().getAge("user", new ElectrodeBridgeResponseListener<Integer>() {
+            @Override
+            public void onSuccess(@Nullable Integer responseData) {
+                fail();
+            }
+
+            @Override
+            public void onFailure(@NonNull FailureMessage failureMessage) {
+                assertNotNull(failureMessage);
+                countDownLatch.countDown();
+            }
+        });
+
+        waitForCountDownToFinishOrFail(countDownLatch);
+    }
 }
diff --git a/...rc/main/java/com/walmartlabs/electrode/reactnative/bridge/ElectrodeBridgeTransceiver.java b/...rc/main/java/com/walmartlabs/electrode/reactnative/bridge/ElectrodeBridgeTransceiver.java
@@ -146,7 +146,7 @@ public static void addConstantsProvider(@NonNull ConstantsProvider constantsProv
         sConstantsProviders.add(constantsProvider);
     }
 
-    @NonNull
+    @Nullable
     @Override
     public UUID getRequestHandlerId(@NonNull String name) {
         return sRequestRegistrar.getRequestHandlerId(name);

diff --git a/...dge/src/main/java/com/walmartlabs/electrode/reactnative/bridge/ElectrodeNativeBridge.java b/...dge/src/main/java/com/walmartlabs/electrode/reactnative/bridge/ElectrodeNativeBridge.java
@@ -17,6 +17,7 @@
 package com.walmartlabs.electrode.reactnative.bridge;
 
 import android.support.annotation.NonNull;
+import android.support.annotation.Nullable;
 
 import java.util.UUID;
 
@@ -66,7 +67,7 @@ public interface ElectrodeNativeBridge {
      * @param name
      * @return {@link UUID} of the request handler
      */
-    @NonNull
+    @Nullable
     UUID getRequestHandlerId(@NonNull String name);
 
     /**

diff --git a/...e/src/main/java/com/walmartlabs/electrode/reactnative/bridge/RequestHandlerProcessor.java b/...e/src/main/java/com/walmartlabs/electrode/reactnative/bridge/RequestHandlerProcessor.java
@@ -91,15 +91,10 @@ public void onSuccess(TResp obj) {
 
     @Override
     public boolean unregister() {
-        Logger.d(TAG, "Removing registered request handler %s with id %s", handler, id);
-        ElectrodeBridgeRequestHandler removedHandler = ElectrodeBridgeHolder.unregisterRequestHandler(id);
-        if (handler != null && intermediateRequestHandler == removedHandler) {
-            intermediateRequestHandler = null;
-            handler = null;
-            return true;
-        } else {
-            Logger.w(TAG, "Not able to unregister a request handler. This is not normal as the request handle should have proper reference of the registered handler. ");
-        }
-        return false;
+        Logger.w(TAG, "Removing registered request handler %s with id %s", handler, id);
+        ElectrodeBridgeHolder.unregisterRequestHandler(id);
+        intermediateRequestHandler = null;
+        handler = null;
+        return true;
     }
 }
diff --git a/...e-bridge/src/main/java/com/walmartlabs/electrode/reactnative/bridge/RequestRegistrar.java b/...e-bridge/src/main/java/com/walmartlabs/electrode/reactnative/bridge/RequestRegistrar.java
@@ -59,6 +59,6 @@ public interface RequestRegistrar<T> {
      * @param name
      * @return {@link UUID} of the request handler
      */
-    @NonNull
+    @Nullable
     UUID getRequestHandlerId(@NonNull String name);
 }
diff --git a/...idge/src/main/java/com/walmartlabs/electrode/reactnative/bridge/RequestRegistrarImpl.java b/...idge/src/main/java/com/walmartlabs/electrode/reactnative/bridge/RequestRegistrarImpl.java
@@ -20,11 +20,14 @@
 import android.support.annotation.Nullable;
 import android.support.annotation.VisibleForTesting;
 
+import com.walmartlabs.electrode.reactnative.bridge.helpers.Logger;
+
 import java.util.Map;
 import java.util.UUID;
 import java.util.concurrent.ConcurrentHashMap;
 
 public class RequestRegistrarImpl<T> implements RequestRegistrar<T> {
+    private static final String TAG = RequestRegistrarImpl.class.getSimpleName();
 
     private final ConcurrentHashMap<UUID, String> mRequestNameByUUID = new ConcurrentHashMap<>();
     private final ConcurrentHashMap<String, T> mRequestHandlerByRequestName = new ConcurrentHashMap<>();
@@ -40,9 +43,14 @@ public class RequestRegistrarImpl<T> implements RequestRegistrar<T> {
     @NonNull
     public boolean registerRequestHandler(@NonNull String name, @NonNull T requestHandler, @NonNull UUID requestHandlerUuid) {
         boolean isRegistered;
+        if (mRequestHandlerByRequestName.get(name) != null) {
+            Logger.d(TAG, "A request handler for request(name: %s) already exist. Replacing with a new request handler", name);
+            removeOldUuidMapping(name);
+        }
         mRequestHandlerByRequestName.put(name, requestHandler);
         mRequestNameByUUID.put(requestHandlerUuid, name);
         isRegistered = true;
+        Logger.d(TAG, "New request handler(id: %s) registered for request: %s", requestHandlerUuid, name);
         return isRegistered;
     }
 
@@ -55,7 +63,10 @@ public boolean registerRequestHandler(@NonNull String name, @NonNull T requestHa
     public T unregisterRequestHandler(@NonNull UUID requestHandlerUuid) {
         String requestName = mRequestNameByUUID.remove(requestHandlerUuid);
         if (requestName != null) {
+            Logger.d(TAG, "Request handler(id: %s) removed for request: %s", requestHandlerUuid, requestName);
             return mRequestHandlerByRequestName.remove(requestName);
+        } else {
+            Logger.d(TAG, "Request handler(id: %s) already removed", requestHandlerUuid);
         }
         return null;
     }
@@ -72,17 +83,25 @@ public T getRequestHandler(@NonNull String name) {
         return mRequestHandlerByRequestName.get(name);
     }
 
-    @NonNull
+    @Nullable
     @Override
     public UUID getRequestHandlerId(@NonNull String name) {
         for (Map.Entry entry : mRequestNameByUUID.entrySet()) {
-            if (name != null && name.equals(entry.getValue())) {
+            if (name.equals(entry.getValue())) {
                 return (UUID) entry.getKey();
             }
         }
         return null;
     }
 
+    private void removeOldUuidMapping(@NonNull String name) {
+        UUID oldUuid = getRequestHandlerId(name);
+        if (oldUuid != null) {
+            Logger.d(TAG, "Removing old request handler(id: %s)", oldUuid);
+            mRequestNameByUUID.remove(oldUuid);
+        }
+    }
+
     /**
      * ONLY FOR TESTING
      * <p>