Skip to content

Commit

Permalink
fix(notifier): Remove the makeDurablePublishKit "onAdvance" option (#…
Browse files Browse the repository at this point in the history 
…7370)

Reverts 312351f (#7341), because #7350
went in a different direction.

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
  • Loading branch information
@gibson042 @mergify
gibson042 and mergify[bot] authored Apr 12, 2023
1 parent f0b37a8 commit 6861f5e
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 99 deletions.
24 changes: 4 additions & 20 deletions packages/notifier/src/publish-kit.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import { makePromiseKit } from '@endo/promise-kit';
import { E, Far } from '@endo/far';
import { callE, isCallback } from '@agoric/internal/src/callback.js';
import { M } from '@agoric/store';
import { canBeDurable, prepareExoClassKit } from '@agoric/vat-data';

Expand Down Expand Up @@ -179,19 +178,14 @@ harden(makePublishKit);
/**
* @param {object} [options]
* @param {DurablePublishKitValueDurability & 'mandatory'} [options.valueDurability='mandatory']
* @param {PublishKitOnUpdate} [options.onUpdate] A direct callback
* to which published values should be sent, useful for receiving validated values
* without consuming heap RAM with ephemeral objects
* @returns {DurablePublishKitState}
*/
const initDurablePublishKitState = (options = {}) => {
const { valueDurability = 'mandatory', onUpdate } = options;
const { valueDurability = 'mandatory' } = options;
assert.equal(valueDurability, 'mandatory');
!onUpdate || assert(isCallback(onUpdate));
return {
// configuration
valueDurability,
onUpdate,

// lifecycle progress
publishCount: 0n,
Expand Down Expand Up @@ -294,7 +288,7 @@ const provideDurablePublishKitEphemeralData = (state, facets) => {
*/
const advanceDurablePublishKit = (context, value, targetStatus = 'live') => {
const { state, facets } = context;
const { valueDurability, onUpdate, status } = state;
const { valueDurability, status } = state;
if (status !== 'live') {
throw new Error('Cannot update state after termination.');
}
Expand All @@ -304,16 +298,6 @@ const advanceDurablePublishKit = (context, value, targetStatus = 'live') => {
}
const { tailP: currentP, tailR: resolveCurrent } =
provideDurablePublishKitEphemeralData(state, facets);
const commit = resolution => {
// Invoke a direct callback, but ignore the result.
if (onUpdate) {
// We use Promise.resolve to ensure that onUpdate always receives
// a promise while preserving the identity of any rejection.
// `resolution` is a safe value created in this module.
void callE(onUpdate, Promise.resolve(resolution));
}
resolveCurrent(resolution);
};

const publishCount = state.publishCount + 1n;
state.publishCount = publishCount;
Expand All @@ -337,7 +321,7 @@ const advanceDurablePublishKit = (context, value, targetStatus = 'live') => {
state.hasValue = true;
state.value = value;
const rejection = makeQuietRejection(value);
commit(rejection);
resolveCurrent(rejection);
} else {
// Persist a terminal value, or a non-terminal value
// if configured as 'mandatory' or 'opportunistic'.
Expand All @@ -349,7 +333,7 @@ const advanceDurablePublishKit = (context, value, targetStatus = 'live') => {
state.value = undefined;
}

commit(
resolveCurrent(
harden({
head: { value, done },
publishCount,
Expand Down
6 changes: 0 additions & 6 deletions packages/notifier/src/types-ambient.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,17 +155,11 @@
* enabled without test coverage.
*/

/**
* @typedef {import('@agoric/internal/src/callback.js').Callback<(publicationRecordP: Promise<PublicationRecord<*>>) => void>} PublishKitOnUpdate
*/

/**
* @typedef {object} DurablePublishKitState
*
* @property {DurablePublishKitValueDurability} valueDurability
*
* @property {PublishKitOnUpdate} [onUpdate]
*
* @property {bigint} publishCount
*
* @property {'live' | 'finished' | 'failed'} status
Expand Down
76 changes: 3 additions & 73 deletions packages/notifier/test/test-publish-kit.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import '@agoric/swingset-vat/tools/prepare-test-env.js';
import test from 'ava';
import { E } from '@endo/far';
// import { makeMethodCallback } from '@agoric/internal/src/callback.js';
import {
buildKernelBundles,
initializeSwingset,
Expand Down Expand Up @@ -39,12 +38,6 @@ const makers = {
),
};

// ava t.like does not support array shapes, but object analogs are fine
const arrayShape = sparseArr => ({
length: sparseArr.length,
...Object.fromEntries(Object.entries(sparseArr)),
});

const assertTransmission = async (t, publishKit, value, method = 'publish') => {
const { publisher, subscriber } = publishKit;
publisher[method](value);
Expand Down Expand Up @@ -284,25 +277,9 @@ test('durable publish kit upgrade trauma (full-vat integration)', async t => {
const sub1 = await run('messageVat', [
{ name: 'pubsub', methodName: 'getSubscriber' },
]);
const spyName = 'receivePublicationRecord';
const directSubscriber = await run('makeRemotable', [
'directSubscriber',
{ [spyName]: undefined },
]);
const pub2Options = {
// onUpdate: makeMethodCallback(directSubscriber, spyName),
onUpdate: { target: directSubscriber, methodName: spyName, bound: [] },
};
const { publisher: pub2 } = await run('messageVat', [
{
name: 'pubsub',
methodName: 'makeDurablePublishKit',
args: [pub2Options],
},
]);

/**
* Advances all publishers.
* Advances the publisher.
*
* @param {unknown} value
* @returns {Promise<void>}
Expand All @@ -311,9 +288,6 @@ test('durable publish kit upgrade trauma (full-vat integration)', async t => {
await run('messageVat', [
{ name: 'pubsub', methodName: 'publish', args: [value] },
]);
await run('messageVatObject', [
{ presence: pub2, methodName: 'publish', args: [value] },
]);
};

// Verify receipt of a published value.
Expand All @@ -324,33 +298,7 @@ test('durable publish kit upgrade trauma (full-vat integration)', async t => {
]);
assertCells(t, 'v1 first', [v1FirstCell], 1n, { value: value1, done: false });

// Verify receipt of published value via direct callback.
let pub2LogConsumedCount = 0;
const shiftPub2Log = async () => {
const log = await run('getLogForRemotable', [directSubscriber]);
const logTail = log.slice(pub2LogConsumedCount);
pub2LogConsumedCount = log.length;
const lastEntry = log
.filter(([methodName, _publicationRecord]) => methodName === spyName)
.at(-1);
const lastPublicationP = lastEntry[1];
const lastPublication = await run('awaitVatObject', [
{ presence: lastPublicationP },
]);
return { logTail, lastPublication };
};
const { logTail: v1Pub2FirstLog, lastPublication: v1Pub2FirstPublication } =
await shiftPub2Log();
// eslint-disable-next-line no-sparse-arrays
const v1Pub2ExpectedFirstLog = [arrayShape([spyName, ,])];
t.like(v1Pub2FirstLog, arrayShape(v1Pub2ExpectedFirstLog));
assertCells(t, 'v1 first callback', [v1Pub2FirstPublication], 1n, {
value: value1,
done: false,
});

// Verify receipt of a second published value via tail and subscribeAfter,
// and independently via direct callback.
// Verify receipt of a second published value via tail and subscribeAfter.
const value2 = Symbol.for('value2');
await publish(value2);
await run('messageVatObject', [
Expand All @@ -373,15 +321,6 @@ test('durable publish kit upgrade trauma (full-vat integration)', async t => {
{ value: value2, done: false },
{ strict: false },
);
const { logTail: v1Pub2SecondLog, lastPublication: v1Pub2SecondPublication } =
await shiftPub2Log();
// eslint-disable-next-line no-sparse-arrays
const v1Pub2ExpectedSecondLog = [arrayShape([spyName, ,])];
t.like(v1Pub2SecondLog, arrayShape(v1Pub2ExpectedSecondLog));
assertCells(t, 'v1 second callback', [v1Pub2SecondPublication], 2n, {
value: value2,
done: false,
});

// Upgrade the vat, breaking promises from v1.
await run('upgradeVat', [
Expand Down Expand Up @@ -415,7 +354,7 @@ test('durable publish kit upgrade trauma (full-vat integration)', async t => {
]);
assertCells(t, 'v2 first', [v2FirstCell], 2n, { value: value2, done: false });

// Verify receipt of published values from v2.
// Verify receipt of a published value from v2.
const value3 = Symbol.for('value3');
await publish(value3);
const v2SecondCells = [
Expand All @@ -435,15 +374,6 @@ test('durable publish kit upgrade trauma (full-vat integration)', async t => {
{ value: value3, done: false },
{ strict: false },
);
const { logTail: v2Pub2FirstLog, lastPublication: v2Pub2FirstPublication } =
await shiftPub2Log();
// eslint-disable-next-line no-sparse-arrays
const v2Pub2ExpectedFirstLog = [arrayShape([spyName, ,])];
t.like(v2Pub2FirstLog, arrayShape(v2Pub2ExpectedFirstLog));
assertCells(t, 'v2 first callback', [v2Pub2FirstPublication], 3n, {
value: value3,
done: false,
});
});

// TODO: Find a way to test virtual object rehydration
Expand Down

0 comments on commit 6861f5e

Please sign in to comment.