NativePlugins API update

Summary:
The main change is is changing from the inheritance model:
`FlipperPlugin > NativePlugin > TablePlugin > ConcretePlugin`

to a composition model where:
`NativePlugin > TablePlugin > ConcretePlugin`

And behind the scenes, there's a `FlipperPlugin` that has a `NativePlugin` reference.

Now your native plugin will call methods like (in the table case) `display.updateRows` instead of `super.updateRows()`

The reasons for this are:

* Testability: we can easily mock the display and assert output.
* Encapsulation: Previously, native plugins could call `mConnection.send(...)` and send completely unsupported messages to the desktop. Now they only have access to the display's public api, which is guaranteed to work.

I've also changed it so that on every row update, we send the latest table metadata along with it. This makes sure we can ensure that the right table schema is displayed for the current data.

QueryResponder interface also added, which will come in useful for queryable datasets.

Reviewed By: passy

Differential Revision: D14751885

fbshipit-source-id: ea0bbd25f7eaa60020f8866fe210d8bd1c22e90b

android/src/main/java/com/facebook/flipper/nativeplugins/NativePlugin.java

@@ -1,31 +1,7 @@
 package com.facebook.flipper.nativeplugins;
 
-import com.facebook.flipper.core.FlipperPlugin;
+public interface NativePlugin {
+  String getTitle();
 
-/**
+  RawNativePlugin asFlipperPlugin();
- * Subclass of {@link FlipperPlugin} for mobile-defined plugins that conform to a template.
- * Implementations should call {@link #NativePlugin(String, String)} to specify which template will
- * be used. See {@link com.facebook.flipper.nativeplugins.table.TablePlugin} for an example
- * subclass.
- */
-public abstract class NativePlugin implements FlipperPlugin {
-  private final String pluginType;
-  private final String id;
-
-  /**
-   * Call super() inside subclass constructors to provide the template name and id of the concrete
-   * plugin instance.
-   *
-   * @param pluginType This needs to correspond to a plugin template defined in Flipper.
-   * @param id This will uniquely
-   */
-  public NativePlugin(final String pluginType, final String id) {
-    this.pluginType = pluginType;
-    this.id = id;
-  }
-
-  @Override
-  public final String getId() {
-    return "_nativeplugin_" + pluginType + "_" + id;
-  }
 }

android/src/main/java/com/facebook/flipper/nativeplugins/NativePluginRegistry.java

@@ -0,0 +1,16 @@
+package com.facebook.flipper.nativeplugins;
+
+import com.facebook.flipper.core.FlipperClient;
+
+public class NativePluginRegistry {
+
+  private final FlipperClient client;
+
+  public NativePluginRegistry(FlipperClient client) {
+    this.client = client;
+  }
+
+  public void register(final NativePlugin plugin) {
+    client.addPlugin(plugin.asFlipperPlugin());
+  }
+}

android/src/main/java/com/facebook/flipper/nativeplugins/RawNativePlugin.java

@@ -0,0 +1,31 @@
+package com.facebook.flipper.nativeplugins;
+
+import com.facebook.flipper.core.FlipperPlugin;
+
+/**
+ * Subclass of {@link FlipperPlugin} for mobile-defined plugins that conform to a template.
+ * Implementations should call {@link #RawNativePlugin(String, String)} to specify which template
+ * will be used. See {@link com.facebook.flipper.nativeplugins.table.TablePlugin} for an example
+ * subclass.
+ */
+public abstract class RawNativePlugin implements FlipperPlugin {
+  private final String pluginType;
+  private final String id;
+
+  /**
+   * Call super() inside subclass constructors to provide the template name and id of the concrete
+   * plugin instance.
+   *
+   * @param pluginType This needs to correspond to a plugin template defined in Flipper.
+   * @param id This will uniquely
+   */
+  public RawNativePlugin(final String pluginType, final String id) {
+    this.pluginType = pluginType;
+    this.id = id;
+  }
+
+  @Override
+  public final String getId() {
+    return "_nativeplugin_" + pluginType + "_" + id;
+  }
+}

android/src/main/java/com/facebook/flipper/nativeplugins/table/Column.java

@@ -0,0 +1,69 @@
+package com.facebook.flipper.nativeplugins.table;
+
+public class Column {
+  public final String id;
+  final String displayName;
+  final String displayWidth;
+  final boolean showByDefault;
+  final boolean isFilterable;
+
+  Column(
+      String id,
+      String displayName,
+      String displayWidth,
+      boolean showByDefault,
+      boolean isFilterable) {
+    if (id == null) {
+      throw new IllegalArgumentException("id must not be null");
+    }
+    if (displayName == null) {
+      throw new IllegalArgumentException("displayName must not be null");
+    }
+    this.id = id;
+    this.displayName = displayName;
+    this.displayWidth = displayWidth;
+    this.showByDefault = showByDefault;
+    this.isFilterable = isFilterable;
+  }
+
+  public static class Builder {
+    private final String id;
+    private String displayName;
+    private String displayWidth;
+    private boolean showByDefault = true;
+    private boolean isFilterable = false;
+
+    public Builder(String id) {
+      this.id = id;
+    }
+
+    public Builder displayName(String displayName) {
+      this.displayName = displayName;
+      return this;
+    }
+
+    public Builder displayWidthPx(int displayWidth) {
+      this.displayWidth = Integer.toString(displayWidth);
+      return this;
+    }
+
+    public Builder displayWidthPercent(int displayWidth) {
+      this.displayWidth = Integer.toString(displayWidth) + "%";
+      return this;
+    }
+
+    public Builder showByDefault(boolean showByDefault) {
+      this.showByDefault = showByDefault;
+      return this;
+    }
+
+    public Builder isFilterable(boolean isFilterable) {
+      this.isFilterable = isFilterable;
+      return this;
+    }
+
+    public Column build() {
+      return new Column(id, displayName, displayWidth, showByDefault, isFilterable);
+    }
+  }
+}

android/src/main/java/com/facebook/flipper/nativeplugins/table/QueryableTableRowProvider.java

@@ -0,0 +1,18 @@
+package com.facebook.flipper.nativeplugins.table;
+
+import java.util.List;
+
+public interface QueryableTableRowProvider {
+
+  TableQueryResult getQueryResults(String query);
+
+  class TableQueryResult {
+    final TableMetadata metadata;
+    final List<? extends TableRow> results;
+
+    public TableQueryResult(final TableMetadata metadata, final List<? extends TableRow> results) {
+      this.metadata = metadata;
+      this.results = results;
+    }
+  }
+}

android/src/main/java/com/facebook/flipper/nativeplugins/table/TableMetadata.java

@@ -1,26 +1,34 @@
 package com.facebook.flipper.nativeplugins.table;
 
+import androidx.annotation.Nullable;
+
 public class TableMetadata {
 
-  final TablePlugin.Column[] mColumns;
+  final Column[] mColumns;
+  final QueryableTableRowProvider mResponder;
 
-  private TableMetadata(TablePlugin.Column[] columns) {
+  private TableMetadata(
-    if (columns == null) {
+      @Nullable Column[] columns, @Nullable QueryableTableRowProvider queryResponder) {
-      throw new IllegalArgumentException("columns must not be null");
+    this.mColumns = columns == null ? new Column[] {} : columns;
-    }
+    this.mResponder = queryResponder;
-    this.mColumns = columns;
   }
 
   public static class Builder {
-    private TablePlugin.Column[] columns;
+    private Column[] columns;
+    private QueryableTableRowProvider queryResponder;
 
-    public Builder columns(TablePlugin.Column... columns) {
+    public Builder columns(Column... columns) {
       this.columns = columns;
       return this;
     }
 
+    public Builder queryResponder(QueryableTableRowProvider responder) {
+      this.queryResponder = responder;
+      return this;
+    }
+
     public TableMetadata build() {
-      return new TableMetadata(columns);
+      return new TableMetadata(columns, queryResponder);
     }
   }
 }

android/src/main/java/com/facebook/flipper/nativeplugins/table/TablePlugin.java

@@ -1,158 +1,37 @@
-/*
- *  Copyright (c) 2018-present, Facebook, Inc.
- *
- *  This source code is licensed under the MIT license found in the LICENSE
- *  file in the root directory of this source tree.
- *
- */
 package com.facebook.flipper.nativeplugins.table;
 
-import com.facebook.flipper.core.FlipperArray;
 import com.facebook.flipper.core.FlipperConnection;
-import com.facebook.flipper.core.FlipperObject;
-import com.facebook.flipper.core.FlipperReceiver;
-import com.facebook.flipper.core.FlipperResponder;
 import com.facebook.flipper.nativeplugins.NativePlugin;
-import java.util.List;
+import com.facebook.flipper.nativeplugins.RawNativePlugin;
 
-public abstract class TablePlugin extends NativePlugin {
+public abstract class TablePlugin implements NativePlugin {
-
-  public static class Column {
-    public final String id;
-    final String displayName;
-    final String displayWidth;
-    final boolean showByDefault;
-    final boolean isFilterable;
-
-    Column(
-        String id,
-        String displayName,
-        String displayWidth,
-        boolean showByDefault,
-        boolean isFilterable) {
-      if (id == null) {
-        throw new IllegalArgumentException("id must not be null");
-      }
-      if (displayName == null) {
-        throw new IllegalArgumentException("displayName must not be null");
-      }
-      this.id = id;
-      this.displayName = displayName;
-      this.displayWidth = displayWidth;
-      this.showByDefault = showByDefault;
-      this.isFilterable = isFilterable;
-    }
-
-    public static class Builder {
-      private final String id;
-      private String displayName;
-      private String displayWidth;
-      private boolean showByDefault = true;
-      private boolean isFilterable = false;
-
-      public Builder(String id) {
-        this.id = id;
-      }
-
-      public Builder displayName(String displayName) {
-        this.displayName = displayName;
-        return this;
-      }
-
-      public Builder displayWidthPx(int displayWidth) {
-        this.displayWidth = Integer.toString(displayWidth);
-        return this;
-      }
-
-      public Builder displayWidthPercent(int displayWidth) {
-        this.displayWidth = Integer.toString(displayWidth) + "%";
-        return this;
-      }
-
-      public Builder showByDefault(boolean showByDefault) {
-        this.showByDefault = showByDefault;
-        return this;
-      }
-
-      public Builder isFilterable(boolean isFilterable) {
-        this.isFilterable = isFilterable;
-        return this;
-      }
-
-      public Column build() {
-        return new Column(id, displayName, displayWidth, showByDefault, isFilterable);
-      }
-    }
-  }
-
-  private FlipperConnection mConnection;
-
-  public TablePlugin(final String id) {
-    super("Table", id);
-  }
-
-  @Override
-  public final void onConnect(FlipperConnection connection) {
-    this.mConnection = connection;
-    connection.receive(
-        "getMetadata",
-        new FlipperReceiver() {
-          @Override
-          public void onReceive(FlipperObject params, FlipperResponder responder) throws Exception {
-            final FlipperObject.Builder columns = new FlipperObject.Builder();
-            final FlipperObject.Builder columnSizes = new FlipperObject.Builder();
-            final FlipperArray.Builder columnOrder = new FlipperArray.Builder();
-            final FlipperArray.Builder filterableColumns = new FlipperArray.Builder();
-            for (Column c : getMetadata().mColumns) {
-              columns.put(c.id, new FlipperObject.Builder().put("value", c.displayName).build());
-              columnSizes.put(c.id, c.displayWidth);
-              columnOrder.put(
-                  new FlipperObject.Builder().put("key", c.id).put("visible", c.showByDefault));
-              if (c.isFilterable) {
-                filterableColumns.put(c.id);
-              }
-            }
-
-            responder.success(
-                new FlipperObject.Builder()
-                    .put("columns", columns.build())
-                    .put("columnSizes", columnSizes.build())
-                    .put("columnOrder", columnOrder.build())
-                    .put("filterableColumns", filterableColumns.build())
-                    .build());
-          }
-        });
-    this.onConnected();
-  }
-
-  protected abstract void onConnected();
-
-  protected abstract void onDisconnected();
-
-  protected final void updateRows(List<? extends TableRow> rows) {
-    final FlipperArray.Builder array = new FlipperArray.Builder();
-    for (TableRow r : rows) {
-      array.put(r.serialize());
-    }
-    this.mConnection.send("updateRows", array.build());
-  }
 
   public abstract TableMetadata getMetadata();
 
-  public List<? extends TableRow> getRows() {
+  public void onConnect(TableRowDisplay display) {}
-    throw new UnsupportedOperationException(
+
-        "getRows not implemented in "
+  public void onDisconnect() {};
-            + getClass().getSimpleName()
-            + ". Perhaps this is a streaming plugin?");
-  }
 
   @Override
-  public void onDisconnect() throws Exception {
+  public final RawNativePlugin asFlipperPlugin() {
-    this.onDisconnected();
+    return new RawNativePlugin("Table", getTitle()) {
-  }
+
+      @Override
+      public void onConnect(final FlipperConnection connection) throws Exception {
+        final TableRowDisplay display = new TableRowDisplayImpl(connection, TablePlugin.this);
+        TablePlugin.this.onConnect(display);
+      }
+
+      @Override
+      public void onDisconnect() throws Exception {
+        TablePlugin.this.onDisconnect();
+      }
+
+      @Override
+      public boolean runInBackground() {
+        return false;
+      }
+    };
+    }
 
-  @Override
-  public final boolean runInBackground() {
-    return false;
-  }
 }

android/src/main/java/com/facebook/flipper/nativeplugins/table/TableRow.java

@@ -5,7 +5,7 @@ import com.facebook.flipper.nativeplugins.components.Sidebar;
 import java.util.Map;
 
 public abstract class TableRow {
-  interface Value {
+  public interface Value {
     FlipperObject serialize();
   }
 
@@ -75,10 +75,10 @@ public abstract class TableRow {
   }
 
   final String id;
-  final Map<TablePlugin.Column, ? extends Value> values;
+  final Map<Column, ? extends Value> values;
   final Sidebar sidebar;
 
-  public TableRow(String id, Map<TablePlugin.Column, ? extends Value> values, Sidebar sidebar) {
+  public TableRow(String id, Map<Column, ? extends Value> values, Sidebar sidebar) {
     this.id = id;
     this.values = values;
     this.sidebar = sidebar;
@@ -86,7 +86,7 @@ public abstract class TableRow {
 
   final FlipperObject serialize() {
     FlipperObject.Builder columnsObject = new FlipperObject.Builder();
-    for (Map.Entry<TablePlugin.Column, ? extends Value> e : values.entrySet()) {
+    for (Map.Entry<Column, ? extends Value> e : values.entrySet()) {
       columnsObject.put(e.getKey().id, e.getValue().serialize());
     }
     columnsObject.put("id", id);
@@ -96,4 +96,18 @@ public abstract class TableRow {
         .put("id", id)
         .build();
   }
+
+  @Override
+  public boolean equals(Object o) {
+    if (this == o) {
+      return true;
+    }
+    if (o == null) {
+      return false;
+    }
+    if (getClass() != o.getClass()) {
+      return false;
+    }
+    return serialize().equals(((TableRow) o).serialize());
+  }
 }

android/src/main/java/com/facebook/flipper/nativeplugins/table/TableRowDisplay.java

@@ -0,0 +1,9 @@
+package com.facebook.flipper.nativeplugins.table;
+
+import java.util.List;
+
+public interface TableRowDisplay {
+  void updateRow(TableRow row, TableMetadata tableMetadata);
+
+  void updateRows(List<? extends TableRow> rows, TableMetadata tableMetadata);
+}

android/src/main/java/com/facebook/flipper/nativeplugins/table/TableRowDisplayImpl.java

@@ -0,0 +1,61 @@
+package com.facebook.flipper.nativeplugins.table;
+
+import com.facebook.flipper.core.FlipperArray;
+import com.facebook.flipper.core.FlipperConnection;
+import com.facebook.flipper.core.FlipperObject;
+import com.facebook.flipper.core.FlipperReceiver;
+import com.facebook.flipper.core.FlipperResponder;
+import java.util.List;
+
+public class TableRowDisplayImpl implements TableRowDisplay {
+
+  private final FlipperConnection mConnection;
+
+  TableRowDisplayImpl(FlipperConnection connection, final TablePlugin subscriber) {
+    this.mConnection = connection;
+    connection.receive(
+        "getMetadata",
+        new FlipperReceiver() {
+          @Override
+          public void onReceive(FlipperObject params, FlipperResponder responder) throws Exception {
+            final FlipperObject.Builder columns = new FlipperObject.Builder();
+            final FlipperObject.Builder columnSizes = new FlipperObject.Builder();
+            final FlipperArray.Builder columnOrder = new FlipperArray.Builder();
+            final FlipperArray.Builder filterableColumns = new FlipperArray.Builder();
+            for (Column c : subscriber.getMetadata().mColumns) {
+              columns.put(c.id, new FlipperObject.Builder().put("value", c.displayName).build());
+              columnSizes.put(c.id, c.displayWidth);
+              columnOrder.put(
+                  new FlipperObject.Builder().put("key", c.id).put("visible", c.showByDefault));
+              if (c.isFilterable) {
+                filterableColumns.put(c.id);
+              }
+            }
+
+            responder.success(
+                new FlipperObject.Builder()
+                    .put("columns", columns.build())
+                    .put("columnSizes", columnSizes.build())
+                    .put("columnOrder", columnOrder.build())
+                    .put("filterableColumns", filterableColumns.build())
+                    .build());
+          }
+        });
+  }
+
+  @Override
+  public final void updateRow(TableRow row, TableMetadata tableMetadata) {
+    final FlipperArray.Builder array = new FlipperArray.Builder();
+    array.put(row.serialize());
+    this.mConnection.send("updateRows", array.build());
+  }
+
+  @Override
+  public final void updateRows(List<? extends TableRow> rows, TableMetadata tableMetadata) {
+    final FlipperArray.Builder array = new FlipperArray.Builder();
+    for (TableRow r : rows) {
+      array.put(r.serialize());
+    }
+    this.mConnection.send("updateRows", array.build());
+  }
+}

android/src/test/java/com/facebook/flipper/nativeplugins/table/MockTablePlugin.java

@@ -0,0 +1,14 @@
+package com.facebook.flipper.nativeplugins.table;
+
+public class MockTablePlugin extends TablePlugin {
+
+  @Override
+  public TableMetadata getMetadata() {
+    return new TableMetadata.Builder().columns().build();
+  }
+
+  @Override
+  public String getTitle() {
+    return "Mock Table Plugin";
+  }
+}

android/src/test/java/com/facebook/flipper/nativeplugins/table/MockTableRow.java

@@ -0,0 +1,10 @@
+package com.facebook.flipper.nativeplugins.table;
+
+import com.facebook.flipper.nativeplugins.components.Sidebar;
+import java.util.Map;
+
+public class MockTableRow extends TableRow {
+  public MockTableRow(String id, Map<Column, ? extends Value> values, Sidebar sidebar) {
+    super(id, values, sidebar);
+  }
+}

android/src/test/java/com/facebook/flipper/nativeplugins/table/TableMetadataTestUtils.java

@@ -0,0 +1,12 @@
+package com.facebook.flipper.nativeplugins.table;
+
+public class TableMetadataTestUtils {
+
+  public static Column[] getColumns(TableMetadata tableMetadata) {
+    return tableMetadata.mColumns;
+  }
+
+  public static QueryableTableRowProvider getQueryResponder(TableMetadata tableMetadata) {
+    return tableMetadata.responder;
+  }
+}

android/src/test/java/com/facebook/flipper/nativeplugins/table/TableRowDisplayImplTest.java

@@ -0,0 +1,91 @@
+package com.facebook.flipper.nativeplugins.table;
+
+import static org.junit.Assert.assertEquals;
+
+import com.facebook.flipper.core.FlipperArray;
+import com.facebook.flipper.core.FlipperObject;
+import com.facebook.flipper.nativeplugins.components.Sidebar;
+import com.facebook.flipper.testing.FlipperConnectionMock;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.robolectric.RobolectricTestRunner;
+
+@RunWith(RobolectricTestRunner.class)
+public class TableRowDisplayImplTest {
+
+  FlipperConnectionMock mConnection;
+  MockTablePlugin mockTablePlugin;
+
+  static final Column NAME_COLUMN =
+      new Column.Builder("name")
+          .displayName("Name")
+          .displayWidthPercent(90)
+          .isFilterable(true)
+          .showByDefault(false)
+          .build();
+  static final Column AGE_COLUMN =
+      new Column.Builder("age")
+          .displayName("Age")
+          .displayWidthPercent(50)
+          .isFilterable(false)
+          .showByDefault(true)
+          .build();
+
+  @Before
+  public void setup() {
+    mConnection = new FlipperConnectionMock();
+    mockTablePlugin = new MockTablePlugin();
+  }
+
+  private TableRow row(String id, String name, int age) {
+    Map<Column, TableRow.Value> map1 = new HashMap<>();
+    map1.put(NAME_COLUMN, new TableRow.StringValue(name));
+    map1.put(AGE_COLUMN, new TableRow.IntValue(age));
+    return new MockTableRow(id, map1, new Sidebar());
+  }
+
+  @Test
+  public void testUpdateRow() {
+    TableRowDisplay display = new TableRowDisplayImpl(mConnection, mockTablePlugin);
+    display.updateRow(row("row1", "santa", 55), null);
+
+    assertEquals(1, mConnection.sent.get("updateRows").size());
+    FlipperArray rowArray = (FlipperArray) mConnection.sent.get("updateRows").get(0);
+    assertEquals(1, rowArray.length());
+    FlipperObject updatedRow = rowArray.getObject(0);
+    assertEquals(serializedRow("row1", "santa", 55), updatedRow);
+  }
+
+  @Test
+  public void testUpdateRows() {
+    TableRowDisplay display = new TableRowDisplayImpl(mConnection, mockTablePlugin);
+    List<TableRow> rows = new ArrayList<>();
+    rows.add(row("row1", "santa", 55));
+    rows.add(row("row2", "elf", 15));
+    display.updateRows(rows, null);
+
+    assertEquals(1, mConnection.sent.get("updateRows").size());
+    FlipperArray rowArray = (FlipperArray) mConnection.sent.get("updateRows").get(0);
+    assertEquals(2, rowArray.length());
+    assertEquals(serializedRow("row1", "santa", 55), rowArray.getObject(0));
+    assertEquals(serializedRow("row2", "elf", 15), rowArray.getObject(1));
+  }
+
+  private FlipperObject serializedRow(String id, String name, int age) {
+    return new FlipperObject(
+        "{\"columns\":{\"name\":{\"type\":\"string\",\"value\":\""
+            + name
+            + "\"},\"id\":\""
+            + id
+            + "\",\"age\":{\"type\":\"int\",\"value\":"
+            + age
+            + "}},\"sidebar\":[],\"id\":\""
+            + id
+            + "\"}");
+  }
+}