Skip to content

Commit

Permalink
Merge ReactMethod and ReactSyncHook
Browse files Browse the repository at this point in the history 
Reviewed By: astreet

Differential Revision: D4409726

fbshipit-source-id: 7a0091da754b114680772aa9c0a898b1aa721ba5
  • Loading branch information
@javache @facebook-github-bot
javache authored and facebook-github-bot committed Jan 30, 2017
1 parent 412acd2 commit 59226f0
Show file tree
Hide file tree
Showing 7 changed files with 85 additions and 133 deletions.
103 changes: 30 additions & 73 deletions ReactAndroid/src/main/java/com/facebook/react/bridge/BaseJavaModule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,12 +176,18 @@ public class JavaMethod implements NativeMethod {
private final int mJSArgumentsNeeded;
private final String mTraceName;

public JavaMethod(Method method) {
public JavaMethod(Method method, boolean isSync) {
mMethod = method;
mMethod.setAccessible(true);

if (isSync) {
mType = METHOD_TYPE_SYNC;
}

// TODO: create these lazily
Class[] parameterTypes = method.getParameterTypes();
mArgumentExtractors = buildArgumentExtractors(parameterTypes);
mSignature = buildSignature(parameterTypes);
mSignature = buildSignature(mMethod, parameterTypes, isSync);
// Since native methods are invoked from a message queue executed on a single thread, it is
// save to allocate only one arguments object per method that can be reused across calls
mArguments = new Object[parameterTypes.length];
Expand All @@ -197,9 +203,16 @@ public String getSignature() {
return mSignature;
}

private String buildSignature(Class[] paramTypes) {
StringBuilder builder = new StringBuilder(paramTypes.length);
builder.append("v.");
private String buildSignature(Method method, Class[] paramTypes, boolean isSync) {
StringBuilder builder = new StringBuilder(paramTypes.length + 2);

if (isSync) {
builder.append(returnTypeToChar(method.getReturnType()));
builder.append('.');
} else {
builder.append("v.");
}

for (int i = 0; i < paramTypes.length; i++) {
Class paramClass = paramTypes[i];
if (paramClass == ExecutorToken.class) {
Expand All @@ -212,7 +225,9 @@ private String buildSignature(Class[] paramTypes) {
} else if (paramClass == Promise.class) {
Assertions.assertCondition(
i == paramTypes.length - 1, "Promise must be used as last parameter only");
mType = METHOD_TYPE_PROMISE;
if (!isSync) {
mType = METHOD_TYPE_PROMISE;
}
}
builder.append(paramTypeToChar(paramClass));
}
Expand Down Expand Up @@ -352,89 +367,36 @@ public void invoke(CatalystInstance catalystInstance, ExecutorToken executorToke
* Determines how the method is exported in JavaScript:
* METHOD_TYPE_ASYNC for regular methods
* METHOD_TYPE_PROMISE for methods that return a promise object to the caller.
* METHOD_TYPE_SYNC for sync methods
*/
@Override
public String getType() {
return mType;
}
}

public class SyncJavaHook implements SyncNativeHook {

private Method mMethod;
private final String mSignature;

public SyncJavaHook(Method method) {
mMethod = method;
mMethod.setAccessible(true);
mSignature = buildSignature(method);
}

public Method getMethod() {
return mMethod;
}

public String getSignature() {
return mSignature;
}

private String buildSignature(Method method) {
Class[] paramTypes = method.getParameterTypes();
StringBuilder builder = new StringBuilder(paramTypes.length + 2);

builder.append(returnTypeToChar(method.getReturnType()));
builder.append('.');

for (int i = 0; i < paramTypes.length; i++) {
Class paramClass = paramTypes[i];
if (paramClass == ExecutorToken.class) {
if (!BaseJavaModule.this.supportsWebWorkers()) {
throw new RuntimeException(
"Module " + BaseJavaModule.this + " doesn't support web workers, but " +
mMethod.getName() +
" takes an ExecutorToken.");
}
} else if (paramClass == Promise.class) {
Assertions.assertCondition(
i == paramTypes.length - 1, "Promise must be used as last parameter only");
}
builder.append(paramTypeToChar(paramClass));
}

return builder.toString();
}
}

private @Nullable Map<String, NativeMethod> mMethods;
private @Nullable Map<String, SyncNativeHook> mHooks;

private void findMethods() {
if (mMethods == null) {
Systrace.beginSection(TRACE_TAG_REACT_JAVA_BRIDGE, "findMethods");
mMethods = new HashMap<>();
mHooks = new HashMap<>();

Method[] targetMethods = getClass().getDeclaredMethods();
for (Method targetMethod : targetMethods) {
if (targetMethod.getAnnotation(ReactMethod.class) != null) {
String methodName = targetMethod.getName();
if (mHooks.containsKey(methodName) || mMethods.containsKey(methodName)) {
// We do not support method overloading since js sees a function as an object regardless
// of number of params.
throw new IllegalArgumentException(
"Java Module " + getName() + " sync method name already registered: " + methodName);
}
mMethods.put(methodName, new JavaMethod(targetMethod));
}
if (targetMethod.getAnnotation(ReactSyncHook.class) != null) {
ReactMethod annotation = targetMethod.getAnnotation(ReactMethod.class);
if (annotation != null) {
String methodName = targetMethod.getName();
if (mHooks.containsKey(methodName) || mMethods.containsKey(methodName)) {
if (mMethods.containsKey(methodName)) {
// We do not support method overloading since js sees a function as an object regardless
// of number of params.
throw new IllegalArgumentException(
"Java Module " + getName() + " sync method name already registered: " + methodName);
"Java Module " + getName() + " method name already registered: " + methodName);
}
mHooks.put(methodName, new SyncJavaHook(targetMethod));
mMethods.put(
methodName,
new JavaMethod(targetMethod,
annotation.isBlockingSynchronousMethod()));
}
}
Systrace.endSection(TRACE_TAG_REACT_JAVA_BRIDGE);
Expand All @@ -454,11 +416,6 @@ public final Map<String, NativeMethod> getMethods() {
return null;
}

public final Map<String, SyncNativeHook> getSyncHooks() {
findMethods();
return assertNotNull(mHooks);
}

@Override
public void initialize() {
// do nothing
Expand Down
7 changes: 0 additions & 7 deletions ReactAndroid/src/main/java/com/facebook/react/bridge/NativeModule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,6 @@ interface NativeMethod {
String getType();
}

/**
* A method that can be called from JS synchronously on the JS thread and return a result.
* @see ReactSyncHook
*/
interface SyncNativeHook {
}

/**
* @return the name of this module. This will be the name used to {@code require()} this module
* from javascript.
Expand Down
27 changes: 21 additions & 6 deletions ReactAndroid/src/main/java/com/facebook/react/bridge/ReactMethod.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,28 @@
import static java.lang.annotation.RetentionPolicy.RUNTIME;

/**
* Annotation which is used to mark methods that are exposed to
* Catalyst. This applies to derived classes of {@link
* BaseJavaModule}, which will generate a list of exported methods by
* searching for those which are annotated with this annotation and
* adding a JS callback for each.
* Annotation which is used to mark methods that are exposed to React Native.
*
* This applies to derived classes of {@link BaseJavaModule}, which will generate a list
* of exported methods by searching for those which are annotated with this annotation
* and adding a JS callback for each.
*/
@Retention(RUNTIME)
public @interface ReactMethod {

/**
* Whether the method can be called from JS synchronously **on the JS thread**,
* possibly returning a result.
*
* WARNING: in the vast majority of cases, you should leave this to false which allows
* your native module methods to be called asynchronously: calling methods
* synchronously can have strong performance penalties and introduce threading-related
* bugs to your native modules.
*
* In order to support remote debugging, both the method args and return type must be
* serializable to JSON: this means that we only support the same args as
* {@link ReactMethod}, and the hook can only be void or return JSON values (e.g. bool,
* number, String, {@link WritableMap}, or {@link WritableArray}). Calling these
* methods when running under the websocket executor is currently not supported.
*/
boolean isBlockingSynchronousMethod() default false;
}
31 changes: 0 additions & 31 deletions ReactAndroid/src/main/java/com/facebook/react/bridge/ReactSyncHook.java
View file

This file was deleted.

18 changes: 13 additions & 5 deletions ReactAndroid/src/main/java/com/facebook/react/cxxbridge/JavaModuleWrapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

package com.facebook.react.cxxbridge;

import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
Expand All @@ -19,6 +20,7 @@
import com.facebook.react.bridge.ExecutorToken;
import com.facebook.react.bridge.NativeArray;
import com.facebook.react.bridge.NativeModuleLogger;
import com.facebook.react.bridge.NativeModule;
import com.facebook.react.bridge.ReadableNativeArray;
import com.facebook.react.bridge.WritableNativeArray;
import com.facebook.react.bridge.WritableNativeMap;
Expand All @@ -38,6 +40,10 @@
/* package */ class JavaModuleWrapper {
@DoNotStrip
public class MethodDescriptor {
@DoNotStrip
Method method;
@DoNotStrip
String signature;
@DoNotStrip
String name;
@DoNotStrip
Expand All @@ -46,7 +52,7 @@ public class MethodDescriptor {

private final CatalystInstance mCatalystInstance;
private final ModuleHolder mModuleHolder;
private final ArrayList<BaseJavaModule.JavaMethod> mMethods;
private final ArrayList<NativeModule.NativeMethod> mMethods;

public JavaModuleWrapper(CatalystInstance catalystinstance, ModuleHolder moduleHolder) {
mCatalystInstance = catalystinstance;
Expand All @@ -67,19 +73,21 @@ public String getName() {
@DoNotStrip
public List<MethodDescriptor> getMethodDescriptors() {
ArrayList<MethodDescriptor> descs = new ArrayList<>();

for (Map.Entry<String, BaseJavaModule.NativeMethod> entry :
getModule().getMethods().entrySet()) {
for (Map.Entry<String, NativeModule.NativeMethod> entry :
getModule().getMethods().entrySet()) {
MethodDescriptor md = new MethodDescriptor();
md.name = entry.getKey();
md.type = entry.getValue().getType();

BaseJavaModule.JavaMethod method = (BaseJavaModule.JavaMethod) entry.getValue();
if (md.type == BaseJavaModule.METHOD_TYPE_SYNC) {
md.signature = method.getSignature();
md.method = method.getMethod();
}
mMethods.add(method);

descs.add(md);
}

return descs;
}

Expand Down
2 changes: 1 addition & 1 deletion ReactAndroid/src/main/jni/xreact/jni/ModuleRegistryHolder.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@

#pragma once

#include <cxxreact/ModuleRegistry.h>
#include <fb/fbjni.h>

#include <cxxreact/ModuleRegistry.h>
#include "CxxModuleWrapper.h"

namespace facebook {
Expand Down
30 changes: 20 additions & 10 deletions ReactAndroid/src/test/java/com/facebook/react/bridge/BaseJavaModuleTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

import java.util.Map;

import com.facebook.react.bridge.Arguments;
import com.facebook.react.bridge.ReadableNativeArray;

import org.junit.Before;
Expand Down Expand Up @@ -55,32 +54,43 @@ public void testCallMethodWithoutEnoughArgs() throws Exception {
regularMethod.invoke(null, null, mArguments);
}

@Test(expected = NativeArgumentsParseException.class)
public void testCallAsyncMethodWithoutEnoughArgs() throws Exception {
BaseJavaModule.NativeMethod asyncMethod = mMethods.get("asyncMethod");
@Test
public void testCallMethodWithEnoughArgs() {
BaseJavaModule.NativeMethod regularMethod = mMethods.get("regularMethod");
Mockito.stub(mArguments.size()).toReturn(2);
asyncMethod.invoke(null, null, mArguments);
regularMethod.invoke(null, null, mArguments);
}

@Test()
public void testCallAsyncMethodWithEnoughArgs() throws Exception {
@Test
public void testCallAsyncMethodWithEnoughArgs() {
// Promise block evaluates to 2 args needing to be passed from JS
BaseJavaModule.NativeMethod asyncMethod = mMethods.get("asyncMethod");
Mockito.stub(mArguments.size()).toReturn(3);
asyncMethod.invoke(null, null, mArguments);
}

@Test
public void testCallSyncMethod() {
BaseJavaModule.NativeMethod syncMethod = mMethods.get("syncMethod");
Mockito.stub(mArguments.size()).toReturn(2);
syncMethod.invoke(null, null, mArguments);
}

private static class MethodsModule extends BaseJavaModule {
@Override
public String getName() {
return "Methods";
}

@ReactMethod
public void regularMethod(String a, int b) {
}
public void regularMethod(String a, int b) {}

@ReactMethod
public void asyncMethod(int a, Promise p) {
public void asyncMethod(int a, Promise p) {}

@ReactMethod(isBlockingSynchronousMethod = true)
public int syncMethod(int a, int b) {
return a + b;
}
}
}

0 comments on commit 59226f0

Please sign in to comment.