RelayPublishQueue: applied selector store updates are not applied cor…

…rectly during rebase (#2489)

Summary:
Fixes an issue with sequential optimistic mutation payloads (not updaters) where a field update from an earlier payload could be overwritten with the *original* value if the same record happened to be updated in a subsequent payload. This happened because the helper `copyFieldsFromRecord` was copying *all* fields from the base version of the record, rather than only updating new fields from the input record.

The PR added new test cases here, the fix on top of that is to update copyFieldsFromRecord to only create a backup record but not re-copy fields from that record to the sink.

-- original pr --

Added some tests that show expected/unexpected behavior I encountered.

Suppose the publish queue has two applied optimistic updates that touch different fields on the same record. If I `RelayEnvironment#commitPayload()`, a backup rebase is executed to commit the payload. During this backup rebase, the second applied optimistic update can wipe the first applied optimistic update.
Pull Request resolved: #2489

Reviewed By: kassens

Differential Revision: D13914926

Pulled By: josephsavona

fbshipit-source-id: 8bbab97a9117af6387e0e60f7b0dbd2454621d94

packages/relay-runtime/mutations/RelayRecordSourceMutator.js

@@ -148,7 +148,7 @@ class RelayRecordSourceMutator {
   }
 
   copyFieldsFromRecord(record: Record, sinkID: DataID): void {
-    this.copyFields(RelayModernRecord.getDataID(record), sinkID);
+    this._createBackupRecord(sinkID);
     const sink = this._getSinkRecord(sinkID);
     RelayModernRecord.copyFields(record, sink);
     this._setSentinelFieldsInBackupRecord(sinkID, sink);

packages/relay-runtime/mutations/RelayRecordSourceProxy.js

@@ -65,7 +65,6 @@ class RelayRecordSourceProxy implements RecordSourceProxy {
             this.create(dataID, RelayModernRecord.getType(sourceRecord));
           }
           this.__mutator.copyFieldsFromRecord(sourceRecord, dataID);
-          delete this._proxies[dataID];
         }
       } else if (status === NONEXISTENT) {
         this.delete(dataID);

packages/relay-runtime/mutations/__tests__/RelayRecordSourceMutator-test.js

@@ -236,7 +236,6 @@ describe('RelayRecordSourceMutator', () => {
         sf: {
           [ID_KEY]: 'sf',
           [TYPENAME_KEY]: 'Page',
-          name: 'San Francisco',
           state: 'California',
         },
       });

packages/relay-runtime/store/__tests__/RelayPublishQueue-test.js

@@ -288,7 +288,7 @@ describe('RelayPublishQueue', () => {
       expect(sourceData['4'].name).toEqual('ZUCK');
     });
 
-    it('applies multiple updaters in subsequent run()s', () => {
+    it('applies updates in subsequent run()s (payload then updater)', () => {
       const queue = new RelayPublishQueue(store);
       queue.applyUpdate({
         operation: operationDescriptor,
@@ -313,6 +313,113 @@ describe('RelayPublishQueue', () => {
       expect(sourceData['4'].name).toEqual('ZUCK');
     });
 
+    it('applies updates in subsequent run()s (updater then updater)', () => {
+      const queue = new RelayPublishQueue(store);
+      const optimisticUpdate = {
+        storeUpdater: storeProxy => {
+          const zuck = storeProxy.get('4');
+          zuck.setValue('zuck', 'name');
+        },
+      };
+      const optimisticUpdate2 = {
+        storeUpdater: storeProxy => {
+          const zuck = storeProxy.get('4');
+          zuck.setValue('zuckerberg', 'lastName');
+        },
+      };
+      queue.applyUpdate(optimisticUpdate);
+      queue.applyUpdate(optimisticUpdate2);
+      queue.run();
+      expect(sourceData).toEqual({
+        ...initialData,
+        4: {
+          ...initialData['4'],
+          name: 'zuck', // from first updater
+          lastName: 'zuckerberg', // from second updater
+        },
+      });
+    });
+
+    it('applies updates in subsequent run()s (payload then payload)', () => {
+      const queue = new RelayPublishQueue(store);
+      // First set `name`.
+      const nameMutation = generateAndCompile(`
+        mutation ChangeNameMutation(
+          $input: ActorNameChangeInput!
+        ) {
+          actorNameChange(input: $input) {
+            actor {
+              name
+            }
+          }
+        }
+      `).ChangeNameMutation;
+      const nameMutationDescriptor = createOperationDescriptor(nameMutation, {
+        input: {},
+      });
+      queue.applyUpdate({
+        operation: nameMutationDescriptor,
+        response: {
+          actorNameChange: {
+            actor: {
+              __typename: 'User',
+              id: '4',
+              name: 'zuck',
+            },
+          },
+        },
+      });
+      // Next set `lastName`.
+      const lastNameMutation = generateAndCompile(`
+        mutation ChangeNameMutation(
+          $input: ActorNameChangeInput!
+        ) {
+          actorNameChange(input: $input) {
+            actor {
+              lastName
+            }
+          }
+        }
+      `).ChangeNameMutation;
+      const lastNameMutationDescriptor = createOperationDescriptor(
+        lastNameMutation,
+        {input: {}},
+      );
+      queue.applyUpdate({
+        operation: lastNameMutationDescriptor,
+        response: {
+          actorNameChange: {
+            actor: {
+              __typename: 'User',
+              id: '4',
+              lastName: 'zuckerberg',
+            },
+          },
+        },
+      });
+      queue.run();
+      expect(sourceData).toEqual({
+        ...initialData,
+        [ROOT_ID]: {
+          ...initialData[ROOT_ID],
+          'actorNameChange(input:{})': {
+            __ref: 'client:root:actorNameChange(input:{})',
+          },
+        },
+        'client:root:actorNameChange(input:{})': {
+          __id: 'client:root:actorNameChange(input:{})',
+          __typename: 'ActorNameChangePayload',
+          actor: {__ref: '4'},
+        },
+        4: {
+          ...initialData['4'],
+          id: '4',
+          name: 'zuck', // from first updater
+          lastName: 'zuckerberg', // from second updater
+        },
+      });
+    });
+
     it('rebases changes when an earlier change is reverted', () => {
       const queue = new RelayPublishQueue(store);
       const optimisticUpdate = {