Add file-meta GC coverage

Author: lukemelia

packages/host/app/lib/gc-card-store.ts

@@ -127,11 +127,11 @@ export default class CardStoreWithGarbageCollection implements CardStore {
   // importantly these properties are not tracked so that we are able
   // to deserialize an instance without glimmer rendering the inner workings of
   // the deserialization process.
-  #nonTrackedCards = new Map<string, StoredInstance>();
-  #nonTrackedCardErrors = new Map<string, CardErrorJSONAPI>();
+  #nonTrackedInstances = new Map<string, StoredInstance>();
+  #nonTrackedInstanceErrors = new Map<string, CardErrorJSONAPI>();
 
   #cards = new TrackedMap<string, StoredInstance>();
-  #cardErrors = new TrackedMap<string, CardErrorJSONAPI>();
+  #instanceErrors = new TrackedMap<string, CardErrorJSONAPI>();
   #gcCandidates: Set<LocalId> = new Set();
   #referenceCount: ReferenceCount;
   #idResolver = new IDResolver();
@@ -299,9 +299,9 @@ export default class CardStoreWithGarbageCollection implements CardStore {
 
   reset() {
     this.#cards.clear();
-    this.#clearEphemeralErrors(this.#cardErrors);
-    this.#nonTrackedCards.clear();
-    this.#clearEphemeralErrors(this.#nonTrackedCardErrors);
+    this.#clearEphemeralErrors(this.#instanceErrors);
+    this.#nonTrackedInstances.clear();
+    this.#clearEphemeralErrors(this.#nonTrackedInstanceErrors);
     this.#gcCandidates.clear();
     this.#cardDocsInFlight.clear();
     this.#fileMetaDocsInFlight.clear();
@@ -334,20 +334,26 @@ export default class CardStoreWithGarbageCollection implements CardStore {
   sweep(api: typeof CardAPI) {
     let dependencyGraph = this.makeDependencyGraph(api);
     let reachable = new Set<string>();
-    let visited = new WeakSet<CardDef>();
+    let visited = new WeakSet<StoredInstance>();
     let rootLocalIds: string[] = [];
 
     for (let instance of this.#cards.values()) {
-      if (!instance || !isCardInstance(instance)) {
+      if (!instance || visited.has(instance)) {
         continue;
       }
-      if (visited.has(instance)) {
+      visited.add(instance);
+      if (isCardInstance(instance)) {
+        let localId = instance[localIdSymbol];
+        if (this.hasReferences(localId)) {
+          rootLocalIds.push(localId);
+        }
         continue;
       }
-      visited.add(instance);
-      let localId = instance[localIdSymbol];
-      if (this.hasReferences(localId)) {
-        rootLocalIds.push(localId);
+      if (isFileDefInstance(instance)) {
+        let fileId = instance.id;
+        if (fileId && this.hasReferences(fileId)) {
+          reachable.add(fileId);
+        }
       }
     }
 
@@ -367,49 +373,57 @@ export default class CardStoreWithGarbageCollection implements CardStore {
       }
     }
 
-    visited = new WeakSet<CardDef>();
+    visited = new WeakSet<StoredInstance>();
     for (let instance of this.#cards.values()) {
-      if (!instance || !isCardInstance(instance)) {
-        continue;
-      }
-      if (visited.has(instance)) {
+      if (!instance || visited.has(instance)) {
         continue;
       }
       visited.add(instance);
-      let localId = instance[localIdSymbol];
-      if (!reachable.has(localId)) {
-        if (this.#gcCandidates.has(localId)) {
+      let gcId: string | undefined;
+      let extraDeleteIds: string[] = [];
+      if (isCardInstance(instance)) {
+        gcId = instance[localIdSymbol];
+        if (instance.id) {
+          extraDeleteIds.push(instance.id);
+        }
+      } else if (isFileDefInstance(instance)) {
+        gcId = instance.id;
+      }
+      if (!gcId) {
+        continue; // we should alwyays have a gcId by this point, but this helps TypeScript know that
+      }
+      if (!reachable.has(gcId)) {
+        if (this.#gcCandidates.has(gcId)) {
           destroy(instance);
-          // brand the instance to make it easier for debugging
           (instance as unknown as any)[
             Symbol.for('__instance_detached_from_store')
           ] = true;
-          this.delete(localId);
-          if (instance.id) {
-            this.delete(instance.id);
+          this.delete(gcId);
+          for (let id of extraDeleteIds) {
+            this.delete(id);
           }
         } else {
-          this.#gcCandidates.add(localId);
+          this.#gcCandidates.add(gcId);
         }
       } else {
-        this.#gcCandidates.delete(localId);
+        this.#gcCandidates.delete(gcId);
       }
     }
   }
 
   makeTracked(remoteId: string) {
     remoteId = remoteId.replace(/\.json$/, '');
-    let instance = this.#nonTrackedCards.get(remoteId);
+    let instance = this.#nonTrackedInstances.get(remoteId);
     if (instance) {
       this.setItem(remoteId, instance);
     }
-    this.#nonTrackedCards.delete(remoteId);
+    this.#nonTrackedInstances.delete(remoteId);
 
-    let error = this.#nonTrackedCardErrors.get(remoteId);
+    let error = this.#nonTrackedInstanceErrors.get(remoteId);
     if (error) {
       this.addInstanceOrError(remoteId, error);
     }
-    this.#nonTrackedCardErrors.delete(remoteId);
+    this.#nonTrackedInstanceErrors.delete(remoteId);
   }
 
   consumersOf(api: typeof CardAPI, instance: CardDef) {
@@ -431,9 +445,9 @@ export default class CardStoreWithGarbageCollection implements CardStore {
   private deleteFromAll(id: string) {
     id = id.replace(/\.json$/, '');
     this.#cards.delete(id);
-    this.#cardErrors.delete(id);
-    this.#nonTrackedCards.delete(id);
-    this.#nonTrackedCardErrors.delete(id);
+    this.#instanceErrors.delete(id);
+    this.#nonTrackedInstances.delete(id);
+    this.#nonTrackedInstanceErrors.delete(id);
   }
 
   private getItem(type: 'instance', id: string): StoredInstance | undefined;
@@ -465,9 +479,11 @@ export default class CardStoreWithGarbageCollection implements CardStore {
     item: StoredInstance | CardErrorJSONAPI | undefined;
     localId: string | undefined;
   } {
-    let bucket = type === 'instance' ? this.#cards : this.#cardErrors;
+    let bucket = type === 'instance' ? this.#cards : this.#instanceErrors;
     let silentBucket =
-      type === 'instance' ? this.#nonTrackedCards : this.#nonTrackedCardErrors;
+      type === 'instance'
+        ? this.#nonTrackedInstances
+        : this.#nonTrackedInstanceErrors;
     let localId = isLocalId(localOrRemoteId) ? localOrRemoteId : undefined;
     let remoteId = !isLocalId(localOrRemoteId) ? localOrRemoteId : undefined;
     let item: StoredInstance | CardErrorJSONAPI | undefined;
@@ -505,10 +521,10 @@ export default class CardStoreWithGarbageCollection implements CardStore {
     notTracked?: true,
   ) {
     id = id.replace(/\.json$/, '');
-    let cardBucket = notTracked ? this.#nonTrackedCards : this.#cards;
+    let cardBucket = notTracked ? this.#nonTrackedInstances : this.#cards;
     let errorBucket = notTracked
-      ? this.#nonTrackedCardErrors
-      : this.#cardErrors;
+      ? this.#nonTrackedInstanceErrors
+      : this.#instanceErrors;
     if (!isLocalId(id) && isCardInstance(item)) {
       this.#idResolver.addIdPair(item[localIdSymbol], id);
     } else if (!isLocalId(id)) {
@@ -569,10 +585,18 @@ export default class CardStoreWithGarbageCollection implements CardStore {
     }
   }
 
-  private hasReferences(localId: string): boolean {
-    let referenceCount = this.#referenceCount.get(localId) ?? 0;
-    for (let remoteId of this.#idResolver.getRemoteIds(localId)) {
-      referenceCount += this.#referenceCount.get(remoteId) ?? 0;
+  private hasReferences(id: string): boolean {
+    let idsToCheck = new Set<string>([id]);
+    let localId = isLocalId(id) ? id : this.#idResolver.getLocalId(id);
+    if (localId) {
+      idsToCheck.add(localId);
+      for (let remoteId of this.#idResolver.getRemoteIds(localId)) {
+        idsToCheck.add(remoteId);
+      }
+    }
+    let referenceCount = 0;
+    for (let refId of idsToCheck) {
+      referenceCount += this.#referenceCount.get(refId) ?? 0;
     }
     return referenceCount > 0;
   }

packages/host/tests/integration/store-test.gts

@@ -377,6 +377,44 @@ module('Integration | Store', function (hooks) {
     );
   });
 
+  test('file-meta instance is retained when referenced', async function (assert) {
+    await testRealm.write('hero.png', 'mock hero image');
+    let fileUrl = `${testRealmURL}hero.png`;
+
+    let fileInstance = await storeService.get(fileUrl);
+    assert.ok(fileInstance, 'file meta instance is loaded');
+    assert.ok(
+      (fileInstance as any).constructor?.isFileDef,
+      'file meta instance is a FileDef',
+    );
+
+    storeService.addReference(fileUrl);
+    forceGC();
+
+    let peekedInstance = cardStore.get(fileUrl) as unknown;
+    assert.strictEqual(
+      peekedInstance,
+      fileInstance,
+      'file meta instance is retained after GC with reference',
+    );
+  });
+
+  test('file-meta instance is garbage collected when references drop', async function (assert) {
+    await testRealm.write('hero.png', 'mock hero image');
+    let fileUrl = `${testRealmURL}hero.png`;
+
+    await storeService.get(fileUrl);
+    storeService.addReference(fileUrl);
+    storeService.dropReference(fileUrl);
+    forceGC();
+
+    assert.strictEqual(
+      cardStore.get(fileUrl),
+      undefined,
+      'file meta instance is garbage collected after reference drop',
+    );
+  });
+
   test('garbage collects cards that only consume each other', async function (assert) {
     let alpha = new PersonDef({ name: 'Alpha' });
     let beta = new PersonDef({ name: 'Beta' });