Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion frontend/src/components/common/Link.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ import { useQueryClient } from '@tanstack/react-query';
import React from 'react';
import { Link as RouterLink } from 'react-router-dom';
import { formatClusterPathParam, getCluster, getSelectedClusters } from '../../lib/cluster';
import { kubeObjectQueryKey, useEndpoints } from '../../lib/k8s/api/v2/hooks';
import { useEndpoints } from '../../lib/k8s/api/v2/hooks';
import { getKubeObjectClassCacheKey, kubeObjectQueryKey } from '../../lib/k8s/api/v2/queryKeys';
import type { KubeObject } from '../../lib/k8s/KubeObject';
import type { RouteURLProps } from '../../lib/router/createRouteURL';
import { createRouteURL } from '../../lib/router/createRouteURL';
Expand Down Expand Up @@ -74,6 +75,7 @@ function KubeObjectLink(props: {
endpoint,
namespace,
name,
kubeObjectClassCacheKey: getKubeObjectClassCacheKey(kubeObject._class()),
});
// prepopulate the query cache with existing object
client.setQueryData(key, kubeObject);
Expand Down
165 changes: 164 additions & 1 deletion frontend/src/lib/k8s/api/v2/hooks.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@ import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
import { renderHook, waitFor } from '@testing-library/react';
import type { ReactNode } from 'react';
import { afterEach, beforeEach, describe, expect, it, type MockedFunction, vi } from 'vitest';
import type { QueryParameters } from '../v1/queryParameters';
import { ApiError } from './ApiError';
import { clusterFetch } from './fetch';
import { useEndpoints, useKubeObject } from './hooks';
import { useEndpoints, useKubeObject, useStableCleanedQueryParams } from './hooks';
import type { KubeObjectEndpoint } from './KubeObjectEndpoint';
import { getKubeObjectClassCacheKey } from './queryKeys';

vi.mock('./fetch', () => ({
clusterFetch: vi.fn(),
Expand Down Expand Up @@ -397,3 +399,164 @@ describe('useKubeObject watch wiring', () => {
});
});
});

describe('useKubeObject class-identity cache key', () => {
beforeEach(() => vi.clearAllMocks());
afterEach(() => vi.unstubAllEnvs());

it('produces distinct query entries for two classes with the same JS name and endpoint', async () => {
vi.stubEnv('REACT_APP_ENABLE_WEBSOCKET_MULTIPLEXER', 'false');
mockClusterFetch.mockResolvedValue(
mockJsonResponse({
apiVersion: 'v1',
kind: 'Pod',
metadata: { name: 'my-pod', namespace: 'my-ns' },
})
);

const PodA = class Pod {
static apiEndpoint = {
apiInfo: [{ version: 'v1', resource: 'pods' }] as KubeObjectEndpoint[],
};
constructor(public jsonData: any, public cluster?: string) {}
} as any;
const PodB = class Pod {
static apiEndpoint = {
apiInfo: [{ version: 'v1', resource: 'pods' }] as KubeObjectEndpoint[],
};
constructor(public jsonData: any, public cluster?: string) {}
} as any;

expect(PodA.name).toBe(PodB.name);
expect(PodA).not.toBe(PodB);
expect(getKubeObjectClassCacheKey(PodA)).not.toBe(getKubeObjectClassCacheKey(PodB));

const queryClient = new QueryClient({
defaultOptions: {
queries: {
retry: false,
refetchOnWindowFocus: false,
refetchOnMount: false,
refetchOnReconnect: false,
},
},
});
const wrapper = ({ children }: { children: ReactNode }) => (
<QueryClientProvider client={queryClient}>{children}</QueryClientProvider>
);

const resultA = renderHook(
() =>
useKubeObject({
kubeObjectClass: PodA,
name: 'my-pod',
namespace: 'my-ns',
cluster: 'test',
}),
{ wrapper }
);
const resultB = renderHook(
() =>
useKubeObject({
kubeObjectClass: PodB,
name: 'my-pod',
namespace: 'my-ns',
cluster: 'test',
}),
{ wrapper }
);

await waitFor(() => {
expect(resultA.result.current.data).toBeTruthy();
expect(resultB.result.current.data).toBeTruthy();
});

expect(resultA.result.current.data).toBeInstanceOf(PodA);
expect(resultB.result.current.data).toBeInstanceOf(PodB);
expect(resultA.result.current.data).not.toBeInstanceOf(PodB);
expect(resultB.result.current.data).not.toBeInstanceOf(PodA);

const objectQueries = queryClient
.getQueryCache()
.getAll()
.filter(q => Array.isArray(q.queryKey) && q.queryKey[0] === 'object');
expect(objectQueries).toHaveLength(2);
});

it('agrees on the same discriminator across separate module instances of queryKeys', async () => {
// Surrogate for the plugin SDK shipping its own copy of `frontend/src/lib/**`:
// re-import `queryKeys` from a fresh module instance and verify that calling
// `getKubeObjectClassCacheKey(Cls)` from either instance yields the same id
// for the same class. The globalThis-stored registry under `Symbol.for(...)`
// means the second call reads the value the first call stamped, regardless
// of module identity.
const SharedCls = class Shared {} as any;
const first = getKubeObjectClassCacheKey(SharedCls);

vi.resetModules();
const fresh = await import('./queryKeys');
expect(fresh.getKubeObjectClassCacheKey).not.toBe(getKubeObjectClassCacheKey);
const second = fresh.getKubeObjectClassCacheKey(SharedCls);

expect(second).toBe(first);
});
});

describe('useStableCleanedQueryParams', () => {
it('returns the same reference when the cleaned entries are unchanged across rerenders', () => {
const { result, rerender } = renderHook<
ReturnType<typeof useStableCleanedQueryParams>,
{ qp: QueryParameters | undefined }
>(({ qp }) => useStableCleanedQueryParams(qp), {
initialProps: { qp: { labelSelector: 'a=b' } },
});
const first = result.current;

// New input object, structurally equal content → same returned reference.
rerender({ qp: { labelSelector: 'a=b' } });
expect(result.current).toBe(first);
});

it('returns the same reference regardless of key insertion order', () => {
const { result, rerender } = renderHook<
ReturnType<typeof useStableCleanedQueryParams>,
{ qp: QueryParameters | undefined }
>(({ qp }) => useStableCleanedQueryParams(qp), {
initialProps: { qp: { labelSelector: 'a=b', fieldSelector: 'metadata.name=foo' } },
});
const first = result.current;

rerender({ qp: { fieldSelector: 'metadata.name=foo', labelSelector: 'a=b' } });
expect(result.current).toBe(first);
});

it('treats undefined/empty values as equivalent to the entry being absent', () => {
const { result, rerender } = renderHook<
ReturnType<typeof useStableCleanedQueryParams>,
{ qp: QueryParameters | undefined }
>(({ qp }) => useStableCleanedQueryParams(qp), {
initialProps: { qp: { labelSelector: 'a=b' } },
});
const first = result.current;

rerender({ qp: { labelSelector: 'a=b', fieldSelector: '' } });
expect(result.current).toBe(first);

rerender({ qp: { labelSelector: 'a=b', fieldSelector: undefined } });
expect(result.current).toBe(first);
});

it('returns a new reference when an effective value actually changes', () => {
const { result, rerender } = renderHook<
ReturnType<typeof useStableCleanedQueryParams>,
{ qp: QueryParameters | undefined }
>(({ qp }) => useStableCleanedQueryParams(qp), {
initialProps: { qp: { labelSelector: 'a=b' } },
});
const first = result.current;

rerender({ qp: { labelSelector: 'a=c' } });
expect(result.current).not.toBe(first);
expect(result.current).toEqual({ labelSelector: 'a=c' });
});
});
71 changes: 48 additions & 23 deletions frontend/src/lib/k8s/api/v2/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,12 @@ import type { KubeListUpdateEvent } from './KubeList';
import { KubeObjectEndpoint } from './KubeObjectEndpoint';
import { makeUrl } from './makeUrl';
import { useWebSocket } from './multiplexer';
import {
getKubeObjectClassCacheKey,
getWebsocketMultiplexerEnabled,
kubeObjectQueryKey,
} from './queryKeys';
import { kubeRequestRetry } from './retry';
import { getWebsocketMultiplexerEnabled } from './useKubeObjectList';
import { useWebSockets } from './webSocket';

export type QueryStatus = 'pending' | 'success' | 'error';
Expand Down Expand Up @@ -82,20 +86,6 @@ export interface QueryListResponse<DataType, ItemType, ErrorType>
errors: ApiError[] | null;
}

export const kubeObjectQueryKey = ({
cluster,
endpoint,
namespace,
name,
queryParams,
}: {
cluster: string;
endpoint?: KubeObjectEndpoint | null;
namespace?: string;
name: string;
queryParams?: QueryParameters;
}) => ['object', cluster, endpoint, namespace ?? '', name, queryParams ?? {}];

/**
* Returns a single KubeObject.
*/
Expand Down Expand Up @@ -124,15 +114,19 @@ export function useKubeObject<K extends KubeObject>({
name
);

const cleanedUpQueryParams = Object.fromEntries(
Object.entries(queryParams ?? {}).filter(([, value]) => value !== undefined && value !== '')
);
const cleanedUpQueryParams = useStableCleanedQueryParams(queryParams);

const queryKey = useMemo(
() =>
kubeObjectQueryKey({ cluster, name, namespace, endpoint, queryParams: cleanedUpQueryParams }),
// eslint-disable-next-line react-hooks/exhaustive-deps
[endpoint, namespace, name]
kubeObjectQueryKey({
cluster,
name,
namespace,
endpoint,
queryParams: cleanedUpQueryParams,
kubeObjectClassCacheKey: getKubeObjectClassCacheKey(kubeObjectClass),
Comment thread
WasThatRudy marked this conversation as resolved.
}),
[cluster, endpoint, namespace, name, kubeObjectClass, cleanedUpQueryParams]
Comment thread
WasThatRudy marked this conversation as resolved.
);

const client = useQueryClient();
Expand Down Expand Up @@ -174,8 +168,7 @@ export function useKubeObject<K extends KubeObject>({
},
},
];
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [endpoint]);
}, [endpoint, namespace, name, cluster, cleanedUpQueryParams, kubeObjectClass, queryKey, client]);

const multiplexerEnabled = getWebsocketMultiplexerEnabled();

Expand Down Expand Up @@ -284,3 +277,35 @@ export const useEndpoints = (

return { endpoint, error };
};

/**
* Filters `queryParams` to drop undefined/empty values and returns a reference
* that's stable across renders when the resulting set of entries is unchanged
* (regardless of input reference identity or key insertion order). The
* stability matters for downstream effect deps (the legacy WebSocket
* connection in `useKubeObject`) that would otherwise churn on each render.
*
* Exported so the contract (stability + reference equality across reorders)
* can be exercised directly in unit tests.
*/
export function useStableCleanedQueryParams(
queryParams: QueryParameters | undefined
): QueryParameters {
const key = useMemo(
() =>
JSON.stringify(
Object.entries(queryParams ?? {})
.filter(([, value]) => value !== undefined && value !== '')
.sort(([a], [b]) => (a < b ? -1 : a > b ? 1 : 0))
),
[queryParams]
);
return useMemo(
() =>
Object.fromEntries(
Object.entries(queryParams ?? {}).filter(([, value]) => value !== undefined && value !== '')
) as QueryParameters,
// eslint-disable-next-line react-hooks/exhaustive-deps -- proxied via the stringified key above
[key]
);
}
Loading
Loading