Fix last item not being removed on initial loadAll

- Add a unit test to cover this case

Signed-off-by: Sebastian Malton <sebastian@malton.name>

src/common/k8s-api/__tests__/kube-object.store.test.ts

@@ -0,0 +1,56 @@
+/**
+ * Copyright (c) OpenLens Authors. All rights reserved.
+ * Licensed under MIT License. See LICENSE in root directory for more information.
+ */
+
+import { when } from "mobx";
+import { KubeObject } from "../kube-object";
+import type { KubeObjectStoreLoadingParams } from "../kube-object.store";
+import { KubeObjectStore } from "../kube-object.store";
+
+class FakeKubeObjectStore extends KubeObjectStore<KubeObject> {
+  get contextReady() {
+    return when(() => true);
+  }
+
+  constructor(private readonly _loadItems: (params: KubeObjectStoreLoadingParams) => KubeObject[]) {
+    super();
+  }
+
+  async loadItems(params: KubeObjectStoreLoadingParams) {
+    return this._loadItems(params);
+  }
+}
+
+describe("KubeObjectStore", () => {
+  it("should remove an object from the list of items after it is not returned from listing the same namespace again", async () => {
+    const loadItems = jest.fn();
+    const obj = new KubeObject({
+      apiVersion: "v1",
+      kind: "Foo",
+      metadata: {
+        name: "some-obj-name",
+        resourceVersion: "1",
+        uid: "some-uid",
+        namespace: "default",
+      },
+    });
+    const store = new FakeKubeObjectStore(loadItems);
+
+    loadItems.mockImplementationOnce(() => [obj]);
+
+    await store.loadAll({
+      namespaces: ["default"],
+    });
+
+    expect(store.items).toContain(obj);
+
+    loadItems.mockImplementationOnce(() => []);
+
+    await store.loadAll({
+      namespaces: ["default"],
+    });
+
+    expect(store.items).not.toContain(obj);
+  });
+});

src/common/k8s-api/kube-object.store.ts

@@ -60,6 +60,14 @@ export interface KubeObjectStoreSubscribeParams {
   abortController?: AbortController;
 }
 
+export interface MergeItemsOptions {
+  merge?: boolean;
+  updateStore?: boolean;
+  sort?: boolean;
+  filter?: boolean;
+  namespaces: string[];
+}
+
 export abstract class KubeObjectStore<T extends KubeObject> extends ItemStore<T> {
   static defaultContext = observable.box<ClusterContext>(); // TODO: support multiple cluster contexts
 
@@ -227,7 +235,7 @@ export abstract class KubeObjectStore<T extends KubeObject> extends ItemStore<T>
   }
 
   @action
-  async loadAll({ namespaces, merge = true, reqInit, onLoadFailure }: KubeObjectStoreLoadAllParams = {}): Promise<void | T[]> {
+  async loadAll({ namespaces, merge = true, reqInit, onLoadFailure }: KubeObjectStoreLoadAllParams = {}): Promise<undefined | T[]> {
     await this.contextReady;
     namespaces ??= this.context.contextNamespaces;
     this.isLoading = true;
@@ -235,7 +243,7 @@ export abstract class KubeObjectStore<T extends KubeObject> extends ItemStore<T>
     try {
       const items = await this.loadItems({ namespaces, reqInit, onLoadFailure });
 
-      this.mergeItems(items, { merge });
+      this.mergeItems(items, { merge, namespaces });
 
       this.isLoaded = true;
       this.failedLoading = false;
@@ -248,29 +256,31 @@ export abstract class KubeObjectStore<T extends KubeObject> extends ItemStore<T>
     } finally {
       this.isLoading = false;
     }
+
+    return undefined;
   }
 
   @action
-  async reloadAll(opts: { force?: boolean; namespaces?: string[]; merge?: boolean } = {}) {
+  async reloadAll(opts: { force?: boolean; namespaces?: string[]; merge?: boolean } = {}): Promise<undefined | T[]> {
     const { force = false, ...loadingOptions } = opts;
 
     if (this.isLoading || (this.isLoaded && !force)) {
-      return;
+      return undefined;
     }
 
     return this.loadAll(loadingOptions);
   }
 
   @action
-  protected mergeItems(partialItems: T[], { merge = true, updateStore = true, sort = true, filter = true } = {}): T[] {
+  protected mergeItems(partialItems: T[], { merge = true, updateStore = true, sort = true, filter = true, namespaces }: MergeItemsOptions): T[] {
     let items = partialItems;
 
     // update existing items
     if (merge) {
-      const namespaces = partialItems.map(item => item.getNs());
+      const ns = new Set(namespaces);
 
       items = [
-        ...this.items.filter(existingItem => !namespaces.includes(existingItem.getNs())),
+        ...this.items.filter(existingItem => !ns.has(existingItem.getNs())),
         ...partialItems,
       ];
     }