From f6be2e2b66d16238408f2b460499eb4dedb0d01c Mon Sep 17 00:00:00 2001 From: Jamie Howard Date: Fri, 30 Sep 2022 11:02:59 +0100 Subject: [PATCH 01/12] fix(content-manager): detect non existant relation on entity update --- .../admin/admin/src/pages/HomePage/index.js | 2 +- .../__tests__/entity-service.test.js | 129 ++++++++++++++++++ .../lib/services/entity-service/index.js | 35 ++++- .../tests/admin/file-folder.test.e2e.js | 1 + 4 files changed, 165 insertions(+), 2 deletions(-) diff --git a/packages/core/admin/admin/src/pages/HomePage/index.js b/packages/core/admin/admin/src/pages/HomePage/index.js index ae1448fdd8..ae08990b0f 100644 --- a/packages/core/admin/admin/src/pages/HomePage/index.js +++ b/packages/core/admin/admin/src/pages/HomePage/index.js @@ -31,7 +31,7 @@ const LogoContainer = styled(Box)` `; const HomePage = () => { - // // Temporary until we develop the menu API + // Temporary until we develop the menu API const { collectionTypes, singleTypes, isLoading: isLoadingForModels } = useModels(); const { guidedTourState, isGuidedTourVisible, isSkipped } = useGuidedTour(); diff --git a/packages/core/strapi/lib/services/entity-service/__tests__/entity-service.test.js b/packages/core/strapi/lib/services/entity-service/__tests__/entity-service.test.js index 3866b8780a..f1358547df 100644 --- a/packages/core/strapi/lib/services/entity-service/__tests__/entity-service.test.js +++ b/packages/core/strapi/lib/services/entity-service/__tests__/entity-service.test.js @@ -186,4 +186,133 @@ describe('Entity service', () => { }); }); }); + + describe('Update', () => { + describe('assign default values', () => { + let instance; + + const entityUID = 'api::entity.entity'; + const relationUID = 'api::relation.relation'; + const fakeEntities = { + 0: { + id: 0, + Name: 'TestEntity', + createdAt: '2022-09-28T15:11:22.995Z', + updatedAt: '2022-09-29T09:01:02.949Z', + publishedAt: null, + }, + 1: { + id: 1, + Name: 'TestRelation', + createdAt: '2022-09-28T15:11:22.995Z', + updatedAt: '2022-09-29T09:01:02.949Z', + publishedAt: null, + }, + 2: null, + }; + beforeAll(() => { + const fakeModel = { + kind: 'collectionType', + modelName: 'entity', + collectionName: 'entity', + uid: entityUID, + privateAttributes: [], + options: {}, + info: { + singularName: 'entity', + pluralName: 'entities', + displayName: 'ENTITY', + }, + attributes: { + Name: { + type: 'string', + }, + addresses: { + type: 'relation', + relation: 'oneToMany', + target: relationUID, + mappedBy: 'entity', + }, + updatedBy: { + type: 'relation', + relation: 'oneToOne', + target: 'admin::user', + configurable: false, + writable: false, + visible: false, + useJoinTable: false, + private: true, + }, + }, + }; + + const fakeQuery = { + findOne: jest.fn(({ where }) => fakeEntities[where.id]), + update: jest.fn(({ where }) => ({ + ...fakeEntities[where.id], + addresses: { + count: 1, + }, + })), + }; + + const fakeDB = { + query: jest.fn(() => fakeQuery), + }; + + const fakeStrapi = { + getModel: jest.fn(() => fakeModel), + }; + + instance = createEntityService({ + strapi: fakeStrapi, + db: fakeDB, + eventHub: null, // bypass event emission for update tests + entityValidator, + }); + }); + + test(`should fail if the entity doesn't exist`, async () => { + expect( + await instance.update(entityUID, Math.random() * (10000 - 100) + 100, {}) + ).toBeNull(); + }); + + test('should successfully update with an existing relation', async () => { + const data = { + Name: 'TestEntry', + addresses: { + connect: [ + { + id: 1, + }, + ], + }, + updatedBy: 1, + }; + expect(await instance.update(entityUID, 0, { data })).toMatchObject({ + ...fakeEntities[0], + addresses: { + count: 1, + }, + }); + }); + + test('should throw an error when trying to associate a relation that does not exist', async () => { + const data = { + Name: 'TestEntry', + addresses: { + connect: [ + { + id: 2, + }, + ], + }, + updatedBy: 1, + }; + + await expect(instance.update(entityUID, 0, { data })).rejects.toThrow(); + }); + }); + }); }); diff --git a/packages/core/strapi/lib/services/entity-service/index.js b/packages/core/strapi/lib/services/entity-service/index.js index 0df7e2ea0d..cafecf6849 100644 --- a/packages/core/strapi/lib/services/entity-service/index.js +++ b/packages/core/strapi/lib/services/entity-service/index.js @@ -9,7 +9,7 @@ const { contentTypes: contentTypesUtils, sanitize, } = require('@strapi/utils'); -const { ValidationError } = require('@strapi/utils').errors; +const { ValidationError, ApplicationError } = require('@strapi/utils').errors; const { transformParamsToQuery } = require('@strapi/utils').convertQueryParams; const uploadFiles = require('../utils/upload-files'); @@ -47,6 +47,7 @@ const createDefaultImplementation = ({ strapi, db, eventHub, entityValidator }) }, async emitEvent(uid, event, entity) { + if (!eventHub) return; const model = strapi.getModel(uid); const sanitizedEntity = await sanitize.sanitizers.defaultSanitizeOutput(model, entity); @@ -168,6 +169,38 @@ const createDefaultImplementation = ({ strapi, db, eventHub, entityValidator }) entityToUpdate ); + // Create an array of the relations we are attempting to associate with this + // entity. + const relationChecks = []; + Object.keys(data).forEach((key) => { + const attribute = model.attributes[key]; + if (attribute?.type !== 'relation') { + return; + } + if (!data[key]?.connect) { + return; + } + relationChecks.push({ uid: attribute.target, data: data[key].connect }); + }); + + // Confirm that these relations exists in the DB before performing the query. + await Promise.all( + relationChecks.map(async (check) => { + await Promise.all( + check.data.map(async (d) => { + const relationEntity = await db.query(check.uid).findOne({ where: { id: d.id } }); + if (relationEntity) { + return; + } + // Trying to associate a relation with this entity that does not exist + throw new ApplicationError( + `Relation of type ${check.uid} with id ${d.id} does not exist` + ); + }) + ); + }) + ); + const query = transformParamsToQuery(uid, pickSelectionParams(wrappedParams)); // TODO: wrap in transaction diff --git a/packages/core/upload/tests/admin/file-folder.test.e2e.js b/packages/core/upload/tests/admin/file-folder.test.e2e.js index 800b684a89..a39e02bd9b 100644 --- a/packages/core/upload/tests/admin/file-folder.test.e2e.js +++ b/packages/core/upload/tests/admin/file-folder.test.e2e.js @@ -209,6 +209,7 @@ describe('File', () => { data.files[1] = file; }); }); + describe('Move a file from root level to a folder', () => { test('when replacing the file', async () => { const res = await rq({ From 646181a70f7b0b6f47907123b5161ed4232ee96e Mon Sep 17 00:00:00 2001 From: Jamie Howard Date: Tue, 11 Oct 2022 11:10:42 +0100 Subject: [PATCH 02/12] feat(entity-validator): detect non existent relations on create and update WIP chore: front-end tests --- .../components/LogoInput/tests/index.test.js | 4 +- .../__tests__/entity-validator.test.js | 6 + .../__tests__/entity-service.test.js | 283 +++++++++++++----- .../lib/services/entity-service/index.js | 35 +-- .../lib/services/entity-validator/index.js | 117 +++++++- .../core/strapi/tests/endpoint.test.e2e.js | 52 ++++ 6 files changed, 386 insertions(+), 111 deletions(-) diff --git a/packages/core/admin/admin/src/pages/SettingsPage/pages/ApplicationInfosPage/components/LogoInput/tests/index.test.js b/packages/core/admin/admin/src/pages/SettingsPage/pages/ApplicationInfosPage/components/LogoInput/tests/index.test.js index f470600f5c..0337830276 100644 --- a/packages/core/admin/admin/src/pages/SettingsPage/pages/ApplicationInfosPage/components/LogoInput/tests/index.test.js +++ b/packages/core/admin/admin/src/pages/SettingsPage/pages/ApplicationInfosPage/components/LogoInput/tests/index.test.js @@ -260,7 +260,7 @@ describe('ApplicationsInfosPage || LogoInput', () => { fireEvent.change(textInput, { target: { - value: 'https://cdn.pixabay.com/photo/2022/01/18/07/38/cat-6946505__340.jpg', + value: 'https://storage.googleapis.com/gtv-videos-bucket/sample/images/TearsOfSteel.jpg', }, }); @@ -279,7 +279,7 @@ describe('ApplicationsInfosPage || LogoInput', () => { fireEvent.change(textInput, { target: { - value: 'https://cdn.pixabay.com/photo/2022/01/18/07/38/cat-6946505__340.jpg', + value: 'https://storage.googleapis.com/gtv-videos-bucket/sample/images/TearsOfSteel.jpg', }, }); diff --git a/packages/core/strapi/lib/services/__tests__/entity-validator.test.js b/packages/core/strapi/lib/services/__tests__/entity-validator.test.js index 0111fdb4fb..28ca9eb960 100644 --- a/packages/core/strapi/lib/services/__tests__/entity-validator.test.js +++ b/packages/core/strapi/lib/services/__tests__/entity-validator.test.js @@ -10,6 +10,7 @@ describe('Entity validator', () => { errors: { badRequest: jest.fn(), }, + getModel: () => ({}), }; const model = { @@ -88,6 +89,7 @@ describe('Entity validator', () => { errors: { badRequest: jest.fn(), }, + getModel: () => ({}), }; const model = { @@ -164,6 +166,7 @@ describe('Entity validator', () => { errors: { badRequest: jest.fn(), }, + getModel: () => ({}), }; const model = { @@ -203,6 +206,7 @@ describe('Entity validator', () => { errors: { badRequest: jest.fn(), }, + getModel: () => ({}), }; const model = { @@ -329,6 +333,7 @@ describe('Entity validator', () => { errors: { badRequest: jest.fn(), }, + getModel: () => ({}), }; const model = { @@ -469,6 +474,7 @@ describe('Entity validator', () => { errors: { badRequest: jest.fn(), }, + getModel: () => ({}), }; const model = { diff --git a/packages/core/strapi/lib/services/entity-service/__tests__/entity-service.test.js b/packages/core/strapi/lib/services/entity-service/__tests__/entity-service.test.js index f1358547df..65a00a8b48 100644 --- a/packages/core/strapi/lib/services/entity-service/__tests__/entity-service.test.js +++ b/packages/core/strapi/lib/services/entity-service/__tests__/entity-service.test.js @@ -3,6 +3,7 @@ jest.mock('bcryptjs', () => ({ hashSync: () => 'secret-password' })); const { EventEmitter } = require('events'); +const { ApplicationError } = require('@strapi/utils').errors; const createEntityService = require('..'); const entityValidator = require('../../entity-validator'); @@ -81,50 +82,107 @@ describe('Entity service', () => { describe('Create', () => { describe('assign default values', () => { let instance; + const entityUID = 'api::entity.entity'; + const relationUID = 'api::relation.relation'; beforeAll(() => { - const fakeQuery = { - count: jest.fn(() => 0), - create: jest.fn(({ data }) => data), - }; - - const fakeModel = { - kind: 'contentType', - modelName: 'test-model', - privateAttributes: [], - options: {}, - attributes: { - attrStringDefaultRequired: { type: 'string', default: 'default value', required: true }, - attrStringDefault: { type: 'string', default: 'default value' }, - attrBoolDefaultRequired: { type: 'boolean', default: true, required: true }, - attrBoolDefault: { type: 'boolean', default: true }, - attrIntDefaultRequired: { type: 'integer', default: 1, required: true }, - attrIntDefault: { type: 'integer', default: 1 }, - attrEnumDefaultRequired: { - type: 'enumeration', - enum: ['a', 'b', 'c'], - default: 'a', - required: true, + const fakeEntities = { + [relationUID]: { + 1: { + id: 1, + Name: 'TestRelation', + createdAt: '2022-09-28T15:11:22.995Z', + updatedAt: '2022-09-29T09:01:02.949Z', + publishedAt: null, }, - attrEnumDefault: { - type: 'enumeration', - enum: ['a', 'b', 'c'], - default: 'b', + 2: { + id: 2, + Name: 'TestRelation2', + createdAt: '2022-09-28T15:11:22.995Z', + updatedAt: '2022-09-29T09:01:02.949Z', + publishedAt: null, }, - attrPassword: { type: 'password' }, }, }; + const fakeModel = { + [entityUID]: { + uid: entityUID, + kind: 'contentType', + modelName: 'test-model', + privateAttributes: [], + options: {}, + attributes: { + attrStringDefaultRequired: { + type: 'string', + default: 'default value', + required: true, + }, + attrStringDefault: { type: 'string', default: 'default value' }, + attrBoolDefaultRequired: { type: 'boolean', default: true, required: true }, + attrBoolDefault: { type: 'boolean', default: true }, + attrIntDefaultRequired: { type: 'integer', default: 1, required: true }, + attrIntDefault: { type: 'integer', default: 1 }, + attrEnumDefaultRequired: { + type: 'enumeration', + enum: ['a', 'b', 'c'], + default: 'a', + required: true, + }, + attrEnumDefault: { + type: 'enumeration', + enum: ['a', 'b', 'c'], + default: 'b', + }, + attrPassword: { type: 'password' }, + attrRelation: { + type: 'relation', + relation: 'oneToMany', + target: relationUID, + mappedBy: 'entity', + }, + }, + }, + [relationUID]: { + uid: relationUID, + kind: 'contentType', + modelName: 'relation', + attributes: { + Name: { + type: 'string', + default: 'default value', + required: true, + }, + }, + }, + }; + const fakeQuery = (uid) => ({ + count: jest.fn(() => 0), + create: jest.fn(({ data }) => data), + findWithCount: jest.fn(({ where }) => { + const ret = []; + where.id.$in.forEach((id) => { + const entity = fakeEntities[uid][id]; + if (!entity) return; + ret.push(entity); + }); + return [ret, ret.length]; + }), + }); + const fakeDB = { - query: jest.fn(() => fakeQuery), + query: jest.fn((uid) => fakeQuery(uid)), }; - const fakeStrapi = { - getModel: jest.fn(() => fakeModel), + global.strapi = { + getModel: jest.fn((uid) => { + return fakeModel[uid]; + }), + db: fakeDB, }; instance = createEntityService({ - strapi: fakeStrapi, + strapi: global.strapi, db: fakeDB, eventHub: new EventEmitter(), entityValidator, @@ -134,7 +192,7 @@ describe('Entity service', () => { test('should create record with all default attributes', async () => { const data = {}; - await expect(instance.create('test-model', { data })).resolves.toMatchObject({ + await expect(instance.create(entityUID, { data })).resolves.toMatchObject({ attrStringDefaultRequired: 'default value', attrStringDefault: 'default value', attrBoolDefaultRequired: true, @@ -154,7 +212,7 @@ describe('Entity service', () => { attrEnumDefault: 'c', }; - await expect(instance.create('test-model', { data })).resolves.toMatchObject({ + await expect(instance.create(entityUID, { data })).resolves.toMatchObject({ attrStringDefault: 'my value', attrBoolDefault: false, attrIntDefault: 2, @@ -179,11 +237,63 @@ describe('Entity service', () => { attrPassword: 'fooBar', }; - await expect(instance.create('test-model', { data })).resolves.toMatchObject({ + await expect(instance.create(entityUID, { data })).resolves.toMatchObject({ ...data, attrPassword: 'secret-password', }); }); + + test('should create record with valid relation', async () => { + const data = { + attrStringDefaultRequired: 'my value', + attrStringDefault: 'my value', + attrBoolDefaultRequired: true, + attrBoolDefault: true, + attrIntDefaultRequired: 10, + attrIntDefault: 10, + attrEnumDefaultRequired: 'c', + attrEnumDefault: 'a', + attrPassword: 'fooBar', + attrRelation: { + connect: [ + { + id: 1, + }, + ], + }, + }; + + const res = instance.create(entityUID, { data }); + + await expect(res).resolves.toMatchObject({ + ...data, + attrPassword: 'secret-password', + }); + }); + + test('should fail to create a record with an invalid relation', async () => { + const data = { + attrStringDefaultRequired: 'my value', + attrStringDefault: 'my value', + attrBoolDefaultRequired: true, + attrBoolDefault: true, + attrIntDefaultRequired: 10, + attrIntDefault: 10, + attrEnumDefaultRequired: 'c', + attrEnumDefault: 'a', + attrPassword: 'fooBar', + attrRelation: { + connect: [ + { + id: 3, + }, + ], + }, + }; + + const res = instance.create(entityUID, { data }); + await expect(res).rejects.toThrow(ApplicationError); + }); }); }); @@ -193,25 +303,36 @@ describe('Entity service', () => { const entityUID = 'api::entity.entity'; const relationUID = 'api::relation.relation'; + const fakeEntities = { - 0: { - id: 0, - Name: 'TestEntity', - createdAt: '2022-09-28T15:11:22.995Z', - updatedAt: '2022-09-29T09:01:02.949Z', - publishedAt: null, + [entityUID]: { + 0: { + id: 0, + Name: 'TestEntity', + createdAt: '2022-09-28T15:11:22.995Z', + updatedAt: '2022-09-29T09:01:02.949Z', + publishedAt: null, + }, }, - 1: { - id: 1, - Name: 'TestRelation', - createdAt: '2022-09-28T15:11:22.995Z', - updatedAt: '2022-09-29T09:01:02.949Z', - publishedAt: null, + [relationUID]: { + 1: { + id: 1, + Name: 'TestRelation', + createdAt: '2022-09-28T15:11:22.995Z', + updatedAt: '2022-09-29T09:01:02.949Z', + publishedAt: null, + }, + 2: { + id: 2, + Name: 'TestRelation2', + createdAt: '2022-09-28T15:11:22.995Z', + updatedAt: '2022-09-29T09:01:02.949Z', + publishedAt: null, + }, }, - 2: null, }; - beforeAll(() => { - const fakeModel = { + const fakeModel = { + [entityUID]: { kind: 'collectionType', modelName: 'entity', collectionName: 'entity', @@ -233,41 +354,56 @@ describe('Entity service', () => { target: relationUID, mappedBy: 'entity', }, - updatedBy: { - type: 'relation', - relation: 'oneToOne', - target: 'admin::user', - configurable: false, - writable: false, - visible: false, - useJoinTable: false, - private: true, + }, + }, + [relationUID]: { + kind: 'contentType', + modelName: 'relation', + attributes: { + Name: { + type: 'string', + default: 'default value', + required: true, }, }, - }; + }, + }; - const fakeQuery = { - findOne: jest.fn(({ where }) => fakeEntities[where.id]), + beforeAll(() => { + const fakeQuery = (key) => ({ + findOne: jest.fn(({ where }) => fakeEntities[key][where.id]), + findWithCount: jest.fn(({ where }) => { + const ret = []; + where.id.$in.forEach((id) => { + const entity = fakeEntities[key][id]; + if (!entity) return; + ret.push(entity); + }); + return [ret, ret.length]; + }), update: jest.fn(({ where }) => ({ - ...fakeEntities[where.id], + ...fakeEntities[key][where.id], addresses: { count: 1, }, })), - }; + }); const fakeDB = { - query: jest.fn(() => fakeQuery), + query: jest.fn((key) => fakeQuery(key)), }; - const fakeStrapi = { - getModel: jest.fn(() => fakeModel), + global.strapi = { + getModel: jest.fn((uid) => { + return fakeModel[uid]; + }), + db: fakeDB, }; instance = createEntityService({ - strapi: fakeStrapi, + strapi: global.strapi, db: fakeDB, - eventHub: null, // bypass event emission for update tests + eventHub: new EventEmitter(), entityValidator, }); }); @@ -278,7 +414,7 @@ describe('Entity service', () => { ).toBeNull(); }); - test('should successfully update with an existing relation', async () => { + test('should successfully update an existing relation', async () => { const data = { Name: 'TestEntry', addresses: { @@ -288,10 +424,9 @@ describe('Entity service', () => { }, ], }, - updatedBy: 1, }; expect(await instance.update(entityUID, 0, { data })).toMatchObject({ - ...fakeEntities[0], + ...fakeEntities[entityUID][0], addresses: { count: 1, }, @@ -304,14 +439,14 @@ describe('Entity service', () => { addresses: { connect: [ { - id: 2, + id: 3, }, ], }, - updatedBy: 1, }; - await expect(instance.update(entityUID, 0, { data })).rejects.toThrow(); + const res = instance.update(entityUID, 0, { data }); + await expect(res).rejects.toThrow(ApplicationError); }); }); }); diff --git a/packages/core/strapi/lib/services/entity-service/index.js b/packages/core/strapi/lib/services/entity-service/index.js index bfd9e54dbb..cc7e99187d 100644 --- a/packages/core/strapi/lib/services/entity-service/index.js +++ b/packages/core/strapi/lib/services/entity-service/index.js @@ -9,7 +9,7 @@ const { contentTypes: contentTypesUtils, sanitize, } = require('@strapi/utils'); -const { ValidationError, ApplicationError } = require('@strapi/utils').errors; +const { ValidationError } = require('@strapi/utils').errors; const { isAnyToMany } = require('@strapi/utils').relations; const { transformParamsToQuery } = require('@strapi/utils').convertQueryParams; const uploadFiles = require('../utils/upload-files'); @@ -48,7 +48,6 @@ const createDefaultImplementation = ({ strapi, db, eventHub, entityValidator }) }, async emitEvent(uid, event, entity) { - if (!eventHub) return; const model = strapi.getModel(uid); const sanitizedEntity = await sanitize.sanitizers.defaultSanitizeOutput(model, entity); @@ -170,38 +169,6 @@ const createDefaultImplementation = ({ strapi, db, eventHub, entityValidator }) entityToUpdate ); - // Create an array of the relations we are attempting to associate with this - // entity. - const relationChecks = []; - Object.keys(data).forEach((key) => { - const attribute = model.attributes[key]; - if (attribute?.type !== 'relation') { - return; - } - if (!data[key]?.connect) { - return; - } - relationChecks.push({ uid: attribute.target, data: data[key].connect }); - }); - - // Confirm that these relations exists in the DB before performing the query. - await Promise.all( - relationChecks.map(async (check) => { - await Promise.all( - check.data.map(async (d) => { - const relationEntity = await db.query(check.uid).findOne({ where: { id: d.id } }); - if (relationEntity) { - return; - } - // Trying to associate a relation with this entity that does not exist - throw new ApplicationError( - `Relation of type ${check.uid} with id ${d.id} does not exist` - ); - }) - ); - }) - ); - const query = transformParamsToQuery(uid, pickSelectionParams(wrappedParams)); // TODO: wrap in transaction diff --git a/packages/core/strapi/lib/services/entity-validator/index.js b/packages/core/strapi/lib/services/entity-validator/index.js index 9473366198..2a0f7608b6 100644 --- a/packages/core/strapi/lib/services/entity-validator/index.js +++ b/packages/core/strapi/lib/services/entity-validator/index.js @@ -5,6 +5,7 @@ 'use strict'; +const { isEmpty, uniqBy, castArray, isNil } = require('lodash'); const { has, assoc, prop, isObject } = require('lodash/fp'); const strapiUtils = require('@strapi/utils'); const validators = require('./validators'); @@ -204,6 +205,107 @@ const createModelValidator = return yup.object().shape(schema); }; +/** + * Builds a map containing all the media and relations being associated with an entity + * @param {String} uid of the model + * @param {Object} data + * @param {Map} relationsMap to be updated and returned + * @returns + */ +const buildRelationsMap = (uid, data, relationsMap = new Map()) => { + const currentModel = strapi.getModel(uid); + if (isEmpty(currentModel)) return; + + Object.keys(currentModel.attributes).forEach((attributeName) => { + const attribute = currentModel.attributes[attributeName]; + const value = data[attributeName]; + if (isEmpty(value) || isNil(value)) { + return; + } + switch (attribute.type) { + case 'relation': { + if (!attribute.target) { + break; + } + + // If the attribute type is a relation keep track of all + // associations being made with relations. These will later be checked + // against the DB to confirm they exist + let directValue = []; + if (Array.isArray(value)) { + directValue = value.map((v) => ({ id: v })); + } + relationsMap.set( + attribute.target, + (relationsMap.get(attribute.target) || []).concat( + ...(value.connect || value.set || directValue) + ) + ); + break; + } + case 'media': { + // For media attribute types keep track of all media associated with + // this entity. These will later be checked against the DB to confirm + // they exist + const mediaUID = 'plugin::upload.file'; + castArray(value).forEach((v) => + relationsMap.set(mediaUID, [...(relationsMap.get(mediaUID) || []), { id: v.id || v }]) + ); + break; + } + case 'component': { + return castArray(value).forEach((componentValue) => + buildRelationsMap(attribute.component, componentValue, relationsMap) + ); + } + case 'dynamiczone': { + return value.forEach((dzValue) => + buildRelationsMap(dzValue.__component, dzValue, relationsMap) + ); + } + default: + break; + } + }); + + return relationsMap; +}; + +/** + * Iterate through the relations map and validate that every relation or media + * mentioned exists + */ +const checkRelationsExist = async (relationsMap = new Map()) => { + const promises = []; + for (const [key, value] of relationsMap) { + const evaluate = async () => { + const uniqueValues = uniqBy(value, `id`); + // eslint-disable-next-line no-unused-vars + const [entities, count] = await strapi.db.query(key).findWithCount({ + where: { + id: { + $in: uniqueValues.map((v) => Number(v.id)), + }, + }, + }); + + if (count !== uniqueValues.length) { + const missingEntities = uniqueValues.filter( + (value) => !entities.find((entity) => entity.id === value.id) + ); + throw new ValidationError( + `Relations of type ${key} associated with this entity do not exist. IDs: ${missingEntities + .map((entity) => entity.id) + .join(',')}` + ); + } + }; + promises.push(evaluate()); + } + + return Promise.all(promises); +}; + const createValidateEntity = (createOrUpdate) => async (model, data, { isDraft = false } = {}, entity = null) => { @@ -222,7 +324,20 @@ const createValidateEntity = entity, }, { isDraft } - ).required(); + ) + .test('relations-test', 'check that all relations exist', async function (data) { + try { + await checkRelationsExist(buildRelationsMap(model.uid, data) || new Map()); + } catch (e) { + return this.createError({ + path: this.path, + message: e.message, + }); + } + return true; + }) + .required(); + return validateYupSchema(validator, { strict: false, abortEarly: false })(data); }; diff --git a/packages/core/strapi/tests/endpoint.test.e2e.js b/packages/core/strapi/tests/endpoint.test.e2e.js index 6739f2e977..05e41bc0c0 100644 --- a/packages/core/strapi/tests/endpoint.test.e2e.js +++ b/packages/core/strapi/tests/endpoint.test.e2e.js @@ -143,6 +143,32 @@ describe('Create Strapi API End to End', () => { expect(body.data.attributes.tags.data[0].id).toBe(data.tags[0].id); }); + test('Create article with non existent tag', async () => { + const entry = { + title: 'Article 3', + content: 'Content 3', + tags: [1000], + }; + + const res = await rq({ + url: '/articles', + method: 'POST', + body: { + data: entry, + }, + qs: { + populate: ['tags'], + }, + }); + + expect(res.statusCode).toBe(400); + expect(JSON.parse(res.error.text).error.message).toContain( + `Relations of type api::tag.tag associated with this entity do not exist. IDs: ${entry.tags.join( + ',' + )}` + ); + }); + test('Update article1 add tag2', async () => { const { id, attributes } = data.articles[0]; const entry = { ...attributes, tags: [data.tags[1].id] }; @@ -197,6 +223,32 @@ describe('Create Strapi API End to End', () => { expect(body.data.attributes.tags.data.length).toBe(3); }); + test('Update article1 adding a non existent tag', async () => { + const { id, attributes } = data.articles[0]; + const entry = { ...attributes }; + entry.tags = [1000, 1001, 1002]; + + cleanDate(entry); + + const res = await rq({ + url: `/articles/${id}`, + method: 'PUT', + body: { + data: entry, + }, + qs: { + populate: ['tags'], + }, + }); + + expect(res.statusCode).toBe(400); + expect(JSON.parse(res.error.text).error.message).toContain( + `Relations of type api::tag.tag associated with this entity do not exist. IDs: ${entry.tags.join( + ',' + )}` + ); + }); + test('Update article1 remove one tag', async () => { const { id, attributes } = data.articles[0]; From 00d6962b5e52e085d2dcf986c64dacef59f9c65e Mon Sep 17 00:00:00 2001 From: Jamie Howard Date: Mon, 24 Oct 2022 12:59:38 +0100 Subject: [PATCH 03/12] feat(strapi): entity validator --- .../__tests__/entity-validator.test.js | 57 ++--- .../__tests__/entity-service-events.test.js | 31 +-- .../__tests__/entity-service.test.js | 17 +- .../lib/services/entity-validator/index.js | 197 +++++++++--------- .../core/strapi/tests/endpoint.test.e2e.js | 8 +- 5 files changed, 143 insertions(+), 167 deletions(-) diff --git a/packages/core/strapi/lib/services/__tests__/entity-validator.test.js b/packages/core/strapi/lib/services/__tests__/entity-validator.test.js index 28ca9eb960..4264975fd6 100644 --- a/packages/core/strapi/lib/services/__tests__/entity-validator.test.js +++ b/packages/core/strapi/lib/services/__tests__/entity-validator.test.js @@ -5,15 +5,16 @@ const entityValidator = require('../entity-validator'); describe('Entity validator', () => { describe('Published input', () => { describe('General Errors', () => { - it('Throws a badRequest error on invalid input', async () => { - global.strapi = { - errors: { - badRequest: jest.fn(), - }, - getModel: () => ({}), - }; + let model; + global.strapi = { + errors: { + badRequest: jest.fn(), + }, + getModel: () => model, + }; - const model = { + it('Throws a badRequest error on invalid input', async () => { + model = { attributes: { title: { type: 'string', @@ -45,7 +46,7 @@ describe('Entity validator', () => { }); it('Returns data on valid input', async () => { - const model = { + model = { attributes: { title: { type: 'string', @@ -62,7 +63,7 @@ describe('Entity validator', () => { }); it('Returns casted data when possible', async () => { - const model = { + model = { attributes: { title: { type: 'string', @@ -85,14 +86,7 @@ describe('Entity validator', () => { }); test('Throws on required not respected', async () => { - global.strapi = { - errors: { - badRequest: jest.fn(), - }, - getModel: () => ({}), - }; - - const model = { + model = { attributes: { title: { type: 'string', @@ -141,7 +135,7 @@ describe('Entity validator', () => { }); it('Supports custom field types', async () => { - const model = { + model = { attributes: { uuid: { type: 'uuid', @@ -166,7 +160,7 @@ describe('Entity validator', () => { errors: { badRequest: jest.fn(), }, - getModel: () => ({}), + getModel: () => model, }; const model = { @@ -202,13 +196,6 @@ describe('Entity validator', () => { }); test('Throws on max length not respected', async () => { - global.strapi = { - errors: { - badRequest: jest.fn(), - }, - getModel: () => ({}), - }; - const model = { attributes: { title: { @@ -333,7 +320,7 @@ describe('Entity validator', () => { errors: { badRequest: jest.fn(), }, - getModel: () => ({}), + getModel: () => model, }; const model = { @@ -461,6 +448,13 @@ describe('Entity validator', () => { }, }; + global.strapi = { + errors: { + badRequest: jest.fn(), + }, + getModel: () => model, + }; + const input = { title: 'tooSmall' }; expect.hasAssertions(); @@ -470,13 +464,6 @@ describe('Entity validator', () => { }); test('Throws on max length not respected', async () => { - global.strapi = { - errors: { - badRequest: jest.fn(), - }, - getModel: () => ({}), - }; - const model = { attributes: { title: { diff --git a/packages/core/strapi/lib/services/entity-service/__tests__/entity-service-events.test.js b/packages/core/strapi/lib/services/entity-service/__tests__/entity-service-events.test.js index dca0c8ffdd..ae7c4e8570 100644 --- a/packages/core/strapi/lib/services/entity-service/__tests__/entity-service-events.test.js +++ b/packages/core/strapi/lib/services/entity-service/__tests__/entity-service-events.test.js @@ -4,28 +4,22 @@ const createEntityService = require('..'); const entityValidator = require('../../entity-validator'); describe('Entity service triggers webhooks', () => { - global.strapi = { - getModel: () => ({}), - config: { - get: () => [], - }, - }; - let instance; const eventHub = { emit: jest.fn() }; let entity = { attr: 'value' }; beforeAll(() => { + const model = { + kind: 'singleType', + modelName: 'test-model', + privateAttributes: [], + attributes: { + attr: { type: 'string' }, + }, + }; instance = createEntityService({ strapi: { - getModel: () => ({ - kind: 'singleType', - modelName: 'test-model', - privateAttributes: [], - attributes: { - attr: { type: 'string' }, - }, - }), + getModel: () => model, }, db: { query: () => ({ @@ -41,6 +35,13 @@ describe('Entity service triggers webhooks', () => { eventHub, entityValidator, }); + + global.strapi = { + getModel: () => model, + config: { + get: () => [], + }, + }; }); test('Emit event: Create', async () => { diff --git a/packages/core/strapi/lib/services/entity-service/__tests__/entity-service.test.js b/packages/core/strapi/lib/services/entity-service/__tests__/entity-service.test.js index 65a00a8b48..7c95ab8b61 100644 --- a/packages/core/strapi/lib/services/entity-service/__tests__/entity-service.test.js +++ b/packages/core/strapi/lib/services/entity-service/__tests__/entity-service.test.js @@ -157,16 +157,15 @@ describe('Entity service', () => { }, }; const fakeQuery = (uid) => ({ - count: jest.fn(() => 0), create: jest.fn(({ data }) => data), - findWithCount: jest.fn(({ where }) => { - const ret = []; + count: jest.fn(({ where }) => { + let ret = 0; where.id.$in.forEach((id) => { const entity = fakeEntities[uid][id]; if (!entity) return; - ret.push(entity); + ret += 1; }); - return [ret, ret.length]; + return ret; }), }); @@ -372,14 +371,14 @@ describe('Entity service', () => { beforeAll(() => { const fakeQuery = (key) => ({ findOne: jest.fn(({ where }) => fakeEntities[key][where.id]), - findWithCount: jest.fn(({ where }) => { - const ret = []; + count: jest.fn(({ where }) => { + let ret = 0; where.id.$in.forEach((id) => { const entity = fakeEntities[key][id]; if (!entity) return; - ret.push(entity); + ret += 1; }); - return [ret, ret.length]; + return ret; }), update: jest.fn(({ where }) => ({ ...fakeEntities[key][where.id], diff --git a/packages/core/strapi/lib/services/entity-validator/index.js b/packages/core/strapi/lib/services/entity-validator/index.js index 2a0f7608b6..ddc340049e 100644 --- a/packages/core/strapi/lib/services/entity-validator/index.js +++ b/packages/core/strapi/lib/services/entity-validator/index.js @@ -205,107 +205,6 @@ const createModelValidator = return yup.object().shape(schema); }; -/** - * Builds a map containing all the media and relations being associated with an entity - * @param {String} uid of the model - * @param {Object} data - * @param {Map} relationsMap to be updated and returned - * @returns - */ -const buildRelationsMap = (uid, data, relationsMap = new Map()) => { - const currentModel = strapi.getModel(uid); - if (isEmpty(currentModel)) return; - - Object.keys(currentModel.attributes).forEach((attributeName) => { - const attribute = currentModel.attributes[attributeName]; - const value = data[attributeName]; - if (isEmpty(value) || isNil(value)) { - return; - } - switch (attribute.type) { - case 'relation': { - if (!attribute.target) { - break; - } - - // If the attribute type is a relation keep track of all - // associations being made with relations. These will later be checked - // against the DB to confirm they exist - let directValue = []; - if (Array.isArray(value)) { - directValue = value.map((v) => ({ id: v })); - } - relationsMap.set( - attribute.target, - (relationsMap.get(attribute.target) || []).concat( - ...(value.connect || value.set || directValue) - ) - ); - break; - } - case 'media': { - // For media attribute types keep track of all media associated with - // this entity. These will later be checked against the DB to confirm - // they exist - const mediaUID = 'plugin::upload.file'; - castArray(value).forEach((v) => - relationsMap.set(mediaUID, [...(relationsMap.get(mediaUID) || []), { id: v.id || v }]) - ); - break; - } - case 'component': { - return castArray(value).forEach((componentValue) => - buildRelationsMap(attribute.component, componentValue, relationsMap) - ); - } - case 'dynamiczone': { - return value.forEach((dzValue) => - buildRelationsMap(dzValue.__component, dzValue, relationsMap) - ); - } - default: - break; - } - }); - - return relationsMap; -}; - -/** - * Iterate through the relations map and validate that every relation or media - * mentioned exists - */ -const checkRelationsExist = async (relationsMap = new Map()) => { - const promises = []; - for (const [key, value] of relationsMap) { - const evaluate = async () => { - const uniqueValues = uniqBy(value, `id`); - // eslint-disable-next-line no-unused-vars - const [entities, count] = await strapi.db.query(key).findWithCount({ - where: { - id: { - $in: uniqueValues.map((v) => Number(v.id)), - }, - }, - }); - - if (count !== uniqueValues.length) { - const missingEntities = uniqueValues.filter( - (value) => !entities.find((entity) => entity.id === value.id) - ); - throw new ValidationError( - `Relations of type ${key} associated with this entity do not exist. IDs: ${missingEntities - .map((entity) => entity.id) - .join(',')}` - ); - } - }; - promises.push(evaluate()); - } - - return Promise.all(promises); -}; - const createValidateEntity = (createOrUpdate) => async (model, data, { isDraft = false } = {}, entity = null) => { @@ -327,7 +226,7 @@ const createValidateEntity = ) .test('relations-test', 'check that all relations exist', async function (data) { try { - await checkRelationsExist(buildRelationsMap(model.uid, data) || new Map()); + await checkRelationsExist(buildRelationsStore(model.uid, data) || {}); } catch (e) { return this.createError({ path: this.path, @@ -341,6 +240,100 @@ const createValidateEntity = return validateYupSchema(validator, { strict: false, abortEarly: false })(data); }; +/** + * Builds an object containing all the media and relations being associated with an entity + * @param {String} uid of the model + * @param {Object} data + * @param {Object} relationsStore to be updated and returned + * @returns + */ +const buildRelationsStore = (uid, data, relationsStore = {}) => { + const currentModel = strapi.getModel(uid); + + Object.keys(currentModel?.attributes || {}).forEach((attributeName) => { + const attribute = currentModel.attributes[attributeName]; + + const value = data[attributeName]; + if (isEmpty(value) || isNil(value)) { + return; + } + + switch (attribute.type) { + case 'relation': { + if (!attribute.target) { + return; + } + // If the attribute type is a relation keep track of all + // associations being made with relations. + let directValue = []; + if (Array.isArray(value)) { + directValue = value.map((v) => ({ id: v })); + } + relationsStore[attribute.target] = relationsStore[attribute.target] || []; + relationsStore[attribute.target].push(...(value.connect || value.set || directValue)); + break; + } + case 'media': { + // For media attribute types keep track of all media associated with + // this entity. + const mediaUID = 'plugin::upload.file'; + castArray(value).forEach((v) => { + relationsStore[mediaUID] = relationsStore[mediaUID] || []; + relationsStore[mediaUID].push({ id: v.id || v }); + }); + break; + } + case 'component': { + return castArray(value).forEach((componentValue) => + buildRelationsStore(attribute.component, componentValue, relationsStore) + ); + } + case 'dynamiczone': { + return value.forEach((dzValue) => + buildRelationsStore(dzValue.__component, dzValue, relationsStore) + ); + } + default: + break; + } + }); + + return relationsStore; +}; + +/** + * Iterate through the relations store and validates that every relation or media + * mentioned exists + */ +const checkRelationsExist = async (relationsStore = {}) => { + const promises = []; + + for (const [key, value] of Object.entries(relationsStore)) { + const evaluate = async () => { + const uniqueValues = uniqBy(value, `id`); + // eslint-disable-next-line no-unused-vars + const count = await strapi.db.query(key).count({ + where: { + id: { + $in: uniqueValues.map((v) => Number(v.id)), + }, + }, + }); + + if (count !== uniqueValues.length) { + throw new ValidationError( + `${ + uniqueValues.length - count + } relation(s) of type ${key} associated with this entity do not exist` + ); + } + }; + promises.push(evaluate()); + } + + return Promise.all(promises); +}; + module.exports = { validateEntityCreation: createValidateEntity('creation'), validateEntityUpdate: createValidateEntity('update'), diff --git a/packages/core/strapi/tests/endpoint.test.e2e.js b/packages/core/strapi/tests/endpoint.test.e2e.js index 05e41bc0c0..d7b19a3bec 100644 --- a/packages/core/strapi/tests/endpoint.test.e2e.js +++ b/packages/core/strapi/tests/endpoint.test.e2e.js @@ -163,9 +163,7 @@ describe('Create Strapi API End to End', () => { expect(res.statusCode).toBe(400); expect(JSON.parse(res.error.text).error.message).toContain( - `Relations of type api::tag.tag associated with this entity do not exist. IDs: ${entry.tags.join( - ',' - )}` + `1 relation(s) of type api::tag.tag associated with this entity do not exist` ); }); @@ -243,9 +241,7 @@ describe('Create Strapi API End to End', () => { expect(res.statusCode).toBe(400); expect(JSON.parse(res.error.text).error.message).toContain( - `Relations of type api::tag.tag associated with this entity do not exist. IDs: ${entry.tags.join( - ',' - )}` + `3 relation(s) of type api::tag.tag associated with this entity do not exist` ); }); From 6aa6f394fe5954b00028e79e7958429c8588face Mon Sep 17 00:00:00 2001 From: Jamie Howard Date: Wed, 2 Nov 2022 10:39:16 +0000 Subject: [PATCH 04/12] fix(entity-validator): support all relation formats --- .../lib/services/entity-validator/index.js | 76 ++++++++++--------- 1 file changed, 42 insertions(+), 34 deletions(-) diff --git a/packages/core/strapi/lib/services/entity-validator/index.js b/packages/core/strapi/lib/services/entity-validator/index.js index ddc340049e..d8998cf9ee 100644 --- a/packages/core/strapi/lib/services/entity-validator/index.js +++ b/packages/core/strapi/lib/services/entity-validator/index.js @@ -226,7 +226,7 @@ const createValidateEntity = ) .test('relations-test', 'check that all relations exist', async function (data) { try { - await checkRelationsExist(buildRelationsStore(model.uid, data) || {}); + await checkRelationsExist(buildRelationsStore({ uid: model.uid, data })); } catch (e) { return this.createError({ path: this.path, @@ -240,6 +240,23 @@ const createValidateEntity = return validateYupSchema(validator, { strict: false, abortEarly: false })(data); }; +/** + * Assign values in the relationsStore + * @param {Object} relationsValue containing the relation IDs + * @param {String} target the UID of these relations + * @param {Object} relationsStore to be updated + */ +const updateRelationsStore = ({ relationsValue = null, target = '', relationsStore = {} }) => { + if (isEmpty(target) || isNil(relationsValue)) { + return; + } + const source = relationsValue.connect || relationsValue.set || castArray(relationsValue); + const idArray = source.map((v) => ({ id: v.id || v })); + + relationsStore[target] = relationsStore[target] || []; + relationsStore[target].push(...idArray); +}; + /** * Builds an object containing all the media and relations being associated with an entity * @param {String} uid of the model @@ -247,51 +264,43 @@ const createValidateEntity = * @param {Object} relationsStore to be updated and returned * @returns */ -const buildRelationsStore = (uid, data, relationsStore = {}) => { +const buildRelationsStore = ({ uid = '', data = null, relationsStore = {} }) => { + if (isEmpty(uid) || isNil(data)) { + return relationsStore; + } + const currentModel = strapi.getModel(uid); - Object.keys(currentModel?.attributes || {}).forEach((attributeName) => { + Object.keys(currentModel.attributes).forEach((attributeName) => { const attribute = currentModel.attributes[attributeName]; const value = data[attributeName]; - if (isEmpty(value) || isNil(value)) { + if (isNil(value)) { return; } switch (attribute.type) { - case 'relation': { - if (!attribute.target) { + case 'relation': + case 'media': { + const target = + attribute.type === 'media' ? 'plugin::upload.file' : attribute?.target ?? null; + if (!target) { return; } - // If the attribute type is a relation keep track of all - // associations being made with relations. - let directValue = []; - if (Array.isArray(value)) { - directValue = value.map((v) => ({ id: v })); - } - relationsStore[attribute.target] = relationsStore[attribute.target] || []; - relationsStore[attribute.target].push(...(value.connect || value.set || directValue)); + // Keep track of all associations being made with relations or media. + updateRelationsStore({ relationsValue: value, target, relationsStore }); break; } - case 'media': { - // For media attribute types keep track of all media associated with - // this entity. - const mediaUID = 'plugin::upload.file'; - castArray(value).forEach((v) => { - relationsStore[mediaUID] = relationsStore[mediaUID] || []; - relationsStore[mediaUID].push({ id: v.id || v }); - }); - break; - } - case 'component': { - return castArray(value).forEach((componentValue) => - buildRelationsStore(attribute.component, componentValue, relationsStore) - ); - } + case 'component': case 'dynamiczone': { - return value.forEach((dzValue) => - buildRelationsStore(dzValue.__component, dzValue, relationsStore) - ); + const key = attribute.target === 'dynamiczone' ? `__component` : `component`; + const values = castArray(value); + return values.forEach((value) => { + if (!value || !value[key]) { + return; + } + buildRelationsStore({ uid: value[key], data: value, relationsStore }); + }); } default: break; @@ -311,11 +320,10 @@ const checkRelationsExist = async (relationsStore = {}) => { for (const [key, value] of Object.entries(relationsStore)) { const evaluate = async () => { const uniqueValues = uniqBy(value, `id`); - // eslint-disable-next-line no-unused-vars const count = await strapi.db.query(key).count({ where: { id: { - $in: uniqueValues.map((v) => Number(v.id)), + $in: uniqueValues.map((v) => v.id), }, }, }); From 3abbc7f89fe9e2ddbc22226d5cde4a758e974640 Mon Sep 17 00:00:00 2001 From: Jamie Howard Date: Thu, 3 Nov 2022 14:47:02 +0000 Subject: [PATCH 05/12] fix: support all relation connection formats --- .../lib/services/entity-validator/index.js | 64 +++++++++---------- 1 file changed, 30 insertions(+), 34 deletions(-) diff --git a/packages/core/strapi/lib/services/entity-validator/index.js b/packages/core/strapi/lib/services/entity-validator/index.js index d8998cf9ee..9ef2c980b5 100644 --- a/packages/core/strapi/lib/services/entity-validator/index.js +++ b/packages/core/strapi/lib/services/entity-validator/index.js @@ -5,7 +5,7 @@ 'use strict'; -const { isEmpty, uniqBy, castArray, isNil } = require('lodash'); +const { uniqBy, castArray, isNil } = require('lodash'); const { has, assoc, prop, isObject } = require('lodash/fp'); const strapiUtils = require('@strapi/utils'); const validators = require('./validators'); @@ -240,23 +240,6 @@ const createValidateEntity = return validateYupSchema(validator, { strict: false, abortEarly: false })(data); }; -/** - * Assign values in the relationsStore - * @param {Object} relationsValue containing the relation IDs - * @param {String} target the UID of these relations - * @param {Object} relationsStore to be updated - */ -const updateRelationsStore = ({ relationsValue = null, target = '', relationsStore = {} }) => { - if (isEmpty(target) || isNil(relationsValue)) { - return; - } - const source = relationsValue.connect || relationsValue.set || castArray(relationsValue); - const idArray = source.map((v) => ({ id: v.id || v })); - - relationsStore[target] = relationsStore[target] || []; - relationsStore[target].push(...idArray); -}; - /** * Builds an object containing all the media and relations being associated with an entity * @param {String} uid of the model @@ -264,8 +247,8 @@ const updateRelationsStore = ({ relationsValue = null, target = '', relationsSto * @param {Object} relationsStore to be updated and returned * @returns */ -const buildRelationsStore = ({ uid = '', data = null, relationsStore = {} }) => { - if (isEmpty(uid) || isNil(data)) { +const buildRelationsStore = ({ uid, data, relationsStore = {} }) => { + if (!uid || !data) { return relationsStore; } @@ -282,25 +265,38 @@ const buildRelationsStore = ({ uid = '', data = null, relationsStore = {} }) => switch (attribute.type) { case 'relation': case 'media': { - const target = - attribute.type === 'media' ? 'plugin::upload.file' : attribute?.target ?? null; + const target = attribute.type === 'media' ? 'plugin::upload.file' : attribute.target; if (!target) { - return; + break; } - // Keep track of all associations being made with relations or media. - updateRelationsStore({ relationsValue: value, target, relationsStore }); + + // As there are multiple formats supported for associating relations + // with an entity, the value here can be an: array, object or number. + let source; + if (Array.isArray(value)) { + source = value; + } else if (isObject(value)) { + source = value?.connect ?? value?.set ?? []; + } else { + source = castArray(value); + } + const idArray = source.map((v) => ({ id: v.id || v })); + + // Update the relationStore to keep track of all associations being made + // with relations and media. + relationsStore[target] = relationsStore[target] || []; + relationsStore[target].push(...idArray); break; } - case 'component': + case 'component': { + return castArray(value).forEach((componentValue) => + buildRelationsStore({ uid: attribute.component, data: componentValue, relationsStore }) + ); + } case 'dynamiczone': { - const key = attribute.target === 'dynamiczone' ? `__component` : `component`; - const values = castArray(value); - return values.forEach((value) => { - if (!value || !value[key]) { - return; - } - buildRelationsStore({ uid: value[key], data: value, relationsStore }); - }); + return value.forEach((dzValue) => + buildRelationsStore({ uid: dzValue.__component, data: dzValue, relationsStore }) + ); } default: break; From 2b56063804c9989ca96da3170e6aabcc158bbcfb Mon Sep 17 00:00:00 2001 From: Jamie Howard Date: Fri, 4 Nov 2022 13:14:16 +0000 Subject: [PATCH 06/12] test(entity-validator): test all relation validation cases --- .../__tests__/entity-validator.test.js | 685 ++++++++++++++++++ .../__tests__/entity-service.test.js | 14 +- 2 files changed, 696 insertions(+), 3 deletions(-) diff --git a/packages/core/strapi/lib/services/__tests__/entity-validator.test.js b/packages/core/strapi/lib/services/__tests__/entity-validator.test.js index 4264975fd6..457b5bf4ba 100644 --- a/packages/core/strapi/lib/services/__tests__/entity-validator.test.js +++ b/packages/core/strapi/lib/services/__tests__/entity-validator.test.js @@ -1,5 +1,8 @@ 'use strict'; +const { sample } = require('lodash'); +const { ValidationError } = require('@strapi/utils').errors; + const entityValidator = require('../entity-validator'); describe('Entity validator', () => { @@ -324,6 +327,7 @@ describe('Entity validator', () => { }; const model = { + uid: 'api::test.test', attributes: { title: { type: 'string', @@ -585,4 +589,685 @@ describe('Entity validator', () => { }); }); }); + + /** + * Test that relations can be successfully validated and non existent relations + * can be detected at every possible level: Attribute, Single/Repeatable + * Component, Media or Dynamic Zone. + */ + describe('Relations', () => { + const models = new Map(); + models.set('api::dev.dev', { + kind: 'collectionType', + collectionName: 'devs', + modelType: 'contentType', + modelName: 'dev', + connection: 'default', + uid: 'api::dev.dev', + apiName: 'dev', + globalId: 'Dev', + info: { + singularName: 'dev', + pluralName: 'devs', + displayName: 'Dev', + description: '', + }, + attributes: { + categories: { + type: 'relation', + relation: 'manyToMany', + target: 'api::category.category', + inversedBy: 'devs', + }, + sCom: { + type: 'component', + repeatable: false, + component: 'basic.dev-compo', + }, + rCom: { + type: 'component', + repeatable: true, + component: 'basic.dev-compo', + }, + DZ: { + type: 'dynamiczone', + components: ['basic.dev-compo'], + }, + media: { + allowedTypes: ['images', 'files', 'videos', 'audios'], + type: 'media', + multiple: true, + }, + createdAt: { + type: 'datetime', + }, + updatedAt: { + type: 'datetime', + }, + publishedAt: { + type: 'datetime', + configurable: false, + writable: true, + visible: false, + }, + createdBy: { + type: 'relation', + relation: 'oneToOne', + target: 'admin::user', + configurable: false, + writable: false, + visible: false, + useJoinTable: false, + private: true, + }, + updatedBy: { + type: 'relation', + relation: 'oneToOne', + target: 'admin::user', + configurable: false, + writable: false, + visible: false, + useJoinTable: false, + private: true, + }, + }, + }); + models.set('api::category.category', { + kind: 'collectionType', + collectionName: 'categories', + modelType: 'contentType', + modelName: 'category', + connection: 'default', + uid: 'api::category.category', + apiName: 'category', + globalId: 'Category', + info: { + displayName: 'Category', + singularName: 'category', + pluralName: 'categories', + description: '', + name: 'Category', + }, + attributes: { + name: { + type: 'string', + pluginOptions: { + i18n: { + localized: true, + }, + }, + }, + }, + }); + models.set('basic.dev-compo', { + collectionName: 'components_basic_dev_compos', + uid: 'basic.dev-compo', + category: 'basic', + modelType: 'component', + modelName: 'dev-compo', + globalId: 'ComponentBasicDevCompo', + info: { + displayName: 'DevCompo', + icon: 'allergies', + }, + attributes: { + categories: { + type: 'relation', + relation: 'oneToMany', + target: 'api::category.category', + }, + }, + }); + models.set('plugin::upload.file', { + collectionName: 'files', + info: { + singularName: 'file', + pluralName: 'files', + displayName: 'File', + description: '', + }, + attributes: { + name: { + type: 'string', + configurable: false, + required: true, + }, + }, + kind: 'collectionType', + modelType: 'contentType', + modelName: 'file', + connection: 'default', + uid: 'plugin::upload.file', + plugin: 'upload', + globalId: 'UploadFile', + }); + + const IDsThatExist = [1, 2, 3, 4, 5, 6]; + const nonExistentIDs = [10, 11, 12, 13, 14, 15, 16]; + const strapi = { + components: { + 'basic.dev-compo': {}, + }, + db: { + query() { + return { + count: ({ + where: { + id: { $in }, + }, + }) => IDsThatExist.filter((value) => $in.includes(value)).length, + }; + }, + }, + errors: { + badRequest: jest.fn(), + }, + getModel: (uid) => models.get(uid), + }; + + describe('Attribute', () => { + describe('Success', () => { + const testData = [ + [ + 'Connect', + { + categories: { + disconnect: [], + connect: [ + { + id: sample(IDsThatExist), + }, + ], + }, + }, + ], + [ + 'Set', + { + categories: { + set: [ + { + id: sample(IDsThatExist), + }, + ], + }, + }, + ], + [ + 'Number', + { + categories: sample(IDsThatExist), + }, + ], + [ + 'Array', + { + categories: IDsThatExist.slice(-Math.floor(IDsThatExist.length / 2)), + }, + ], + ]; + test.each(testData)('%s', async (__, input = {}) => { + global.strapi = strapi; + const res = entityValidator.validateEntityCreation(models.get('api::dev.dev'), input, { + isDraft: true, + }); + await expect(res).resolves.not.toThrowError(); + }); + }); + + describe('Error', () => { + const expectError = new ValidationError( + `2 relation(s) of type api::category.category associated with this entity do not exist` + ); + const testData = [ + [ + 'Connect', + { + categories: { + disconnect: [], + connect: [sample(IDsThatExist), ...nonExistentIDs.slice(-2)].map((id) => ({ + id, + })), + }, + }, + ], + [ + 'Set', + { + categories: { + set: [sample(IDsThatExist), ...nonExistentIDs.slice(-2)].map((id) => ({ id })), + }, + }, + ], + [ + 'Number', + { + categories: nonExistentIDs.slice(-2), + }, + ], + ]; + + test.each(testData)('%s', async (__, input = {}) => { + global.strapi = strapi; + const res = entityValidator.validateEntityCreation(models.get('api::dev.dev'), input, { + isDraft: true, + }); + await expect(res).rejects.toThrowError(expectError); + }); + }); + }); + + describe('Single Component', () => { + describe('Success', () => { + const testData = [ + [ + 'Connect', + { + sCom: { + categories: { + disconnect: [], + connect: [ + { + id: sample(IDsThatExist), + }, + ], + }, + }, + }, + ], + [ + 'Set', + { + sCom: { + categories: { + set: [ + { + id: sample(IDsThatExist), + }, + ], + }, + }, + }, + ], + [ + 'Number', + { + sCom: { + categories: sample(IDsThatExist), + }, + }, + ], + [ + 'Array', + { + sCom: { + categories: IDsThatExist.slice(-3), + }, + }, + ], + ]; + + test.each(testData)('%s', async (__, input = {}) => { + global.strapi = strapi; + const res = entityValidator.validateEntityCreation(models.get('api::dev.dev'), input, { + isDraft: true, + }); + await expect(res).resolves.not.toThrowError(); + }); + }); + + describe('Error', () => { + const expectedError = new ValidationError( + `1 relation(s) of type api::category.category associated with this entity do not exist` + ); + const testData = [ + [ + 'Connect', + { + sCom: { + categories: { + disconnect: [], + connect: [ + { + id: sample(nonExistentIDs), + }, + ], + }, + }, + }, + ], + [ + 'Set', + { + sCom: { + categories: { + set: [ + { + id: sample(nonExistentIDs), + }, + ], + }, + }, + }, + ], + [ + 'Number', + { + sCom: { + categories: sample(nonExistentIDs), + }, + }, + ], + [ + 'Array', + { + sCom: { + categories: [sample(nonExistentIDs)], + }, + }, + ], + ]; + + test.each(testData)('%s', async (__, input = {}) => { + global.strapi = strapi; + const res = entityValidator.validateEntityCreation(models.get('api::dev.dev'), input, { + isDraft: true, + }); + await expect(res).rejects.toThrowError(expectedError); + }); + }); + }); + + describe('Repeatable Component', () => { + describe('Success', () => { + const testData = [ + [ + 'Connect', + { + rCom: [ + { + categories: { + disconnect: [], + connect: [ + { + id: sample(IDsThatExist), + }, + ], + }, + }, + ], + }, + ], + [ + 'Set', + { + rCom: [ + { + categories: { + set: IDsThatExist.slice(-Math.floor(IDsThatExist.length / 2)).map((id) => ({ + id, + })), + }, + }, + ], + }, + ], + [ + 'Number', + { + rCom: [ + { + categories: IDsThatExist[0], + }, + ], + }, + ], + [ + 'Array', + { + rCom: [ + { + categories: IDsThatExist.slice(-Math.floor(IDsThatExist.length / 2)), + }, + ], + }, + ], + ]; + + test.each(testData)('%s', async (__, input = {}) => { + global.strapi = strapi; + const res = entityValidator.validateEntityCreation(models.get('api::dev.dev'), input, { + isDraft: true, + }); + await expect(res).resolves.not.toThrowError(); + }); + }); + + describe('Error', () => { + const expectedError = new ValidationError( + `4 relation(s) of type api::category.category associated with this entity do not exist` + ); + const testData = [ + [ + 'Connect', + { + rCom: [ + { + categories: { + disconnect: [], + connect: [sample(IDsThatExist), ...nonExistentIDs.slice(-4)].map((id) => ({ + id, + })), + }, + }, + ], + }, + ], + [ + 'Set', + { + rCom: [ + { + categories: { + set: [sample(IDsThatExist), ...nonExistentIDs.slice(-4)].map((id) => ({ + id, + })), + }, + }, + ], + }, + ], + [ + 'Array', + { + rCom: [ + { + categories: nonExistentIDs.slice(-4), + }, + ], + }, + ], + ]; + + test.each(testData)('%s', async (__, input = {}) => { + global.strapi = strapi; + const res = entityValidator.validateEntityCreation(models.get('api::dev.dev'), input, { + isDraft: true, + }); + await expect(res).rejects.toThrowError(expectedError); + }); + }); + }); + + describe('Dynamic Zones', () => { + describe('Success', () => { + const testData = [ + [ + 'Connect', + { + DZ: [ + { + __component: 'basic.dev-compo', + categories: { + disconnect: [], + connect: IDsThatExist.slice(-3).map((id) => ({ + id, + })), + }, + }, + ], + }, + ], + [ + 'Set', + { + DZ: [ + { + __component: 'basic.dev-compo', + categories: { + set: IDsThatExist.slice(-3).map((id) => ({ + id, + })), + }, + }, + ], + }, + ], + [ + 'Number', + { + DZ: [ + { + __component: 'basic.dev-compo', + categories: IDsThatExist[0], + }, + ], + }, + ], + [ + 'Array', + { + DZ: [ + { + __component: 'basic.dev-compo', + categories: IDsThatExist.slice(-3), + }, + ], + }, + ], + ]; + + test.each(testData)('%s', async (__, input = {}) => { + global.strapi = strapi; + const res = entityValidator.validateEntityCreation(models.get('api::dev.dev'), input, { + isDraft: true, + }); + await expect(res).resolves.not.toThrowError(); + }); + }); + + describe('Error', () => { + const expectedError = new ValidationError( + `2 relation(s) of type api::category.category associated with this entity do not exist` + ); + const testData = [ + [ + 'Connect', + { + DZ: [ + { + __component: 'basic.dev-compo', + categories: { + disconnect: [], + connect: [sample(IDsThatExist), ...nonExistentIDs.slice(-2)].map((id) => ({ + id, + })), + }, + }, + ], + }, + ], + [ + 'Set', + { + DZ: [ + { + __component: 'basic.dev-compo', + categories: { + set: [sample(IDsThatExist), ...nonExistentIDs.slice(-2)].map((id) => ({ + id, + })), + }, + }, + ], + }, + ], + [ + 'Array', + { + DZ: [ + { + __component: 'basic.dev-compo', + categories: [sample(IDsThatExist), ...nonExistentIDs.slice(-2)].map((id) => ({ + id, + })), + }, + ], + }, + ], + ]; + + test.each(testData)('%s', async (__, input = {}) => { + global.strapi = strapi; + const res = entityValidator.validateEntityCreation(models.get('api::dev.dev'), input, { + isDraft: true, + }); + await expect(res).rejects.toThrowError(expectedError); + }); + }); + }); + + describe('Media', () => { + it('Success', async () => { + global.strapi = strapi; + const input = { + media: [ + { + id: sample(IDsThatExist), + name: 'img.jpeg', + }, + ], + }; + + const res = entityValidator.validateEntityCreation(models.get('api::dev.dev'), input, { + isDraft: true, + }); + await expect(res).resolves.not.toThrowError(); + }); + + it('Error', async () => { + global.strapi = strapi; + const expectedError = new ValidationError( + `1 relation(s) of type plugin::upload.file associated with this entity do not exist` + ); + const input = { + media: [ + { + id: sample(nonExistentIDs), + name: 'img.jpeg', + }, + { + id: sample(IDsThatExist), + name: 'img.jpeg', + }, + ], + }; + + const res = entityValidator.validateEntityCreation(models.get('api::dev.dev'), input, { + isDraft: true, + }); + await expect(res).rejects.toThrowError(expectedError); + }); + }); + }); }); diff --git a/packages/core/strapi/lib/services/entity-service/__tests__/entity-service.test.js b/packages/core/strapi/lib/services/entity-service/__tests__/entity-service.test.js index 7c95ab8b61..a56705003a 100644 --- a/packages/core/strapi/lib/services/entity-service/__tests__/entity-service.test.js +++ b/packages/core/strapi/lib/services/entity-service/__tests__/entity-service.test.js @@ -3,7 +3,7 @@ jest.mock('bcryptjs', () => ({ hashSync: () => 'secret-password' })); const { EventEmitter } = require('events'); -const { ApplicationError } = require('@strapi/utils').errors; +const { ValidationError } = require('@strapi/utils').errors; const createEntityService = require('..'); const entityValidator = require('../../entity-validator'); @@ -291,7 +291,11 @@ describe('Entity service', () => { }; const res = instance.create(entityUID, { data }); - await expect(res).rejects.toThrow(ApplicationError); + await expect(res).rejects.toThrowError( + new ValidationError( + `1 relation(s) of type api::relation.relation associated with this entity do not exist` + ) + ); }); }); }); @@ -445,7 +449,11 @@ describe('Entity service', () => { }; const res = instance.update(entityUID, 0, { data }); - await expect(res).rejects.toThrow(ApplicationError); + await expect(res).rejects.toThrowError( + new ValidationError( + `1 relation(s) of type api::relation.relation associated with this entity do not exist` + ) + ); }); }); }); From f8f330d2e6f34829771ea6f6c00e54efdbcae901 Mon Sep 17 00:00:00 2001 From: Jamie Howard Date: Wed, 9 Nov 2022 15:08:06 +0000 Subject: [PATCH 07/12] chore: test organisation fix: relations existence checks --- jest.base-config.js | 1 + .../server/tests/api/basic-dp-dz.test.e2e.js | 7 +- .../__tests__/entity-validator.test.js | 1273 ----------------- .../__tests__/entity-service.test.js | 8 +- .../entity-validator/__tests__/index.test.js | 589 ++++++++ .../relations/attribute-level.test.js | 123 ++ .../relations/component-level.test.js | 275 ++++ .../relations/dynamic-zone-level.test.js | 159 ++ .../__tests__/relations/media-level.test.js | 74 + .../relations/utils/relations.testdata.js | 153 ++ .../lib/services/entity-validator/index.js | 9 +- .../strapi/tests/api/basic-dz.test.e2e.js | 7 +- .../core/strapi/tests/endpoint.test.e2e.js | 4 +- 13 files changed, 1397 insertions(+), 1285 deletions(-) delete mode 100644 packages/core/strapi/lib/services/__tests__/entity-validator.test.js create mode 100644 packages/core/strapi/lib/services/entity-validator/__tests__/index.test.js create mode 100644 packages/core/strapi/lib/services/entity-validator/__tests__/relations/attribute-level.test.js create mode 100644 packages/core/strapi/lib/services/entity-validator/__tests__/relations/component-level.test.js create mode 100644 packages/core/strapi/lib/services/entity-validator/__tests__/relations/dynamic-zone-level.test.js create mode 100644 packages/core/strapi/lib/services/entity-validator/__tests__/relations/media-level.test.js create mode 100644 packages/core/strapi/lib/services/entity-validator/__tests__/relations/utils/relations.testdata.js diff --git a/jest.base-config.js b/jest.base-config.js index 17f346681b..ea542b67ee 100644 --- a/jest.base-config.js +++ b/jest.base-config.js @@ -2,6 +2,7 @@ module.exports = { rootDir: __dirname, setupFilesAfterEnv: ['/test/unit.setup.js'], modulePathIgnorePatterns: ['.cache'], + testPathIgnorePatterns: ['.testdata.js'], testMatch: ['/**/__tests__/**/*.[jt]s?(x)'], // Use `jest-watch-typeahead` version 0.6.5. Newest version 1.0.0 does not support jest@26 // Reference: https://github.com/jest-community/jest-watch-typeahead/releases/tag/v1.0.0 diff --git a/packages/core/content-manager/server/tests/api/basic-dp-dz.test.e2e.js b/packages/core/content-manager/server/tests/api/basic-dp-dz.test.e2e.js index 6802a743f3..74ecb55b5b 100644 --- a/packages/core/content-manager/server/tests/api/basic-dp-dz.test.e2e.js +++ b/packages/core/content-manager/server/tests/api/basic-dp-dz.test.e2e.js @@ -271,7 +271,7 @@ describe('CM API - Basic + dz + draftAndPublish', () => { error: { status: 400, name: 'ValidationError', - message: 'dz[0].__component is a required field', + message: '2 errors occurred', details: { errors: [ { @@ -279,6 +279,11 @@ describe('CM API - Basic + dz + draftAndPublish', () => { message: 'dz[0].__component is a required field', name: 'ValidationError', }, + { + message: "Cannot read properties of undefined (reading 'attributes')", + name: 'ValidationError', + path: [], + }, ], }, }, diff --git a/packages/core/strapi/lib/services/__tests__/entity-validator.test.js b/packages/core/strapi/lib/services/__tests__/entity-validator.test.js deleted file mode 100644 index 457b5bf4ba..0000000000 --- a/packages/core/strapi/lib/services/__tests__/entity-validator.test.js +++ /dev/null @@ -1,1273 +0,0 @@ -'use strict'; - -const { sample } = require('lodash'); -const { ValidationError } = require('@strapi/utils').errors; - -const entityValidator = require('../entity-validator'); - -describe('Entity validator', () => { - describe('Published input', () => { - describe('General Errors', () => { - let model; - global.strapi = { - errors: { - badRequest: jest.fn(), - }, - getModel: () => model, - }; - - it('Throws a badRequest error on invalid input', async () => { - model = { - attributes: { - title: { - type: 'string', - }, - }, - }; - - const input = { title: 1234 }; - - expect.hasAssertions(); - - try { - await entityValidator.validateEntityCreation(model, input); - } catch (e) { - expect(e).toMatchObject({ - name: 'ValidationError', - message: 'title must be a `string` type, but the final value was: `1234`.', - details: { - errors: [ - { - path: ['title'], - message: 'title must be a `string` type, but the final value was: `1234`.', - name: 'ValidationError', - }, - ], - }, - }); - } - }); - - it('Returns data on valid input', async () => { - model = { - attributes: { - title: { - type: 'string', - }, - }, - }; - - const input = { title: 'test Title' }; - - expect.hasAssertions(); - - const data = await entityValidator.validateEntityCreation(model, input); - expect(data).toEqual(input); - }); - - it('Returns casted data when possible', async () => { - model = { - attributes: { - title: { - type: 'string', - }, - number: { - type: 'integer', - }, - }, - }; - - const input = { title: 'Test', number: '123' }; - - expect.hasAssertions(); - - const data = await entityValidator.validateEntityCreation(model, input); - expect(data).toEqual({ - title: 'Test', - number: 123, - }); - }); - - test('Throws on required not respected', async () => { - model = { - attributes: { - title: { - type: 'string', - required: true, - }, - }, - }; - - expect.hasAssertions(); - - try { - await entityValidator.validateEntityCreation(model, {}); - } catch (e) { - expect(e).toMatchObject({ - name: 'ValidationError', - message: 'title must be defined.', - details: { - errors: [ - { - path: ['title'], - message: 'title must be defined.', - name: 'ValidationError', - }, - ], - }, - }); - } - - try { - await entityValidator.validateEntityCreation(model, { title: null }); - } catch (e) { - expect(e).toMatchObject({ - name: 'ValidationError', - message: 'title must be a `string` type, but the final value was: `null`.', - details: { - errors: [ - { - path: ['title'], - message: 'title must be a `string` type, but the final value was: `null`.', - name: 'ValidationError', - }, - ], - }, - }); - } - }); - - it('Supports custom field types', async () => { - model = { - attributes: { - uuid: { - type: 'uuid', - }, - }, - }; - - const input = { uuid: '2479d6d7-2497-478d-8a34-a9e8ce45f8a7' }; - - expect.hasAssertions(); - - const data = await entityValidator.validateEntityCreation(model, input); - expect(data).toEqual({ - uuid: '2479d6d7-2497-478d-8a34-a9e8ce45f8a7', - }); - }); - }); - - describe('String validator', () => { - test('Throws on min length not respected', async () => { - global.strapi = { - errors: { - badRequest: jest.fn(), - }, - getModel: () => model, - }; - - const model = { - attributes: { - title: { - type: 'string', - minLength: 10, - }, - }, - }; - - const input = { title: 'tooSmall' }; - - expect.hasAssertions(); - - try { - await entityValidator.validateEntityCreation(model, input); - } catch (e) { - expect(e).toMatchObject({ - name: 'ValidationError', - message: 'title must be at least 10 characters', - details: { - errors: [ - { - path: ['title'], - message: 'title must be at least 10 characters', - name: 'ValidationError', - }, - ], - }, - }); - } - }); - - test('Throws on max length not respected', async () => { - const model = { - attributes: { - title: { - type: 'string', - maxLength: 2, - }, - }, - }; - - const input = { title: 'tooLong' }; - - expect.hasAssertions(); - - try { - await entityValidator.validateEntityCreation(model, input); - } catch (e) { - expect(e).toMatchObject({ - name: 'ValidationError', - message: 'title must be at most 2 characters', - details: { - errors: [ - { - path: ['title'], - message: 'title must be at most 2 characters', - name: 'ValidationError', - }, - ], - }, - }); - } - }); - - test('Allows empty strings even when required', async () => { - const model = { - attributes: { - title: { - type: 'string', - required: true, - }, - }, - }; - - const input = { title: '' }; - - expect.hasAssertions(); - - const data = await entityValidator.validateEntityCreation(model, input); - expect(data).toEqual(input); - }); - - test('Assign default values', async () => { - const model = { - attributes: { - title: { - type: 'string', - required: true, - default: 'New', - }, - type: { - type: 'string', - default: 'test', - }, - testDate: { - type: 'date', - required: true, - default: '2020-04-01T04:00:00.000Z', - }, - testJSON: { - type: 'date', - required: true, - default: { - foo: 1, - bar: 2, - }, - }, - }, - }; - - await expect(entityValidator.validateEntityCreation(model, {})).resolves.toMatchObject({ - title: 'New', - type: 'test', - testDate: '2020-04-01T04:00:00.000Z', - testJSON: { - foo: 1, - bar: 2, - }, - }); - }); - - test("Don't assign default value if empty string", async () => { - const model = { - attributes: { - title: { - type: 'string', - required: true, - default: 'default', - }, - content: { - type: 'string', - default: 'default', - }, - }, - }; - - await expect( - entityValidator.validateEntityCreation(model, { - title: '', - content: '', - }) - ).resolves.toMatchObject({ - title: '', - content: '', - }); - }); - }); - }); - - describe('Draft input', () => { - describe('General Errors', () => { - it('Throws a badRequest error on invalid input', async () => { - global.strapi = { - errors: { - badRequest: jest.fn(), - }, - getModel: () => model, - }; - - const model = { - uid: 'api::test.test', - attributes: { - title: { - type: 'string', - }, - }, - }; - - const input = { title: 1234 }; - - expect.hasAssertions(); - - try { - await entityValidator.validateEntityCreation(model, input, { isDraft: true }); - } catch (e) { - expect(e).toMatchObject({ - name: 'ValidationError', - message: 'title must be a `string` type, but the final value was: `1234`.', - details: { - errors: [ - { - path: ['title'], - message: 'title must be a `string` type, but the final value was: `1234`.', - name: 'ValidationError', - }, - ], - }, - }); - } - }); - - it('Returns data on valid input', async () => { - const model = { - attributes: { - title: { - type: 'string', - }, - }, - }; - - const input = { title: 'test Title' }; - - expect.hasAssertions(); - - const data = await entityValidator.validateEntityCreation(model, input, { isDraft: true }); - expect(data).toEqual(input); - }); - - it('Returns casted data when possible', async () => { - const model = { - attributes: { - title: { - type: 'string', - }, - number: { - type: 'integer', - }, - }, - }; - - const input = { title: 'Test', number: '123' }; - - expect.hasAssertions(); - - const data = await entityValidator.validateEntityCreation(model, input, { isDraft: true }); - expect(data).toEqual({ - title: 'Test', - number: 123, - }); - }); - - test('Does not throws on required not respected', async () => { - const model = { - attributes: { - title: { - type: 'string', - required: true, - }, - }, - }; - - expect.hasAssertions(); - - let data = await entityValidator.validateEntityCreation(model, {}, { isDraft: true }); - expect(data).toEqual({}); - - data = await entityValidator.validateEntityCreation( - model, - { title: null }, - { isDraft: true } - ); - expect(data).toEqual({ title: null }); - }); - - it('Supports custom field types', async () => { - const model = { - attributes: { - uuid: { - type: 'uuid', - }, - }, - }; - - const input = { uuid: '2479d6d7-2497-478d-8a34-a9e8ce45f8a7' }; - - expect.hasAssertions(); - - const data = await entityValidator.validateEntityCreation(model, input, { isDraft: true }); - expect(data).toEqual({ - uuid: '2479d6d7-2497-478d-8a34-a9e8ce45f8a7', - }); - }); - }); - - describe('String validator', () => { - test('Does not throws on min length not respected', async () => { - const model = { - attributes: { - title: { - type: 'string', - minLength: 10, - }, - }, - }; - - global.strapi = { - errors: { - badRequest: jest.fn(), - }, - getModel: () => model, - }; - - const input = { title: 'tooSmall' }; - - expect.hasAssertions(); - - const data = await entityValidator.validateEntityCreation(model, input, { isDraft: true }); - expect(data).toEqual(input); - }); - - test('Throws on max length not respected', async () => { - const model = { - attributes: { - title: { - type: 'string', - maxLength: 2, - }, - }, - }; - - const input = { title: 'tooLong' }; - - expect.hasAssertions(); - - try { - await entityValidator.validateEntityCreation(model, input, { isDraft: true }); - } catch (e) { - expect(e).toMatchObject({ - name: 'ValidationError', - message: 'title must be at most 2 characters', - details: { - errors: [ - { - path: ['title'], - message: 'title must be at most 2 characters', - name: 'ValidationError', - }, - ], - }, - }); - } - }); - - test('Allows empty strings even when required', async () => { - const model = { - attributes: { - title: { - type: 'string', - }, - }, - }; - - const input = { title: '' }; - - expect.hasAssertions(); - - const data = await entityValidator.validateEntityCreation(model, input, { isDraft: true }); - expect(data).toEqual(input); - }); - - test('Assign default values', async () => { - const model = { - attributes: { - title: { - type: 'string', - required: true, - default: 'New', - }, - type: { - type: 'string', - default: 'test', - }, - testDate: { - type: 'date', - required: true, - default: '2020-04-01T04:00:00.000Z', - }, - testJSON: { - type: 'date', - required: true, - default: { - foo: 1, - bar: 2, - }, - }, - }, - }; - - await expect( - entityValidator.validateEntityCreation(model, {}, { isDraft: true }) - ).resolves.toMatchObject({ - title: 'New', - type: 'test', - testDate: '2020-04-01T04:00:00.000Z', - testJSON: { - foo: 1, - bar: 2, - }, - }); - }); - - test("Don't assign default value if empty string", async () => { - const model = { - attributes: { - title: { - type: 'string', - required: true, - default: 'default', - }, - content: { - type: 'string', - default: 'default', - }, - }, - }; - - await expect( - entityValidator.validateEntityCreation( - model, - { - title: '', - content: '', - }, - { isDraft: true } - ) - ).resolves.toMatchObject({ - title: '', - content: '', - }); - }); - }); - }); - - /** - * Test that relations can be successfully validated and non existent relations - * can be detected at every possible level: Attribute, Single/Repeatable - * Component, Media or Dynamic Zone. - */ - describe('Relations', () => { - const models = new Map(); - models.set('api::dev.dev', { - kind: 'collectionType', - collectionName: 'devs', - modelType: 'contentType', - modelName: 'dev', - connection: 'default', - uid: 'api::dev.dev', - apiName: 'dev', - globalId: 'Dev', - info: { - singularName: 'dev', - pluralName: 'devs', - displayName: 'Dev', - description: '', - }, - attributes: { - categories: { - type: 'relation', - relation: 'manyToMany', - target: 'api::category.category', - inversedBy: 'devs', - }, - sCom: { - type: 'component', - repeatable: false, - component: 'basic.dev-compo', - }, - rCom: { - type: 'component', - repeatable: true, - component: 'basic.dev-compo', - }, - DZ: { - type: 'dynamiczone', - components: ['basic.dev-compo'], - }, - media: { - allowedTypes: ['images', 'files', 'videos', 'audios'], - type: 'media', - multiple: true, - }, - createdAt: { - type: 'datetime', - }, - updatedAt: { - type: 'datetime', - }, - publishedAt: { - type: 'datetime', - configurable: false, - writable: true, - visible: false, - }, - createdBy: { - type: 'relation', - relation: 'oneToOne', - target: 'admin::user', - configurable: false, - writable: false, - visible: false, - useJoinTable: false, - private: true, - }, - updatedBy: { - type: 'relation', - relation: 'oneToOne', - target: 'admin::user', - configurable: false, - writable: false, - visible: false, - useJoinTable: false, - private: true, - }, - }, - }); - models.set('api::category.category', { - kind: 'collectionType', - collectionName: 'categories', - modelType: 'contentType', - modelName: 'category', - connection: 'default', - uid: 'api::category.category', - apiName: 'category', - globalId: 'Category', - info: { - displayName: 'Category', - singularName: 'category', - pluralName: 'categories', - description: '', - name: 'Category', - }, - attributes: { - name: { - type: 'string', - pluginOptions: { - i18n: { - localized: true, - }, - }, - }, - }, - }); - models.set('basic.dev-compo', { - collectionName: 'components_basic_dev_compos', - uid: 'basic.dev-compo', - category: 'basic', - modelType: 'component', - modelName: 'dev-compo', - globalId: 'ComponentBasicDevCompo', - info: { - displayName: 'DevCompo', - icon: 'allergies', - }, - attributes: { - categories: { - type: 'relation', - relation: 'oneToMany', - target: 'api::category.category', - }, - }, - }); - models.set('plugin::upload.file', { - collectionName: 'files', - info: { - singularName: 'file', - pluralName: 'files', - displayName: 'File', - description: '', - }, - attributes: { - name: { - type: 'string', - configurable: false, - required: true, - }, - }, - kind: 'collectionType', - modelType: 'contentType', - modelName: 'file', - connection: 'default', - uid: 'plugin::upload.file', - plugin: 'upload', - globalId: 'UploadFile', - }); - - const IDsThatExist = [1, 2, 3, 4, 5, 6]; - const nonExistentIDs = [10, 11, 12, 13, 14, 15, 16]; - const strapi = { - components: { - 'basic.dev-compo': {}, - }, - db: { - query() { - return { - count: ({ - where: { - id: { $in }, - }, - }) => IDsThatExist.filter((value) => $in.includes(value)).length, - }; - }, - }, - errors: { - badRequest: jest.fn(), - }, - getModel: (uid) => models.get(uid), - }; - - describe('Attribute', () => { - describe('Success', () => { - const testData = [ - [ - 'Connect', - { - categories: { - disconnect: [], - connect: [ - { - id: sample(IDsThatExist), - }, - ], - }, - }, - ], - [ - 'Set', - { - categories: { - set: [ - { - id: sample(IDsThatExist), - }, - ], - }, - }, - ], - [ - 'Number', - { - categories: sample(IDsThatExist), - }, - ], - [ - 'Array', - { - categories: IDsThatExist.slice(-Math.floor(IDsThatExist.length / 2)), - }, - ], - ]; - test.each(testData)('%s', async (__, input = {}) => { - global.strapi = strapi; - const res = entityValidator.validateEntityCreation(models.get('api::dev.dev'), input, { - isDraft: true, - }); - await expect(res).resolves.not.toThrowError(); - }); - }); - - describe('Error', () => { - const expectError = new ValidationError( - `2 relation(s) of type api::category.category associated with this entity do not exist` - ); - const testData = [ - [ - 'Connect', - { - categories: { - disconnect: [], - connect: [sample(IDsThatExist), ...nonExistentIDs.slice(-2)].map((id) => ({ - id, - })), - }, - }, - ], - [ - 'Set', - { - categories: { - set: [sample(IDsThatExist), ...nonExistentIDs.slice(-2)].map((id) => ({ id })), - }, - }, - ], - [ - 'Number', - { - categories: nonExistentIDs.slice(-2), - }, - ], - ]; - - test.each(testData)('%s', async (__, input = {}) => { - global.strapi = strapi; - const res = entityValidator.validateEntityCreation(models.get('api::dev.dev'), input, { - isDraft: true, - }); - await expect(res).rejects.toThrowError(expectError); - }); - }); - }); - - describe('Single Component', () => { - describe('Success', () => { - const testData = [ - [ - 'Connect', - { - sCom: { - categories: { - disconnect: [], - connect: [ - { - id: sample(IDsThatExist), - }, - ], - }, - }, - }, - ], - [ - 'Set', - { - sCom: { - categories: { - set: [ - { - id: sample(IDsThatExist), - }, - ], - }, - }, - }, - ], - [ - 'Number', - { - sCom: { - categories: sample(IDsThatExist), - }, - }, - ], - [ - 'Array', - { - sCom: { - categories: IDsThatExist.slice(-3), - }, - }, - ], - ]; - - test.each(testData)('%s', async (__, input = {}) => { - global.strapi = strapi; - const res = entityValidator.validateEntityCreation(models.get('api::dev.dev'), input, { - isDraft: true, - }); - await expect(res).resolves.not.toThrowError(); - }); - }); - - describe('Error', () => { - const expectedError = new ValidationError( - `1 relation(s) of type api::category.category associated with this entity do not exist` - ); - const testData = [ - [ - 'Connect', - { - sCom: { - categories: { - disconnect: [], - connect: [ - { - id: sample(nonExistentIDs), - }, - ], - }, - }, - }, - ], - [ - 'Set', - { - sCom: { - categories: { - set: [ - { - id: sample(nonExistentIDs), - }, - ], - }, - }, - }, - ], - [ - 'Number', - { - sCom: { - categories: sample(nonExistentIDs), - }, - }, - ], - [ - 'Array', - { - sCom: { - categories: [sample(nonExistentIDs)], - }, - }, - ], - ]; - - test.each(testData)('%s', async (__, input = {}) => { - global.strapi = strapi; - const res = entityValidator.validateEntityCreation(models.get('api::dev.dev'), input, { - isDraft: true, - }); - await expect(res).rejects.toThrowError(expectedError); - }); - }); - }); - - describe('Repeatable Component', () => { - describe('Success', () => { - const testData = [ - [ - 'Connect', - { - rCom: [ - { - categories: { - disconnect: [], - connect: [ - { - id: sample(IDsThatExist), - }, - ], - }, - }, - ], - }, - ], - [ - 'Set', - { - rCom: [ - { - categories: { - set: IDsThatExist.slice(-Math.floor(IDsThatExist.length / 2)).map((id) => ({ - id, - })), - }, - }, - ], - }, - ], - [ - 'Number', - { - rCom: [ - { - categories: IDsThatExist[0], - }, - ], - }, - ], - [ - 'Array', - { - rCom: [ - { - categories: IDsThatExist.slice(-Math.floor(IDsThatExist.length / 2)), - }, - ], - }, - ], - ]; - - test.each(testData)('%s', async (__, input = {}) => { - global.strapi = strapi; - const res = entityValidator.validateEntityCreation(models.get('api::dev.dev'), input, { - isDraft: true, - }); - await expect(res).resolves.not.toThrowError(); - }); - }); - - describe('Error', () => { - const expectedError = new ValidationError( - `4 relation(s) of type api::category.category associated with this entity do not exist` - ); - const testData = [ - [ - 'Connect', - { - rCom: [ - { - categories: { - disconnect: [], - connect: [sample(IDsThatExist), ...nonExistentIDs.slice(-4)].map((id) => ({ - id, - })), - }, - }, - ], - }, - ], - [ - 'Set', - { - rCom: [ - { - categories: { - set: [sample(IDsThatExist), ...nonExistentIDs.slice(-4)].map((id) => ({ - id, - })), - }, - }, - ], - }, - ], - [ - 'Array', - { - rCom: [ - { - categories: nonExistentIDs.slice(-4), - }, - ], - }, - ], - ]; - - test.each(testData)('%s', async (__, input = {}) => { - global.strapi = strapi; - const res = entityValidator.validateEntityCreation(models.get('api::dev.dev'), input, { - isDraft: true, - }); - await expect(res).rejects.toThrowError(expectedError); - }); - }); - }); - - describe('Dynamic Zones', () => { - describe('Success', () => { - const testData = [ - [ - 'Connect', - { - DZ: [ - { - __component: 'basic.dev-compo', - categories: { - disconnect: [], - connect: IDsThatExist.slice(-3).map((id) => ({ - id, - })), - }, - }, - ], - }, - ], - [ - 'Set', - { - DZ: [ - { - __component: 'basic.dev-compo', - categories: { - set: IDsThatExist.slice(-3).map((id) => ({ - id, - })), - }, - }, - ], - }, - ], - [ - 'Number', - { - DZ: [ - { - __component: 'basic.dev-compo', - categories: IDsThatExist[0], - }, - ], - }, - ], - [ - 'Array', - { - DZ: [ - { - __component: 'basic.dev-compo', - categories: IDsThatExist.slice(-3), - }, - ], - }, - ], - ]; - - test.each(testData)('%s', async (__, input = {}) => { - global.strapi = strapi; - const res = entityValidator.validateEntityCreation(models.get('api::dev.dev'), input, { - isDraft: true, - }); - await expect(res).resolves.not.toThrowError(); - }); - }); - - describe('Error', () => { - const expectedError = new ValidationError( - `2 relation(s) of type api::category.category associated with this entity do not exist` - ); - const testData = [ - [ - 'Connect', - { - DZ: [ - { - __component: 'basic.dev-compo', - categories: { - disconnect: [], - connect: [sample(IDsThatExist), ...nonExistentIDs.slice(-2)].map((id) => ({ - id, - })), - }, - }, - ], - }, - ], - [ - 'Set', - { - DZ: [ - { - __component: 'basic.dev-compo', - categories: { - set: [sample(IDsThatExist), ...nonExistentIDs.slice(-2)].map((id) => ({ - id, - })), - }, - }, - ], - }, - ], - [ - 'Array', - { - DZ: [ - { - __component: 'basic.dev-compo', - categories: [sample(IDsThatExist), ...nonExistentIDs.slice(-2)].map((id) => ({ - id, - })), - }, - ], - }, - ], - ]; - - test.each(testData)('%s', async (__, input = {}) => { - global.strapi = strapi; - const res = entityValidator.validateEntityCreation(models.get('api::dev.dev'), input, { - isDraft: true, - }); - await expect(res).rejects.toThrowError(expectedError); - }); - }); - }); - - describe('Media', () => { - it('Success', async () => { - global.strapi = strapi; - const input = { - media: [ - { - id: sample(IDsThatExist), - name: 'img.jpeg', - }, - ], - }; - - const res = entityValidator.validateEntityCreation(models.get('api::dev.dev'), input, { - isDraft: true, - }); - await expect(res).resolves.not.toThrowError(); - }); - - it('Error', async () => { - global.strapi = strapi; - const expectedError = new ValidationError( - `1 relation(s) of type plugin::upload.file associated with this entity do not exist` - ); - const input = { - media: [ - { - id: sample(nonExistentIDs), - name: 'img.jpeg', - }, - { - id: sample(IDsThatExist), - name: 'img.jpeg', - }, - ], - }; - - const res = entityValidator.validateEntityCreation(models.get('api::dev.dev'), input, { - isDraft: true, - }); - await expect(res).rejects.toThrowError(expectedError); - }); - }); - }); -}); diff --git a/packages/core/strapi/lib/services/entity-service/__tests__/entity-service.test.js b/packages/core/strapi/lib/services/entity-service/__tests__/entity-service.test.js index a56705003a..3d713e2778 100644 --- a/packages/core/strapi/lib/services/entity-service/__tests__/entity-service.test.js +++ b/packages/core/strapi/lib/services/entity-service/__tests__/entity-service.test.js @@ -105,7 +105,7 @@ describe('Entity service', () => { }, }; - const fakeModel = { + const fakeModels = { [entityUID]: { uid: entityUID, kind: 'contentType', @@ -175,7 +175,7 @@ describe('Entity service', () => { global.strapi = { getModel: jest.fn((uid) => { - return fakeModel[uid]; + return fakeModels[uid]; }), db: fakeDB, }; @@ -334,7 +334,7 @@ describe('Entity service', () => { }, }, }; - const fakeModel = { + const fakeModels = { [entityUID]: { kind: 'collectionType', modelName: 'entity', @@ -398,7 +398,7 @@ describe('Entity service', () => { global.strapi = { getModel: jest.fn((uid) => { - return fakeModel[uid]; + return fakeModels[uid]; }), db: fakeDB, }; diff --git a/packages/core/strapi/lib/services/entity-validator/__tests__/index.test.js b/packages/core/strapi/lib/services/entity-validator/__tests__/index.test.js new file mode 100644 index 0000000000..720fb2e795 --- /dev/null +++ b/packages/core/strapi/lib/services/entity-validator/__tests__/index.test.js @@ -0,0 +1,589 @@ +'use strict'; + +const entityValidator = require('..'); + +describe('Entity validator', () => { + describe('Published input', () => { + describe('General Errors', () => { + let model; + global.strapi = { + errors: { + badRequest: jest.fn(), + }, + getModel: () => model, + }; + + it('Throws a badRequest error on invalid input', async () => { + model = { + attributes: { + title: { + type: 'string', + }, + }, + }; + + const input = { title: 1234 }; + + expect.hasAssertions(); + + try { + await entityValidator.validateEntityCreation(model, input); + } catch (e) { + expect(e).toMatchObject({ + name: 'ValidationError', + message: 'title must be a `string` type, but the final value was: `1234`.', + details: { + errors: [ + { + path: ['title'], + message: 'title must be a `string` type, but the final value was: `1234`.', + name: 'ValidationError', + }, + ], + }, + }); + } + }); + + it('Returns data on valid input', async () => { + model = { + attributes: { + title: { + type: 'string', + }, + }, + }; + + const input = { title: 'test Title' }; + + expect.hasAssertions(); + + const data = await entityValidator.validateEntityCreation(model, input); + expect(data).toEqual(input); + }); + + it('Returns casted data when possible', async () => { + model = { + attributes: { + title: { + type: 'string', + }, + number: { + type: 'integer', + }, + }, + }; + + const input = { title: 'Test', number: '123' }; + + expect.hasAssertions(); + + const data = await entityValidator.validateEntityCreation(model, input); + expect(data).toEqual({ + title: 'Test', + number: 123, + }); + }); + + test('Throws on required not respected', async () => { + model = { + attributes: { + title: { + type: 'string', + required: true, + }, + }, + }; + + expect.hasAssertions(); + + try { + await entityValidator.validateEntityCreation(model, {}); + } catch (e) { + expect(e).toMatchObject({ + name: 'ValidationError', + message: 'title must be defined.', + details: { + errors: [ + { + path: ['title'], + message: 'title must be defined.', + name: 'ValidationError', + }, + ], + }, + }); + } + + try { + await entityValidator.validateEntityCreation(model, { title: null }); + } catch (e) { + expect(e).toMatchObject({ + name: 'ValidationError', + message: 'title must be a `string` type, but the final value was: `null`.', + details: { + errors: [ + { + path: ['title'], + message: 'title must be a `string` type, but the final value was: `null`.', + name: 'ValidationError', + }, + ], + }, + }); + } + }); + + it('Supports custom field types', async () => { + model = { + attributes: { + uuid: { + type: 'uuid', + }, + }, + }; + + const input = { uuid: '2479d6d7-2497-478d-8a34-a9e8ce45f8a7' }; + + expect.hasAssertions(); + + const data = await entityValidator.validateEntityCreation(model, input); + expect(data).toEqual({ + uuid: '2479d6d7-2497-478d-8a34-a9e8ce45f8a7', + }); + }); + }); + + describe('String validator', () => { + test('Throws on min length not respected', async () => { + global.strapi = { + errors: { + badRequest: jest.fn(), + }, + getModel: () => model, + }; + + const model = { + attributes: { + title: { + type: 'string', + minLength: 10, + }, + }, + }; + + const input = { title: 'tooSmall' }; + + expect.hasAssertions(); + + try { + await entityValidator.validateEntityCreation(model, input); + } catch (e) { + expect(e).toMatchObject({ + name: 'ValidationError', + message: 'title must be at least 10 characters', + details: { + errors: [ + { + path: ['title'], + message: 'title must be at least 10 characters', + name: 'ValidationError', + }, + ], + }, + }); + } + }); + + test('Throws on max length not respected', async () => { + const model = { + attributes: { + title: { + type: 'string', + maxLength: 2, + }, + }, + }; + + const input = { title: 'tooLong' }; + + expect.hasAssertions(); + + try { + await entityValidator.validateEntityCreation(model, input); + } catch (e) { + expect(e).toMatchObject({ + name: 'ValidationError', + message: 'title must be at most 2 characters', + details: { + errors: [ + { + path: ['title'], + message: 'title must be at most 2 characters', + name: 'ValidationError', + }, + ], + }, + }); + } + }); + + test('Allows empty strings even when required', async () => { + const model = { + attributes: { + title: { + type: 'string', + required: true, + }, + }, + }; + + const input = { title: '' }; + + expect.hasAssertions(); + + const data = await entityValidator.validateEntityCreation(model, input); + expect(data).toEqual(input); + }); + + test('Assign default values', async () => { + const model = { + attributes: { + title: { + type: 'string', + required: true, + default: 'New', + }, + type: { + type: 'string', + default: 'test', + }, + testDate: { + type: 'date', + required: true, + default: '2020-04-01T04:00:00.000Z', + }, + testJSON: { + type: 'date', + required: true, + default: { + foo: 1, + bar: 2, + }, + }, + }, + }; + + await expect(entityValidator.validateEntityCreation(model, {})).resolves.toMatchObject({ + title: 'New', + type: 'test', + testDate: '2020-04-01T04:00:00.000Z', + testJSON: { + foo: 1, + bar: 2, + }, + }); + }); + + test("Don't assign default value if empty string", async () => { + const model = { + attributes: { + title: { + type: 'string', + required: true, + default: 'default', + }, + content: { + type: 'string', + default: 'default', + }, + }, + }; + + await expect( + entityValidator.validateEntityCreation(model, { + title: '', + content: '', + }) + ).resolves.toMatchObject({ + title: '', + content: '', + }); + }); + }); + }); + + describe('Draft input', () => { + describe('General Errors', () => { + it('Throws a badRequest error on invalid input', async () => { + global.strapi = { + errors: { + badRequest: jest.fn(), + }, + getModel: () => model, + }; + + const model = { + uid: 'api::test.test', + attributes: { + title: { + type: 'string', + }, + }, + }; + + const input = { title: 1234 }; + + expect.hasAssertions(); + + try { + await entityValidator.validateEntityCreation(model, input, { isDraft: true }); + } catch (e) { + expect(e).toMatchObject({ + name: 'ValidationError', + message: 'title must be a `string` type, but the final value was: `1234`.', + details: { + errors: [ + { + path: ['title'], + message: 'title must be a `string` type, but the final value was: `1234`.', + name: 'ValidationError', + }, + ], + }, + }); + } + }); + + it('Returns data on valid input', async () => { + const model = { + attributes: { + title: { + type: 'string', + }, + }, + }; + + const input = { title: 'test Title' }; + + expect.hasAssertions(); + + const data = await entityValidator.validateEntityCreation(model, input, { isDraft: true }); + expect(data).toEqual(input); + }); + + it('Returns casted data when possible', async () => { + const model = { + attributes: { + title: { + type: 'string', + }, + number: { + type: 'integer', + }, + }, + }; + + const input = { title: 'Test', number: '123' }; + + expect.hasAssertions(); + + const data = await entityValidator.validateEntityCreation(model, input, { isDraft: true }); + expect(data).toEqual({ + title: 'Test', + number: 123, + }); + }); + + test('Does not throws on required not respected', async () => { + const model = { + attributes: { + title: { + type: 'string', + required: true, + }, + }, + }; + + expect.hasAssertions(); + + let data = await entityValidator.validateEntityCreation(model, {}, { isDraft: true }); + expect(data).toEqual({}); + + data = await entityValidator.validateEntityCreation( + model, + { title: null }, + { isDraft: true } + ); + expect(data).toEqual({ title: null }); + }); + + it('Supports custom field types', async () => { + const model = { + attributes: { + uuid: { + type: 'uuid', + }, + }, + }; + + const input = { uuid: '2479d6d7-2497-478d-8a34-a9e8ce45f8a7' }; + + expect.hasAssertions(); + + const data = await entityValidator.validateEntityCreation(model, input, { isDraft: true }); + expect(data).toEqual({ + uuid: '2479d6d7-2497-478d-8a34-a9e8ce45f8a7', + }); + }); + }); + + describe('String validator', () => { + test('Does not throws on min length not respected', async () => { + const model = { + attributes: { + title: { + type: 'string', + minLength: 10, + }, + }, + }; + + global.strapi = { + errors: { + badRequest: jest.fn(), + }, + getModel: () => model, + }; + + const input = { title: 'tooSmall' }; + + expect.hasAssertions(); + + const data = await entityValidator.validateEntityCreation(model, input, { isDraft: true }); + expect(data).toEqual(input); + }); + + test('Throws on max length not respected', async () => { + const model = { + attributes: { + title: { + type: 'string', + maxLength: 2, + }, + }, + }; + + const input = { title: 'tooLong' }; + + expect.hasAssertions(); + + try { + await entityValidator.validateEntityCreation(model, input, { isDraft: true }); + } catch (e) { + expect(e).toMatchObject({ + name: 'ValidationError', + message: 'title must be at most 2 characters', + details: { + errors: [ + { + path: ['title'], + message: 'title must be at most 2 characters', + name: 'ValidationError', + }, + ], + }, + }); + } + }); + + test('Allows empty strings even when required', async () => { + const model = { + attributes: { + title: { + type: 'string', + }, + }, + }; + + const input = { title: '' }; + + expect.hasAssertions(); + + const data = await entityValidator.validateEntityCreation(model, input, { isDraft: true }); + expect(data).toEqual(input); + }); + + test('Assign default values', async () => { + const model = { + attributes: { + title: { + type: 'string', + required: true, + default: 'New', + }, + type: { + type: 'string', + default: 'test', + }, + testDate: { + type: 'date', + required: true, + default: '2020-04-01T04:00:00.000Z', + }, + testJSON: { + type: 'date', + required: true, + default: { + foo: 1, + bar: 2, + }, + }, + }, + }; + + await expect( + entityValidator.validateEntityCreation(model, {}, { isDraft: true }) + ).resolves.toMatchObject({ + title: 'New', + type: 'test', + testDate: '2020-04-01T04:00:00.000Z', + testJSON: { + foo: 1, + bar: 2, + }, + }); + }); + + test("Don't assign default value if empty string", async () => { + const model = { + attributes: { + title: { + type: 'string', + required: true, + default: 'default', + }, + content: { + type: 'string', + default: 'default', + }, + }, + }; + + await expect( + entityValidator.validateEntityCreation( + model, + { + title: '', + content: '', + }, + { isDraft: true } + ) + ).resolves.toMatchObject({ + title: '', + content: '', + }); + }); + }); + }); +}); diff --git a/packages/core/strapi/lib/services/entity-validator/__tests__/relations/attribute-level.test.js b/packages/core/strapi/lib/services/entity-validator/__tests__/relations/attribute-level.test.js new file mode 100644 index 0000000000..39041f972f --- /dev/null +++ b/packages/core/strapi/lib/services/entity-validator/__tests__/relations/attribute-level.test.js @@ -0,0 +1,123 @@ +'use strict'; + +const { ValidationError } = require('@strapi/utils').errors; + +const entityValidator = require('../..'); +const { models, existentIDs, nonExistentIds } = require('./utils/relations.testdata'); + +/** + * Test that relations can be successfully validated and non existent relations + * can be detected at the Attribute level. + */ +describe('Entity validator | Relations | Attribute', () => { + const strapi = { + components: { + 'basic.dev-compo': {}, + }, + db: { + query() { + return { + count: ({ + where: { + id: { $in }, + }, + }) => existentIDs.filter((value) => $in.includes(value)).length, + }; + }, + }, + errors: { + badRequest: jest.fn(), + }, + getModel: (uid) => models.get(uid), + }; + + describe('Success', () => { + const testData = [ + [ + 'Connect', + { + categories: { + disconnect: [], + connect: [ + { + id: existentIDs[0], + }, + ], + }, + }, + ], + [ + 'Set', + { + categories: { + set: [ + { + id: existentIDs[0], + }, + ], + }, + }, + ], + [ + 'Number', + { + categories: existentIDs[0], + }, + ], + [ + 'Array', + { + categories: existentIDs.slice(-Math.floor(existentIDs.length / 2)), + }, + ], + ]; + test.each(testData)('%s', async (__, input = {}) => { + global.strapi = strapi; + const res = entityValidator.validateEntityCreation(models.get('api::dev.dev'), input, { + isDraft: true, + }); + await expect(res).resolves.not.toThrowError(); + }); + }); + + describe('Error', () => { + const expectError = new ValidationError( + `2 relation(s) of type api::category.category associated with this entity do not exist` + ); + const testData = [ + [ + 'Connect', + { + categories: { + disconnect: [], + connect: [existentIDs[0], ...nonExistentIds.slice(-2)].map((id) => ({ + id, + })), + }, + }, + ], + [ + 'Set', + { + categories: { + set: [existentIDs[0], ...nonExistentIds.slice(-2)].map((id) => ({ id })), + }, + }, + ], + [ + 'Number', + { + categories: nonExistentIds.slice(-2), + }, + ], + ]; + + test.each(testData)('%s', async (__, input = {}) => { + global.strapi = strapi; + const res = entityValidator.validateEntityCreation(models.get('api::dev.dev'), input, { + isDraft: true, + }); + await expect(res).rejects.toThrowError(expectError); + }); + }); +}); diff --git a/packages/core/strapi/lib/services/entity-validator/__tests__/relations/component-level.test.js b/packages/core/strapi/lib/services/entity-validator/__tests__/relations/component-level.test.js new file mode 100644 index 0000000000..ac03644ea8 --- /dev/null +++ b/packages/core/strapi/lib/services/entity-validator/__tests__/relations/component-level.test.js @@ -0,0 +1,275 @@ +'use strict'; + +const { ValidationError } = require('@strapi/utils').errors; + +const entityValidator = require('../..'); +const { models, nonExistentIds, existentIDs } = require('./utils/relations.testdata'); + +/** + * Test that relations can be successfully validated and non existent relations + * can be detected at the Component level. + */ +describe('Entity validator | Relations | Component Level', () => { + const strapi = { + components: { + 'basic.dev-compo': {}, + }, + db: { + query() { + return { + count: ({ + where: { + id: { $in }, + }, + }) => existentIDs.filter((value) => $in.includes(value)).length, + }; + }, + }, + errors: { + badRequest: jest.fn(), + }, + getModel: (uid) => models.get(uid), + }; + + describe('Single Component', () => { + describe('Success', () => { + const testData = [ + [ + 'Connect', + { + sCom: { + categories: { + disconnect: [], + connect: [ + { + id: existentIDs[0], + }, + ], + }, + }, + }, + ], + [ + 'Set', + { + sCom: { + categories: { + set: [ + { + id: existentIDs[0], + }, + ], + }, + }, + }, + ], + [ + 'Number', + { + sCom: { + categories: existentIDs[0], + }, + }, + ], + [ + 'Array', + { + sCom: { + categories: existentIDs.slice(-3), + }, + }, + ], + ]; + + test.each(testData)('%s', async (__, input = {}) => { + global.strapi = strapi; + const res = entityValidator.validateEntityCreation(models.get('api::dev.dev'), input, { + isDraft: true, + }); + await expect(res).resolves.not.toThrowError(); + }); + }); + + describe('Error', () => { + const expectedError = new ValidationError( + `1 relation(s) of type api::category.category associated with this entity do not exist` + ); + const testData = [ + [ + 'Connect', + { + sCom: { + categories: { + disconnect: [], + connect: [ + { + id: nonExistentIds[0], + }, + ], + }, + }, + }, + ], + [ + 'Set', + { + sCom: { + categories: { + set: [ + { + id: nonExistentIds[0], + }, + ], + }, + }, + }, + ], + [ + 'Number', + { + sCom: { + categories: nonExistentIds[0], + }, + }, + ], + [ + 'Array', + { + sCom: { + categories: [nonExistentIds[0]], + }, + }, + ], + ]; + + test.each(testData)('%s', async (__, input = {}) => { + global.strapi = strapi; + const res = entityValidator.validateEntityCreation(models.get('api::dev.dev'), input, { + isDraft: true, + }); + await expect(res).rejects.toThrowError(expectedError); + }); + }); + }); + + describe('Repeatable Component', () => { + describe('Success', () => { + const testData = [ + [ + 'Connect', + { + rCom: [ + { + categories: { + disconnect: [], + connect: [ + { + id: existentIDs[0], + }, + ], + }, + }, + ], + }, + ], + [ + 'Set', + { + rCom: [ + { + categories: { + set: existentIDs.slice(-Math.floor(existentIDs.length / 2)).map((id) => ({ + id, + })), + }, + }, + ], + }, + ], + [ + 'Number', + { + rCom: [ + { + categories: existentIDs[0], + }, + ], + }, + ], + [ + 'Array', + { + rCom: [ + { + categories: existentIDs.slice(-Math.floor(existentIDs.length / 2)), + }, + ], + }, + ], + ]; + + test.each(testData)('%s', async (__, input = {}) => { + global.strapi = strapi; + const res = entityValidator.validateEntityCreation(models.get('api::dev.dev'), input, { + isDraft: true, + }); + await expect(res).resolves.not.toThrowError(); + }); + }); + + describe('Error', () => { + const expectedError = new ValidationError( + `4 relation(s) of type api::category.category associated with this entity do not exist` + ); + const testData = [ + [ + 'Connect', + { + rCom: [ + { + categories: { + disconnect: [], + connect: [existentIDs[0], ...nonExistentIds.slice(-4)].map((id) => ({ + id, + })), + }, + }, + ], + }, + ], + [ + 'Set', + { + rCom: [ + { + categories: { + set: [existentIDs[0], ...nonExistentIds.slice(-4)].map((id) => ({ + id, + })), + }, + }, + ], + }, + ], + [ + 'Array', + { + rCom: [ + { + categories: nonExistentIds.slice(-4), + }, + ], + }, + ], + ]; + + test.each(testData)('%s', async (__, input = {}) => { + global.strapi = strapi; + const res = entityValidator.validateEntityCreation(models.get('api::dev.dev'), input, { + isDraft: true, + }); + await expect(res).rejects.toThrowError(expectedError); + }); + }); + }); +}); diff --git a/packages/core/strapi/lib/services/entity-validator/__tests__/relations/dynamic-zone-level.test.js b/packages/core/strapi/lib/services/entity-validator/__tests__/relations/dynamic-zone-level.test.js new file mode 100644 index 0000000000..4e838838bc --- /dev/null +++ b/packages/core/strapi/lib/services/entity-validator/__tests__/relations/dynamic-zone-level.test.js @@ -0,0 +1,159 @@ +'use strict'; + +const { ValidationError } = require('@strapi/utils').errors; + +const entityValidator = require('../..'); +const { models, nonExistentIds, existentIDs } = require('./utils/relations.testdata'); + +/** + * Test that relations can be successfully validated and non existent relations + * can be detected at the Dynamic Zone level. + */ +describe('Entity validator | Relations | Dynamic Zone', () => { + const strapi = { + components: { + 'basic.dev-compo': {}, + }, + db: { + query() { + return { + count: ({ + where: { + id: { $in }, + }, + }) => existentIDs.filter((value) => $in.includes(value)).length, + }; + }, + }, + errors: { + badRequest: jest.fn(), + }, + getModel: (uid) => models.get(uid), + }; + + describe('Success', () => { + const testData = [ + [ + 'Connect', + { + DZ: [ + { + __component: 'basic.dev-compo', + categories: { + disconnect: [], + connect: existentIDs.slice(-3).map((id) => ({ + id, + })), + }, + }, + ], + }, + ], + [ + 'Set', + { + DZ: [ + { + __component: 'basic.dev-compo', + categories: { + set: existentIDs.slice(-3).map((id) => ({ + id, + })), + }, + }, + ], + }, + ], + [ + 'Number', + { + DZ: [ + { + __component: 'basic.dev-compo', + categories: existentIDs[0], + }, + ], + }, + ], + [ + 'Array', + { + DZ: [ + { + __component: 'basic.dev-compo', + categories: existentIDs.slice(-3), + }, + ], + }, + ], + ]; + + test.each(testData)('%s', async (__, input = {}) => { + global.strapi = strapi; + const res = entityValidator.validateEntityCreation(models.get('api::dev.dev'), input, { + isDraft: true, + }); + await expect(res).resolves.not.toThrowError(); + }); + }); + + describe('Error', () => { + const expectedError = new ValidationError( + `2 relation(s) of type api::category.category associated with this entity do not exist` + ); + const testData = [ + [ + 'Connect', + { + DZ: [ + { + __component: 'basic.dev-compo', + categories: { + disconnect: [], + connect: [existentIDs[0], ...nonExistentIds.slice(-2)].map((id) => ({ + id, + })), + }, + }, + ], + }, + ], + [ + 'Set', + { + DZ: [ + { + __component: 'basic.dev-compo', + categories: { + set: [existentIDs[0], ...nonExistentIds.slice(-2)].map((id) => ({ + id, + })), + }, + }, + ], + }, + ], + [ + 'Array', + { + DZ: [ + { + __component: 'basic.dev-compo', + categories: [existentIDs[0], ...nonExistentIds.slice(-2)].map((id) => ({ + id, + })), + }, + ], + }, + ], + ]; + + test.each(testData)('%s', async (__, input = {}) => { + global.strapi = strapi; + const res = entityValidator.validateEntityCreation(models.get('api::dev.dev'), input, { + isDraft: true, + }); + await expect(res).rejects.toThrowError(expectedError); + }); + }); +}); diff --git a/packages/core/strapi/lib/services/entity-validator/__tests__/relations/media-level.test.js b/packages/core/strapi/lib/services/entity-validator/__tests__/relations/media-level.test.js new file mode 100644 index 0000000000..0a08552027 --- /dev/null +++ b/packages/core/strapi/lib/services/entity-validator/__tests__/relations/media-level.test.js @@ -0,0 +1,74 @@ +'use strict'; + +const { ValidationError } = require('@strapi/utils').errors; + +const entityValidator = require('../..'); +const { models, existentIDs, nonExistentIds } = require('./utils/relations.testdata'); + +/** + * Test that relations can be successfully validated and non existent relations + * can be detected at the Media level. + */ +describe('Entity validator | Relations | Media', () => { + const strapi = { + components: { + 'basic.dev-compo': {}, + }, + db: { + query() { + return { + count: ({ + where: { + id: { $in }, + }, + }) => existentIDs.filter((value) => $in.includes(value)).length, + }; + }, + }, + errors: { + badRequest: jest.fn(), + }, + getModel: (uid) => models.get(uid), + }; + + it('Success', async () => { + global.strapi = strapi; + const input = { + media: [ + { + id: existentIDs[0], + name: 'img.jpeg', + }, + ], + }; + + const res = entityValidator.validateEntityCreation(models.get('api::dev.dev'), input, { + isDraft: true, + }); + await expect(res).resolves.not.toThrowError(); + }); + + it('Error', async () => { + global.strapi = strapi; + const expectedError = new ValidationError( + `1 relation(s) of type plugin::upload.file associated with this entity do not exist` + ); + const input = { + media: [ + { + id: nonExistentIds[0], + name: 'img.jpeg', + }, + { + id: existentIDs[0], + name: 'img.jpeg', + }, + ], + }; + + const res = entityValidator.validateEntityCreation(models.get('api::dev.dev'), input, { + isDraft: true, + }); + await expect(res).rejects.toThrowError(expectedError); + }); +}); diff --git a/packages/core/strapi/lib/services/entity-validator/__tests__/relations/utils/relations.testdata.js b/packages/core/strapi/lib/services/entity-validator/__tests__/relations/utils/relations.testdata.js new file mode 100644 index 0000000000..1131c91150 --- /dev/null +++ b/packages/core/strapi/lib/services/entity-validator/__tests__/relations/utils/relations.testdata.js @@ -0,0 +1,153 @@ +'use strict'; + +const models = new Map(); +models.set('api::dev.dev', { + kind: 'collectionType', + collectionName: 'devs', + modelType: 'contentType', + modelName: 'dev', + connection: 'default', + uid: 'api::dev.dev', + apiName: 'dev', + globalId: 'Dev', + info: { + singularName: 'dev', + pluralName: 'devs', + displayName: 'Dev', + description: '', + }, + attributes: { + categories: { + type: 'relation', + relation: 'manyToMany', + target: 'api::category.category', + inversedBy: 'devs', + }, + sCom: { + type: 'component', + repeatable: false, + component: 'basic.dev-compo', + }, + rCom: { + type: 'component', + repeatable: true, + component: 'basic.dev-compo', + }, + DZ: { + type: 'dynamiczone', + components: ['basic.dev-compo'], + }, + media: { + allowedTypes: ['images', 'files', 'videos', 'audios'], + type: 'media', + multiple: true, + }, + createdAt: { + type: 'datetime', + }, + updatedAt: { + type: 'datetime', + }, + publishedAt: { + type: 'datetime', + configurable: false, + writable: true, + visible: false, + }, + createdBy: { + type: 'relation', + relation: 'oneToOne', + target: 'admin::user', + configurable: false, + writable: false, + visible: false, + useJoinTable: false, + private: true, + }, + updatedBy: { + type: 'relation', + relation: 'oneToOne', + target: 'admin::user', + configurable: false, + writable: false, + visible: false, + useJoinTable: false, + private: true, + }, + }, +}); +models.set('api::category.category', { + kind: 'collectionType', + collectionName: 'categories', + modelType: 'contentType', + modelName: 'category', + connection: 'default', + uid: 'api::category.category', + apiName: 'category', + globalId: 'Category', + info: { + displayName: 'Category', + singularName: 'category', + pluralName: 'categories', + description: '', + name: 'Category', + }, + attributes: { + name: { + type: 'string', + pluginOptions: { + i18n: { + localized: true, + }, + }, + }, + }, +}); +models.set('basic.dev-compo', { + collectionName: 'components_basic_dev_compos', + uid: 'basic.dev-compo', + category: 'basic', + modelType: 'component', + modelName: 'dev-compo', + globalId: 'ComponentBasicDevCompo', + info: { + displayName: 'DevCompo', + icon: 'allergies', + }, + attributes: { + categories: { + type: 'relation', + relation: 'oneToMany', + target: 'api::category.category', + }, + }, +}); +models.set('plugin::upload.file', { + collectionName: 'files', + info: { + singularName: 'file', + pluralName: 'files', + displayName: 'File', + description: '', + }, + attributes: { + name: { + type: 'string', + configurable: false, + required: true, + }, + }, + kind: 'collectionType', + modelType: 'contentType', + modelName: 'file', + connection: 'default', + uid: 'plugin::upload.file', + plugin: 'upload', + globalId: 'UploadFile', +}); + +module.exports = { + models, + existentIDs: [1, 2, 3, 4, 5, 6], + nonExistentIds: [10, 11, 12, 13, 14, 15, 16], +}; diff --git a/packages/core/strapi/lib/services/entity-validator/index.js b/packages/core/strapi/lib/services/entity-validator/index.js index 9ef2c980b5..befb97f08b 100644 --- a/packages/core/strapi/lib/services/entity-validator/index.js +++ b/packages/core/strapi/lib/services/entity-validator/index.js @@ -6,7 +6,7 @@ 'use strict'; const { uniqBy, castArray, isNil } = require('lodash'); -const { has, assoc, prop, isObject } = require('lodash/fp'); +const { has, assoc, prop, isObject, isEmpty } = require('lodash/fp'); const strapiUtils = require('@strapi/utils'); const validators = require('./validators'); @@ -248,7 +248,7 @@ const createValidateEntity = * @returns */ const buildRelationsStore = ({ uid, data, relationsStore = {} }) => { - if (!uid || !data) { + if (isEmpty(data)) { return relationsStore; } @@ -265,11 +265,12 @@ const buildRelationsStore = ({ uid, data, relationsStore = {} }) => { switch (attribute.type) { case 'relation': case 'media': { - const target = attribute.type === 'media' ? 'plugin::upload.file' : attribute.target; - if (!target) { + if (attribute.relation === 'morphToMany' || attribute.relation === 'morphToOne') { + // TODO: handle polymorphic relations break; } + const target = attribute.type === 'media' ? 'plugin::upload.file' : attribute.target; // As there are multiple formats supported for associating relations // with an entity, the value here can be an: array, object or number. let source; diff --git a/packages/core/strapi/tests/api/basic-dz.test.e2e.js b/packages/core/strapi/tests/api/basic-dz.test.e2e.js index 6791f4c8c2..868041c9be 100644 --- a/packages/core/strapi/tests/api/basic-dz.test.e2e.js +++ b/packages/core/strapi/tests/api/basic-dz.test.e2e.js @@ -341,7 +341,7 @@ describe('Core API - Basic + dz', () => { error: { status: 400, name: 'ValidationError', - message: 'dz[0].__component is a required field', + message: '2 errors occurred', details: { errors: [ { @@ -349,6 +349,11 @@ describe('Core API - Basic + dz', () => { message: 'dz[0].__component is a required field', name: 'ValidationError', }, + { + message: "Cannot read properties of undefined (reading 'attributes')", + name: 'ValidationError', + path: [], + }, ], }, }, diff --git a/packages/core/strapi/tests/endpoint.test.e2e.js b/packages/core/strapi/tests/endpoint.test.e2e.js index d7b19a3bec..c60d94c94c 100644 --- a/packages/core/strapi/tests/endpoint.test.e2e.js +++ b/packages/core/strapi/tests/endpoint.test.e2e.js @@ -221,10 +221,10 @@ describe('Create Strapi API End to End', () => { expect(body.data.attributes.tags.data.length).toBe(3); }); - test('Update article1 adding a non existent tag', async () => { + test('Error when updating article1 with some non existent tags', async () => { const { id, attributes } = data.articles[0]; const entry = { ...attributes }; - entry.tags = [1000, 1001, 1002]; + entry.tags = [1000, 1001, 1002, ...data.tags.slice(-1).map((t) => t.id)]; cleanDate(entry); From 3ef2eabdadd86c9e968bfa0ee8f1508c084fb144 Mon Sep 17 00:00:00 2001 From: Alexandre Bodin Date: Wed, 9 Nov 2022 19:10:41 +0100 Subject: [PATCH 08/12] Fix validation on user update --- .../server/controllers/validation/user.js | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/packages/plugins/users-permissions/server/controllers/validation/user.js b/packages/plugins/users-permissions/server/controllers/validation/user.js index e0407f138f..d62f3f77bd 100644 --- a/packages/plugins/users-permissions/server/controllers/validation/user.js +++ b/packages/plugins/users-permissions/server/controllers/validation/user.js @@ -18,7 +18,7 @@ const createUserBodySchema = yup.object().shape({ connect: yup .array() .of(yup.object().shape({ id: yup.strapiID().required() })) - .min(1) + .min(1, 'Users must have a role') .required(), }) .required() @@ -36,7 +36,16 @@ const updateUserBodySchema = yup.object().shape({ connect: yup .array() .of(yup.object().shape({ id: yup.strapiID().required() })) - .min(1) + .required(), + disconnect: yup + .array() + .test('CheckDisconnect', 'Cannot remove role', function test(disconnectValue) { + if (value.connect.length === 0 && disconnectValue.length > 0) { + return false; + } + + return true; + }) .required(), }) : yup.strapiID() From 13cbc3149767e0072c137364acfca086663b7662 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre=20No=C3=ABl?= Date: Wed, 9 Nov 2022 19:35:56 +0100 Subject: [PATCH 09/12] remove tests relation-list.test.api.js & relations.test.api.js --- .../content-manager/relations.test.api.js | 75 ------- .../server/tests/relation-list.test.api.js | 196 ------------------ 2 files changed, 271 deletions(-) delete mode 100644 packages/core/content-manager/server/tests/content-manager/relations.test.api.js delete mode 100644 packages/core/content-manager/server/tests/relation-list.test.api.js diff --git a/packages/core/content-manager/server/tests/content-manager/relations.test.api.js b/packages/core/content-manager/server/tests/content-manager/relations.test.api.js deleted file mode 100644 index cc7dbadcde..0000000000 --- a/packages/core/content-manager/server/tests/content-manager/relations.test.api.js +++ /dev/null @@ -1,75 +0,0 @@ -'use strict'; - -// Helpers. -const { createTestBuilder } = require('../../../../../../test/helpers/builder'); -const { createStrapiInstance } = require('../../../../../../test/helpers/strapi'); -const form = require('../../../../../../test/helpers/generators'); -const { createAuthRequest } = require('../../../../../../test/helpers/request'); - -const builder = createTestBuilder(); -let strapi; -let rq; - -const restart = async () => { - await strapi.destroy(); - strapi = await createStrapiInstance(); - rq = await createAuthRequest({ strapi }); -}; - -describe('Content Manager - Hide relations', () => { - beforeAll(async () => { - await builder.addContentTypes([form.article]).build(); - - strapi = await createStrapiInstance(); - rq = await createAuthRequest({ strapi }); - }); - - afterAll(async () => { - await strapi.destroy(); - await builder.cleanup(); - }); - - test('Hide relations', async () => { - await rq({ - url: '/content-manager/content-types/api::article.article/configuration', - method: 'PUT', - body: { - layouts: { - edit: [], - editRelations: [], - list: [], - }, - }, - }); - - const { body } = await rq({ - url: '/content-manager/content-types/api::article.article/configuration', - method: 'GET', - }); - - expect(body.data.contentType.layouts.editRelations).toStrictEqual([]); - }); - - test('Hide relations after server restart', async () => { - await rq({ - url: '/content-manager/content-types/api::article.article/configuration', - method: 'PUT', - body: { - layouts: { - edit: [], - editRelations: [], - list: [], - }, - }, - }); - - await restart(); - - const { body } = await rq({ - url: '/content-manager/content-types/api::article.article/configuration', - method: 'GET', - }); - - expect(body.data.contentType.layouts.editRelations).toStrictEqual([]); - }); -}); diff --git a/packages/core/content-manager/server/tests/relation-list.test.api.js b/packages/core/content-manager/server/tests/relation-list.test.api.js deleted file mode 100644 index ef0e7c65af..0000000000 --- a/packages/core/content-manager/server/tests/relation-list.test.api.js +++ /dev/null @@ -1,196 +0,0 @@ -'use strict'; - -// Test a simple default API with no relations - -const { omit, pick } = require('lodash/fp'); - -const { createTestBuilder } = require('../../../../../test/helpers/builder'); -const { createStrapiInstance } = require('../../../../../test/helpers/strapi'); -const { createAuthRequest } = require('../../../../../test/helpers/request'); - -let strapi; -let rq; -const data = { - products: [], - shops: [], -}; - -const productModel = { - attributes: { - name: { - type: 'string', - }, - }, - displayName: 'Product', - singularName: 'product', - pluralName: 'products', - description: '', - collectionName: '', -}; - -const productWithDPModel = { - attributes: { - name: { - type: 'string', - }, - }, - displayName: 'Product', - singularName: 'product', - pluralName: 'products', - draftAndPublish: true, - description: '', - collectionName: '', -}; - -const shopModel = { - attributes: { - name: { - type: 'string', - }, - products: { - type: 'relation', - relation: 'manyToMany', - target: 'api::product.product', - targetAttribute: 'shops', - }, - }, - displayName: 'Shop', - singularName: 'shop', - pluralName: 'shops', -}; - -const shops = [ - { - name: 'market', - }, -]; - -const products = - ({ withPublished = false }) => - ({ shop }) => { - const shops = [shop[0].id]; - - const entries = [ - { - name: 'tomato', - shops, - publishedAt: new Date(), - }, - { - name: 'apple', - shops, - publishedAt: null, - }, - ]; - - if (withPublished) { - return entries; - } - - return entries.map(omit('publishedAt')); - }; - -describe('Relation-list route', () => { - describe('without draftAndPublish', () => { - const builder = createTestBuilder(); - - beforeAll(async () => { - await builder - .addContentTypes([productModel, shopModel]) - .addFixtures(shopModel.singularName, shops) - .addFixtures(productModel.singularName, products({ withPublished: false })) - .build(); - - strapi = await createStrapiInstance(); - rq = await createAuthRequest({ strapi }); - - data.shops = await builder.sanitizedFixturesFor(shopModel.singularName, strapi); - data.products = await builder.sanitizedFixturesFor(productModel.singularName, strapi); - }); - - afterAll(async () => { - await strapi.destroy(); - await builder.cleanup(); - }); - - test('Can get relation-list for products of a shop', async () => { - const res = await rq({ - method: 'POST', - url: '/content-manager/relations/api::shop.shop/products', - }); - - expect(res.body).toHaveLength(data.products.length); - data.products.forEach((product, index) => { - expect(res.body[index]).toStrictEqual(pick(['_id', 'id', 'name'], product)); - }); - }); - - test('Can get relation-list for products of a shop and omit some results', async () => { - const res = await rq({ - method: 'POST', - url: '/content-manager/relations/api::shop.shop/products', - body: { - idsToOmit: [data.products[0].id], - }, - }); - - expect(res.body).toHaveLength(1); - expect(res.body[0]).toStrictEqual(pick(['_id', 'id', 'name'], data.products[1])); - }); - }); - - describe('with draftAndPublish', () => { - const builder = createTestBuilder(); - - beforeAll(async () => { - await builder - .addContentTypes([productWithDPModel, shopModel]) - .addFixtures(shopModel.singularName, shops) - .addFixtures(productWithDPModel.singularName, products({ withPublished: true })) - .build(); - - strapi = await createStrapiInstance(); - rq = await createAuthRequest({ strapi }); - - data.shops = await builder.sanitizedFixturesFor(shopModel.singularName, strapi); - data.products = await builder.sanitizedFixturesFor(productWithDPModel.singularName, strapi); - }); - - afterAll(async () => { - await strapi.destroy(); - await builder.cleanup(); - }); - - test('Can get relation-list for products of a shop', async () => { - const res = await rq({ - method: 'POST', - url: '/content-manager/relations/api::shop.shop/products', - }); - - expect(res.body).toHaveLength(data.products.length); - - const tomatoProductRes = res.body.find((p) => p.name === 'tomato'); - const appleProductRes = res.body.find((p) => p.name === 'apple'); - - expect(tomatoProductRes).toMatchObject(pick(['_id', 'id', 'name'], data.products[0])); - expect(tomatoProductRes.publishedAt).toBeISODate(); - expect(appleProductRes).toStrictEqual({ - ...pick(['_id', 'id', 'name'], data.products[1]), - publishedAt: null, - }); - }); - - test('Can get relation-list for products of a shop and omit some results', async () => { - const res = await rq({ - method: 'POST', - url: '/content-manager/relations/api::shop.shop/products', - body: { - idsToOmit: [data.products[1].id], - }, - }); - - expect(res.body).toHaveLength(1); - expect(res.body[0]).toMatchObject(pick(['_id', 'id', 'name'], data.products[0])); - }); - }); -}); From 37d14ac49b467f750535cce4a42734492da287a9 Mon Sep 17 00:00:00 2001 From: Jamie Howard Date: Wed, 9 Nov 2022 16:31:43 +0000 Subject: [PATCH 10/12] test(content-manager): basic-dz api --- .../server/tests/api/basic-dz.test.api.js | 7 ++++++- .../strapi/lib/services/entity-validator/index.js | 14 +++++++++++--- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/packages/core/content-manager/server/tests/api/basic-dz.test.api.js b/packages/core/content-manager/server/tests/api/basic-dz.test.api.js index a49a73ce6c..7ba785cde5 100644 --- a/packages/core/content-manager/server/tests/api/basic-dz.test.api.js +++ b/packages/core/content-manager/server/tests/api/basic-dz.test.api.js @@ -301,7 +301,7 @@ describe('CM API - Basic + dz', () => { error: { status: 400, name: 'ValidationError', - message: 'dz[0].__component is a required field', + message: '2 errors occurred', details: { errors: [ { @@ -309,6 +309,11 @@ describe('CM API - Basic + dz', () => { message: 'dz[0].__component is a required field', name: 'ValidationError', }, + { + message: "Cannot read properties of undefined (reading 'attributes')", + name: 'ValidationError', + path: [], + }, ], }, }, diff --git a/packages/core/strapi/lib/services/entity-validator/index.js b/packages/core/strapi/lib/services/entity-validator/index.js index befb97f08b..5e906a033e 100644 --- a/packages/core/strapi/lib/services/entity-validator/index.js +++ b/packages/core/strapi/lib/services/entity-validator/index.js @@ -277,7 +277,7 @@ const buildRelationsStore = ({ uid, data, relationsStore = {} }) => { if (Array.isArray(value)) { source = value; } else if (isObject(value)) { - source = value?.connect ?? value?.set ?? []; + source = value.connect ?? value.set ?? []; } else { source = castArray(value); } @@ -291,12 +291,20 @@ const buildRelationsStore = ({ uid, data, relationsStore = {} }) => { } case 'component': { return castArray(value).forEach((componentValue) => - buildRelationsStore({ uid: attribute.component, data: componentValue, relationsStore }) + buildRelationsStore({ + uid: attribute.component, + data: componentValue, + relationsStore, + }) ); } case 'dynamiczone': { return value.forEach((dzValue) => - buildRelationsStore({ uid: dzValue.__component, data: dzValue, relationsStore }) + buildRelationsStore({ + uid: dzValue.__component, + data: dzValue, + relationsStore, + }) ); } default: From 47975952a11bb3954841f0940b1b6b4499d5359e Mon Sep 17 00:00:00 2001 From: Jamie Howard Date: Thu, 10 Nov 2022 15:20:31 +0000 Subject: [PATCH 11/12] chore(entity-validator): refactor to use reduce --- .../lib/services/entity-validator/index.js | 56 ++++++++++--------- 1 file changed, 31 insertions(+), 25 deletions(-) diff --git a/packages/core/strapi/lib/services/entity-validator/index.js b/packages/core/strapi/lib/services/entity-validator/index.js index 5e906a033e..b9603146a7 100644 --- a/packages/core/strapi/lib/services/entity-validator/index.js +++ b/packages/core/strapi/lib/services/entity-validator/index.js @@ -6,7 +6,7 @@ 'use strict'; const { uniqBy, castArray, isNil } = require('lodash'); -const { has, assoc, prop, isObject, isEmpty } = require('lodash/fp'); +const { has, assoc, prop, isObject, isEmpty, merge } = require('lodash/fp'); const strapiUtils = require('@strapi/utils'); const validators = require('./validators'); @@ -244,22 +244,20 @@ const createValidateEntity = * Builds an object containing all the media and relations being associated with an entity * @param {String} uid of the model * @param {Object} data - * @param {Object} relationsStore to be updated and returned - * @returns + * @returns {Object} */ -const buildRelationsStore = ({ uid, data, relationsStore = {} }) => { +const buildRelationsStore = ({ uid, data }) => { if (isEmpty(data)) { - return relationsStore; + return {}; } - const currentModel = strapi.getModel(uid); - Object.keys(currentModel.attributes).forEach((attributeName) => { + return Object.keys(currentModel.attributes).reduce((result, attributeName) => { const attribute = currentModel.attributes[attributeName]; - const value = data[attributeName]; + if (isNil(value)) { - return; + return result; } switch (attribute.type) { @@ -285,34 +283,42 @@ const buildRelationsStore = ({ uid, data, relationsStore = {} }) => { // Update the relationStore to keep track of all associations being made // with relations and media. - relationsStore[target] = relationsStore[target] || []; - relationsStore[target].push(...idArray); + result[target] = result[target] || []; + result[target].push(...idArray); break; } case 'component': { - return castArray(value).forEach((componentValue) => - buildRelationsStore({ - uid: attribute.component, - data: componentValue, - relationsStore, - }) + return castArray(value).reduce( + (relationsStore, componentValue) => + merge( + relationsStore, + buildRelationsStore({ + uid: attribute.component, + data: componentValue, + }) + ), + result ); } case 'dynamiczone': { - return value.forEach((dzValue) => - buildRelationsStore({ - uid: dzValue.__component, - data: dzValue, - relationsStore, - }) + return value.reduce( + (relationsStore, dzValue) => + merge( + relationsStore, + buildRelationsStore({ + uid: dzValue.__component, + data: dzValue, + }) + ), + result ); } default: break; } - }); - return relationsStore; + return result; + }, {}); }; /** From 2ab5f01d64fea9d8d9ec197af481c090df00b9c5 Mon Sep 17 00:00:00 2001 From: Gustav Hansen Date: Thu, 10 Nov 2022 17:37:40 +0100 Subject: [PATCH 12/12] CM: Remove unused icon components Co-authored-by: Josh Ellis --- .../src/content-manager/icons/Bold/index.js | 22 ----------- .../src/content-manager/icons/Code/index.js | 13 ------- .../src/content-manager/icons/Cross/index.js | 28 ------------- .../src/content-manager/icons/Italic/index.js | 23 ----------- .../src/content-manager/icons/Link/index.js | 21 ---------- .../src/content-manager/icons/Media/index.js | 14 ------- .../src/content-manager/icons/Na/index.js | 39 ------------------- .../src/content-manager/icons/Ol/index.js | 13 ------- .../src/content-manager/icons/Quote/index.js | 13 ------- .../content-manager/icons/Striked/index.js | 24 ------------ .../src/content-manager/icons/Ul/index.js | 15 ------- .../content-manager/icons/Underline/index.js | 22 ----------- 12 files changed, 247 deletions(-) delete mode 100644 packages/core/admin/admin/src/content-manager/icons/Bold/index.js delete mode 100644 packages/core/admin/admin/src/content-manager/icons/Code/index.js delete mode 100644 packages/core/admin/admin/src/content-manager/icons/Cross/index.js delete mode 100644 packages/core/admin/admin/src/content-manager/icons/Italic/index.js delete mode 100644 packages/core/admin/admin/src/content-manager/icons/Link/index.js delete mode 100644 packages/core/admin/admin/src/content-manager/icons/Media/index.js delete mode 100644 packages/core/admin/admin/src/content-manager/icons/Na/index.js delete mode 100644 packages/core/admin/admin/src/content-manager/icons/Ol/index.js delete mode 100644 packages/core/admin/admin/src/content-manager/icons/Quote/index.js delete mode 100644 packages/core/admin/admin/src/content-manager/icons/Striked/index.js delete mode 100644 packages/core/admin/admin/src/content-manager/icons/Ul/index.js delete mode 100644 packages/core/admin/admin/src/content-manager/icons/Underline/index.js diff --git a/packages/core/admin/admin/src/content-manager/icons/Bold/index.js b/packages/core/admin/admin/src/content-manager/icons/Bold/index.js deleted file mode 100644 index b55992e6e2..0000000000 --- a/packages/core/admin/admin/src/content-manager/icons/Bold/index.js +++ /dev/null @@ -1,22 +0,0 @@ -import React from 'react'; - -const Bold = () => { - return ( - - - - B - - - - ); -}; - -export default Bold; diff --git a/packages/core/admin/admin/src/content-manager/icons/Code/index.js b/packages/core/admin/admin/src/content-manager/icons/Code/index.js deleted file mode 100644 index 4837b6fd2a..0000000000 --- a/packages/core/admin/admin/src/content-manager/icons/Code/index.js +++ /dev/null @@ -1,13 +0,0 @@ -import React from 'react'; - -const Code = () => { - return ( - - - - - - ); -}; - -export default Code; diff --git a/packages/core/admin/admin/src/content-manager/icons/Cross/index.js b/packages/core/admin/admin/src/content-manager/icons/Cross/index.js deleted file mode 100644 index 72e5c9fdd7..0000000000 --- a/packages/core/admin/admin/src/content-manager/icons/Cross/index.js +++ /dev/null @@ -1,28 +0,0 @@ -import React from 'react'; -import PropTypes from 'prop-types'; - -const Cross = ({ fill, height, width, ...rest }) => { - return ( - - - - ); -}; - -Cross.defaultProps = { - fill: '#b3b5b9', - height: '8', - width: '8', -}; - -Cross.propTypes = { - fill: PropTypes.string, - height: PropTypes.oneOfType([PropTypes.number, PropTypes.string]), - width: PropTypes.oneOfType([PropTypes.number, PropTypes.string]), -}; - -export default Cross; diff --git a/packages/core/admin/admin/src/content-manager/icons/Italic/index.js b/packages/core/admin/admin/src/content-manager/icons/Italic/index.js deleted file mode 100644 index 9295a7f5ab..0000000000 --- a/packages/core/admin/admin/src/content-manager/icons/Italic/index.js +++ /dev/null @@ -1,23 +0,0 @@ -import React from 'react'; - -const Italic = () => { - return ( - - - - I - - - - ); -}; - -export default Italic; diff --git a/packages/core/admin/admin/src/content-manager/icons/Link/index.js b/packages/core/admin/admin/src/content-manager/icons/Link/index.js deleted file mode 100644 index 79b461a819..0000000000 --- a/packages/core/admin/admin/src/content-manager/icons/Link/index.js +++ /dev/null @@ -1,21 +0,0 @@ -import React from 'react'; - -const Link = () => { - return ( - - - - - - - - ); -}; - -export default Link; diff --git a/packages/core/admin/admin/src/content-manager/icons/Media/index.js b/packages/core/admin/admin/src/content-manager/icons/Media/index.js deleted file mode 100644 index 80841145a7..0000000000 --- a/packages/core/admin/admin/src/content-manager/icons/Media/index.js +++ /dev/null @@ -1,14 +0,0 @@ -import React from 'react'; - -const Media = () => { - return ( - - - - - - - ); -}; - -export default Media; diff --git a/packages/core/admin/admin/src/content-manager/icons/Na/index.js b/packages/core/admin/admin/src/content-manager/icons/Na/index.js deleted file mode 100644 index 90157de9e2..0000000000 --- a/packages/core/admin/admin/src/content-manager/icons/Na/index.js +++ /dev/null @@ -1,39 +0,0 @@ -import React from 'react'; -import PropTypes from 'prop-types'; - -const Na = ({ fill, fontFamily, fontSize, fontWeight, height, textFill, width, ...rest }) => { - return ( - - - - - - N/A - - - - - ); -}; - -Na.defaultProps = { - fill: '#fafafb', - fontFamily: 'Lato-Medium, Lato', - fontSize: '12', - fontWeight: '400', - height: '35', - textFill: '#838383', - width: '35', -}; - -Na.propTypes = { - fill: PropTypes.string, - fontFamily: PropTypes.string, - fontSize: PropTypes.string, - fontWeight: PropTypes.string, - height: PropTypes.oneOfType([PropTypes.number, PropTypes.string]), - textFill: PropTypes.string, - width: PropTypes.oneOfType([PropTypes.number, PropTypes.string]), -}; - -export default Na; diff --git a/packages/core/admin/admin/src/content-manager/icons/Ol/index.js b/packages/core/admin/admin/src/content-manager/icons/Ol/index.js deleted file mode 100644 index 1fe01741ae..0000000000 --- a/packages/core/admin/admin/src/content-manager/icons/Ol/index.js +++ /dev/null @@ -1,13 +0,0 @@ -import React from 'react'; - -const Ol = () => { - return ( - - - - - - ); -}; - -export default Ol; diff --git a/packages/core/admin/admin/src/content-manager/icons/Quote/index.js b/packages/core/admin/admin/src/content-manager/icons/Quote/index.js deleted file mode 100644 index 830f5a75c6..0000000000 --- a/packages/core/admin/admin/src/content-manager/icons/Quote/index.js +++ /dev/null @@ -1,13 +0,0 @@ -import React from 'react'; - -const Quote = () => { - return ( - - - - - - ); -}; - -export default Quote; diff --git a/packages/core/admin/admin/src/content-manager/icons/Striked/index.js b/packages/core/admin/admin/src/content-manager/icons/Striked/index.js deleted file mode 100644 index 534e61cfc9..0000000000 --- a/packages/core/admin/admin/src/content-manager/icons/Striked/index.js +++ /dev/null @@ -1,24 +0,0 @@ -import React from 'react'; - -const Striked = () => { - return ( - - - - - abc - - - - - - ); -}; - -export default Striked; diff --git a/packages/core/admin/admin/src/content-manager/icons/Ul/index.js b/packages/core/admin/admin/src/content-manager/icons/Ul/index.js deleted file mode 100644 index 6a3284b2a2..0000000000 --- a/packages/core/admin/admin/src/content-manager/icons/Ul/index.js +++ /dev/null @@ -1,15 +0,0 @@ -import React from 'react'; - -const Ul = () => { - return ( - - - - - - - - ); -}; - -export default Ul; diff --git a/packages/core/admin/admin/src/content-manager/icons/Underline/index.js b/packages/core/admin/admin/src/content-manager/icons/Underline/index.js deleted file mode 100644 index ac02a6306e..0000000000 --- a/packages/core/admin/admin/src/content-manager/icons/Underline/index.js +++ /dev/null @@ -1,22 +0,0 @@ -import React from 'react'; - -const Underline = () => { - return ( - - - - U - - - - ); -}; - -export default Underline;