fix: break up incoherent GetApply function into SyncImpl record

Agoric · Jul 16, 2021 · 1455298 · 1455298
1 parent b90ae08
commit 1455298
Show file tree

Hide file tree

Showing 4 changed files with 82 additions and 58 deletions.
diff --git a/packages/captp/lib/captp.js b/packages/captp/lib/captp.js
@@ -15,7 +15,7 @@ import { E, HandledPromise } from '@agoric/eventual-send';
 import { isPromise } from '@agoric/promise-kit';
 import { assert, details as X } from '@agoric/assert';
 
-import { makeSync, nearGetApplySync } from './sync.js';
+import { makeSync, nearSyncImpl } from './sync.js';
 
 import './types.js';
 
@@ -34,7 +34,7 @@ export { E };
  * @property {typeof defaultMakeMarshal} makeMarshal
  * @property {typeof defaultGetInterfaceOf} getInterfaceOf
  * @property {number} epoch
- * @property {GetApplySync} getApplySync
+ * @property {SyncImpl} syncImpl
  */
 /**
  * Create a CapTP connection.
@@ -51,7 +51,7 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) {
     makeMarshal = defaultMakeMarshal,
     getInterfaceOf = defaultGetInterfaceOf,
     epoch = 0,
-    getApplySync: rawGetApplySync,
+    syncImpl: rawSyncImpl,
   } = opts;
 
   const disconnectReason = id =>
@@ -457,9 +457,8 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) {
   /** @type {{Sync?: Sync}} */
   const addSync = {};
 
-  if (rawGetApplySync) {
-    /** @type {GetApplySync} */
-    const getApplySync = (target, prop, methodArgs = undefined) => {
+  if (rawSyncImpl) {
+    const makeSyncImpl = implMethod => (target, ...args) => {
       assert(
         Promise.resolve(target) !== target,
         X`Sync(${target}) target cannot be a promise`,
@@ -475,13 +474,21 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) {
         X`Sync(${target}) imported target was not exportAsSyncable`,
       );
       assert(
-        rawGetApplySync,
-        X`Sync(${target}) failed; no opts.getApplySync supplied to makeCapTP`,
+        rawSyncImpl,
+        X`Sync(${target}) failed; no opts.syncImpl supplied to makeCapTP`,
       );
-      return rawGetApplySync(slot, prop, methodArgs);
+      return rawSyncImpl[implMethod](slot, ...args);
     };
 
-    addSync.Sync = makeSync(getApplySync);
+    /** @type {SyncImpl} */
+    const syncImpl = {
+      applyFunction: makeSyncImpl('applyFunction'),
+      applyMethod: makeSyncImpl('applyMethod'),
+      get: makeSyncImpl('get'),
+    };
+    harden(syncImpl);
+
+    addSync.Sync = makeSync(syncImpl);
   }
 
   return harden({
@@ -529,18 +536,24 @@ export function makeLoopback(ourId, opts = {}) {
     index: 0,
   });
 
+  const makeFarSyncImpl = implMethod => (slot, ...args) => {
+    // Cross the boundary to pull out the far object.
+    // eslint-disable-next-line no-use-before-define
+    const far = farUnserialize({ body: slotBody, slots: [slot] });
+    return nearSyncImpl[implMethod](far, ...args);
+  };
+
   // Create the tunnel.
   let farDispatch;
   const {
     dispatch: nearDispatch,
     Sync,
     getBootstrap: getFarBootstrap,
   } = makeCapTP(`near-${ourId}`, o => farDispatch(o), bootstrap, {
-    getApplySync(slot, prop, methodArgs = undefined) {
-      // Cross the boundary to pull out the far object.
-      // eslint-disable-next-line no-use-before-define
-      const far = farUnserialize({ body: slotBody, slots: [slot] });
-      return nearGetApplySync(far, prop, methodArgs);
+    syncImpl: {
+      applyFunction: makeFarSyncImpl('applyFunction'),
+      applyMethod: makeFarSyncImpl('applyMethod'),
+      get: makeFarSyncImpl('get'),
     },
   });
   assert(Sync);

diff --git a/packages/captp/lib/sync.js b/packages/captp/lib/sync.js
@@ -1,26 +1,24 @@
+// @ts-check
 // Lifted mostly from `@agoric/eventual-send/src/E.js`.
 
 import './types';
 
 /**
- * Default implementation of GetApplySync.
+ * Default implementation of Sync for near objects.
  *
- * @type {GetApplySync}
+ * @type {SyncImpl}
  */
-export const nearGetApplySync = harden(
-  (target, prop, methodArgs = undefined) => {
-    if (Array.isArray(methodArgs)) {
-      if (prop === null) {
-        // Function application.
-        return target(...methodArgs);
-      }
-      // Method application.
-      return target[prop](...methodArgs);
-    }
-    // Property get.
+export const nearSyncImpl = harden({
+  applyFunction(target, args) {
+    return target(...args);
+  },
+  applyMethod(target, prop, args) {
+    return target[prop](...args);
+  },
+  get(target, prop) {
     return target[prop];
   },
-);
+});
 
 const readOnlyProxyHandler = {
   set(_target, _prop, _value) {
@@ -41,45 +39,48 @@ const readOnlyProxyHandler = {
  * A Proxy handler for Sync(x)
  *
  * @param {*} x Any value passed to Sync(x)
- * @param {GetApplySync} getApplySync
+ * @param {SyncImpl} syncImpl
  * @returns {ProxyHandler}
  */
-function SyncProxyHandler(x, getApplySync) {
+function SyncProxyHandler(x, syncImpl) {
   return harden({
     ...readOnlyProxyHandler,
     get(_target, p, _receiver) {
-      return (...args) => getApplySync(x, p, args);
+      return (...args) => syncImpl.applyMethod(x, p, args);
     },
     apply(_target, _thisArg, argArray = []) {
-      return getApplySync(x, null, argArray);
+      return syncImpl.applyFunction(x, argArray);
     },
     has(_target, _p) {
-      // We just pretend everything exists.
+      // TODO: has property is not yet transferrable over captp.
       return true;
     },
   });
 }
 
 /**
- * @param {GetApplySync} getApplySync
+ * @param {SyncImpl} syncImpl
  * @returns {Sync}
  */
-export function makeSync(getApplySync) {
+export function makeSync(syncImpl) {
   function Sync(x) {
-    const handler = SyncProxyHandler(x, getApplySync);
+    const handler = SyncProxyHandler(x, syncImpl);
     return harden(new Proxy(() => {}, handler));
   }
 
-  const makeSyncGetterProxy = x =>
-    new Proxy(Object.create(null), {
+  const makeSyncGetterProxy = x => {
+    const handler = harden({
       ...readOnlyProxyHandler,
-      has(_target, prop) {
-        return getApplySync(x, prop) !== undefined;
+      has(_target, _prop) {
+        // TODO: has property is not yet transferrable over captp.
+        return true;
       },
       get(_target, prop) {
-        return getApplySync(x, prop);
+        return syncImpl.get(x, prop);
       },
     });
+    return new Proxy(Object.create(null), handler);
+  };
   Sync.get = makeSyncGetterProxy;
 
   return harden(Sync);

diff --git a/packages/captp/lib/types.js b/packages/captp/lib/types.js
@@ -1,14 +1,17 @@
 /**
- * @callback GetApplySync
- * @param {any} target
- * @param {string | symbol | number | null} prop the property or method name,
- * null if this is a function call of target
- * @param {Array<any>} [methodArgs] any function or method arguments
+ * @typedef {Object} SyncImpl
+ * @property {(target: any, args: Array<any>) => any} applyFunction function
+ * application
+ * @property {(target: any, method: string | symbol | number, args: Array<any>)
+ * => any} applyMethod method invocation, which is an atomic lookup of method and
+ * apply
+ * @property {(target: any, prop: string | symbol | number) => any} get property
+ * lookup
  */
 
 /** @typedef {import('./ts-types').Sync} Sync */
 
 /**
  * @template T
- * @typedef {import('./ts-types').Syncable} Syncable
+ * @typedef {import('./ts-types').Syncable<T>} Syncable
  */
diff --git a/packages/captp/test/test-sync.js b/packages/captp/test/test-sync.js
@@ -1,7 +1,7 @@
 import { test } from '@agoric/swingset-vat/tools/prepare-test-env-ava';
 import { Far } from '@agoric/marshal';
 import { E, makeCapTP, makeLoopback } from '../lib/captp';
-import { nearGetApplySync } from '../lib/sync';
+import { nearSyncImpl } from '../lib/sync';
 
 function createFarBootstrap(exportAsSyncable) {
   // Create a remotable that has a syncable return value.
@@ -52,21 +52,28 @@ test('try loopback syncable', async t => {
 });
 
 test('try explicit syncable', async t => {
+  const makeFarSyncImpl = implMethod => (slot, ...args) => {
+    // Cross the boundary to pull out the far object.
+    const body = JSON.stringify({
+      '@qclass': 'slot',
+      index: 0,
+    });
+    // eslint-disable-next-line no-use-before-define
+    const far = farUnserialize({ body, slots: [slot] });
+    return nearSyncImpl[implMethod](far, ...args);
+  };
+
   let farDispatch;
   const { dispatch: nearDispatch, getBootstrap, Sync } = makeCapTP(
     'near',
     o => farDispatch(o),
     undefined,
     {
-      getApplySync(slot, prop, methodArgs = undefined) {
-        // Cross the boundary to pull out the far object.
-        const body = JSON.stringify({
-          '@qclass': 'slot',
-          index: 0,
-        });
-        // eslint-disable-next-line no-use-before-define
-        const far = farUnserialize({ body, slots: [slot] });
-        return nearGetApplySync(far, prop, methodArgs);
+      syncImpl: {
+        applyFunction: makeFarSyncImpl('applyFunction'),
+        applyMethod: makeFarSyncImpl('applyMethod'),
+        get: makeFarSyncImpl('get'),
+        has: makeFarSyncImpl('has'),
       },
     },
   );