Fix last item not being removed on initial loadAll (#5309)

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

@@ -0,0 +1,146 @@
+/**
+ * Copyright (c) OpenLens Authors. All rights reserved.
+ * Licensed under MIT License. See LICENSE in root directory for more information.
+ */
+
+import type { ClusterContext } from "../cluster-context";
+import type { KubeApi } from "../kube-api";
+import { KubeObject } from "../kube-object";
+import type { KubeObjectStoreLoadingParams } from "../kube-object.store";
+import { KubeObjectStore } from "../kube-object.store";
+
+class FakeKubeObjectStore extends KubeObjectStore<KubeObject> {
+  _context = {
+    allNamespaces: [],
+    contextNamespaces: [],
+    hasSelectedAll: false,
+  } as ClusterContext;
+
+  get context() {
+    return this._context;
+  }
+
+  constructor(private readonly _loadItems: (params: KubeObjectStoreLoadingParams) => KubeObject[], api: Partial<KubeApi<KubeObject>>) {
+    super(api as KubeApi<KubeObject>);
+  }
+
+  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, {
+      isNamespaced: true,
+    });
+
+    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);
+  });
+
+  it("should not remove an object that is not returned, if it is in a different namespace", async () => {
+    const loadItems = jest.fn();
+    const objInDefaultNamespace = new KubeObject({
+      apiVersion: "v1",
+      kind: "Foo",
+      metadata: {
+        name: "some-obj-name",
+        resourceVersion: "1",
+        uid: "some-uid",
+        namespace: "default",
+      },
+    });
+    const objNotInDefaultNamespace = new KubeObject({
+      apiVersion: "v1",
+      kind: "Foo",
+      metadata: {
+        name: "some-obj-name",
+        resourceVersion: "1",
+        uid: "some-uid",
+        namespace: "not-default",
+      },
+    });
+    const store = new FakeKubeObjectStore(loadItems, {
+      isNamespaced: true,
+    });
+
+    loadItems.mockImplementationOnce(() => [objInDefaultNamespace]);
+
+    await store.loadAll({
+      namespaces: ["default"],
+    });
+
+    expect(store.items).toContain(objInDefaultNamespace);
+
+    loadItems.mockImplementationOnce(() => [objNotInDefaultNamespace]);
+
+    await store.loadAll({
+      namespaces: ["not-default"],
+    });
+
+    expect(store.items).toContain(objInDefaultNamespace);
+  });
+
+  it("should remove all objects not returned if the api is cluster-scoped", async () => {
+    const loadItems = jest.fn();
+    const clusterScopedObject1 = new KubeObject({
+      apiVersion: "v1",
+      kind: "Foo",
+      metadata: {
+        name: "some-obj-name",
+        resourceVersion: "1",
+        uid: "some-uid",
+      },
+    });
+    const clusterScopedObject2 = new KubeObject({
+      apiVersion: "v1",
+      kind: "Foo",
+      metadata: {
+        name: "some-obj-name",
+        resourceVersion: "1",
+        uid: "some-uid",
+        namespace: "not-default",
+      },
+    });
+    const store = new FakeKubeObjectStore(loadItems, {
+      isNamespaced: false,
+    });
+
+    loadItems.mockImplementationOnce(() => [clusterScopedObject1]);
+
+    await store.loadAll({});
+
+    expect(store.items).toContain(clusterScopedObject1);
+
+    loadItems.mockImplementationOnce(() => [clusterScopedObject2]);
+
+    await store.loadAll({});
+
+    expect(store.items).not.toContain(clusterScopedObject1);
+  });
+});

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

@@ -60,10 +60,18 @@ 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
 
-  public api: KubeApi<T>;
+  public readonly api: KubeApi<T>;
   public readonly limit?: number;
   public readonly bufferSize: number = 50000;
   @observable private loadedNamespaces?: string[];
@@ -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());
+    if (merge && this.api.isNamespaced) {
+      const ns = new Set(namespaces);
 
       items = [
-        ...this.items.filter(existingItem => !namespaces.includes(existingItem.getNs())),
+        ...this.items.filter(existingItem => !ns.has(existingItem.getNs())),
         ...partialItems,
       ];
     }