Support invoking and returning arbitrary types from Java sync hooks

Reviewed By: mhorowitz

Differential Revision: D4409900

fbshipit-source-id: 347e33c442b32f64355d343308c218c15cf5a70f

ReactAndroid/src/androidTest/java/com/facebook/react/tests/BUCK

@@ -8,6 +8,7 @@ deps = [
   react_native_target('java/com/facebook/react:react'),
   react_native_target('java/com/facebook/react/bridge:bridge'),
   react_native_target('java/com/facebook/react/common:common'),
+  react_native_target('java/com/facebook/react/module/annotations:annotations'),
   react_native_target('java/com/facebook/react/modules/core:core'),
   react_native_target('java/com/facebook/react/modules/datepicker:datepicker'),
   react_native_target('java/com/facebook/react/modules/share:share'),

ReactAndroid/src/androidTest/java/com/facebook/react/tests/CatalystNativeJSToJavaParametersTestCase.java

@@ -41,11 +41,15 @@
 
 /**
  * Integration test to verify passing various types of parameters from JS to Java works
+ *
+ * TODO: we should run these tests with isBlockingSynchronousMethod = true as well,
+ * since they currrently use a completely different codepath
  */
 public class CatalystNativeJSToJavaParametersTestCase extends ReactIntegrationTestCase {
 
   private interface TestJSToJavaParametersModule extends JavaScriptModule {
     void returnBasicTypes();
+    void returnBoxedTypes();
     void returnDynamicTypes();
 
     void returnArrayWithBasicTypes();
@@ -116,6 +120,19 @@ public void testBasicTypes() {
     assertNull(args[3]);
   }
 
+  public void testBoxedTypes() {
+    mCatalystInstance.getJSModule(TestJSToJavaParametersModule.class).returnBoxedTypes();
+    waitForBridgeAndUIIdle();
+
+    List<Object[]> boxedTypesCalls = mRecordingTestModule.getBoxedTypesCalls();
+    assertEquals(1, boxedTypesCalls.size());
+
+    Object[] args = boxedTypesCalls.get(0);
+    assertEquals(Integer.valueOf(42), args[0]);
+    assertEquals(Double.valueOf(3.14), args[1]);
+    assertEquals(Boolean.valueOf(true), args[2]);
+  }
+
   public void testDynamicType() {
     mCatalystInstance.getJSModule(TestJSToJavaParametersModule.class).returnDynamicTypes();
     waitForBridgeAndUIIdle();
@@ -682,9 +699,10 @@ private void mapGetByType(ReadableMap map, String key, String typeToAskFor) {
     }
   }
 
-  private class RecordingTestModule extends BaseJavaModule {
+  private static class RecordingTestModule extends BaseJavaModule {
 
     private final List<Object[]> mBasicTypesCalls = new ArrayList<Object[]>();
+    private final List<Object[]> mBoxedTypesCalls = new ArrayList<Object[]>();
     private final List<ReadableArray> mArrayCalls = new ArrayList<ReadableArray>();
     private final List<ReadableMap> mMapCalls = new ArrayList<ReadableMap>();
     private final List<Dynamic> mDynamicCalls = new ArrayList<Dynamic>();
@@ -699,6 +717,11 @@ public void receiveBasicTypes(String s, double d, boolean b, String nullableStri
       mBasicTypesCalls.add(new Object[]{s, d, b, nullableString});
     }
 
+    @ReactMethod
+    public void receiveBoxedTypes(Integer i, Double d, Boolean b) {
+      mBoxedTypesCalls.add(new Object[]{i, d, b});
+    }
+
     @ReactMethod
     public void receiveArray(ReadableArray array) {
       mArrayCalls.add(array);
@@ -718,6 +741,10 @@ public List<Object[]> getBasicTypesCalls() {
       return mBasicTypesCalls;
     }
 
+    public List<Object[]> getBoxedTypesCalls() {
+      return mBoxedTypesCalls;
+    }
+
     public List<ReadableArray> getArrayCalls() {
       return mArrayCalls;
     }

ReactAndroid/src/androidTest/java/com/facebook/react/tests/CatalystNativeJavaToJSReturnValuesTestCase.java

@@ -0,0 +1,163 @@
+/**
+ * Copyright (c) 2014-present, Facebook, Inc.
+ * All rights reserved.
+ * This source code is licensed under the BSD-style license found in the
+ * LICENSE file in the root directory of this source tree. An additional grant
+ * of patent rights can be found in the PATENTS file in the same directory.
+ */
+
+package com.facebook.react.tests;
+
+import java.util.ArrayList;
+
+import com.facebook.react.bridge.BaseJavaModule;
+import com.facebook.react.bridge.CatalystInstance;
+import com.facebook.react.bridge.JavaScriptModule;
+import com.facebook.react.bridge.ReactMethod;
+import com.facebook.react.bridge.WritableArray;
+import com.facebook.react.bridge.WritableMap;
+import com.facebook.react.bridge.WritableNativeArray;
+import com.facebook.react.bridge.WritableNativeMap;
+import com.facebook.react.module.annotations.ReactModule;
+import com.facebook.react.testing.AssertModule;
+import com.facebook.react.testing.FakeWebSocketModule;
+import com.facebook.react.testing.ReactIntegrationTestCase;
+import com.facebook.react.testing.ReactTestHelper;
+import com.facebook.react.uimanager.UIImplementationProvider;
+import com.facebook.react.uimanager.UIManagerModule;
+import com.facebook.react.uimanager.ViewManager;
+
+/**
+ * Test marshalling return values from Java to JS
+ */
+public class CatalystNativeJavaToJSReturnValuesTestCase extends ReactIntegrationTestCase {
+
+  private interface TestJavaToJSReturnValuesModule extends JavaScriptModule {
+    void callMethod(String methodName, String expectedReturnType, String expectedJSON);
+    void triggerException();
+  }
+
+  @ReactModule(name = "TestModule")
+  private static class TestModule extends BaseJavaModule {
+    @Override
+    public String getName() {
+      return "TestModule";
+    }
+
+    @ReactMethod(isBlockingSynchronousMethod = true)
+    boolean getBoolean() {
+      return true;
+    }
+
+    @ReactMethod(isBlockingSynchronousMethod = true)
+    Boolean getBoxedBoolean() {
+      return Boolean.valueOf(true);
+    }
+
+    @ReactMethod(isBlockingSynchronousMethod = true)
+    int getInt() {
+      return 42;
+    }
+
+    @ReactMethod(isBlockingSynchronousMethod = true)
+    Integer getBoxedInt() {
+      return Integer.valueOf(42);
+    }
+
+    @ReactMethod(isBlockingSynchronousMethod = true)
+    double getDouble() {
+      return 3.14159;
+    }
+
+    @ReactMethod(isBlockingSynchronousMethod = true)
+    Double getBoxedDouble() {
+      return Double.valueOf(3.14159);
+    }
+
+    @ReactMethod(isBlockingSynchronousMethod = true)
+    String getString() {
+      return "Hello world!";
+    }
+
+    @ReactMethod(isBlockingSynchronousMethod = true)
+    WritableArray getArray() {
+      WritableArray arr = new WritableNativeArray();
+      arr.pushString("a");
+      arr.pushBoolean(true);
+      return arr;
+    }
+
+    @ReactMethod(isBlockingSynchronousMethod = true)
+    WritableMap getMap() {
+      WritableMap map = new WritableNativeMap();
+      map.putBoolean("a", true);
+      map.putBoolean("b", false);
+      return map;
+    }
+
+    @ReactMethod(isBlockingSynchronousMethod = true)
+    boolean triggerException() {
+      throw new RuntimeException("Exception triggered");
+    }
+  }
+
+  private AssertModule mAssertModule;
+  private CatalystInstance mInstance;
+
+  @Override
+  protected void setUp() throws Exception {
+    super.setUp();
+
+    final UIManagerModule mUIManager = new UIManagerModule(
+        getContext(),
+        new ArrayList<ViewManager>(),
+        new UIImplementationProvider(),
+        false);
+
+    mAssertModule = new AssertModule();
+
+    mInstance = ReactTestHelper.catalystInstanceBuilder(this)
+        .addNativeModule(mAssertModule)
+        .addNativeModule(new FakeWebSocketModule())
+        .addJSModule(TestJavaToJSReturnValuesModule.class)
+        .addNativeModule(mUIManager)
+        .addNativeModule(new TestModule())
+        .build();
+  }
+
+  public void testGetPrimitives() {
+    TestJavaToJSReturnValuesModule m = mInstance.getJSModule(TestJavaToJSReturnValuesModule.class);
+
+    // jboolean is actually an unsigned char, so we don't get JS booleans
+    m.callMethod("getBoolean", "number", "1");
+    m.callMethod("getBoxedBoolean", "number", "1");
+
+    m.callMethod("getInt", "number", "42");
+    m.callMethod("getBoxedInt", "number", "42");
+
+    m.callMethod("getDouble", "number", "3.14159");
+    m.callMethod("getBoxedDouble", "number", "3.14159");
+
+    waitForBridgeAndUIIdle();
+    mAssertModule.verifyAssertsAndReset();
+  }
+
+  public void testObjectTypes() {
+    TestJavaToJSReturnValuesModule m = mInstance.getJSModule(TestJavaToJSReturnValuesModule.class);
+
+    m.callMethod("getString", "string", "\"Hello world!\"");
+    m.callMethod("getArray", "object", "[\"a\",true]");
+    m.callMethod("getMap", "object", "{\"b\":false,\"a\":true}");
+
+    waitForBridgeAndUIIdle();
+    mAssertModule.verifyAssertsAndReset();
+  }
+
+  public void testThrowsException() {
+    TestJavaToJSReturnValuesModule m = mInstance.getJSModule(TestJavaToJSReturnValuesModule.class);
+    m.triggerException();
+
+    waitForBridgeAndUIIdle();
+    mAssertModule.verifyAssertsAndReset();
+  }
+}

ReactAndroid/src/androidTest/js/TestBundle.js

@@ -18,6 +18,7 @@ require('ViewRenderingTestModule');
 require('TestJavaToJSArgumentsModule');
 require('TestJSLocaleModule');
 require('TestJSToJavaParametersModule');
+require('TestJavaToJSReturnValuesModule');
 require('UIManagerTestModule');
 
 require('CatalystRootViewTestModule');

ReactAndroid/src/androidTest/js/TestJSToJavaParametersModule.js

@@ -18,6 +18,9 @@ var TestJSToJavaParametersModule = {
   returnBasicTypes: function() {
     Recording.receiveBasicTypes('foo', 3.14, true, null);
   },
+  returnBoxedTypes: function() {
+    Recording.receiveBoxedTypes(42, 3.14, true);
+  },
   returnDynamicTypes: function() {
     Recording.receiveDynamic('foo');
     Recording.receiveDynamic(3.14);

ReactAndroid/src/androidTest/js/TestJavaToJSReturnValuesModule.js

@@ -0,0 +1,40 @@
+/**
+ * Copyright (c) 2013-present, Facebook, Inc.
+ * All rights reserved.
+ *
+ * This source code is licensed under the BSD-style license found in the
+ * LICENSE file in the root directory of this source tree. An additional grant
+ * of patent rights can be found in the PATENTS file in the same directory.
+ *
+ * @providesModule TestJavaToJSReturnValuesModule
+ */
+
+'use strict';
+
+const BatchedBridge = require('BatchedBridge');
+
+const {assertEquals, assertTrue} = require('Asserts');
+const {TestModule} = require('NativeModules');
+
+var TestJavaToJSReturnValuesModule = {
+  callMethod: function(methodName, expectedType, expectedJSON) {
+    const result = TestModule[methodName]();
+    assertEquals(expectedType, typeof result);
+    assertEquals(expectedJSON, JSON.stringify(result));
+  },
+
+  triggerException: function() {
+    try {
+      TestModule.triggerException();
+    } catch (ex) {
+      assertTrue(ex.message.indexOf('Exception triggered') !== -1);
+    }
+  }
+};
+
+BatchedBridge.registerCallableModule(
+  'TestJavaToJSReturnValuesModule',
+  TestJavaToJSReturnValuesModule
+);
+
+module.exports = TestJavaToJSReturnValuesModule;

ReactAndroid/src/main/java/com/facebook/react/bridge/BaseJavaModule.java

@@ -461,6 +461,7 @@ private static char paramTypeToChar(Class paramClass) {
   }
 
   private static char returnTypeToChar(Class returnClass) {
+    // Keep this in sync with MethodInvoker
     char tryCommon = commonTypeToChar(returnClass);
     if (tryCommon != '\0') {
       return tryCommon;

ReactAndroid/src/main/jni/xreact/jni/JavaModuleWrapper.cpp

@@ -2,10 +2,8 @@
 
 #include "JavaModuleWrapper.h"
 
-#include <folly/json.h>
-
 #include <fb/fbjni.h>
-
+#include <folly/json.h>
 #include <cxxreact/CxxModule.h>
 #include <cxxreact/CxxNativeModule.h>
 #include <cxxreact/Instance.h>
@@ -42,9 +40,30 @@ std::string JavaNativeModule::getName() {
 
 std::vector<MethodDescriptor> JavaNativeModule::getMethods() {
   std::vector<MethodDescriptor> ret;
+  syncMethods_.clear();
   auto descs = wrapper_->getMethodDescriptors();
   for (const auto& desc : *descs) {
-    ret.emplace_back(desc->getName(), desc->getType());
+    auto methodName = desc->getName();
+    auto methodType = desc->getType();
+
+    if (methodType == "sync") {
+      // allow for the sync methods vector to have empty values, resize on demand
+      size_t methodIndex = ret.size();
+      if (methodIndex >= syncMethods_.size()) {
+        syncMethods_.resize(methodIndex + 1);
+      }
+      syncMethods_.insert(syncMethods_.begin() + methodIndex, MethodInvoker(
+        desc->getMethod(),
+        desc->getSignature(),
+        getName() + "." + methodName,
+        true
+      ));
+    }
+
+    ret.emplace_back(
+      std::move(methodName),
+      std::move(methodType)
+    );
   }
   return ret;
 }
@@ -75,7 +94,15 @@ void JavaNativeModule::invoke(ExecutorToken token, unsigned int reactMethodId, f
 }
 
 MethodCallResult JavaNativeModule::callSerializableNativeHook(ExecutorToken token, unsigned int reactMethodId, folly::dynamic&& params) {
-  throw std::runtime_error("Unsupported operation.");
+    // TODO: evaluate whether calling through invoke is potentially faster
+    if (reactMethodId >= syncMethods_.size()) {
+      throw std::invalid_argument(
+        folly::to<std::string>("methodId ", reactMethodId, " out of range [0..", syncMethods_.size(), "]"));
+    }
+
+    auto& method = syncMethods_[reactMethodId];
+    CHECK(method.hasValue() && method->isSyncHook()) << "Trying to invoke a asynchronous method as synchronous hook";
+    return method->invoke(instance_, wrapper_->getModule(), token, params);
 }
 
 NewJavaNativeModule::NewJavaNativeModule(

ReactAndroid/src/main/jni/xreact/jni/JavaModuleWrapper.h

@@ -4,6 +4,7 @@
 
 #include <cxxreact/NativeModule.h>
 #include <fb/fbjni.h>
+#include <folly/Optional.h>
 
 #include "MethodInvoker.h"
 
@@ -54,6 +55,7 @@ class JavaNativeModule : public NativeModule {
  private:
   std::weak_ptr<Instance> instance_;
   jni::global_ref<JavaModuleWrapper::javaobject> wrapper_;
+  std::vector<folly::Optional<MethodInvoker>> syncMethods_;
 };
 
 // Experimental new implementation that uses direct method invocation