add native module overriding

Differential Revision: D2700638

fb-gh-sync-id: a88ffaf864be848e1bba22e443d301e4623f04ec

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

@@ -327,6 +327,11 @@ public abstract class BaseJavaModule implements NativeModule {
     // do nothing
   }
 
+  @Override
+  public boolean canOverrideExistingModule() {
+    return false;
+  }
+
   @Override
   public void onCatalystInstanceDestroy() {
     // do nothing

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

@@ -22,7 +22,7 @@ import com.fasterxml.jackson.core.JsonGenerator;
  * register themselves using {@link CxxModuleWrapper}.
  */
 public interface NativeModule {
-  public static interface NativeMethod {
+  interface NativeMethod {
     void invoke(CatalystInstance catalystInstance, ReadableNativeArray parameters);
     String getType();
   }
@@ -31,28 +31,36 @@ public interface NativeModule {
    * @return the name of this module. This will be the name used to {@code require()} this module
    * from javascript.
    */
-  public String getName();
+  String getName();
 
   /**
    * @return methods callable from JS on this module
    */
-  public Map<String, NativeMethod> getMethods();
+  Map<String, NativeMethod> getMethods();
 
   /**
    * Append a field which represents the constants this module exports
    * to JS.  If no constants are exported this should do nothing.
    */
-  public void writeConstantsField(JsonGenerator jg, String fieldName) throws IOException;
+  void writeConstantsField(JsonGenerator jg, String fieldName) throws IOException;
 
   /**
    * This is called at the end of {@link CatalystApplicationFragment#createCatalystInstance()}
    * after the CatalystInstance has been created, in order to initialize NativeModules that require
    * the CatalystInstance or JS modules.
    */
-  public void initialize();
+  void initialize();
+
+  /**
+   * Return true if you intend to override some other native module that was registered e.g. as part
+   * of a different package (such as the core one). Trying to override without returning true from
+   * this method is considered an error and will throw an exception during initialization. By
+   * default all modules return false.
+   */
+  boolean canOverrideExistingModule();
 
   /**
    * Called before {CatalystInstance#onHostDestroy}
    */
-  public void onCatalystInstanceDestroy();
+  void onCatalystInstanceDestroy();
 }

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

@@ -13,11 +13,10 @@ import java.io.IOException;
 import java.io.StringWriter;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.HashMap;
 import java.util.Map;
-import java.util.Set;
 
 import com.facebook.react.common.MapBuilder;
-import com.facebook.react.common.SetBuilder;
 import com.facebook.infer.annotation.Assertions;
 import com.facebook.systrace.Systrace;
 
@@ -30,13 +29,13 @@ import com.fasterxml.jackson.core.JsonGenerator;
 public class NativeModuleRegistry {
 
   private final ArrayList<ModuleDefinition> mModuleTable;
-  private final Map<Class<NativeModule>, NativeModule> mModuleInstances;
+  private final Map<Class<? extends NativeModule>, NativeModule> mModuleInstances;
   private final String mModuleDescriptions;
   private final ArrayList<OnBatchCompleteListener> mBatchCompleteListenerModules;
 
   private NativeModuleRegistry(
       ArrayList<ModuleDefinition> moduleTable,
-      Map<Class<NativeModule>, NativeModule> moduleInstances,
+      Map<Class<? extends NativeModule>, NativeModule> moduleInstances,
       String moduleDescriptions) {
     mModuleTable = moduleTable;
     mModuleInstances = moduleInstances;
@@ -160,32 +159,24 @@ public class NativeModuleRegistry {
 
   public static class Builder {
 
-    private ArrayList<ModuleDefinition> mModuleDefinitions;
+    private final HashMap<String, NativeModule> mModules = MapBuilder.newHashMap();
-    private Map<Class<NativeModule>, NativeModule> mModuleInstances;
-    private Set<String> mSeenModuleNames;
-
-    public Builder() {
-      mModuleDefinitions = new ArrayList<ModuleDefinition>();
-      mModuleInstances = MapBuilder.newHashMap();
-      mSeenModuleNames = SetBuilder.newHashSet();
-    }
 
     public Builder add(NativeModule module) {
-      ModuleDefinition registration = new ModuleDefinition(
+      NativeModule existing = mModules.get(module.getName());
-          mModuleDefinitions.size(),
+      if (existing != null && !module.canOverrideExistingModule()) {
-          module.getName(),
+        throw new IllegalStateException("Native module " + module.getClass().getSimpleName() +
-          module);
+            " tried to override " + existing.getClass().getSimpleName() + " for module name " +
-      Assertions.assertCondition(
+            module.getName() + ". If this was your intention, return true from " +
-          !mSeenModuleNames.contains(module.getName()),
+            module.getClass().getSimpleName() + "#canOverrideExistingModule()");
-          "Module " + module.getName() + " was already registered!");
+      }
-      mSeenModuleNames.add(module.getName());
+      mModules.put(module.getName(), module);
-      mModuleDefinitions.add(registration);
-      mModuleInstances.put((Class<NativeModule>) module.getClass(), module);
       return this;
     }
 
     public NativeModuleRegistry build() {
       Systrace.beginSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "CreateJSON");
+      ArrayList<ModuleDefinition> moduleTable = new ArrayList<>();
+      Map<Class<? extends NativeModule>, NativeModule> moduleInstances = MapBuilder.newHashMap();
       String moduleDefinitionJson;
       try {
         JsonFactory jsonFactory = new JsonFactory();
@@ -193,19 +184,23 @@ public class NativeModuleRegistry {
         try {
           JsonGenerator jg = jsonFactory.createGenerator(writer);
           jg.writeStartObject();
-          for (ModuleDefinition module : mModuleDefinitions) {
+          int idx = 0;
-            jg.writeObjectFieldStart(module.name);
+          for (NativeModule module : mModules.values()) {
-            jg.writeNumberField("moduleID", module.id);
+            ModuleDefinition moduleDef = new ModuleDefinition(idx++, module.getName(), module);
+            moduleTable.add(moduleDef);
+            moduleInstances.put(module.getClass(), module);
+            jg.writeObjectFieldStart(moduleDef.name);
+            jg.writeNumberField("moduleID", moduleDef.id);
             jg.writeObjectFieldStart("methods");
-            for (int i = 0; i < module.methods.size(); i++) {
+            for (int i = 0; i < moduleDef.methods.size(); i++) {
-              MethodRegistration method = module.methods.get(i);
+              MethodRegistration method = moduleDef.methods.get(i);
               jg.writeObjectFieldStart(method.name);
               jg.writeNumberField("methodID", i);
               jg.writeStringField("type", method.method.getType());
               jg.writeEndObject();
             }
             jg.writeEndObject();
-            module.target.writeConstantsField(jg, "constants");
+            moduleDef.target.writeConstantsField(jg, "constants");
             jg.writeEndObject();
           }
           jg.writeEndObject();
@@ -217,7 +212,7 @@ public class NativeModuleRegistry {
       } finally {
         Systrace.endSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE);
       }
-      return new NativeModuleRegistry(mModuleDefinitions, mModuleInstances, moduleDefinitionJson);
+      return new NativeModuleRegistry(moduleTable, moduleInstances, moduleDefinitionJson);
     }
   }
 }