mirror: add EAV reading to `extract`, behind flag (#1343)

Summary:
This completes the end-to-end EAV mode pipeline, but does not yet set it
as default or use it in production.

A note about indentation: we take care to avoid reindenting the entire
block of `extract` test cases, which is over 900 lines long. As to the
implementation code, reindenting the legacy type-specific primitives
branch is not easily avoidable, but when we remove that branch we won’t
have to reindent the EAV mode branch: we can replace its `if` block with
two scope blocks (which is the right thing to do, anyway).

Test Plan:
We reuse existing tests, which suffice for full coverage in both
implementation branches. Note that these tests cover the case of object
types with no primitive fields (the `Feline` and `Socket` types), which
are more likely to fail in a broken EAV implementation than in a broken
type-specific primitives implementation due to deletion anomalies.

To check that all relevant calls to `mirror.extract(…)` have been
properly replaced with `extract(mirror, …)`, run

    yarn coverage -f graphql/mirror -t 'EAV primitives'

and note that the “else” path of the `if (fullOptions.useEavPrimitives)`
branch is not taken; then, run

    yarn coverage -f graphql/mirror -t 'legacy type-specific primitives'

and note that the “if” path of the same branch is not taken.

To check that the table hiding logic is working, invert the branch that
checks `if (fullOptions.useEavPrimitives)`, and note that every test
case using the table hiding logic fails (except for some of the error
handling test cases, which do not actually need to read primitive data).

Finally, `yarn test --full` passes after flipping the `useEavPrimitives`
default to `true`.

wchargin-branch: mirror-eav-extract

src/graphql/mirror.js

@@ -1636,8 +1636,19 @@ export class Mirror {
    * error will be thrown. Furthermore, all transitive dependencies of
    * the object must have been at least partially loaded at some point,
    * or an error will be thrown.
+   *
+   * The `options` argument to `extract` has no stability guarantees and
+   * may be removed or changed at any time. For information about its
+   * semantics, read the current source code.
    */
-  extract(rootId: Schema.ObjectId): mixed {
+  extract(
+    rootId: Schema.ObjectId,
+    options?: {|+useEavPrimitives?: boolean|}
+  ): mixed {
+    const fullOptions = {
+      ...{useEavPrimitives: false},
+      ...(options || {}),
+    };
     const db = this._db;
     return _inTransaction(db, () => {
       // We'll compute the transitive dependencies and store them into a
@@ -1726,9 +1737,73 @@ export class Mirror {
         // Constructing the result set inherently requires mutation,
         // because the object graph can have cycles. We start by
         // creating a record for each object, with just that object's
-        // primitive data and typename. Then, we link in node references
-        // and connection entries.
+        // typename and ID (and, in legacy non-EAV mode, all primitive
+        // data). Then, we link in primitives (except in legacy non-EAV
+        // mode), node references, and connection entries.
         const allObjects: Map<Schema.ObjectId, Object> = new Map();
+        if (fullOptions.useEavPrimitives) {
+          // Initialize `allObjects`.
+          const getObjects = db.prepare(
+            `SELECT id AS id, typename AS typename FROM ${temporaryTableName}`
+          );
+          for (const object of getObjects.iterate()) {
+            allObjects.set(object.id, {
+              id: object.id,
+              __typename: object.typename,
+            });
+          }
+
+          // Fill in primitive data.
+          const getPrimitives = db.prepare(
+            dedent`\
+              SELECT
+                object_id AS objectId,
+                fieldname AS name,
+                value AS value
+              FROM ${temporaryTableName} JOIN primitives
+              ON ${temporaryTableName}.id = primitives.object_id
+              -- Order by field name to ensure that nested fields appear
+              -- before their eggs: e.g., "author" before "author.user".
+              ORDER BY fieldname ASC
+            `
+          );
+          for (const field: {|
+            +objectId: Schema.ObjectId,
+            +name: Schema.Fieldname,
+            +value: string | 0 | 1,
+          |} of getPrimitives.iterate()) {
+            const object = NullUtil.get(allObjects.get(field.objectId));
+            if (field.value === 0) {
+              // Nested object is absent.
+              object[field.name] = null;
+            } else if (field.value === 1) {
+              // Nested object is present.
+              object[field.name] = {};
+            } else {
+              // Normal field, not nested object indicator.
+              const value = JSON.parse(field.value);
+              const parts = field.name.split(".");
+              switch (parts.length) {
+                case 1:
+                  object[field.name] = value;
+                  break;
+                case 2: {
+                  const [nestName, eggName] = parts;
+                  if (object[nestName] !== null) {
+                    object[nestName][eggName] = value;
+                  }
+                  break;
+                }
+                // istanbul ignore next: should not be possible
+                default:
+                  throw new Error(
+                    `Corruption: bad field name: ${JSON.stringify(field.name)}`
+                  );
+              }
+            }
+          }
+        } else {
+          // Legacy non-EAV mode.
           for (const typename of typenames) {
             const objectType = this._schemaInfo.objectTypes[typename];
             // istanbul ignore if: should not be possible using the
@@ -1751,7 +1826,8 @@ export class Mirror {
               ),
               ...objectType.nestedFieldNames.map((f1) =>
                 Object.keys(objectType.nestedFields[f1].primitives).map(
-                (f2) => `${primitivesTableName}."${f1}.${f2}" AS "${f1}.${f2}"`
+                  (f2) =>
+                    `${primitivesTableName}."${f1}.${f2}" AS "${f1}.${f2}"`
                 )
               )
             );
@@ -1819,6 +1895,7 @@ export class Mirror {
               allObjects.set(object.id, object);
             }
           }
+        }
 
         // Add links.
         {

src/graphql/mirror.test.js

@@ -2502,7 +2502,7 @@ describe("graphql/mirror", () => {
       });
     });
 
-    describe("extract", () => {
+    function testExtract(extract: (Mirror, Schema.ObjectId) => mixed) {
       // A schema with some useful edge cases.
       function buildTestSchema(): Schema.Schema {
         const s = Schema;
@@ -2571,7 +2571,7 @@ describe("graphql/mirror", () => {
         const db = new Database(":memory:");
         const mirror = new Mirror(db, buildTestSchema());
         expect(() => {
-          mirror.extract("wat");
+          extract(mirror, "wat");
         }).toThrow('No such object: "wat"');
       });
 
@@ -2580,7 +2580,7 @@ describe("graphql/mirror", () => {
         const mirror = new Mirror(db, buildTestSchema());
         mirror.registerObject({typename: "Caveman", id: "brog"});
         expect(() => {
-          mirror.extract("brog");
+          extract(mirror, "brog");
         }).toThrow(
           '"brog" transitively depends on "brog", ' +
             "but that object's own data has never been fetched"
@@ -2604,7 +2604,7 @@ describe("graphql/mirror", () => {
           nodes: [],
         });
         expect(() => {
-          mirror.extract("localhost");
+          extract(mirror, "localhost");
         }).toThrow(
           '"localhost" transitively depends on "localhost", ' +
             'but that object\'s "only" connection has never been fetched'
@@ -2616,7 +2616,7 @@ describe("graphql/mirror", () => {
           nodes: [],
         });
         expect(() => {
-          mirror.extract("loopback");
+          extract(mirror, "loopback");
         }).toThrow(
           '"loopback" transitively depends on "loopback", ' +
             'but that object\'s "connections" connection has never been fetched'
@@ -2645,7 +2645,7 @@ describe("graphql/mirror", () => {
           },
         ]);
         expect(() => {
-          mirror.extract("alpha");
+          extract(mirror, "alpha");
         }).toThrow(
           '"alpha" transitively depends on "gamma", ' +
             "but that object's own data has never been fetched"
@@ -2681,7 +2681,7 @@ describe("graphql/mirror", () => {
         updateConnection("localhost:8080", "connections", ["localhost:7070"]);
         updateConnection("localhost:7070", "connections", ["localhost:6060"]);
         expect(() => {
-          mirror.extract("localhost:8080");
+          extract(mirror, "localhost:8080");
         }).toThrow(
           '"localhost:8080" transitively depends on "localhost:6060", ' +
             'but that object\'s "connections" connection has never been fetched'
@@ -2696,7 +2696,7 @@ describe("graphql/mirror", () => {
         mirror._updateOwnData(updateId, [
           {__typename: "Caveman", id: "brog", only: "ugg", primitives: "ook"},
         ]);
-        const result: Caveman = (mirror.extract("brog"): any);
+        const result: Caveman = (extract(mirror, "brog"): any);
         expect(result).toEqual({
           __typename: "Caveman",
           id: "brog",
@@ -2732,7 +2732,7 @@ describe("graphql/mirror", () => {
             lynx: null,
           },
         ]);
-        const result = mirror.extract("alpha");
+        const result = extract(mirror, "alpha");
         expect(result).toEqual({
           __typename: "Feline",
           id: "alpha",
@@ -2783,7 +2783,7 @@ describe("graphql/mirror", () => {
           "localhost:6060",
         ]);
         updateConnection("localhost:6060", "connections", []);
-        const result = mirror.extract("localhost:8080");
+        const result = extract(mirror, "localhost:8080");
         expect(result).toEqual({
           __typename: "Socket",
           id: "localhost:8080",
@@ -2818,7 +2818,7 @@ describe("graphql/mirror", () => {
         mirror.registerObject({typename: "Empty", id: "mt"});
         const updateId = mirror._createUpdate(new Date(123));
         mirror._updateOwnData(updateId, [{__typename: "Empty", id: "mt"}]);
-        const result = mirror.extract("mt");
+        const result = extract(mirror, "mt");
         expect(result).toEqual({__typename: "Empty", id: "mt"});
       });
 
@@ -2830,7 +2830,7 @@ describe("graphql/mirror", () => {
         mirror._updateOwnData(updateId, [
           {__typename: "Caveman", id: "brog", only: false, primitives: true},
         ]);
-        expect(mirror.extract("brog")).toEqual({
+        expect(extract(mirror, "brog")).toEqual({
           __typename: "Caveman",
           id: "brog",
           only: false,
@@ -2846,7 +2846,7 @@ describe("graphql/mirror", () => {
         mirror._updateOwnData(updateId, [
           {__typename: "Caveman", id: "brog", only: null, primitives: null},
         ]);
-        expect(mirror.extract("brog")).toEqual({
+        expect(extract(mirror, "brog")).toEqual({
           __typename: "Caveman",
           id: "brog",
           only: null,
@@ -2862,7 +2862,7 @@ describe("graphql/mirror", () => {
         mirror._updateOwnData(updateId, [
           {__typename: "Caveman", id: "brog", only: 123, primitives: 987},
         ]);
-        expect(mirror.extract("brog")).toEqual({
+        expect(extract(mirror, "brog")).toEqual({
           __typename: "Caveman",
           id: "brog",
           only: 123,
@@ -2895,7 +2895,7 @@ describe("graphql/mirror", () => {
             lynx: null,
           },
         ]);
-        const result: Nest = (mirror.extract("eyrie"): any);
+        const result: Nest = (extract(mirror, "eyrie"): any);
         expect(result).toEqual({
           __typename: "Nest",
           id: "eyrie",
@@ -2939,7 +2939,7 @@ describe("graphql/mirror", () => {
             lynx: null,
           },
         ]);
-        const result: Feline = (mirror.extract("alpha"): any);
+        const result: Feline = (extract(mirror, "alpha"): any);
         expect(result).toEqual({
           __typename: "Feline",
           id: "alpha",
@@ -2993,7 +2993,7 @@ describe("graphql/mirror", () => {
           "localhost:6060",
         ]);
         updateConnection("localhost:6060", "connections", ["localhost:7070"]);
-        const result: Socket = (mirror.extract("localhost:8080"): any);
+        const result: Socket = (extract(mirror, "localhost:8080"): any);
         expect(result).toEqual({
           __typename: "Socket",
           id: "localhost:8080",
@@ -3050,7 +3050,7 @@ describe("graphql/mirror", () => {
             null,
           ],
         });
-        const result: Socket = (mirror.extract("localhost"): any);
+        const result: Socket = (extract(mirror, "localhost"): any);
         expect(result).toEqual({
           __typename: "Socket",
           id: "localhost",
@@ -3323,7 +3323,7 @@ describe("graphql/mirror", () => {
         mirror.registerObject(objects.noboty());
         mirror.registerObject(objects.issue3());
 
-        const result = mirror.extract("repo:foo/bar");
+        const result = extract(mirror, "repo:foo/bar");
         expect(result).toEqual({
           __typename: "Repository",
           id: "repo:foo/bar",
@@ -3413,6 +3413,75 @@ describe("graphql/mirror", () => {
           ],
         });
       });
+    }
+
+    describe("extract", () => {
+      // We'll run under both legacy and EAV modes. In each case, hide
+      // the tables corresponding to the other mode to catch any
+      // accidental reads. (We hide and unhide rather than deleting
+      // because some test cases call `extract` multiple times.)
+      function hiddenName(name) {
+        return `${name}_DO_NOT_READ`;
+      }
+      function hideTable(db, name) {
+        db.prepare(`ALTER TABLE ${name} RENAME TO ${hiddenName(name)}`).run();
+      }
+      function unhideTable(db, name) {
+        db.prepare(`ALTER TABLE ${hiddenName(name)} RENAME TO ${name}`).run();
+      }
+
+      describe("with legacy type-specific primitives", () => {
+        testExtract((mirror, id) => {
+          hideTable(mirror._db, "primitives");
+          try {
+            return mirror.extract(id, {useEavPrimitives: false});
+          } finally {
+            unhideTable(mirror._db, "primitives");
+          }
+        });
+      });
+
+      describe("with EAV primitives", () => {
+        testExtract((mirror, id) => {
+          const legacyTables = mirror._db
+            .prepare(
+              "SELECT name FROM sqlite_master " +
+                "WHERE type = 'table' AND name LIKE 'primitives_%'"
+            )
+            .pluck()
+            .all();
+          if (legacyTables.length === 0) {
+            throw new Error("Found no type-specific primitives tables?");
+          }
+          for (const table of legacyTables) {
+            hideTable(mirror._db, table);
+          }
+          try {
+            return mirror.extract(id, {useEavPrimitives: true});
+          } finally {
+            for (const table of legacyTables) {
+              unhideTable(mirror._db, table);
+            }
+          }
+        });
+      });
+
+      it("works with default options", () => {
+        // Simple sanity check.
+        const db = new Database(":memory:");
+        const schema = buildGithubSchema();
+        const mirror = new Mirror(db, schema);
+        mirror.registerObject({typename: "SubscribedEvent", id: "sub"});
+        const update = mirror._createUpdate(new Date(123));
+        mirror._updateOwnData(update, [
+          {__typename: "SubscribedEvent", id: "sub", actor: null},
+        ]);
+        expect(mirror.extract("sub")).toEqual({
+          __typename: "SubscribedEvent",
+          id: "sub",
+          actor: null,
+        });
+      });
     });
 
     describe("end-to-end typename guessing", () => {