diff --git a/packages/core/content-manager/server/src/controllers/collection-types.ts b/packages/core/content-manager/server/src/controllers/collection-types.ts index 37e66bd66d..9ad3018ced 100644 --- a/packages/core/content-manager/server/src/controllers/collection-types.ts +++ b/packages/core/content-manager/server/src/controllers/collection-types.ts @@ -1,10 +1,15 @@ import { setCreatorFields, async, errors } from '@strapi/utils'; + +import type { Modules, UID } from '@strapi/types'; + import { getService } from '../utils'; import { validateBulkActionInput } from './validation'; import { getProhibitedCloningFields, excludeNotCreatableFields } from './utils/clone'; import { getDocumentLocaleAndStatus } from './validation/dimensions'; import { formatDocumentWithMetadata } from './utils/metadata'; +type Options = Modules.Documents.Params.Pick; + /** * Create a new document. * @@ -13,7 +18,7 @@ import { formatDocumentWithMetadata } from './utils/metadata'; * @param opts.populate - Populate options of the returned document. * By default documentManager will populate all relations. */ -const createDocument = async (ctx: any, opts?: { populate?: object }) => { +const createDocument = async (ctx: any, opts?: Options) => { const { userAbility, user } = ctx.state; const { model } = ctx.params; const { body } = ctx.request; @@ -55,7 +60,7 @@ const createDocument = async (ctx: any, opts?: { populate?: object }) => { * @param opts - Options * @param opts.populate - Populate options of the returned document */ -const updateDocument = async (ctx: any, opts?: { populate?: object }) => { +const updateDocument = async (ctx: any, opts?: Options) => { const { userAbility, user } = ctx.state; const { id, model } = ctx.params; const { body } = ctx.request; diff --git a/packages/core/content-manager/server/src/controllers/single-types.ts b/packages/core/content-manager/server/src/controllers/single-types.ts index 9cde14e0db..67f398fcd3 100644 --- a/packages/core/content-manager/server/src/controllers/single-types.ts +++ b/packages/core/content-manager/server/src/controllers/single-types.ts @@ -1,10 +1,12 @@ -import type { UID } from '@strapi/types'; +import type { UID, Modules } from '@strapi/types'; import { setCreatorFields, async, errors } from '@strapi/utils'; import { getDocumentLocaleAndStatus } from './validation/dimensions'; import { getService } from '../utils'; import { formatDocumentWithMetadata } from './utils/metadata'; +type OptionsWithPopulate = Modules.Documents.Params.Pick; + const buildPopulateFromQuery = async (query: any, model: any) => { return getService('populate-builder')(model) .populateFromQuery(query) @@ -25,7 +27,7 @@ const findDocument = async (query: any, uid: UID.SingleType, opts: any = {}) => ); }; -const createOrUpdateDocument = async (ctx: any, opts?: { populate: object }) => { +const createOrUpdateDocument = async (ctx: any, opts?: OptionsWithPopulate) => { const { user, userAbility } = ctx.state; const { model } = ctx.params; const { body, query } = ctx.request; diff --git a/packages/core/content-manager/server/src/services/utils/populate.ts b/packages/core/content-manager/server/src/services/utils/populate.ts index 08a9ced46d..4000f634a5 100644 --- a/packages/core/content-manager/server/src/services/utils/populate.ts +++ b/packages/core/content-manager/server/src/services/utils/populate.ts @@ -240,7 +240,7 @@ const getDeepPopulateDraftCount = (uid: UID.Schema) => { let hasRelations = false; const populate = Object.keys(model.attributes).reduce((populateAcc: any, attributeName) => { - const attribute: any = model.attributes[attributeName]; + const attribute: Schema.Attribute.AnyAttribute = model.attributes[attributeName]; switch (attribute.type) { case 'relation': { @@ -258,24 +258,29 @@ const getDeepPopulateDraftCount = (uid: UID.Schema) => { attribute.component ); if (childHasRelations) { - populateAcc[attributeName] = { populate }; + populateAcc[attributeName] = { + populate, + }; hasRelations = true; } break; } case 'dynamiczone': { - const dzPopulate = (attribute.components || []).reduce((acc: any, componentUID: any) => { - const { populate, hasRelations: childHasRelations } = + const dzPopulateFragment = attribute.components?.reduce((acc, componentUID) => { + const { populate: componentPopulate, hasRelations: componentHasRelations } = getDeepPopulateDraftCount(componentUID); - if (childHasRelations) { + + if (componentHasRelations) { hasRelations = true; - return merge(acc, populate); + + return { ...acc, [componentUID]: { populate: componentPopulate } }; } + return acc; }, {}); - if (!isEmpty(dzPopulate)) { - populateAcc[attributeName] = { populate: dzPopulate }; + if (!isEmpty(dzPopulateFragment)) { + populateAcc[attributeName] = { on: dzPopulateFragment }; } break; } diff --git a/packages/core/types/src/modules/documents/params/populate.ts b/packages/core/types/src/modules/documents/params/populate.ts index 0dd73f766c..506f16b537 100644 --- a/packages/core/types/src/modules/documents/params/populate.ts +++ b/packages/core/types/src/modules/documents/params/populate.ts @@ -1,7 +1,7 @@ import type * as Schema from '../../../schema'; import type * as UID from '../../../uid'; -import type { Constants, Guard, If, And, DoesNotExtends, IsNotNever } from '../../../utils'; +import type { Constants, Guard, If, And, DoesNotExtends, IsNotNever, XOR } from '../../../utils'; import type { Params } from '..'; @@ -108,20 +108,16 @@ export type ObjectNotation = [ Schema.Attribute.MorphTargets>, UID.Schema > - > - // TODO: V5: Remove root-level nested params for morph data structures and only allow fragments - | NestedParams; + >; } >, // Loose fallback when registries are not extended - | { [TKey in string]?: boolean | NestedParams } - | { - [TKey in string]?: - | boolean - | Fragment - // TODO: V5: Remove root-level nested params for morph data structures and only allow fragments - | NestedParams; - } + { + [key: string]: + | boolean + // We can't have both populate fragments and nested params, hence the xor + | XOR, Fragment>; + } > : never; diff --git a/packages/core/types/src/modules/entity-service/params/populate.ts b/packages/core/types/src/modules/entity-service/params/populate.ts index df0df2d899..1684bee50a 100644 --- a/packages/core/types/src/modules/entity-service/params/populate.ts +++ b/packages/core/types/src/modules/entity-service/params/populate.ts @@ -1,7 +1,7 @@ import type * as Schema from '../../../schema'; import type * as UID from '../../../uid'; -import type { Constants, Guard, If, And, DoesNotExtends, IsNotNever } from '../../../utils'; +import type { Constants, Guard, If, And, DoesNotExtends, IsNotNever, XOR } from '../../../utils'; import type { Params } from '..'; @@ -60,7 +60,7 @@ type GetPopulatableKeysWithoutTarget = Exclude< * Fragment populate notation for polymorphic attributes */ export type Fragment = { - on?: { [TKey in TMaybeTargets]: boolean | NestedParams }; + on?: { [TKey in TMaybeTargets]?: boolean | NestedParams }; }; type PopulateClause< @@ -108,20 +108,16 @@ export type ObjectNotation = [ Schema.Attribute.MorphTargets>, UID.Schema > - > - // TODO: V5: Remove root-level nested params for morph data structures and only allow fragments - | NestedParams; + >; } >, // Loose fallback when registries are not extended - | { [key: string]: boolean | NestedParams } - | { - [key: string]: - | boolean - | Fragment - // TODO: V5: Remove root-level nested params for morph data structures and only allow fragments - | NestedParams; - } + { + [key: string]: + | boolean + // We can't have both populate fragments and nested params, hence the xor + | XOR, Fragment>; + } > : never; diff --git a/packages/core/utils/src/convert-query-params.ts b/packages/core/utils/src/convert-query-params.ts index ce43c3482f..4d390d783d 100644 --- a/packages/core/utils/src/convert-query-params.ts +++ b/packages/core/utils/src/convert-query-params.ts @@ -13,7 +13,6 @@ import { isObject, cloneDeep, get, - mergeAll, isArray, isString, } from 'lodash/fp'; @@ -341,10 +340,20 @@ const createTransformer = ({ getModel }: TransformerOptions) => { } const { attributes } = schema; - return Object.entries(populate).reduce((acc, [key, subPopulate]) => { + if (_.isString(subPopulate)) { + try { + const subPopulateAsBoolean = parseType({ type: 'boolean', value: subPopulate }); + // Only true is accepted as a boolean populate value + return subPopulateAsBoolean === true ? { ...acc, [key]: true } : acc; + } catch { + // ignore + } + } + if (_.isBoolean(subPopulate)) { - return { ...acc, [key]: subPopulate }; + // Only true is accepted as a boolean populate value + return subPopulate === true ? { ...acc, [key]: true } : acc; } const attribute = attributes[key]; @@ -357,42 +366,29 @@ const createTransformer = ({ getModel }: TransformerOptions) => { const isAllowedAttributeForFragmentPopulate = isDynamicZoneAttribute(attribute) || isMorphToRelationalAttribute(attribute); - if (isAllowedAttributeForFragmentPopulate && hasFragmentPopulateDefined(subPopulate)) { - return { - ...acc, - [key]: { - on: Object.entries(subPopulate.on).reduce( - (acc, [type, typeSubPopulate]) => ({ - ...acc, - [type]: convertNestedPopulate(typeSubPopulate, getModel(type)), - }), - {} - ), - }, - }; - } - - // TODO: This is a query's populate fallback for DynamicZone and is kept for legacy purpose. - // Removing it could break existing user queries but it should be removed in V5. - if (isDynamicZoneAttribute(attribute)) { - const populates = attribute.components - .map((uid) => getModel(uid)) - .map((schema) => convertNestedPopulate(subPopulate, schema)) - .map((populate) => (populate === true ? {} : populate)) // cast boolean to empty object to avoid merging issues - .filter((populate) => populate !== false); - - if (isEmpty(populates)) { - return acc; + if (isAllowedAttributeForFragmentPopulate) { + if (hasFragmentPopulateDefined(subPopulate)) { + // does it have 'on' AND no other property + return { + ...acc, + [key]: { + on: Object.entries(subPopulate.on).reduce( + (acc, [type, typeSubPopulate]) => ({ + ...acc, + [type]: convertNestedPopulate(typeSubPopulate, getModel(type)), + }), + {} + ), + }, + }; } - - return { - ...acc, - [key]: mergeAll(populates), - }; + throw new Error( + `Invalid nested populate. Expected a fragment ("on") but found ${JSON.stringify(subPopulate)}` + ); } - if (isMorphToRelationalAttribute(attribute)) { - return { ...acc, [key]: convertNestedPopulate(subPopulate, undefined) }; + if (!isAllowedAttributeForFragmentPopulate && hasFragmentPopulateDefined(subPopulate)) { + throw new Error(`Using fragments is not permitted to populate "${key}" in "${schema.uid}"`); } // NOTE: Retrieve the target schema UID. diff --git a/packages/core/utils/src/traverse/query-populate.ts b/packages/core/utils/src/traverse/query-populate.ts index 60e1716245..9f632b4fdb 100644 --- a/packages/core/utils/src/traverse/query-populate.ts +++ b/packages/core/utils/src/traverse/query-populate.ts @@ -12,8 +12,6 @@ import { cloneDeep, join, first, - omit, - merge, } from 'lodash/fp'; import traverseFactory from './factory'; @@ -178,6 +176,8 @@ const populate = traverseFactory() const newValue = await recurse(visitor, { schema, path, getModel }, { on: value?.on }); set(key, { on: newValue }); + + return; } const targetSchemaUID = attribute.target; @@ -214,48 +214,17 @@ const populate = traverseFactory() set(key, newValue); }) // Handle populate on dynamic zones - .onDynamicZone( - async ({ key, value, attribute, schema, visitor, path, getModel }, { set, recurse }) => { - if (isNil(value)) { - return; - } - - if (isObject(value)) { - const { components } = attribute; - - const newValue = {}; - - // Handle legacy DZ params - let newProperties: unknown = omit('on', value); - - for (const componentUID of components) { - const componentSchema = getModel(componentUID); - - const properties = await recurse( - visitor, - { schema: componentSchema, path, getModel }, - value - ); - newProperties = merge(newProperties, properties); - } - - Object.assign(newValue, newProperties); - - // Handle new morph fragment syntax - if ('on' in value && value.on) { - const newOn = await recurse(visitor, { schema, path, getModel }, { on: value.on }); - - // Recompose both syntaxes - Object.assign(newValue, newOn); - } - - set(key, newValue); - } else { - const newValue = await recurse(visitor, { schema, path, getModel }, value); - - set(key, newValue); - } + .onDynamicZone(async ({ key, value, schema, visitor, path, getModel }, { set, recurse }) => { + if (isNil(value) || !isObject(value)) { + return; } - ); + + // Handle fragment syntax + if ('on' in value && value.on) { + const newOn = await recurse(visitor, { schema, path, getModel }, { on: value.on }); + + set(key, newOn); + } + }); export default curry(populate.traverse); diff --git a/tests/api/core/strapi/api/populate/filtering/index.test.api.js b/tests/api/core/strapi/api/populate/filtering/index.test.api.js index 8d9fba9b51..f28285c549 100644 --- a/tests/api/core/strapi/api/populate/filtering/index.test.api.js +++ b/tests/api/core/strapi/api/populate/filtering/index.test.api.js @@ -325,7 +325,12 @@ describe('Populate filters', () => { test('Populate every component in the dynamic zone', async () => { const qs = { populate: { - dz: '*', + dz: { + on: { + 'default.foo': true, + 'default.bar': true, + }, + }, }, }; diff --git a/tests/api/core/strapi/api/populate/sanitize.test.api.js b/tests/api/core/strapi/api/populate/sanitize.test.api.js index 4d644fd3ce..aa4caab708 100644 --- a/tests/api/core/strapi/api/populate/sanitize.test.api.js +++ b/tests/api/core/strapi/api/populate/sanitize.test.api.js @@ -21,7 +21,9 @@ const schemas = { singularName: 'a', pluralName: 'as', attributes: { - cover: { type: 'media' }, + cover: { + type: 'media', + }, }, }, b: { @@ -143,9 +145,12 @@ describe('Sanitize populated entries', () => { test("Media's relations (from related) can be populated without restricted attributes", async () => { const { status, body } = await contentAPIRequest.get(`/upload/files/${file.id}`, { - qs: { populate: { related: { populate: '*' } } }, + qs: { + populate: { + related: true, + }, + }, }); - expect(status).toBe(200); expect(body.related).toBeDefined(); expect(Array.isArray(body.related)).toBeTruthy(); @@ -170,7 +175,10 @@ describe('Sanitize populated entries', () => { }); const { status } = await contentAPIRequest.get(`/${schemas.contentTypes.b.pluralName}`, { - qs: { fields: ['id'], populate: '*' }, + qs: { + fields: ['id'], + populate: '*', + }, }); expect(status).toBe(200); diff --git a/tests/api/core/strapi/api/validate/validate-query.test.api.js b/tests/api/core/strapi/api/validate/validate-query.test.api.js index 6a5b0db16c..9a03e6f827 100644 --- a/tests/api/core/strapi/api/validate/validate-query.test.api.js +++ b/tests/api/core/strapi/api/validate/validate-query.test.api.js @@ -941,30 +941,11 @@ describe('Core API - Validate', () => { it.todo('Populates a media'); describe('Dynamic Zone', () => { - it.each([{ dz: { populate: '*' } }, { dz: { populate: true } }, { dz: '*' }, { dz: true }])( - 'Populates a dynamic-zone (%s)', - async (populate) => { - const res = await rq.get('/api/documents', { qs: { populate } }); - - checkAPIResultValidity(res); - - res.body.data.forEach((document) => { - expect(document).toHaveProperty( - 'dz', - expect.arrayContaining([ - expect.objectContaining({ __component: expect.any(String) }), - ]) - ); - expect(document.dz).toHaveLength(3); - }); - } - ); - it.each([ [{ dz: { on: { 'default.component-a': true } } }, 'default.component-a', 2], [{ dz: { on: { 'default.component-b': true } } }, 'default.component-b', 1], ])( - 'Populates a dynamic-use using populate fragments (%s)', + 'Populates a dynamic-zone using populate fragments (%s)', async (populate, componentUID, expectedLength) => { const res = await rq.get('/api/documents', { qs: { populate } });