fix(daemon): Publish removal of name mappings during pet store renames

While consolidating id and name change publications in the pet store
during a refactor, it was discovered that the removal event for
overwritten pet names was omitted. This event is now published as
expected, and a test is added for the same.

packages/daemon/src/pet-store.js

@@ -27,6 +27,40 @@ export const makePetStoreMaker = (filePowers, locator) => {
     /** @type {Map<string, ReturnType<typeof makeIdChangeTopic>>} */
     const idsToTopics = new Map();
 
+    /**
+     * Publishes an id change to its subscribers, if any.
+     *
+     * @param {string} id - The id to publish a change for.
+     * @param {import('./types.js').PetStoreIdDiff} payload - The payload to publish.
+     */
+    const publishIdChangeToSubscribers = (id, payload) => {
+      const idTopic = idsToTopics.get(id);
+      if (idTopic !== undefined) {
+        idTopic.publisher.next(payload);
+      }
+    };
+
+    /**
+     * @param {string} id - The id receiving a name new name.
+     * @param {string} petName - The new name.
+     */
+    const publishNameAddition = (id, petName) => {
+      const idRecord = parseId(id);
+      nameChangesTopic.publisher.next({ add: petName, value: idRecord });
+      publishIdChangeToSubscribers(id, { add: idRecord, names: [petName] });
+    };
+
+    /**
+     * @param {string} id - The id from which a name is being removed.
+     * @param {string} petName - The removed name.
+     */
+    const publishNameRemoval = (id, petName) => {
+      nameChangesTopic.publisher.next({ remove: petName });
+      if (id !== undefined) {
+        publishIdChangeToSubscribers(id, { remove: parseId(id), names: [petName] });
+      }
+    };
+
     /** @param {string} petName */
     const read = async petName => {
       const petNamePath = filePowers.joinPath(petNameDirectoryPath, petName);
@@ -73,7 +107,7 @@ export const makePetStoreMaker = (filePowers, locator) => {
         if (oldFormulaIdentifier !== undefined) {
           // Perform cleanup on the overwritten pet name.
           idsToPetNames.delete(oldFormulaIdentifier, petName);
-          nameChangesTopic.publisher.next({ remove: petName });
+          publishNameRemoval(oldFormulaIdentifier, petName);
         }
       }
 
@@ -82,22 +116,7 @@ export const makePetStoreMaker = (filePowers, locator) => {
       const petNamePath = filePowers.joinPath(petNameDirectoryPath, petName);
       const petNameText = `${formulaIdentifier}\n`;
       await filePowers.writeFileText(petNamePath, petNameText);
-      nameChangesTopic.publisher.next({
-        add: petName,
-        value: parseId(formulaIdentifier),
-      });
-    };
-
-    /**
-     * @param {string} petName
-     * @returns {import('./types.js').IdRecord}
-     */
-    const idRecordForName = petName => {
-      const formulaIdentifier = idsToPetNames.getKey(petName);
-      if (formulaIdentifier === undefined) {
-        throw new Error(`Formula does not exist for pet name ${q(petName)}`);
-      }
-      return parseId(formulaIdentifier);
+      publishNameAddition(formulaIdentifier, petName);
     };
 
     /** @type {import('./types.js').PetStore['list']} */
@@ -107,9 +126,13 @@ export const makePetStoreMaker = (filePowers, locator) => {
     const followNames = async function* currentAndSubsequentNames() {
       const subscription = nameChangesTopic.subscribe();
       for (const name of idsToPetNames.getAll().sort()) {
+        const idRecord = parseId(
+          /** @type {string} */ (idsToPetNames.getKey(name)),
+        );
+
         yield /** @type {import('./types.js').PetStoreNameDiff} */ ({
           add: name,
-          value: idRecordForName(name),
+          value: idRecord,
         });
       }
       yield* subscription;
@@ -148,7 +171,7 @@ export const makePetStoreMaker = (filePowers, locator) => {
       const petNamePath = filePowers.joinPath(petNameDirectoryPath, petName);
       await filePowers.removePath(petNamePath);
       idsToPetNames.delete(formulaIdentifier, petName);
-      nameChangesTopic.publisher.next({ remove: petName });
+      publishNameRemoval(formulaIdentifier, petName);
       // TODO consider retaining a backlog of deleted names for recovery
       // TODO consider tracking historical pet names for formulas
     };
@@ -168,9 +191,6 @@ export const makePetStoreMaker = (filePowers, locator) => {
         );
       }
       assertValidId(formulaIdentifier, fromName);
-      if (overwrittenId !== undefined) {
-        assertValidId(overwrittenId, toName);
-      }
 
       const fromPath = filePowers.joinPath(petNameDirectoryPath, fromName);
       const toPath = filePowers.joinPath(petNameDirectoryPath, toName);
@@ -179,16 +199,14 @@ export const makePetStoreMaker = (filePowers, locator) => {
       // Delete the back-reference for the overwritten pet name if it existed.
       if (overwrittenId !== undefined) {
         idsToPetNames.delete(overwrittenId, toName);
+        publishNameRemoval(overwrittenId, toName);
       }
 
       // Update the mapping for the pet name.
       idsToPetNames.add(formulaIdentifier, toName);
 
-      nameChangesTopic.publisher.next({
-        add: toName,
-        value: parseId(formulaIdentifier),
-      });
-      nameChangesTopic.publisher.next({ remove: fromName });
+      publishNameRemoval(formulaIdentifier, fromName);
+      publishNameAddition(formulaIdentifier, toName);
       // TODO consider retaining a backlog of overwritten names for recovery
     };
 

packages/daemon/test/test-endo.js

@@ -639,9 +639,30 @@ test('followNames publishes renamed names', async t => {
   await E(host).move(['ten'], ['zehn']);
 
   let { value } = await changesIterator.next();
+  t.is(value.remove, 'ten');
+  value = (await changesIterator.next()).value;
   t.is(value.add, 'zehn');
+});
+
+test('followNames publishes renamed names (existing mappings for both names)', async t => {
+  const { cancelled, locator } = await prepareLocator(t);
+  const { host } = await makeHost(locator, cancelled);
+
+  const changesIterator = await prepareFollowChangesIterator(host);
+
+  await E(host).evaluate('MAIN', '10', [], [], 'ten');
+  await changesIterator.next();
+  await E(host).evaluate('MAIN', '"german 10"', [], [], 'zehn');
+  await changesIterator.next();
+
+  await E(host).move(['ten'], ['zehn']);
+
+  let { value } = await changesIterator.next();
+  t.is(value.remove, 'zehn');
   value = (await changesIterator.next()).value;
   t.is(value.remove, 'ten');
+  value = (await changesIterator.next()).value;
+  t.is(value.add, 'zehn');
 });
 
 test('followNames does not notify of redundant pet store writes', async t => {