Extensions: Fix recursions when determining plugin deps (#97295)
* fix(extensions): stop recursion when determining plugin deps * chore: update the names of test case
This commit is contained in:
@@ -17,6 +17,7 @@ import {
|
|||||||
getExposedComponentPluginDependencies,
|
getExposedComponentPluginDependencies,
|
||||||
getAppPluginConfigs,
|
getAppPluginConfigs,
|
||||||
getAppPluginIdFromExposedComponentId,
|
getAppPluginIdFromExposedComponentId,
|
||||||
|
getAppPluginDependencies,
|
||||||
} from './utils';
|
} from './utils';
|
||||||
|
|
||||||
jest.mock('app/features/plugins/pluginSettings', () => ({
|
jest.mock('app/features/plugins/pluginSettings', () => ({
|
||||||
@@ -879,4 +880,93 @@ describe('Plugin Extensions / Utils', () => {
|
|||||||
expect(appPluginIds).toEqual(['myorg-second-app', 'myorg-fourth-app', 'myorg-fifth-app']);
|
expect(appPluginIds).toEqual(['myorg-second-app', 'myorg-fourth-app', 'myorg-fifth-app']);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('getAppPluginDependencies()', () => {
|
||||||
|
const originalApps = config.apps;
|
||||||
|
const genereicAppPluginConfig = {
|
||||||
|
path: '',
|
||||||
|
version: '',
|
||||||
|
preload: false,
|
||||||
|
angular: {
|
||||||
|
detected: false,
|
||||||
|
hideDeprecation: false,
|
||||||
|
},
|
||||||
|
loadingStrategy: PluginLoadingStrategy.fetch,
|
||||||
|
dependencies: {
|
||||||
|
grafanaVersion: '8.0.0',
|
||||||
|
plugins: [],
|
||||||
|
extensions: {
|
||||||
|
exposedComponents: [],
|
||||||
|
},
|
||||||
|
},
|
||||||
|
extensions: {
|
||||||
|
addedLinks: [],
|
||||||
|
addedComponents: [],
|
||||||
|
exposedComponents: [],
|
||||||
|
extensionPoints: [],
|
||||||
|
},
|
||||||
|
};
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
config.apps = originalApps;
|
||||||
|
});
|
||||||
|
|
||||||
|
test('should not end up in an infinite loop if there are circular dependencies', () => {
|
||||||
|
config.apps = {
|
||||||
|
'myorg-first-app': {
|
||||||
|
...genereicAppPluginConfig,
|
||||||
|
id: 'myorg-first-app',
|
||||||
|
},
|
||||||
|
'myorg-second-app': {
|
||||||
|
...genereicAppPluginConfig,
|
||||||
|
id: 'myorg-second-app',
|
||||||
|
dependencies: {
|
||||||
|
...genereicAppPluginConfig.dependencies,
|
||||||
|
extensions: {
|
||||||
|
exposedComponents: ['myorg-third-app/link/v1'],
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
'myorg-third-app': {
|
||||||
|
...genereicAppPluginConfig,
|
||||||
|
id: 'myorg-third-app',
|
||||||
|
dependencies: {
|
||||||
|
...genereicAppPluginConfig.dependencies,
|
||||||
|
extensions: {
|
||||||
|
exposedComponents: ['myorg-second-app/link/v1'],
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
};
|
||||||
|
|
||||||
|
const appPluginIds = getAppPluginDependencies('myorg-second-app');
|
||||||
|
|
||||||
|
expect(appPluginIds).toEqual(['myorg-third-app']);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('should not end up in an infinite loop if a plugin depends on itself', () => {
|
||||||
|
config.apps = {
|
||||||
|
'myorg-first-app': {
|
||||||
|
...genereicAppPluginConfig,
|
||||||
|
id: 'myorg-first-app',
|
||||||
|
},
|
||||||
|
'myorg-second-app': {
|
||||||
|
...genereicAppPluginConfig,
|
||||||
|
id: 'myorg-second-app',
|
||||||
|
dependencies: {
|
||||||
|
...genereicAppPluginConfig.dependencies,
|
||||||
|
extensions: {
|
||||||
|
// Not a valid scenario!
|
||||||
|
// (As this is sometimes happening out in the wild, we thought it's better to also cover it with a test-case.)
|
||||||
|
exposedComponents: ['myorg-second-app/link/v1'],
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
};
|
||||||
|
|
||||||
|
const appPluginIds = getAppPluginDependencies('myorg-second-app');
|
||||||
|
|
||||||
|
expect(appPluginIds).toEqual([]);
|
||||||
|
});
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -459,18 +459,28 @@ export const getExposedComponentPluginDependencies = (exposedComponentId: string
|
|||||||
// Returns a list of app plugin ids that are necessary to be loaded, based on the `dependencies.extensions`
|
// Returns a list of app plugin ids that are necessary to be loaded, based on the `dependencies.extensions`
|
||||||
// metadata field. (For example the plugins that expose components that the app depends on.)
|
// metadata field. (For example the plugins that expose components that the app depends on.)
|
||||||
// Heads up! This is a recursive function.
|
// Heads up! This is a recursive function.
|
||||||
export const getAppPluginDependencies = (pluginId: string): string[] => {
|
export const getAppPluginDependencies = (pluginId: string, visited: string[] = []): string[] => {
|
||||||
if (!config.apps[pluginId]) {
|
if (!config.apps[pluginId]) {
|
||||||
return [];
|
return [];
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Prevent infinite recursion (it would happen if there is a circular dependency between app plugins)
|
||||||
|
if (visited.includes(pluginId)) {
|
||||||
|
return [];
|
||||||
|
}
|
||||||
|
|
||||||
const pluginIdDependencies = config.apps[pluginId].dependencies.extensions.exposedComponents.map(
|
const pluginIdDependencies = config.apps[pluginId].dependencies.extensions.exposedComponents.map(
|
||||||
getAppPluginIdFromExposedComponentId
|
getAppPluginIdFromExposedComponentId
|
||||||
);
|
);
|
||||||
|
|
||||||
return pluginIdDependencies.reduce((acc, pluginId) => {
|
return (
|
||||||
return [...acc, ...getAppPluginDependencies(pluginId)];
|
pluginIdDependencies
|
||||||
}, pluginIdDependencies);
|
.reduce((acc, _pluginId) => {
|
||||||
|
return [...acc, ...getAppPluginDependencies(_pluginId, [...visited, pluginId])];
|
||||||
|
}, pluginIdDependencies)
|
||||||
|
// We don't want the plugin to "depend on itself"
|
||||||
|
.filter((id) => id !== pluginId)
|
||||||
|
);
|
||||||
};
|
};
|
||||||
|
|
||||||
// Returns a list of app plugins that has to be loaded before core Grafana could finish the initialization.
|
// Returns a list of app plugins that has to be loaded before core Grafana could finish the initialization.
|
||||||
|
|||||||
Reference in New Issue
Block a user