Ensure that CatalogEntity.getName() and CatalogEntity.getId() are always used (#4763)

2025-05-20 05:10:56 +00:00 · 2022-01-27 14:42:19 -05:00 · 2022-01-27 14:42:19 -05:00 · 8d8491a035
commit 8d8491a035
parent 1cac3ca74c
17 changed files with 98 additions and 166 deletions
--- a/src/common/tests/hotbar-store.test.ts
+++ b/src/common/tests/hotbar-store.test.ts
@ -4,45 +4,61 @@
 */

 import { anyObject } from "jest-mock-extended";
-import { merge } from "lodash";
 import mockFs from "mock-fs";
 import logger from "../../main/logger";
 import type { CatalogEntity, CatalogEntityData, CatalogEntityKindData } from "../catalog";
 import { HotbarStore } from "../hotbar-store";
 import { getDiForUnitTesting } from "../../main/getDiForUnitTesting";
-import directoryForUserDataInjectable
-  from "../app-paths/directory-for-user-data/directory-for-user-data.injectable";
+import directoryForUserDataInjectable from "../app-paths/directory-for-user-data/directory-for-user-data.injectable";

 jest.mock("../../main/catalog/catalog-entity-registry", () => ({
  catalogEntityRegistry: {
    items: [
-      {
+      getMockCatalogEntity({
+        apiVersion: "v1",
+        kind: "Cluster",
+        status: {
+          phase: "Running",
+        },
        metadata: {
          uid: "1dfa26e2ebab15780a3547e9c7fa785c",
          name: "mycluster",
          source: "local",
+          labels: {},
        },
+      }),
+      getMockCatalogEntity({
+        apiVersion: "v1",
+        kind: "Cluster",
+        status: {
+          phase: "Running",
        },
-      {
        metadata: {
          uid: "55b42c3c7ba3b04193416cda405269a5",
          name: "my_shiny_cluster",
          source: "remote",
+          labels: {},
        },
+      }),
+      getMockCatalogEntity({
+        apiVersion: "v1",
+        kind: "Cluster",
+        status: {
+          phase: "Running",
        },
-      {
        metadata: {
          uid: "catalog-entity",
          name: "Catalog",
          source: "app",
+          labels: {},
        },
-      },
+      }),
    ],
  },
 }));

 function getMockCatalogEntity(data: Partial<CatalogEntityData> & CatalogEntityKindData): CatalogEntity {
-  return merge(data, {
+  return {
    getName: jest.fn(() => data.metadata?.name),
    getId: jest.fn(() => data.metadata?.uid),
    getSource: jest.fn(() => data.metadata?.source ?? "unknown"),
@ -52,7 +68,8 @@ function getMockCatalogEntity(data: Partial<CatalogEntityData> & CatalogEntityKi
    metadata: {},
    spec: {},
    status: {},
-  }) as CatalogEntity;
+    ...data,
+  } as CatalogEntity;
 }

 const testCluster = getMockCatalogEntity({
--- a/src/common/catalog-entities/kubernetes-cluster.ts
+++ b/src/common/catalog-entities/kubernetes-cluster.ts
@ -67,22 +67,22 @@ export class KubernetesCluster extends CatalogEntity<KubernetesClusterMetadata,

  async connect(): Promise<void> {
    if (app) {
-      await ClusterStore.getInstance().getById(this.metadata.uid)?.activate();
+      await ClusterStore.getInstance().getById(this.getId())?.activate();
    } else {
-      await requestClusterActivation(this.metadata.uid, false);
+      await requestClusterActivation(this.getId(), false);
    }
  }

  async disconnect(): Promise<void> {
    if (app) {
-      ClusterStore.getInstance().getById(this.metadata.uid)?.disconnect();
+      ClusterStore.getInstance().getById(this.getId())?.disconnect();
    } else {
-      await requestClusterDisconnection(this.metadata.uid, false);
+      await requestClusterDisconnection(this.getId(), false);
    }
  }

  async onRun(context: CatalogEntityActionContext) {
-    context.navigate(`/cluster/${this.metadata.uid}`);
+    context.navigate(`/cluster/${this.getId()}`);
  }

  onDetailsOpen(): void {
@ -100,7 +100,7 @@ export class KubernetesCluster extends CatalogEntity<KubernetesClusterMetadata,
        icon: "settings",
        onClick: () => broadcastMessage(
          IpcRendererNavigationEvents.NAVIGATE_IN_APP,
-          `/entity/${this.metadata.uid}/settings`,
+          `/entity/${this.getId()}/settings`,
        ),
      });
    }
@ -111,14 +111,14 @@ export class KubernetesCluster extends CatalogEntity<KubernetesClusterMetadata,
        context.menuItems.push({
          title: "Disconnect",
          icon: "link_off",
-          onClick: () => requestClusterDisconnection(this.metadata.uid),
+          onClick: () => requestClusterDisconnection(this.getId()),
        });
        break;
      case LensKubernetesClusterStatus.DISCONNECTED:
        context.menuItems.push({
          title: "Connect",
          icon: "link",
-          onClick: () => context.navigate(`/cluster/${this.metadata.uid}`),
+          onClick: () => context.navigate(`/cluster/${this.getId()}`),
        });
        break;
    }
--- a/src/common/catalog-entities/web-link.ts
+++ b/src/common/catalog-entities/web-link.ts
@ -38,9 +38,9 @@ export class WebLink extends CatalogEntity<CatalogEntityMetadata, WebLinkStatus,
      context.menuItems.push({
        title: "Delete",
        icon: "delete",
-        onClick: async () => WeblinkStore.getInstance().removeById(this.metadata.uid),
+        onClick: async () => WeblinkStore.getInstance().removeById(this.getId()),
        confirm: {
-          message: `Remove Web Link "${this.metadata.name}" from ${productName}?`,
+          message: `Remove Web Link "${this.getName()}" from ${productName}?`,
        },
      });
    }
--- a/src/common/catalog/catalog-entity.ts
+++ b/src/common/catalog/catalog-entity.ts
@ -315,11 +315,24 @@ export abstract class CatalogEntity<
  @observable status: Status;
  @observable spec: Spec;

-  constructor(data: CatalogEntityData<Metadata, Status, Spec>) {
+  constructor({ metadata, status, spec }: CatalogEntityData<Metadata, Status, Spec>) {
    makeObservable(this);
-    this.metadata = data.metadata;
-    this.status = data.status;
-    this.spec = data.spec;
+
+    if (!metadata || typeof metadata !== "object") {
+      throw new TypeError("CatalogEntity's metadata must be a defined object");
+    }
+
+    if (!status || typeof status !== "object") {
+      throw new TypeError("CatalogEntity's status must be a defined object");
+    }
+
+    if (!spec || typeof spec !== "object") {
+      throw new TypeError("CatalogEntity's spec must be a defined object");
+    }
+
+    this.metadata = metadata;
+    this.status = status;
+    this.spec = spec;
  }

  /**
--- a/src/common/hotbar-store.ts
+++ b/src/common/hotbar-store.ts
@ -154,28 +154,28 @@ export class HotbarStore extends BaseStore<HotbarStoreModel> {
  @action
  addToHotbar(item: CatalogEntity, cellIndex?: number) {
    const hotbar = this.getActive();
-    const uid = item.metadata?.uid;
-    const name = item.metadata?.name;
+    const uid = item.getId();
+    const name = item.getName();

    if (typeof uid !== "string") {
-      throw new TypeError("CatalogEntity.metadata.uid must be a string");
+      throw new TypeError("CatalogEntity's ID must be a string");
    }

    if (typeof name !== "string") {
-      throw new TypeError("CatalogEntity.metadata.name must be a string");
+      throw new TypeError("CatalogEntity's NAME must be a string");
    }

-    const newItem = { entity: {
-      uid,
-      name,
-      source: item.metadata.source,
-    }};
-
-
    if (this.isAddedToActive(item)) {
      return;
    }

+    const entity = {
+      uid,
+      name,
+      source: item.metadata.source,
+    };
+    const newItem = { entity };
+
    if (cellIndex === undefined) {
      // Add item to empty cell
      const emptyCellIndex = hotbar.items.indexOf(null);
@ -278,11 +278,14 @@ export class HotbarStore extends BaseStore<HotbarStoreModel> {
  }

  /**
-   * Checks if entity already pinned to hotbar
-   * @returns boolean
+   * Checks if entity already pinned to the active hotbar
   */
-  isAddedToActive(entity: CatalogEntity) {
-    return !!this.getActive().items.find(item => item?.entity.uid === entity.metadata.uid);
+  isAddedToActive(entity: CatalogEntity | null | undefined): boolean {
+    if (!entity) {
+      return false;
+    }
+
+    return this.getActive().items.findIndex(item => item?.entity.uid === entity.getId()) >= 0;
  }

  getDisplayLabel(hotbar: Hotbar): string {
--- a/src/main/catalog/catalog-entity-registry.ts
+++ b/src/main/catalog/catalog-entity-registry.ts
@ -36,7 +36,7 @@ export class CatalogEntityRegistry {
  }

  getById<T extends CatalogEntity>(id: string): T | undefined {
-    return this.items.find((entity) => entity.metadata.uid === id) as T | undefined;
+    return this.items.find(entity => entity.getId() === id) as T | undefined;
  }

  getItemsForApiKind<T extends CatalogEntity>(apiVersion: string, kind: string): T[] {
--- a/src/main/cluster-manager.ts
+++ b/src/main/cluster-manager.ts
@ -85,7 +85,7 @@ export class ClusterManager extends Singleton {
  }

  protected updateEntityFromCluster(cluster: Cluster) {
-    const index = catalogEntityRegistry.items.findIndex((entity) => entity.metadata.uid === cluster.id);
+    const index = catalogEntityRegistry.items.findIndex((entity) => entity.getId() === cluster.id);

    if (index === -1) {
      return;
@ -169,11 +169,11 @@ export class ClusterManager extends Singleton {
  @action
  protected syncClustersFromCatalog(entities: KubernetesCluster[]) {
    for (const entity of entities) {
-      const cluster = this.store.getById(entity.metadata.uid);
+      const cluster = this.store.getById(entity.getId());

      if (!cluster) {
        const model = {
-          id: entity.metadata.uid,
+          id: entity.getId(),
          kubeConfigPath: entity.spec.kubeconfigPath,
          contextName: entity.spec.kubeconfigContext,
          accessibleNamespaces: entity.spec.accessibleNamespaces ?? [],
--- a/src/migrations/hotbar-store/5.0.0-beta.5.ts
+++ b/src/migrations/hotbar-store/5.0.0-beta.5.ts
@ -16,7 +16,7 @@ export default {
    for (const hotbar of hotbars) {
      for (let i = 0; i < hotbar.items.length; i += 1) {
        const item = hotbar.items[i];
-        const entity = catalogEntityRegistry.items.find((entity) => entity.metadata.uid === item?.entity.uid);
+        const entity = catalogEntityRegistry.items.find((entity) => entity.getId() === item?.entity.uid);

        if (!entity) {
          // Clear disabled item
--- a/src/renderer/api/catalog-entity-registry.ts
+++ b/src/renderer/api/catalog-entity-registry.ts
@ -120,7 +120,7 @@ export class CatalogEntityRegistry {
      const entity = this.categoryRegistry.getEntityForData(item);

      if (entity) {
-        this._entities.set(entity.metadata.uid, entity);
+        this._entities.set(entity.getId(), entity);
      } else {
        this.rawEntities.push(item);
      }
--- a/src/renderer/components/+catalog/catalog-entity-item.tsx
+++ b/src/renderer/components/+catalog/catalog-entity-item.tsx
@ -1,100 +0,0 @@
-/**
- * Copyright (c) OpenLens Authors. All rights reserved.
- * Licensed under MIT License. See LICENSE in root directory for more information.
- */
-import styles from "./catalog.module.scss";
-import React from "react";
-import { action, computed } from "mobx";
-import { CatalogEntity } from "../../api/catalog-entity";
-import type { ItemObject } from "../../../common/item.store";
-import { Badge } from "../badge";
-import { navigation } from "../../navigation";
-import { searchUrlParam } from "../input";
-import { KubeObject } from "../../../common/k8s-api/kube-object";
-import type { CatalogEntityRegistry } from "../../api/catalog-entity-registry";
-
-export class CatalogEntityItem<T extends CatalogEntity> implements ItemObject {
-  constructor(public entity: T, private registry: CatalogEntityRegistry) {
-    if (!(entity instanceof CatalogEntity)) {
-      throw Object.assign(new TypeError("CatalogEntityItem cannot wrap a non-CatalogEntity type"), { typeof: typeof entity, prototype: Object.getPrototypeOf(entity) });
-    }
-  }
-
-  get kind() {
-    return this.entity.kind;
-  }
-
-  get apiVersion() {
-    return this.entity.apiVersion;
-  }
-
-  get name() {
-    return this.entity.metadata.name;
-  }
-
-  getName() {
-    return this.entity.metadata.name;
-  }
-
-  get id() {
-    return this.entity.metadata.uid;
-  }
-
-  getId() {
-    return this.id;
-  }
-
-  @computed get phase() {
-    return this.entity.status.phase;
-  }
-
-  get enabled() {
-    return this.entity.status.enabled ?? true;
-  }
-
-  get labels() {
-    return KubeObject.stringifyLabels(this.entity.metadata.labels);
-  }
-
-  getLabelBadges(onClick?: React.MouseEventHandler<any>) {
-    return this.labels
-      .map(label => (
-        <Badge
-          scrollable
-          className={styles.badge}
-          key={label}
-          label={label}
-          title={label}
-          onClick={(event) => {
-            navigation.searchParams.set(searchUrlParam.name, label);
-            onClick?.(event);
-            event.stopPropagation();
-          }}
-          expandable={false}
-        />
-      ));
-  }
-
-  get source() {
-    return this.entity.metadata.source || "unknown";
-  }
-
-  get searchFields() {
-    return [
-      this.name,
-      this.id,
-      this.phase,
-      `source=${this.source}`,
-      ...this.labels,
-    ];
-  }
-
-  onRun() {
-    this.registry.onRun(this.entity);
-  }
-
-  @action
-  async onContextMenuOpen(ctx: any) {
-    return this.entity.onContextMenuOpen(ctx);
-  }
-}
--- a/src/renderer/components/+entity-settings/entity-settings.tsx
+++ b/src/renderer/components/+entity-settings/entity-settings.tsx
@ -81,14 +81,14 @@ export class EntitySettings extends React.Component<Props> {
      <>
        <div className="flex items-center pb-8">
          <Avatar
-            title={this.entity.metadata.name}
-            colorHash={`${this.entity.metadata.name}-${this.entity.metadata.source}`}
+            title={this.entity.getName()}
+            colorHash={`${this.entity.getName()}-${this.entity.metadata.source}`}
            src={this.entity.spec.icon?.src}
            className={styles.settingsAvatar}
            size={40}
          />
          <div className={styles.entityName}>
-            {this.entity.metadata.name}
+            {this.entity.getName()}
          </div>
        </div>
        <Tabs className="flex column" scrollable={false} onChange={this.onTabChange} value={this.activeTab}>
--- a/src/renderer/components/cluster-settings/cluster-settings.tsx
+++ b/src/renderer/components/cluster-settings/cluster-settings.tsx
@ -11,7 +11,7 @@ import type { CatalogEntity } from "../../api/catalog-entity";
 import * as components from "./components";

 function getClusterForEntity(entity: CatalogEntity) {
-  return ClusterStore.getInstance().getById(entity.metadata.uid);
+  return ClusterStore.getInstance().getById(entity.getId());
 }

 export function GeneralSettings({ entity }: EntitySettingViewProps) {
--- a/src/renderer/components/cluster-settings/components/cluster-icon-settings.tsx
+++ b/src/renderer/components/cluster-settings/components/cluster-icon-settings.tsx
@ -75,8 +75,8 @@ export class ClusterIconSetting extends React.Component<Props> {
              accept="image/*"
              label={
                <Avatar
-                  colorHash={`${entity.metadata.name}-${entity.metadata.source}`}
-                  title={entity.metadata.name}
+                  colorHash={`${entity.getName()}-${entity.metadata.source}`}
+                  title={entity.getName()}
                  src={entity.spec.icon?.src}
                  size={53}
                />
--- a/src/renderer/components/cluster-settings/components/cluster-name-setting.tsx
+++ b/src/renderer/components/cluster-settings/components/cluster-name-setting.tsx
@ -29,7 +29,7 @@ export class ClusterNameSetting extends React.Component<Props> {
  componentDidMount() {
    disposeOnUnmount(this,
      autorun(() => {
-        this.name = this.props.cluster.preferences.clusterName || this.props.entity.metadata.name;
+        this.name = this.props.cluster.preferences.clusterName || this.props.entity.getName();
      }),
    );
  }
--- a/src/renderer/components/hotbar/hotbar-entity-icon.tsx
+++ b/src/renderer/components/hotbar/hotbar-entity-icon.tsx
@ -73,7 +73,7 @@ export class HotbarEntityIcon extends React.Component<Props> {

    menuItems.unshift({
      title: "Remove from Hotbar",
-      onClick: () => this.props.remove(this.props.entity.metadata.uid),
+      onClick: () => this.props.remove(this.props.entity.getId()),
    });

    this.contextMenu.menuItems = menuItems;
@ -90,8 +90,8 @@ export class HotbarEntityIcon extends React.Component<Props> {

    return (
      <HotbarIcon
-        uid={entity.metadata.uid}
-        title={entity.metadata.name}
+        uid={entity.getId()}
+        title={entity.getName()}
        source={entity.metadata.source}
        src={entity.spec.icon?.src}
        material={entity.spec.icon?.material}
@ -101,7 +101,7 @@ export class HotbarEntityIcon extends React.Component<Props> {
        onMenuOpen={() => this.onMenuOpen()}
        disabled={!entity}
        menuItems={this.contextMenu.menuItems}
-        tooltip={`${entity.metadata.name} (${entity.metadata.source})`}
+        tooltip={`${entity.getName()} (${entity.metadata.source})`}
        {...elemProps}
      >
        { this.ledIcon }
--- a/src/renderer/components/layout/sidebar-cluster.tsx
+++ b/src/renderer/components/layout/sidebar-cluster.tsx
@ -73,7 +73,7 @@ export function SidebarCluster({ clusterEntity }: { clusterEntity: CatalogEntity
      ? "Remove from Hotbar"
      : "Add to Hotbar";
    const onClick = isAddedToActive
-      ? () => hotbarStore.removeFromHotbar(metadata.uid)
+      ? () => hotbarStore.removeFromHotbar(clusterEntity.getId())
      : () => hotbarStore.addToHotbar(clusterEntity);

    contextMenu.menuItems = [{ title, onClick }];
@ -92,8 +92,7 @@ export function SidebarCluster({ clusterEntity }: { clusterEntity: CatalogEntity
    setOpened(!opened);
  };

-  const { metadata, spec } = clusterEntity;
-  const id = `cluster-${metadata.uid}`;
+  const id = `cluster-${clusterEntity.getId()}`;
  const tooltipId = `tooltip-${id}`;

  return (
@ -106,17 +105,17 @@ export function SidebarCluster({ clusterEntity }: { clusterEntity: CatalogEntity
      data-testid="sidebar-cluster-dropdown"
    >
      <Avatar
-        title={metadata.name}
-        colorHash={`${metadata.name}-${metadata.source}`}
+        title={clusterEntity.getName()}
+        colorHash={`${clusterEntity.getName()}-${clusterEntity.metadata.source}`}
        size={40}
-        src={spec.icon?.src}
+        src={clusterEntity.spec.icon?.src}
        className={styles.avatar}
      />
      <div className={styles.clusterName} id={tooltipId}>
-        {metadata.name}
+        {clusterEntity.getName()}
      </div>
      <Tooltip targetId={tooltipId}>
-        {metadata.name}
+        {clusterEntity.getName()}
      </Tooltip>
      <Icon material="arrow_drop_down" className={styles.dropdown}/>
      <Menu
--- a/src/renderer/initializers/catalog.tsx
+++ b/src/renderer/initializers/catalog.tsx
@ -50,7 +50,7 @@ export function initCatalog({ openCommandDialog }: Dependencies) {
        context.menuItems.push({
          title: "Delete",
          icon: "delete",
-          onClick: () => onClusterDelete(entity.metadata.uid),
+          onClick: () => onClusterDelete(entity.getId()),
        });
      }
    });