Remove kludge from KubeApi (#6867)

* Add support for concating iterators Signed-off-by: Sebastian Malton <sebastian@malton.name> * Make clear the seperation of extenal and internal stores and apis Signed-off-by: Sebastian Malton <sebastian@malton.name> * Remove old kludge Signed-off-by: Sebastian Malton <sebastian@malton.name> * Add kludge to extension api to maintain functionality Signed-off-by: Sebastian Malton <sebastian@malton.name> * Fix imports Signed-off-by: Sebastian Malton <sebastian@malton.name> * Fix KubeApi tests Signed-off-by: Sebastian Malton <sebastian@malton.name> * Add failing test to maintain behaviour Signed-off-by: Sebastian Malton <sebastian@malton.name> * Fix tests for KubeApi Signed-off-by: Sebastian Malton <sebastian@malton.name> * Fix build Signed-off-by: Sebastian Malton <sebastian@malton.name> * Fix reactively-hide-kube-object-detail-item tests Signed-off-by: Sebastian Malton <sebastian@malton.name> * Update snapshots Signed-off-by: Sebastian Malton <sebastian@malton.name> * Update snapshots Signed-off-by: Sebastian Malton <sebastian@malton.name> * Add some technical tests Signed-off-by: Sebastian Malton <sebastian@malton.name> * More clear apiBase initialization Signed-off-by: Sebastian Malton <sebastian@malton.name> Signed-off-by: Sebastian Malton <sebastian@malton.name>
lensapp · Jan 11, 2023 · bb7bdf2 · bb7bdf2
1 parent 0ec8cbd
commit bb7bdf2
Show file tree

Hide file tree

Showing 44 changed files with 263 additions and 221 deletions.
diff --git a/src/common/k8s-api/__tests__/kube-api-version-detection.test.ts b/src/common/k8s-api/__tests__/kube-api-version-detection.test.ts
diff --git a/src/common/k8s-api/api-manager/api-manager.ts b/src/common/k8s-api/api-manager/api-manager.ts
@@ -5,11 +5,12 @@
 
 import type { KubeObjectStore } from "../kube-object.store";
 
-import { action, observable, makeObservable } from "mobx";
-import { autoBind, isDefined, iter } from "../../utils";
+import type { IComputedValue } from "mobx";
+import { autorun,  action, observable } from "mobx";
 import type { KubeApi } from "../kube-api";
-import type { KubeJsonApiDataFor, KubeObject, ObjectReference } from "../kube-object";
+import type { KubeObject, ObjectReference } from "../kube-object";
 import { parseKubeApi, createKubeApiURL } from "../kube-api-parse";
+import { chain, find } from "../../utils/iter";
 
 export type RegisterableStore<Store> = Store extends KubeObjectStore<any, any, any>
   ? Store
@@ -21,87 +22,89 @@ export type KubeObjectStoreFrom<Api> = Api extends KubeApi<infer KubeObj, infer
   ? KubeObjectStore<KubeObj, Api, ApiData>
   : never;
 
+export type FindApiCallback = (api: KubeApi<KubeObject>) => boolean;
+
+interface Dependencies {
+  readonly apis: IComputedValue<KubeApi[]>;
+  readonly stores: IComputedValue<KubeObjectStore[]>;
+}
+
 export class ApiManager {
+  private readonly externalApis = observable.array<KubeApi>();
+  private readonly externalStores = observable.array<KubeObjectStore>();
+
   private readonly apis = observable.map<string, KubeApi>();
-  private readonly stores = observable.map<string, KubeObjectStore>();
 
-  constructor() {
-    makeObservable(this);
-    autoBind(this);
+  constructor(private readonly dependencies: Dependencies) {
+    // NOTE: this is done to preserve the old behaviour of an API being discoverable using all previous apiBases
+    autorun(() => {
+      const apis = chain(this.dependencies.apis.get().values())
+        .concat(this.externalApis.values());
+      const removedApis = new Set(this.apis.values());
+
+      for (const api of apis) {
+        removedApis.delete(api);
+        this.apis.set(api.apiBase, api);
+      }
+
+      for (const api of removedApis) {
+        for (const [apiBase, storedApi] of this.apis) {
+          if (storedApi === api) {
+            this.apis.delete(apiBase);
+          }
+        }
+      }
+    });
   }
 
-  getApi(pathOrCallback: string | ((api: KubeApi<KubeObject>) => boolean)) {
-    if (typeof pathOrCallback === "string") {
-      return this.apis.get(pathOrCallback) || this.apis.get(parseKubeApi(pathOrCallback).apiBase);
+  getApi(pathOrCallback: string | FindApiCallback) {
+    if (typeof pathOrCallback === "function") {
+      return find(this.apis.values(), pathOrCallback);
     }
 
-    return iter.find(this.apis.values(), pathOrCallback ?? (() => true));
+    const { apiBase } = parseKubeApi(pathOrCallback);
+
+    return this.apis.get(apiBase);
   }
 
   getApiByKind(kind: string, apiVersion: string) {
-    return iter.find(this.apis.values(), api => api.kind === kind && api.apiVersionWithGroup === apiVersion);
+    return this.getApi(api => api.kind === kind && api.apiVersionWithGroup === apiVersion);
   }
 
   registerApi<Api>(api: RegisterableApi<Api>): void;
   /**
    * @deprecated Just register the `api` by itself
    */
   registerApi<Api>(apiBase: string, api: RegisterableApi<Api>): void;
-  registerApi<Api>(apiBaseRaw: string | RegisterableApi<Api>, apiRaw?: RegisterableApi<Api>) {
-    const api = typeof apiBaseRaw === "string"
-      ? apiRaw
-      : apiBaseRaw;
-
-    if (!api?.apiBase) {
-      return;
-    }
-
-    if (!this.apis.has(api.apiBase)) {
-      this.stores.forEach((store) => {
-        if (store.api === api) {
-          this.stores.set(api.apiBase, store);
-        }
-      });
-
-      this.apis.set(api.apiBase, api);
+  registerApi<Api>(...args: [RegisterableApi<Api>] | [string, RegisterableApi<Api>]) {
+    if (args.length === 1) {
+      this.externalApis.push(args[0]);
+    } else {
+      this.externalApis.push(args[1]);
     }
   }
 
-  protected resolveApi(api: undefined | string | KubeApi): KubeApi | undefined {
-    if (!api) {
-      return undefined;
-    }
-
-    if (typeof api === "string") {
-      return this.getApi(api);
-    }
+  unregisterApi(apiOrBase: string | KubeApi<KubeObject>) {
+    if (typeof apiOrBase === "string") {
+      const api = this.externalApis.find(api => api.apiBase === apiOrBase);
 
-    return api;
-  }
-
-  unregisterApi(api: string | KubeApi<KubeObject>) {
-    if (typeof api === "string") this.apis.delete(api);
-    else {
-      const apis = Array.from(this.apis.entries());
-      const entry = apis.find(entry => entry[1] === api);
-
-      if (entry) this.unregisterApi(entry[0]);
+      if (api) {
+        this.externalApis.remove(api);
+      }
+    } else {
+      this.unregisterApi(apiOrBase.apiBase);
     }
   }
 
   registerStore<KubeObj>(store: RegisterableStore<KubeObj>): void;
   /**
    * @deprecated KubeObjectStore's should only every be about a single KubeApi type
    */
-  registerStore<KubeObj extends KubeObject>(store: KubeObjectStore<KubeObj, KubeApi<KubeObj>, KubeJsonApiDataFor<KubeObj>>, apis: KubeApi<KubeObj>[]): void;
+  registerStore<KubeObj>(store: RegisterableStore<KubeObj>, apis: KubeApi<KubeObject>[]): void;
 
   @action
-  registerStore<KubeObj extends KubeObject>(store: KubeObjectStore<KubeObj, KubeApi<KubeObj>, KubeJsonApiDataFor<KubeObj>>, apis: KubeApi<KubeObj>[] = [store.api]): void {
-    for (const api of apis.filter(isDefined)) {
-      if (api.apiBase) {
-        this.stores.set(api.apiBase, store as never);
-      }
-    }
+  registerStore<KubeObj>(store: RegisterableStore<KubeObj>): void {
+    this.externalStores.push(store);
   }
 
   getStore(api: string | undefined): KubeObjectStore | undefined;
@@ -110,14 +113,23 @@ export class ApiManager {
    * @deprecated use an actual cast instead of hiding it with this unused type param
    */
   getStore<Store extends KubeObjectStore>(api: string | KubeApi): Store | undefined ;
-  getStore(api: string | KubeApi | undefined): KubeObjectStore | undefined {
-    const { apiBase } = this.resolveApi(api) ?? {};
+  getStore(apiOrBase: string | KubeApi | undefined): KubeObjectStore | undefined {
+    if (!apiOrBase) {
+      return undefined;
+    }
 
-    if (apiBase) {
-      return this.stores.get(apiBase);
+    const { apiBase } = typeof apiOrBase === "string"
+      ? parseKubeApi(apiOrBase)
+      : apiOrBase;
+    const api = this.getApi(apiBase);
+
+    if (!api) {
+      return undefined;
     }
 
-    return undefined;
+    return chain(this.dependencies.stores.get().values())
+      .concat(this.externalStores.values())
+      .find(store => store.api.apiBase === api.apiBase);
   }
 
   lookupApiLink(ref: ObjectReference, parentObject?: KubeObject): string {
@@ -132,7 +144,7 @@ export class ApiManager {
     const api = this.getApi(api => api.kind === kind && api.apiVersionWithGroup == apiVersion);
 
     if (api) {
-      return api.getUrl({ namespace, name });
+      return api.formatUrlForNotListing({ namespace, name });
     }
 
     // lookup api by generated resource link
@@ -151,7 +163,7 @@ export class ApiManager {
     const apiByKind = this.getApi(api => api.kind === kind);
 
     if (apiByKind) {
-      return apiByKind.getUrl({ name, namespace });
+      return apiByKind.formatUrlForNotListing({ name, namespace });
     }
 
     // otherwise generate link with default prefix

diff --git a/src/common/k8s-api/api-manager/kube-object-store-token.ts b/src/common/k8s-api/api-manager/kube-object-store-token.ts
@@ -0,0 +1,10 @@
+/**
+ * Copyright (c) OpenLens Authors. All rights reserved.
+ * Licensed under MIT License. See LICENSE in root directory for more information.
+ */
+import { getInjectionToken } from "@ogre-tools/injectable";
+import type { KubeObjectStore } from "../kube-object.store";
+
+export const kubeObjectStoreInjectionToken = getInjectionToken<KubeObjectStore<any, any, any>>({
+  id: "kube-object-store-token",
+});
diff --git a/src/common/k8s-api/api-manager/manager.injectable.ts b/src/common/k8s-api/api-manager/manager.injectable.ts
@@ -2,29 +2,28 @@
  * Copyright (c) OpenLens Authors. All rights reserved.
  * Licensed under MIT License. See LICENSE in root directory for more information.
  */
-import { getInjectable, getInjectionToken } from "@ogre-tools/injectable";
+import { getInjectable } from "@ogre-tools/injectable";
 import { storesAndApisCanBeCreatedInjectionToken } from "../stores-apis-can-be-created.token";
-import type { KubeObjectStore } from "../kube-object.store";
 import { ApiManager } from "./api-manager";
-
-export const kubeObjectStoreInjectionToken = getInjectionToken<KubeObjectStore<any, any, any>>({
-  id: "kube-object-store-token",
-});
+import { computedInjectManyInjectable } from "@ogre-tools/injectable-extension-for-mobx";
+import { kubeObjectStoreInjectionToken } from "./kube-object-store-token";
+import { kubeApiInjectionToken } from "../kube-api/kube-api-injection-token";
+import { computed } from "mobx";
 
 const apiManagerInjectable = getInjectable({
   id: "api-manager",
   instantiate: (di) => {
-    const apiManager = new ApiManager();
-
-    if (di.inject(storesAndApisCanBeCreatedInjectionToken)) {
-      const stores = di.injectMany(kubeObjectStoreInjectionToken);
-
-      for (const store of stores) {
-        apiManager.registerStore(store);
-      }
-    }
+    const computedInjectMany = di.inject(computedInjectManyInjectable);
+    const storesAndApisCanBeCreated = di.inject(storesAndApisCanBeCreatedInjectionToken);
 
-    return apiManager;
+    return new ApiManager({
+      apis: storesAndApisCanBeCreated
+        ? computedInjectMany(kubeApiInjectionToken)
+        : computed(() => []),
+      stores: storesAndApisCanBeCreated
+        ? computedInjectMany(kubeObjectStoreInjectionToken)
+        : computed(() => []),
+    });
   },
 });
 

diff --git a/src/common/k8s-api/kube-api.ts b/src/common/k8s-api/kube-api.ts
@@ -20,10 +20,9 @@ import type { Patch } from "rfc6902";
 import assert from "assert";
 import type { PartialDeep } from "type-fest";
 import type { Logger } from "../logger";
-import { Environments, getEnvironmentSpecificLegacyGlobalDiForExtensionApi } from "../../extensions/as-legacy-globals-for-extension-api/legacy-global-di-for-extension-api";
-import autoRegistrationEmitterInjectable from "./api-manager/auto-registration-emitter.injectable";
 import type AbortController from "abort-controller";
 import { matches } from "lodash/fp";
+import { makeObservable, observable } from "mobx";
 
 /**
  * The options used for creating a `KubeApi`
@@ -206,30 +205,6 @@ export interface DeleteResourceDescriptor extends ResourceDescriptor {
   propagationPolicy?: PropagationPolicy;
 }
 
-/**
- * @deprecated In the new extension API, don't expose `KubeApi`'s constructor
- */
-function legacyRegisterApi(api: KubeApi<any, any>): void {
-  try {
-    /**
-     * This function throws if called in `main`, so the `try..catch` is to make sure that doesn't
-     * leak.
-     *
-     * However, we need this code to be run in `renderer` so that the auto registering of `KubeApi`
-     * instances still works. That auto registering never worked or was applicable in `main` because
-     * there is no "single cluster" on `main`.
-     *
-     * TODO: rearchitect this design pattern in the new extension API
-     */
-    const di = getEnvironmentSpecificLegacyGlobalDiForExtensionApi(Environments.renderer);
-    const autoRegistrationEmitter = di.inject(autoRegistrationEmitterInjectable);
-
-    setImmediate(() => autoRegistrationEmitter.emit("kubeApi", api));
-  } catch {
-    // ignore error
-  }
-}
-
 export interface KubeApiDependencies {
   readonly logger: Logger;
   readonly maybeKubeApi: KubeJsonApi | undefined;
@@ -241,7 +216,9 @@ export class KubeApi<
 > {
   readonly kind: string;
   readonly apiVersion: string;
-  apiBase: string;
+
+  @observable apiBase: string;
+
   apiPrefix: string;
   apiGroup: string;
   apiVersionPreferred: string | undefined;
@@ -288,7 +265,7 @@ export class KubeApi<
     this.apiResource = resource;
     this.request = request;
     this.objectConstructor = objectConstructor;
-    legacyRegisterApi(this);
+    makeObservable(this);
   }
 
   get apiVersionWithGroup() {
@@ -347,7 +324,6 @@ export class KubeApi<
       this.apiGroup = apiGroup;
       this.apiVersionPreferred = apiVersionPreferred;
       this.apiBase = this.computeApiBase();
-      legacyRegisterApi(this);
     }
   }
 

diff --git a/src/common/utils/__tests__/iter.test.ts b/src/common/utils/__tests__/iter.test.ts
@@ -3,7 +3,7 @@
  * Licensed under MIT License. See LICENSE in root directory for more information.
  */
 
-import { join, nth, reduce } from "../iter";
+import { join, nth, reduce, concat } from "../iter";
 
 describe("iter", () => {
   describe("reduce", () => {
@@ -39,4 +39,30 @@ describe("iter", () => {
       expect(nth(["a", "b"], 0)).toBe("a");
     });
   });
+
+  describe("concat", () => {
+    it("should yield undefined for empty args", () => {
+      const iter = concat();
+
+      expect(iter.next()).toEqual({ done: true });
+    });
+
+    it("should yield undefined for only empty args", () => {
+      const iter = concat([].values(), [].values(), [].values(), [].values());
+
+      expect(iter.next()).toEqual({ done: true });
+    });
+
+    it("should yield all of the first and then all of the second", () => {
+      const iter = concat([1, 2, 3].values(), [4, 5, 6].values());
+
+      expect(iter.next()).toEqual({ done: false, value: 1 });
+      expect(iter.next()).toEqual({ done: false, value: 2 });
+      expect(iter.next()).toEqual({ done: false, value: 3 });
+      expect(iter.next()).toEqual({ done: false, value: 4 });
+      expect(iter.next()).toEqual({ done: false, value: 5 });
+      expect(iter.next()).toEqual({ done: false, value: 6 });
+      expect(iter.next()).toEqual({ done: true });
+    });
+  });
 });
diff --git a/src/common/utils/iter.ts b/src/common/utils/iter.ts
@@ -12,6 +12,7 @@ interface Iterator<T> extends Iterable<T> {
   collect<U>(fn: (values: Iterable<T>) => U): U;
   map<U>(fn: (val: T) => U): Iterator<U>;
   flatMap<U>(fn: (val: T) => U[]): Iterator<U>;
+  concat(src2: IterableIterator<T>): Iterator<T>;
   join(sep?: string): string;
 }
 
@@ -24,6 +25,7 @@ export function chain<T>(src: IterableIterator<T>): Iterator<T> {
     find: (fn) => find(src, fn),
     join: (sep) => join(src, sep),
     collect: (fn) => fn(src),
+    concat: (src2) => chain(concat(src, src2)),
     [Symbol.iterator]: () => src,
   };
 }
@@ -236,3 +238,11 @@ export function every<T>(src: Iterable<T>, fn: (val: T) => any): boolean {
 
   return true;
 }
+
+export function* concat<T>(...sources: IterableIterator<T>[]): IterableIterator<T> {
+  for (const source of sources) {
+    for (const val of source) {
+      yield val;
+    }
+  }
+}