From 080808f1727375e6cec23bf144d1c54d613e7533 Mon Sep 17 00:00:00 2001
From: Dan Gebhardt <dan@cerebris.com>
Date: Thu, 5 Sep 2019 17:23:25 -0400
Subject: [PATCH] Ensure that RequestStrategy handler functions receive all
 args

Custom handler functions on a RequestStrategy, such as `action`, `blocking`, and `filter`, should receive all args emitted from the source event:

* For `beforeX` events such as `beforeQuery`, the handler functions will receive the second `hints` argument.
* For `x` events such as `query`, the handler functions will receive the second `result` argument.
* For `xFail` events such as `queryFail`, the handler functions will receive the second `error` argument.
---
 .../src/strategies/request-strategy.ts        | 22 +++++-----
 .../test/strategies/request-strategy-test.ts  | 44 +++++++++++++++++++
 2 files changed, 56 insertions(+), 10 deletions(-)

diff --git a/packages/@orbit/coordinator/src/strategies/request-strategy.ts b/packages/@orbit/coordinator/src/strategies/request-strategy.ts
index 273a8b385..6519f949f 100644
--- a/packages/@orbit/coordinator/src/strategies/request-strategy.ts
+++ b/packages/@orbit/coordinator/src/strategies/request-strategy.ts
@@ -33,31 +33,31 @@ export class RequestStrategy extends ConnectionStrategy {
   protected generateListener(): Listener {
     const target = this.target as any;
 
-    return (data: any, hints: any): any => {
+    return (...args: any[]): any => {
       let result;
 
       if (this._filter) {
-        if (!this._filter(data, hints)) {
+        if (!this._filter(...args)) {
           return;
         }
       }
 
       if (typeof this._action === 'string') {
-        result = target[this._action](data);
+        result = target[this._action](args[0]);
       } else {
-        result = this._action(data);
+        result = this._action(...args);
       }
 
       if (this._catch && result && result.catch) {
         result = result.catch((e: Error) => {
-          return this._catch(e, data);
+          return this._catch(e, ...args);
         });
       }
 
       if (result && result.then) {
         let blocking = false;
         if (typeof this._blocking === 'function') {
-          if (this._blocking(data)) {
+          if (this._blocking(...args)) {
             blocking = true;
           }
         } else if (this._blocking) {
@@ -65,11 +65,13 @@ export class RequestStrategy extends ConnectionStrategy {
         }
 
         if (blocking) {
-          if (this.passHints && typeof hints === 'object') {
-            return this.applyHint(hints, result);
-          } else {
-            return result;
+          if (this.passHints) {
+            const hints = args[1];
+            if (typeof hints === 'object') {
+              return this.applyHint(hints, result);
+            }
           }
+          return result;
         }
       }
     };
diff --git a/packages/@orbit/coordinator/test/strategies/request-strategy-test.ts b/packages/@orbit/coordinator/test/strategies/request-strategy-test.ts
index 8e3a55724..b31968c50 100644
--- a/packages/@orbit/coordinator/test/strategies/request-strategy-test.ts
+++ b/packages/@orbit/coordinator/test/strategies/request-strategy-test.ts
@@ -290,5 +290,49 @@ module('RequestStrategy', function(hooks) {
     await s1.update(tA);
   });
 
+  test('for events that are invoked with more than one arg, pass all args to any custom handler functions', async function(assert) {
+    assert.expect(9);
+
+    strategy = new RequestStrategy({
+      source: 's1',
+      on: 'updateFail',
+      async action(transform: Transform, e: Error): Promise<void> {
+        assert.ok(
+          this instanceof RequestStrategy,
+          '`action` is bound to the strategy'
+        );
+        assert.strictEqual(transform, tA, 'transform is passed to `action`');
+        assert.equal(e.message, ':(', 'error is passed to `action`');
+      },
+      blocking(transform: Transform, e: Error): boolean {
+        assert.ok(
+          this instanceof RequestStrategy,
+          '`blocking` is bound to the strategy'
+        );
+        assert.strictEqual(transform, tA, 'transform is passed to `blocking`');
+        assert.equal(e.message, ':(', 'error is passed to `blocking`');
+        return false;
+      },
+      filter(transform: Transform, e: Error): boolean {
+        assert.ok(
+          this instanceof RequestStrategy,
+          '`filter` is bound to the strategy'
+        );
+        assert.strictEqual(transform, tA, 'transform is passed to `filter`');
+        assert.equal(e.message, ':(', 'error is passed to `filter`');
+        return true;
+      }
+    });
+
+    coordinator = new Coordinator({
+      sources: [s1, s2],
+      strategies: [strategy]
+    });
+
+    await coordinator.activate();
+
+    s1.emit('updateFail', tA, new Error(':('));
+  });
+
   // TODO - test blocking option
 });