diff --git a/superset-frontend/src/extensions/ExtensionsManager.ts b/superset-frontend/src/extensions/ExtensionsManager.ts index 49de8d705b9e..00b447819d0d 100644 --- a/superset-frontend/src/extensions/ExtensionsManager.ts +++ b/superset-frontend/src/extensions/ExtensionsManager.ts @@ -16,12 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { - FeatureFlag, - SupersetClient, - isFeatureEnabled, - logging, -} from '@superset-ui/core'; +import { SupersetClient, logging } from '@superset-ui/core'; import type { contributions, core } from '@apache-superset/core'; import { ExtensionContext } from '../core/models'; @@ -62,9 +57,6 @@ class ExtensionsManager { * @throws Error if initialization fails. */ public async initializeExtensions(): Promise { - if (!isFeatureEnabled(FeatureFlag.EnableExtensions)) { - return; - } const response = await SupersetClient.get({ endpoint: '/api/v1/extensions/', }); diff --git a/superset-frontend/src/extensions/ExtensionsStartup.test.tsx b/superset-frontend/src/extensions/ExtensionsStartup.test.tsx index 02010a4e5b9e..d1af44098fe2 100644 --- a/superset-frontend/src/extensions/ExtensionsStartup.test.tsx +++ b/superset-frontend/src/extensions/ExtensionsStartup.test.tsx @@ -17,10 +17,21 @@ * under the License. */ import { render, waitFor } from 'spec/helpers/testing-library'; -import { logging } from '@superset-ui/core'; +import { logging, FeatureFlag, isFeatureEnabled } from '@superset-ui/core'; +import fetchMock from 'fetch-mock'; import ExtensionsStartup from './ExtensionsStartup'; import ExtensionsManager from './ExtensionsManager'; +// Mock the isFeatureEnabled function +jest.mock('@superset-ui/core', () => ({ + ...jest.requireActual('@superset-ui/core'), + isFeatureEnabled: jest.fn(), +})); + +const mockIsFeatureEnabled = isFeatureEnabled as jest.MockedFunction< + typeof isFeatureEnabled +>; + const mockInitialState = { user: { userId: 1 }, }; @@ -36,12 +47,26 @@ beforeEach(() => { // Clear any existing ExtensionsManager instance (ExtensionsManager as any).instance = undefined; + + // Reset feature flag mock to enabled by default + mockIsFeatureEnabled.mockReset(); + mockIsFeatureEnabled.mockReturnValue(true); + + // Setup fetch mocks for API calls + fetchMock.restore(); + fetchMock.get('glob:*/api/v1/extensions/', { + result: [], + }); }); afterEach(() => { // Clean up after each test delete (window as any).superset; (ExtensionsManager as any).instance = undefined; + + // Reset mocks + mockIsFeatureEnabled.mockReset(); + fetchMock.restore(); }); test('renders without crashing', () => { @@ -55,6 +80,12 @@ test('renders without crashing', () => { }); test('sets up global superset object when user is logged in', async () => { + // Mock initializeExtensions to avoid API calls in this test + const manager = ExtensionsManager.getInstance(); + const initializeSpy = jest + .spyOn(manager, 'initializeExtensions') + .mockImplementation(() => Promise.resolve()); + render(, { useRedux: true, initialState: mockInitialState, @@ -70,6 +101,8 @@ test('sets up global superset object when user is logged in', async () => { expect((window as any).superset.extensions).toBeDefined(); expect((window as any).superset.sqlLab).toBeDefined(); }); + + initializeSpy.mockRestore(); }); test('does not set up global superset object when user is not logged in', async () => { @@ -85,18 +118,26 @@ test('does not set up global superset object when user is not logged in', async }); test('initializes ExtensionsManager when user is logged in', async () => { + // Mock initializeExtensions to avoid API calls, but track that it was called + const manager = ExtensionsManager.getInstance(); + const initializeSpy = jest + .spyOn(manager, 'initializeExtensions') + .mockImplementation(() => Promise.resolve()); + render(, { useRedux: true, initialState: mockInitialState, }); await waitFor(() => { - // Verify ExtensionsManager has been initialized by checking if it has extensions loaded - const manager = ExtensionsManager.getInstance(); + // Verify ExtensionsManager initialization was called + expect(initializeSpy).toHaveBeenCalledTimes(1); // The manager should exist and be ready to use expect(manager).toBeDefined(); expect(manager.getExtensions).toBeDefined(); }); + + initializeSpy.mockRestore(); }); test('does not initialize ExtensionsManager when user is not logged in', async () => { @@ -114,36 +155,47 @@ test('does not initialize ExtensionsManager when user is not logged in', async ( }); }); -test('handles ExtensionsManager initialization errors gracefully', async () => { - const errorSpy = jest.spyOn(logging, 'error').mockImplementation(); +test('only initializes once even with multiple renders', async () => { + // Track calls to the manager's public API + const manager = ExtensionsManager.getInstance(); + const originalInitialize = manager.initializeExtensions; + let initializeCallCount = 0; - // Mock the initializeExtensions method to throw an error - const originalInitialize = ExtensionsManager.prototype.initializeExtensions; - ExtensionsManager.prototype.initializeExtensions = jest - .fn() - .mockImplementation(() => { - throw new Error('Test initialization error'); - }); + manager.initializeExtensions = jest.fn().mockImplementation(() => { + initializeCallCount += 1; + return Promise.resolve(); + }); - render(, { + const { rerender } = render(, { useRedux: true, initialState: mockInitialState, }); await waitFor(() => { - // Verify error was logged - expect(errorSpy).toHaveBeenCalledWith( - 'Error setting up extensions:', - expect.any(Error), - ); + expect(initializeCallCount).toBe(1); }); + // Re-render the component + rerender(); + + // Wait for any potential async operations, but expect no additional calls + await waitFor(() => { + expect(initializeCallCount).toBe(1); + }); + + // Verify initialization is still only called once + expect(initializeCallCount).toBe(1); + // Restore original method - ExtensionsManager.prototype.initializeExtensions = originalInitialize; - errorSpy.mockRestore(); + manager.initializeExtensions = originalInitialize; }); -test('logs success message when ExtensionsManager initializes successfully', async () => { +test('initializes ExtensionsManager and logs success when EnableExtensions feature flag is enabled', async () => { + // Ensure feature flag is enabled + mockIsFeatureEnabled.mockImplementation( + (flag: FeatureFlag) => flag === FeatureFlag.EnableExtensions, + ); + const infoSpy = jest.spyOn(logging, 'info').mockImplementation(); // Mock the initializeExtensions method to succeed @@ -158,6 +210,14 @@ test('logs success message when ExtensionsManager initializes successfully', asy }); await waitFor(() => { + // Verify feature flag was checked + expect(mockIsFeatureEnabled).toHaveBeenCalledWith( + FeatureFlag.EnableExtensions, + ); + // Verify initialization was called + expect( + ExtensionsManager.prototype.initializeExtensions, + ).toHaveBeenCalledTimes(1); // Verify success message was logged expect(infoSpy).toHaveBeenCalledWith( 'Extensions initialized successfully.', @@ -169,37 +229,66 @@ test('logs success message when ExtensionsManager initializes successfully', asy infoSpy.mockRestore(); }); -test('only initializes once even with multiple renders', async () => { - // Track calls to the manager's public API - const manager = ExtensionsManager.getInstance(); - const originalInitialize = manager.initializeExtensions; - let initializeCallCount = 0; +test('does not initialize ExtensionsManager when EnableExtensions feature flag is disabled', async () => { + // Disable the feature flag + mockIsFeatureEnabled.mockReturnValue(false); - manager.initializeExtensions = jest.fn().mockImplementation(() => { - initializeCallCount += 1; - return Promise.resolve(); - }); + const manager = ExtensionsManager.getInstance(); + const initializeSpy = jest + .spyOn(manager, 'initializeExtensions') + .mockImplementation(); - const { rerender } = render(, { + render(, { useRedux: true, initialState: mockInitialState, }); await waitFor(() => { - expect(initializeCallCount).toBe(1); + // Verify feature flag was checked + expect(mockIsFeatureEnabled).toHaveBeenCalledWith( + FeatureFlag.EnableExtensions, + ); + // Verify the global superset object is still set up + expect((window as any).superset).toBeDefined(); + // But extensions should not be initialized + expect(initializeSpy).not.toHaveBeenCalled(); }); - // Re-render the component - rerender(); + initializeSpy.mockRestore(); +}); - // Wait for any potential async operations, but expect no additional calls - await waitFor(() => { - expect(initializeCallCount).toBe(1); +test('logs error when ExtensionsManager initialization fails', async () => { + // Ensure feature flag is enabled + mockIsFeatureEnabled.mockReturnValue(true); + + const errorSpy = jest.spyOn(logging, 'error').mockImplementation(); + + // Mock the initializeExtensions method to throw an error + const originalInitialize = ExtensionsManager.prototype.initializeExtensions; + ExtensionsManager.prototype.initializeExtensions = jest + .fn() + .mockImplementation(() => { + throw new Error('Test initialization error'); + }); + + render(, { + useRedux: true, + initialState: mockInitialState, }); - // Verify initialization is still only called once - expect(initializeCallCount).toBe(1); + await waitFor(() => { + // Verify feature flag was checked + expect(mockIsFeatureEnabled).toHaveBeenCalledWith( + FeatureFlag.EnableExtensions, + ); + // Verify error was logged + expect(errorSpy).toHaveBeenCalledWith( + 'Error setting up extensions:', + expect.any(Error), + ); + }); // Restore original method - manager.initializeExtensions = originalInitialize; + ExtensionsManager.prototype.initializeExtensions = originalInitialize; + errorSpy.mockRestore(); }); diff --git a/superset-frontend/src/extensions/ExtensionsStartup.tsx b/superset-frontend/src/extensions/ExtensionsStartup.tsx index 2706bb73121f..41bb842a89be 100644 --- a/superset-frontend/src/extensions/ExtensionsStartup.tsx +++ b/superset-frontend/src/extensions/ExtensionsStartup.tsx @@ -19,7 +19,7 @@ import { useEffect, useState } from 'react'; // eslint-disable-next-line no-restricted-syntax import * as supersetCore from '@apache-superset/core'; -import { logging } from '@superset-ui/core'; +import { FeatureFlag, isFeatureEnabled, logging } from '@superset-ui/core'; import { authentication, core, @@ -75,14 +75,15 @@ const ExtensionsStartup = () => { }; // Initialize extensions - try { - ExtensionsManager.getInstance().initializeExtensions(); - logging.info('Extensions initialized successfully.'); - } catch (error) { - logging.error('Error setting up extensions:', error); - } finally { - setInitialized(true); + if (isFeatureEnabled(FeatureFlag.EnableExtensions)) { + try { + ExtensionsManager.getInstance().initializeExtensions(); + logging.info('Extensions initialized successfully.'); + } catch (error) { + logging.error('Error setting up extensions:', error); + } } + setInitialized(true); }, [initialized, userId]); return null;