diff --git a/frontend/packages/console-shared/src/utils/__tests__/console-fetch.spec.ts b/frontend/packages/console-shared/src/utils/__tests__/console-fetch.spec.ts index 527ee36306a..79a7a752714 100644 --- a/frontend/packages/console-shared/src/utils/__tests__/console-fetch.spec.ts +++ b/frontend/packages/console-shared/src/utils/__tests__/console-fetch.spec.ts @@ -103,6 +103,32 @@ describe('consoleFetch', () => { ).rejects.not.toBeInstanceOf(RetryError); }); + it('does not console.warn when fetch is aborted via AbortController', async () => { + const consoleSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + const controller = new AbortController(); + window.fetch = jest.fn(() => + Promise.reject(new DOMException('The operation was aborted.', 'AbortError')), + ); + + await expect( + coFetch('/api/lifecycle/test', { signal: controller.signal }), + ).rejects.toMatchObject({ name: 'AbortError' }); + expect(consoleSpy).not.toHaveBeenCalled(); + consoleSpy.mockRestore(); + }); + + it('does console.warn when fetch fails with a real network error', async () => { + const consoleSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + window.fetch = jest.fn(() => Promise.reject(new TypeError('Network failure'))); + + await expect(coFetch('/api/lifecycle/test')).rejects.toThrow('Network failure'); + expect(consoleSpy).toHaveBeenCalledWith( + expect.stringContaining('/api/lifecycle/test'), + expect.any(TypeError), + ); + consoleSpy.mockRestore(); + }); + it('should retry up to 3 times when RetryError is thrown', async () => { window.fetch = jest.fn(() => Promise.resolve({ status: 404, headers: emptyHeaders } as Response), diff --git a/frontend/packages/console-shared/src/utils/console-fetch.ts b/frontend/packages/console-shared/src/utils/console-fetch.ts index 7adf9a5cf55..d6fedcdb246 100644 --- a/frontend/packages/console-shared/src/utils/console-fetch.ts +++ b/frontend/packages/console-shared/src/utils/console-fetch.ts @@ -39,6 +39,8 @@ export const coFetch: ConsoleFetch = async (url, options = {}, timeout = 60000) } catch (e) { if (e instanceof RetryError) { retry = true; + } else if (e instanceof DOMException && e.name === 'AbortError') { + throw e; // expected cancellation — don't warn } else { // eslint-disable-next-line no-console console.warn(`consoleFetch failed for url ${url}`, e); diff --git a/frontend/packages/operator-lifecycle-manager/src/components/clusterserviceversion.tsx b/frontend/packages/operator-lifecycle-manager/src/components/clusterserviceversion.tsx index d7a5de3124e..b97656eb2fe 100644 --- a/frontend/packages/operator-lifecycle-manager/src/components/clusterserviceversion.tsx +++ b/frontend/packages/operator-lifecycle-manager/src/components/clusterserviceversion.tsx @@ -166,8 +166,8 @@ const managedNamespacesColumnClass = css('pf-m-hidden', 'pf-m-visible-on-sm'); const statusColumnClass = css('pf-m-hidden', 'pf-m-visible-on-lg'); const lastUpdatedColumnClass = css('pf-m-hidden', 'pf-m-visible-on-2xl'); const providedAPIsColumnClass = css('pf-m-hidden', 'pf-m-visible-on-xl'); -const clusterCompatibilityColumnClass = css('pf-m-hidden', 'pf-m-visible-on-xl'); -const supportPhaseColumnClass = css('pf-m-hidden', 'pf-m-visible-on-xl'); +const clusterCompatibilityColumnClass = css('pf-m-hidden', 'pf-m-visible-on-xl', 'pf-m-width-15'); +const supportPhaseColumnClass = css('pf-m-hidden', 'pf-m-visible-on-xl', 'pf-m-width-15'); const SubscriptionStatus: FC<{ muted?: boolean; subscription: SubscriptionKind }> = ({ muted = false, diff --git a/frontend/packages/operator-lifecycle-manager/src/hooks/__tests__/useOperatorLifecycle.spec.ts b/frontend/packages/operator-lifecycle-manager/src/hooks/__tests__/useOperatorLifecycle.spec.ts new file mode 100644 index 00000000000..0ecad10ca07 --- /dev/null +++ b/frontend/packages/operator-lifecycle-manager/src/hooks/__tests__/useOperatorLifecycle.spec.ts @@ -0,0 +1,114 @@ +import { act, renderHook } from '@testing-library/react'; +import { coFetchJSON } from '@console/shared/src/utils/console-fetch'; +import { useOperatorLifecycle } from '../useOperatorLifecycle'; + +jest.mock('@console/shared/src/utils/console-fetch', () => ({ + coFetchJSON: jest.fn(), +})); +const mockCoFetchJSON = (coFetchJSON as unknown) as jest.Mock; + +// Each test uses a unique packageName to avoid sharing module-level cache entries. +const CATALOG = 'redhat-operators'; +const NS = 'openshift-marketplace'; + +describe('useOperatorLifecycle', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('returns data when fetch succeeds', async () => { + const mockData = { lifecycleSchema: 'test', properties: '{}' }; + mockCoFetchJSON.mockResolvedValueOnce(mockData); + + const { result } = renderHook(() => useOperatorLifecycle('pkg-success', CATALOG, NS)); + + expect(result.current[1]).toBe(true); // loading initially + + await act(async () => { + await Promise.resolve(); + }); + + const [data, loading, error] = result.current; + expect(data).toEqual(mockData); + expect(loading).toBe(false); + expect(error).toBeNull(); + }); + + it('surfaces real network errors as errors', async () => { + const networkError = new Error('Network failure'); + mockCoFetchJSON.mockRejectedValueOnce(networkError); + + const { result } = renderHook(() => useOperatorLifecycle('pkg-network-error', CATALOG, NS)); + + await act(async () => { + await Promise.resolve(); + await Promise.resolve(); + }); + + const [data, loading, error] = result.current; + expect(error).toBe(networkError); + expect(data).toBeNull(); + expect(loading).toBe(false); + }); + + it('skips fetch when packageName is missing', () => { + renderHook(() => useOperatorLifecycle(undefined, CATALOG, NS)); + + expect(mockCoFetchJSON).not.toHaveBeenCalled(); + const [data, loading, error] = renderHook(() => + useOperatorLifecycle(undefined, CATALOG, NS), + ).result.current; + expect(data).toBeNull(); + expect(loading).toBe(false); + expect(error).toBeNull(); + }); + + it('multiple callers sharing the same cache key all receive data from a single fetch', async () => { + // Core deduplication invariant: N components for the same operator should never + // issue more than one in-flight request. + const sharedData = { lifecycleSchema: 'shared', properties: '{}' }; + mockCoFetchJSON.mockResolvedValueOnce(sharedData); + + const PKG = 'pkg-shared-dedup'; + + // All three mount and subscribe to the same in-flight promise + const { result: resultA } = renderHook(() => useOperatorLifecycle(PKG, CATALOG, NS)); + const { result: resultB } = renderHook(() => useOperatorLifecycle(PKG, CATALOG, NS)); + const { result: resultC } = renderHook(() => useOperatorLifecycle(PKG, CATALOG, NS)); + + await act(async () => { + await Promise.resolve(); + await Promise.resolve(); + }); + + expect(resultA.current[0]).toEqual(sharedData); + expect(resultB.current[0]).toEqual(sharedData); + expect(resultC.current[0]).toEqual(sharedData); + // Only one fetch regardless of how many components share the key + expect(mockCoFetchJSON).toHaveBeenCalledTimes(1); + }); + + it('a component mounting after the cache is populated serves data immediately without fetching', async () => { + const cachedData = { lifecycleSchema: 'cached', properties: '{}' }; + mockCoFetchJSON.mockResolvedValueOnce(cachedData); + + const PKG = 'pkg-cache-hit'; + + // First component populates the cache + const { result: result1 } = renderHook(() => useOperatorLifecycle(PKG, CATALOG, NS)); + + await act(async () => { + await Promise.resolve(); + }); + + expect(result1.current[0]).toEqual(cachedData); + expect(mockCoFetchJSON).toHaveBeenCalledTimes(1); + + // Second component — cache hit, no new fetch + const { result: result2 } = renderHook(() => useOperatorLifecycle(PKG, CATALOG, NS)); + expect(result2.current[0]).toEqual(cachedData); + expect(result2.current[1]).toBe(false); // not loading + expect(result2.current[2]).toBeNull(); + expect(mockCoFetchJSON).toHaveBeenCalledTimes(1); // still just one fetch + }); +}); diff --git a/frontend/packages/operator-lifecycle-manager/src/hooks/useOperatorLifecycle.ts b/frontend/packages/operator-lifecycle-manager/src/hooks/useOperatorLifecycle.ts index 8426da67ee5..2a7a2cf51d4 100644 --- a/frontend/packages/operator-lifecycle-manager/src/hooks/useOperatorLifecycle.ts +++ b/frontend/packages/operator-lifecycle-manager/src/hooks/useOperatorLifecycle.ts @@ -1,4 +1,4 @@ -import { useEffect, useRef, useState } from 'react'; +import { useEffect, useState } from 'react'; import { coFetchJSON } from '@console/shared/src/utils/console-fetch'; import type { LifecycleData } from '../components/operator-lifecycle-status'; import { DEFAULT_SOURCE_NAMESPACE } from '../const'; @@ -41,46 +41,28 @@ const extractPackageName = (olmProperties: string): string | undefined => { } }; -const fetchLifecycleData = ( - cacheKey: string, - url: string, - signal: AbortSignal, -): Promise => { +// Fetch lifecycle data and deduplicate concurrent requests via a module-level cache. +// Requests are intentionally not cancelled on component unmount: the response +// populates the cache so any later render of the same operator avoids a re-fetch, +// and React silently ignores state updates after unmount. +const fetchLifecycleData = (cacheKey: string, url: string): Promise => { const existing = lifecycleCache.get(cacheKey); if (existing?.promise) { - return existing.promise; + return existing.promise; // deduplicate: all callers share the one in-flight request } - const promise = coFetchJSON(url, 'GET', { signal }).then( + const promise = coFetchJSON(url, 'GET', {}).then( (result: LifecycleData) => { - lifecycleCache.set(cacheKey, { - data: result, - error: null, - timestamp: Date.now(), - }); + lifecycleCache.set(cacheKey, { data: result, error: null, timestamp: Date.now() }); return result; }, (err: Error) => { - if (err.name === 'AbortError') { - lifecycleCache.delete(cacheKey); - } else { - lifecycleCache.set(cacheKey, { - data: null, - error: err, - timestamp: Date.now(), - }); - } + lifecycleCache.set(cacheKey, { data: null, error: err, timestamp: Date.now() }); throw err; }, ); - lifecycleCache.set(cacheKey, { - data: null, - error: null, - timestamp: Date.now(), - promise, - }); - + lifecycleCache.set(cacheKey, { data: null, error: null, timestamp: Date.now(), promise }); return promise; }; @@ -92,7 +74,6 @@ export const useOperatorLifecycle = ( const [data, setData] = useState(null); const [loading, setLoading] = useState(false); const [error, setError] = useState(null); - const abortRef = useRef(); useEffect(() => { if (!packageName || !catalogName || !catalogNamespace) { @@ -110,35 +91,25 @@ export const useOperatorLifecycle = ( return undefined; } - abortRef.current?.abort(); - const controller = new AbortController(); - abortRef.current = controller; - setLoading(true); const url = `/api/olm/lifecycle/${encodeURIComponent(catalogNamespace)}/${encodeURIComponent( catalogName, )}/${encodeURIComponent(packageName)}`; - fetchLifecycleData(cacheKey, url, controller.signal) + fetchLifecycleData(cacheKey, url) .then((result) => { - if (!controller.signal.aborted) { - setData(result); - setError(null); - setLoading(false); - } + setData(result); + setError(null); + setLoading(false); }) .catch((err: Error) => { - if (!controller.signal.aborted) { - setData(null); - setError(err); - setLoading(false); - } + setData(null); + setError(err); + setLoading(false); }); - return () => { - controller.abort(); - }; + return undefined; // no cleanup: requests complete naturally }, [packageName, catalogName, catalogNamespace]); return [data, loading, error];