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

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
+import directoryForUserDataInjectable from "../app-paths/directory-for-user-data/directory-for-user-data.injectable";
-  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({

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;
     }

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}?`,
         },
       });
     }

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;
+    if (!metadata || typeof metadata !== "object") {
-    this.spec = data.spec;
+      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;
   }
 
   /**

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

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[] {

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 ?? [],

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

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);
       }

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);
-  }
-}

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}
+            title={this.entity.getName()}
-            colorHash={`${this.entity.metadata.name}-${this.entity.metadata.source}`}
+            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}>

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) {

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}`}
+                  colorHash={`${entity.getName()}-${entity.metadata.source}`}
-                  title={entity.metadata.name}
+                  title={entity.getName()}
                   src={entity.spec.icon?.src}
                   size={53}
                 />

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();
       }),
     );
   }

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}
+        uid={entity.getId()}
-        title={entity.metadata.name}
+        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 }

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-${clusterEntity.getId()}`;
-  const id = `cluster-${metadata.uid}`;
   const tooltipId = `tooltip-${id}`;
 
   return (
@@ -106,17 +105,17 @@ export function SidebarCluster({ clusterEntity }: { clusterEntity: CatalogEntity
       data-testid="sidebar-cluster-dropdown"
     >
       <Avatar
-        title={metadata.name}
+        title={clusterEntity.getName()}
-        colorHash={`${metadata.name}-${metadata.source}`}
+        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

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()),
         });
       }
     });