fix(core): use module uid for config namespace instead of dot notation

This commit is contained in:
Ben Irvin 2024-03-11 12:28:46 +01:00 committed by GitHub
parent 0031649552
commit ab2af1e539
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
35 changed files with 200 additions and 56 deletions

View File

@ -50,7 +50,7 @@ NOTE: Most of this is defined by Knex, be careful when adding options that they
#### api
Content API configuraiton, for example the 'rest.maxLimit' option to set the maximum `limit` available in a query.
Content API configuration, for example the 'rest.maxLimit' option to set the maximum `limit` available in a query.
#### features
@ -60,7 +60,8 @@ Feature flags for enabling and configuring future features that would be breakin
Contains plugin configurations, with each root-level value the id of a plugin, for example, 'users-permissions'
IMPORTANT: Plugins is an exception to the normal namespacing in config.get(). An example plugins.js file has this shape:
IMPORTANT: Plugins are not loaded like other Strapi configurations into the `plugins.` namespace, because they are loaded during the module loading.
That means that in an example plugins.js file like below, its values are accessible at, for example, `strapi.config.get('plugin::graphql.endpoint')`
```
module.exports = () => ({
@ -80,8 +81,6 @@ module.exports = () => ({
});
```
However, the `config` attribute is flattened and removed so that its values are accessible at, for example, `strapi.config.get('plugins.graphql.endpoint')`
NOTE: Because this configuration contains user and plugin keys and Strapi does not yet provide a method to extend the definitions, this configuration is not strictly typed and will not be loaded from the environment variables
#### middlewares

View File

@ -59,7 +59,7 @@ const parseFilesData = async (files: UpdateProjectSettings.Request['files']) =>
tmpPath: file.path,
// TODO
// @ts-expect-error define the correct return type
provider: strapi.config.get('plugin.upload').provider,
provider: strapi.config.get('plugin::upload').provider,
});
})
);
@ -134,7 +134,7 @@ const deleteOldFiles = async ({ previousSettings, newSettings }: any) => {
// Skip if the file was not uploaded with the current provider
// TODO
// @ts-expect-error define the correct return type
if (strapi.config.get('plugin.upload').provider !== previousSettings[inputName].provider) {
if (strapi.config.get('plugin::upload').provider !== previousSettings[inputName].provider) {
return;
}

View File

@ -183,7 +183,7 @@ class Strapi extends Container implements StrapiI {
const appConfig = loadConfiguration(rootDirs, opts);
// Instantiate the Strapi container
this.add('config', registries.config(appConfig))
this.add('config', registries.config(appConfig, this))
.add('content-types', registries.contentTypes())
.add('components', registries.components())
.add('services', registries.services(this))

View File

@ -68,6 +68,7 @@ export default (dirs: { app: string; dist: string }, initialConfig: any = {}) =>
},
};
// See packages/core/core/src/domain/module/index.ts for plugin config loading
const baseConfig = omit('plugins', loadConfigDir(configDir)); // plugin config will be loaded later
const envDir = path.resolve(configDir, 'env', process.env.NODE_ENV as string);

View File

@ -43,8 +43,6 @@ export interface Module {
controllers: Record<string, Common.Controller>;
}
const uidToPath = (uid: string) => uid.replace('::', '.');
// Removes the namespace from a map with keys prefixed with a namespace
const removeNamespacedKeys = <T extends Record<string, unknown>>(map: T, namespace: string) => {
return _.mapKeys(map, (value, key) => removeNamespace(key, namespace));
@ -100,13 +98,13 @@ export const createModule = (namespace: string, rawModule: RawModule, strapi: St
strapi.get('policies').add(namespace, rawModule.policies);
strapi.get('middlewares').add(namespace, rawModule.middlewares);
strapi.get('controllers').add(namespace, rawModule.controllers);
strapi.get('config').set(uidToPath(namespace), rawModule.config);
strapi.get('config').set(namespace, rawModule.config);
},
get routes() {
return rawModule.routes ?? {};
},
config(path: string, defaultValue: unknown) {
return strapi.get('config').get(`${uidToPath(namespace)}.${path}`, defaultValue);
return strapi.get('config').get(`${namespace}.${path}`, defaultValue);
},
contentType(ctName: Common.UID.ContentType) {
return strapi.get('content-types').get(`${namespace}.${ctName}`);

View File

@ -0,0 +1,86 @@
import configProvider from '../config';
const logLevel = 'warn';
describe('config', () => {
test('returns objects for partial paths', () => {
const config = configProvider({ default: { child: 'val' } });
expect(config.get('default')).toEqual({ child: 'val' });
});
test('supports full paths', () => {
const config = configProvider({ default: { child: 'val' } });
expect(config.get('default.child')).toEqual('val');
});
test('accepts initial values', () => {
const config = configProvider({ default: 'val', foo: 'bar' });
expect(config.get('default')).toEqual('val');
expect(config.get('foo')).toEqual('bar');
});
test('accepts uid in paths', () => {
const config = configProvider({
'api::myapi': { foo: 'val' },
'plugin::myplugin': { foo: 'bar' },
});
expect(config.get('api::myapi.foo')).toEqual('val');
expect(config.get('api::myapi')).toEqual({ foo: 'val' });
expect(config.get('plugin::myplugin.foo')).toEqual('bar');
expect(config.get('plugin::myplugin')).toEqual({ foo: 'bar' });
});
test('get supports deprecation for plugin.', () => {
const consoleSpy = jest.spyOn(console, logLevel).mockImplementation(() => {});
const config = configProvider({
'plugin::myplugin': { foo: 'bar' },
});
expect(config.get('plugin.myplugin.foo')).toEqual('bar');
expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining('deprecated'));
consoleSpy.mockRestore();
});
test('set supports deprecation for plugin.', () => {
const consoleSpy = jest.spyOn(console, logLevel).mockImplementation(() => {});
const config = configProvider({
'plugin::myplugin': { foo: 'bar' },
});
config.set('plugin.myplugin.thing', 'val');
expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining('deprecated'));
expect(config.get('plugin::myplugin.thing')).toEqual('val');
consoleSpy.mockRestore();
});
test('logs deprecation with strapi logger if available', () => {
const consoleSpy = jest.spyOn(console, logLevel).mockImplementation(() => {});
const logSpy = jest.fn();
const config = configProvider(
{
'plugin::myplugin': { foo: 'bar' },
},
{ log: { [logLevel]: logSpy } } as any
);
expect(config.get('plugin.myplugin.foo')).toEqual('bar');
expect(logSpy).toHaveBeenCalledWith(expect.stringContaining('deprecated'));
expect(consoleSpy).not.toHaveBeenCalled();
consoleSpy.mockRestore();
});
test('get does NOT support deprecation for other prefixes', () => {
const config = configProvider({
'api::myapi': { foo: 'bar' },
});
expect(config.get('api.myapi')).toEqual(undefined);
});
test('set does NOT support deprecation for other prefixes', () => {
const config = configProvider({
'api::myapi': { foo: 'bar' },
});
config.set('api.myapi.foo', 'nope');
expect(config.get('api.myapi.foo')).toEqual('nope');
expect(config.get('api::myapi.foo')).toEqual('bar');
});
});

View File

@ -1,22 +1,38 @@
import type { ConfigProvider } from '@strapi/types';
import _, { PropertyName } from 'lodash';
import type { ConfigProvider, LoadedStrapi, Strapi } from '@strapi/types';
import { get, set, has, isString, type PropertyName } from 'lodash';
type Config = Record<string, unknown>;
export default (initialConfig = {}): ConfigProvider => {
export default (initialConfig = {}, strapi?: Strapi | LoadedStrapi): ConfigProvider => {
const _config: Config = { ...initialConfig }; // not deep clone because it would break some config
// Accessing model configs with dot (.) was deprecated between v4->v5, but to avoid a major breaking change
// we will still support certain namespaces, currently only 'plugin.'
const transformDeprecatedPaths = (path: PropertyName) => {
if (isString(path) && path.startsWith('plugin.')) {
const newPath = path.replace('plugin.', 'plugin::');
// strapi logger may not be loaded yet, so fall back to console
(strapi?.log?.warn ?? console.warn)(
`Using dot notation for model config namespaces is deprecated, for example "plugin::myplugin" should be used instead of "plugin.myplugin". Modifying requested path ${path} to ${newPath}`
);
return newPath;
}
return path;
};
return {
..._config, // TODO: to remove
get(path: PropertyName, defaultValue?: unknown) {
return _.get(_config, path, defaultValue);
return get(_config, transformDeprecatedPaths(path), defaultValue);
},
set(path: PropertyName, val: unknown) {
_.set(_config, path, val);
set(_config, transformDeprecatedPaths(path), val);
return this;
},
has(path: PropertyName) {
return _.has(_config, path);
return has(_config, path);
},
};
};

View File

@ -33,7 +33,7 @@ const strapiFactory = getStrapiFactory({
db: { transaction },
config: {
get(service) {
if (service === 'plugin.upload') {
if (service === 'plugin::upload') {
return {
provider: 'local',
};

View File

@ -21,7 +21,7 @@ jest.mock('../strategies/restore', () => {
const strapiCommonProperties = {
config: {
get(service) {
if (service === 'plugin.upload') {
if (service === 'plugin::upload') {
return { provider: 'local' };
}
},

View File

@ -226,7 +226,7 @@ class LocalStrapiDestinationProvider implements IDestinationProvider {
return;
}
if (this.strapi.config.get<{ provider: string }>('plugin.upload').provider === 'local') {
if (this.strapi.config.get<{ provider: string }>('plugin::upload').provider === 'local') {
const assetsDirectory = path.join(this.strapi.dirs.static.public, 'uploads');
const backupDirectory = path.join(
this.strapi.dirs.static.public,
@ -267,7 +267,7 @@ class LocalStrapiDestinationProvider implements IDestinationProvider {
}
// TODO: this should catch all thrown errors and bubble it up to engine so it can be reported as a non-fatal diagnostic message telling the user they may need to manually delete assets
if (this.strapi.config.get<{ provider: string }>('plugin.upload').provider === 'local') {
if (this.strapi.config.get<{ provider: string }>('plugin::upload').provider === 'local') {
assertValidStrapi(this.strapi);
const backupDirectory = path.join(
this.strapi.dirs.static.public,
@ -345,7 +345,7 @@ class LocalStrapiDestinationProvider implements IDestinationProvider {
buffer: chunk?.buffer,
};
const provider = strapi.config.get<{ provider: string }>('plugin.upload').provider;
const provider = strapi.config.get<{ provider: string }>('plugin::upload').provider;
try {
await strapi.plugin('upload').provider.uploadStream(uploadData);

View File

@ -39,7 +39,7 @@ const createProvider = (emailConfig: EmailConfig) => {
};
export const bootstrap = async ({ strapi }: { strapi: Strapi }) => {
const emailConfig: EmailConfig = strapi.config.get('plugin.email');
const emailConfig: EmailConfig = strapi.config.get('plugin::email');
strapi.plugin('email').provider = createProvider(emailConfig);
// Add permissions

View File

@ -43,7 +43,7 @@ const emailController = {
to,
subject: `Strapi test mail to: ${to}`,
text: `Great! You have correctly configured the Strapi email plugin with the ${strapi.config.get(
'plugin.email.provider'
'plugin::email.provider'
)} provider. \r\nFor documentation on how to use the email plugin checkout: https://docs.strapi.io/developer-docs/latest/plugins/email.html`,
};

View File

@ -11,7 +11,7 @@ import type {
const { createStrictInterpolationRegExp } = template;
const getProviderSettings = (): EmailConfig => strapi.config.get('plugin.email');
const getProviderSettings = (): EmailConfig => strapi.config.get('plugin::email');
const send = async (options: SendOptions) => strapi.plugin('email').provider.send(options);

View File

@ -41,7 +41,7 @@ module.exports = async ({ strapi }) => {
await getService('weeklyMetrics').registerCron();
getService('metrics').sendUploadPluginMetrics();
if (strapi.config.get('plugin.upload.signAdminURLsOnly', false)) {
if (strapi.config.get('plugin::upload.signAdminURLsOnly', false)) {
getService('extensions').contentManager.entityManager.addSignedFileUrlsToAdmin();
} else {
getService('extensions').core.entityService.addSignedFileUrlsToEntityService();

View File

@ -19,7 +19,7 @@ module.exports = ({ strapi }) => {
strapi.server.app.onerror(err);
});
const localServerConfig = strapi.config.get('plugin.upload.providerOptions.localServer', {});
const localServerConfig = strapi.config.get('plugin::upload.providerOptions.localServer', {});
strapi.server.routes([
{

View File

@ -13,7 +13,7 @@ const spec = require('../documentation/content-api.json');
* @param {{ strapi: import('@strapi/strapi').Strapi }}
*/
module.exports = async ({ strapi }) => {
strapi.plugin('upload').provider = createProvider(strapi.config.get('plugin.upload', {}));
strapi.plugin('upload').provider = createProvider(strapi.config.get('plugin::upload', {}));
await registerUploadMiddleware({ strapi });

View File

@ -76,7 +76,7 @@ describe('file', () => {
},
config: {
get: jest.fn((key) => {
if (key === 'plugin.upload') {
if (key === 'plugin::upload') {
return {
provider: 'private-provider',
};

View File

@ -14,12 +14,10 @@ function mockUploadProvider(uploadFunc, props) {
const { responsiveDimensions = false } = props || {};
const defaultConfig = {
plugin: {
upload: {
breakpoints: {
large: 1000,
medium: 750,
},
'plugin::upload': {
breakpoints: {
large: 1000,
medium: 750,
},
},
};

View File

@ -25,7 +25,7 @@ const deleteByIds = async (ids = []) => {
const signFileUrls = async (file) => {
const { provider } = strapi.plugins.upload;
const { provider: providerConfig } = strapi.config.get('plugin.upload');
const { provider: providerConfig } = strapi.config.get('plugin::upload');
const isPrivate = await provider.isPrivate();
file.isUrlSigned = false;

View File

@ -128,7 +128,7 @@ const DEFAULT_BREAKPOINTS = {
small: 500,
};
const getBreakpoints = () => strapi.config.get('plugin.upload.breakpoints', DEFAULT_BREAKPOINTS);
const getBreakpoints = () => strapi.config.get('plugin::upload.breakpoints', DEFAULT_BREAKPOINTS);
const generateResponsiveFormats = async (file) => {
const { responsiveDimensions = false } = await getService('upload').getSettings();

View File

@ -1,6 +1,6 @@
'use strict';
const getProviderName = () => strapi.config.get('plugin.upload.provider', 'local');
const getProviderName = () => strapi.config.get('plugin::upload.provider', 'local');
const isProviderPrivate = async () => strapi.plugin('upload').provider.isPrivate();
module.exports = ({ strapi }) => ({

View File

@ -7,7 +7,7 @@ const {
module.exports = ({ strapi }) => ({
async checkFileSize(file) {
const { sizeLimit } = strapi.config.get('plugin.upload', {});
const { sizeLimit } = strapi.config.get('plugin::upload', {});
await strapi.plugin('upload').provider.checkFileSize(file, { sizeLimit });
},
async upload(file) {

View File

@ -244,7 +244,7 @@ module.exports = ({ strapi }) => ({
* and responsive formats (if enabled).
*/
async uploadFileAndPersist(fileData, { user } = {}) {
const config = strapi.config.get('plugin.upload');
const config = strapi.config.get('plugin::upload');
const { isImage } = getService('image-manipulation');
await getService('provider').checkFileSize(fileData);
@ -283,7 +283,7 @@ module.exports = ({ strapi }) => ({
},
async replace(id, { data, file }, { user } = {}) {
const config = strapi.config.get('plugin.upload');
const config = strapi.config.get('plugin::upload');
const { isImage } = getService('image-manipulation');
@ -382,7 +382,7 @@ module.exports = ({ strapi }) => ({
},
async remove(file) {
const config = strapi.config.get('plugin.upload');
const config = strapi.config.get('plugin::upload');
// execute delete function of the provider
if (file.provider === config.provider) {

View File

@ -108,7 +108,7 @@ module.exports = {
const layout = fs.readFileSync(path.join(__dirname, '..', 'public', 'login.html'));
const filledLayout = _.template(layout)({
actionUrl: `${strapi.config.server.url}${
strapi.config.get('plugin.documentation.x-strapi-config').path
strapi.config.get('plugin::documentation.x-strapi-config').path
}/login`,
});
const $ = cheerio.load(filledLayout);
@ -164,7 +164,7 @@ module.exports = {
ctx.redirect(
`${strapi.config.server.url}${
strapi.config.get('plugin.documentation.x-strapi-config').path
strapi.config.get('plugin::documentation.x-strapi-config').path
}${querystring}`
);
},

View File

@ -14,7 +14,7 @@ module.exports = async (ctx, next) => {
return ctx.redirect(
`${strapi.config.server.url}${
strapi.config.get('plugin.documentation.x-strapi-config').path
strapi.config.get('plugin::documentation.x-strapi-config').path
}/login${querystring}`
);
}

View File

@ -9,7 +9,7 @@ const defaultOpenApiComponents = require('./utils/default-openapi-components');
const { getPluginsThatNeedDocumentation } = require('./utils/get-plugins-that-need-documentation');
module.exports = ({ strapi }) => {
const config = strapi.config.get('plugin.documentation');
const config = strapi.config.get('plugin::documentation');
const pluginsThatNeedDocumentation = getPluginsThatNeedDocumentation(config);
const overrideService = strapi.plugin('documentation').service('override');

View File

@ -31,7 +31,7 @@ module.exports = ({ strapi }) => {
*/
registerOverride(override, { pluginOrigin, excludeFromGeneration = [] } = {}) {
const pluginsThatNeedDocumentation = getPluginsThatNeedDocumentation(
strapi.config.get('plugin.documentation')
strapi.config.get('plugin::documentation')
);
// Don't apply the override if the plugin is not in the list of plugins that need documentation
if (pluginOrigin && !pluginsThatNeedDocumentation.includes(pluginOrigin)) return;

View File

@ -139,7 +139,7 @@ const mergeCustomizer = (dest: any, src: any) => {
* @param {object} schema
*/
const addGraphqlSchema = (schema: any) => {
_.mergeWith(strapi.config.get('plugin.i18n.schema.graphql'), schema, mergeCustomizer);
_.mergeWith(strapi.config.get('plugin::i18n.schema.graphql'), schema, mergeCustomizer);
};
/**

View File

@ -7,7 +7,7 @@ const createSentryService = (strapi: Strapi) => {
let instance: typeof Sentry | null = null;
// Retrieve user config and merge it with the default one
const config = strapi.config.get('plugin.sentry') as Config;
const config = strapi.config.get('plugin::sentry') as Config;
return {
/**

View File

@ -109,7 +109,7 @@ module.exports = async ({ strapi }) => {
await getService('users-permissions').initialize();
if (!strapi.config.get('plugin.users-permissions.jwtSecret')) {
if (!strapi.config.get('plugin::users-permissions.jwtSecret')) {
if (process.env.NODE_ENV !== 'development') {
throw new Error(
`Missing jwtSecret. Please, set configuration variable "jwtSecret" for the users-permissions plugin in config/plugins.js (ex: you can generate one using Node with \`crypto.randomBytes(16).toString('base64')\`).
@ -119,7 +119,7 @@ For security reasons, prefer storing the secret in an environment variable and r
const jwtSecret = crypto.randomBytes(16).toString('base64');
strapi.config.set('plugin.users-permissions.jwtSecret', jwtSecret);
strapi.config.set('plugin::users-permissions.jwtSecret', jwtSecret);
if (!process.env.JWT_SECRET) {
const envPath = process.env.ENV_PATH || '.env';

View File

@ -278,7 +278,7 @@ module.exports = {
throw new ApplicationError('Register action is currently disabled');
}
const { register } = strapi.config.get('plugin.users-permissions');
const { register } = strapi.config.get('plugin::users-permissions');
const alwaysAllowedKeys = ['username', 'password', 'email'];
// Note that we intentionally do not filter allowedFields to allow a project to explicitly accept private or other Strapi field on registration

View File

@ -9,7 +9,7 @@ const { RateLimitError } = utils.errors;
module.exports =
(config, { strapi }) =>
async (ctx, next) => {
let rateLimitConfig = strapi.config.get('plugin.users-permissions.ratelimit');
let rateLimitConfig = strapi.config.get('plugin::users-permissions.ratelimit');
if (!rateLimitConfig) {
rateLimitConfig = {

View File

@ -29,10 +29,10 @@ module.exports = ({ strapi }) => ({
},
issue(payload, jwtOptions = {}) {
_.defaults(jwtOptions, strapi.config.get('plugin.users-permissions.jwt'));
_.defaults(jwtOptions, strapi.config.get('plugin::users-permissions.jwt'));
return jwt.sign(
_.clone(payload.toJSON ? payload.toJSON() : payload),
strapi.config.get('plugin.users-permissions.jwtSecret'),
strapi.config.get('plugin::users-permissions.jwtSecret'),
jwtOptions
);
},
@ -41,7 +41,7 @@ module.exports = ({ strapi }) => ({
return new Promise((resolve, reject) => {
jwt.verify(
token,
strapi.config.get('plugin.users-permissions.jwtSecret'),
strapi.config.get('plugin::users-permissions.jwtSecret'),
{},
(err, tokenPayload = {}) => {
if (err) {

View File

@ -0,0 +1,45 @@
import { Transform } from 'jscodeshift';
/**
* Replaces string dot format for config get/set/has with uid format for 'plugin' and 'api' namespace where possible
* For example, `strapi.config.get('plugin.anyString')` will become `strapi.config.get('plugin::anyString')`
* Ignores api followed by 'rest' or 'responses' because those are the valid Strapi api config values in v4
*/
const transform: Transform = (file, api) => {
const j = api.jscodeshift;
const root = j(file.source);
const ignoreList = ['api.rest', 'api.responses'];
['get', 'has', 'set'].forEach((configMethod) => {
root
.find(j.CallExpression, {
callee: {
type: 'MemberExpression',
object: {
type: 'MemberExpression',
object: { type: 'Identifier', name: 'strapi' },
property: { type: 'Identifier', name: 'config' },
},
property: { type: 'Identifier', name: configMethod },
},
// Note: we can't filter by arguments because it won't find them in typescript files
})
.forEach((path) => {
const argumentNode = path.node.arguments[0];
if (j.StringLiteral.check(argumentNode)) {
const value = argumentNode.value;
const isTargeted = value.startsWith('plugin.') || value.startsWith('api.');
const isIgnored = ignoreList.some((ignoreItem) => value.startsWith(ignoreItem));
if (!isTargeted || isIgnored) {
return;
}
argumentNode.value = value.replace('.', '::');
}
});
});
return root.toSource();
};
export default transform;

View File

@ -86,6 +86,7 @@ export class Project implements ProjectInterface {
runInBand: true,
verbose: 0,
babel: true,
parser: 'ts',
});
const jsonRunner = jsonRunnerFactory(jsonFiles, { dry, cwd: this.cwd });