From 3525066d46edaf0f7574633dc3f5cfdf899ecbe0 Mon Sep 17 00:00:00 2001 From: Cyberxon Date: Mon, 12 Aug 2024 15:37:55 +0200 Subject: [PATCH 1/3] feat: sort entries by references before add --- .../tasks/create-add-entities-task.ts | 3 +- src/engine/utils/sort-entries-by-reference.ts | 113 +++++++++++++++++ .../create-changeset-item-with-data.ts | 27 ++++ .../utils/sort-entries-by-reference.test.ts | 117 ++++++++++++++++++ 4 files changed, 259 insertions(+), 1 deletion(-) create mode 100644 src/engine/utils/sort-entries-by-reference.ts create mode 100644 src/test/helpers/create-changeset-item-with-data.ts create mode 100644 test/unit/engine/utils/sort-entries-by-reference.test.ts diff --git a/src/engine/apply-changeset/tasks/create-add-entities-task.ts b/src/engine/apply-changeset/tasks/create-add-entities-task.ts index d2cfe221..cccfa66d 100644 --- a/src/engine/apply-changeset/tasks/create-add-entities-task.ts +++ b/src/engine/apply-changeset/tasks/create-add-entities-task.ts @@ -6,6 +6,7 @@ import { ApplyChangesetContext } from '../types' import { pluralizeEntry } from '../../utils' import { isString } from 'lodash' import { publishEntity } from '../actions/publish-entity' +import sortEntries from '../../utils/sort-entries-by-reference' export const createAddEntitiesTask = (): ListrTask => { return { @@ -13,7 +14,7 @@ export const createAddEntitiesTask = (): ListrTask => { task: async (context: ApplyChangesetContext, task) => { const { client, changeset, environmentId, logger, responseCollector } = context logger.info('Start createAddEntitiesTask') - const entries = changeset.items.filter((item) => item.changeType === 'add') as AddedChangesetItem[] + const entries = sortEntries(changeset.items.filter((item) => item.changeType === 'add') as AddedChangesetItem[]) const entityCount = entries.length task.title = `Adding ${entityCount} ${pluralizeEntry(entityCount)}` diff --git a/src/engine/utils/sort-entries-by-reference.ts b/src/engine/utils/sort-entries-by-reference.ts new file mode 100644 index 00000000..46172d67 --- /dev/null +++ b/src/engine/utils/sort-entries-by-reference.ts @@ -0,0 +1,113 @@ +import { some, filter, map, flatten, values, get, has } from 'lodash' +import { AddedChangesetItem } from '../types' +import { EntryField } from 'contentful' +import { EntrySkeletonType } from 'contentful/dist/types/types/query' + +type LinksPerEntry = { index: number; linkIndexes: number[] } + +/** + * Given a list of entries, this function reorders them so that entries which + * are linked from other entries always come first in the order. This ensures + * that when we publish the newly added entries, we are not publishing entries + * which contain links to other entries that haven't been published yet. + */ +export default function sortEntries(entries: AddedChangesetItem[]): AddedChangesetItem[] { + const linkedEntries = getLinkedEntries(entries) + + const mergedLinkedEntries = mergeSort(linkedEntries, (a: LinksPerEntry) => { + return hasLinkedIndexesInFront(a) + }) + + return map(mergedLinkedEntries, (linkInfo: LinksPerEntry) => entries[linkInfo.index]) + + function hasLinkedIndexesInFront(item: LinksPerEntry): number { + if (hasLinkedIndexes(item)) { + return some(item.linkIndexes, (index) => index > item.index) ? 1 : -1 + } + return 0 + } + + function hasLinkedIndexes(item: LinksPerEntry): boolean { + return item.linkIndexes.length > 0 + } +} + +function getLinkedEntries(entries: AddedChangesetItem[]): LinksPerEntry[] { + return map(entries, function (entry: AddedChangesetItem) { + const entryIndex = entries.indexOf(entry) + + const rawLinks = map(entry.data.fields, (field) => { + field = values(field)[0] + if (isEntryLink(field)) { + return getFieldEntriesIndex(field, entries) + } else if (isEntityArray(field) && isEntryLink(field[0])) { + return map(field, (item: EntryField) => getFieldEntriesIndex(item, entries)) + } + }) + + return { + index: entryIndex, + linkIndexes: filter(flatten(rawLinks), (index: number) => index >= 0) as number[], + } + }) +} + +function getFieldEntriesIndex(field: EntryField, entries: AddedChangesetItem[]): number { + const id = get(field, 'sys.id') + return entries.findIndex((entry) => entry.data.sys.id === id) +} + +function isEntryLink(item: EntryField): boolean { + return get(item, 'sys.type') === 'Entry' || get(item, 'sys.linkType') === 'Entry' +} + +function isEntityArray(item: EntryField): boolean { + return Array.isArray(item) && item.length > 0 && has(item[0], 'sys') +} + +/** + * From https://github.com/millermedeiros/amd-utils/blob/master/src/array/sort.js + * MIT Licensed + * Merge sort (http://en.wikipedia.org/wiki/Merge_sort) + * @version 0.1.0 (2012/05/23) + */ +function mergeSort( + arr: LinksPerEntry[], + compareFn: ((a: LinksPerEntry) => number) | ((a: LinksPerEntry, b: LinksPerEntry) => number), +): LinksPerEntry[] { + if (arr.length < 2) return arr + + if (compareFn == null) compareFn = defaultCompare + + const mid = ~~(arr.length / 2) + const left = mergeSort(arr.slice(0, mid), compareFn) + const right = mergeSort(arr.slice(mid, arr.length), compareFn) + + return merge(left, right, compareFn) +} + +function defaultCompare(a: LinksPerEntry, b: LinksPerEntry): -1 | 1 | 0 { + return a < b ? -1 : a > b ? 1 : 0 +} + +function merge( + left: LinksPerEntry[], + right: LinksPerEntry[], + compareFn: ((a: LinksPerEntry) => number) | ((a: LinksPerEntry, b: LinksPerEntry) => number), +): LinksPerEntry[] { + const result: LinksPerEntry[] = [] + + while (left.length > 0 && right.length > 0) { + if (compareFn(left[0], right[0]) <= 0) { + // if 0 it should preserve same order (stable) + result.push(left.shift() as LinksPerEntry) + } else { + result.push(right.shift() as LinksPerEntry) + } + } + + if (left.length) result.push(...left) + if (right.length) result.push(...right) + + return result +} diff --git a/src/test/helpers/create-changeset-item-with-data.ts b/src/test/helpers/create-changeset-item-with-data.ts new file mode 100644 index 00000000..620597fe --- /dev/null +++ b/src/test/helpers/create-changeset-item-with-data.ts @@ -0,0 +1,27 @@ +import { AddedChangesetItem } from '../../engine/types' +import { createLinkObject } from '../../engine/utils/create-link-object' +import { EntryField } from 'contentful' +import { EntrySkeletonType } from 'contentful/dist/types/types/query' + +export const createChangesetItemWithData = ( + contentTypeId: string, + entryId: string, + fields: EntryField = {}, +): AddedChangesetItem => { + const referencedItem: AddedChangesetItem = createLinkObject(entryId, 'add', 'Entry') + referencedItem.data = { + sys: { + space: { sys: { type: 'Link', linkType: 'Space', id: '529ziq3ce86u' } }, + id: entryId, + type: 'Entry', + createdAt: '2023-05-17T10:36:22.538Z', + updatedAt: '2023-05-17T10:36:40.280Z', + environment: { sys: { id: 'master', type: 'Link', linkType: 'Environment' } }, + revision: 1, + version: 1, + contentType: { sys: { type: 'Link', linkType: 'ContentType', id: contentTypeId } }, + }, + fields, + } + return referencedItem +} diff --git a/test/unit/engine/utils/sort-entries-by-reference.test.ts b/test/unit/engine/utils/sort-entries-by-reference.test.ts new file mode 100644 index 00000000..af0db6b7 --- /dev/null +++ b/test/unit/engine/utils/sort-entries-by-reference.test.ts @@ -0,0 +1,117 @@ +import { AddedChangesetItem } from '../../../../src/engine/types' +import sortEntries from '../../../../src/engine/utils/sort-entries-by-reference' +import { expect } from 'chai' +import { createChangesetItemWithData } from '../../../../src/test/helpers/create-changeset-item-with-data' + +describe('sortEntriesByReference', () => { + const referencedItem: AddedChangesetItem = createChangesetItemWithData('lesson', 'added-entry') + const referencedItem1: AddedChangesetItem = createChangesetItemWithData('lesson', 'added-entry1') + const itemWithOneLink: AddedChangesetItem = createChangesetItemWithData('lesson1', 'added-entry-with-one-link', { + referenceField: { + 'en-US': { + sys: { type: 'Link', linkType: 'Entry', id: 'added-entry' }, + }, + }, + }) + + const itemWithArrayLinks: AddedChangesetItem = createChangesetItemWithData('lesson2', 'added-entry-with-multi-link', { + referencesField: { + 'en-US': [ + { + sys: { type: 'Link', linkType: 'Entry', id: 'added-entry' }, + }, + { + sys: { type: 'Link', linkType: 'Entry', id: 'added-entry1' }, + }, + ], + }, + }) + + const itemWithNonExistentLinks: AddedChangesetItem = createChangesetItemWithData( + 'lesson3', + 'added-entry-with-non-existent-link', + { + referenceField: { + 'en-US': { + sys: { type: 'Link', linkType: 'Entry', id: 'random-entry' }, + }, + }, + }, + ) + + const nonReferencedItem = createChangesetItemWithData('lesson', 'non-referenced-entry') + + it('should order based on references if the linked entry is present in the changeset', () => { + // one link + expect(sortEntries([itemWithOneLink, referencedItem, referencedItem1])).to.deep.equal([ + referencedItem, + referencedItem1, + itemWithOneLink, + ]) + // multi link + expect(sortEntries([itemWithArrayLinks, referencedItem, referencedItem1])).to.deep.equal([ + referencedItem, + referencedItem1, + itemWithArrayLinks, + ]) + // both + expect(sortEntries([itemWithArrayLinks, itemWithOneLink, referencedItem, referencedItem1])).to.deep.equal([ + referencedItem, + referencedItem1, + itemWithOneLink, + itemWithArrayLinks, + ]) + }) + + it('should not reorder if the linked entry is not present in the changeset', () => { + // one Link + expect(sortEntries([itemWithOneLink, nonReferencedItem])).to.deep.equal([itemWithOneLink, nonReferencedItem]) + // Muli links + expect(sortEntries([itemWithArrayLinks, nonReferencedItem])).to.deep.equal([itemWithArrayLinks, nonReferencedItem]) + //both + expect(sortEntries([itemWithArrayLinks, itemWithOneLink, nonReferencedItem])).to.deep.equal([ + itemWithArrayLinks, + itemWithOneLink, + nonReferencedItem, + ]) + }) + + it('should not reorder there are no links in the changeset', () => { + expect(sortEntries([nonReferencedItem, referencedItem, referencedItem1])).to.deep.equal([ + nonReferencedItem, + referencedItem, + referencedItem1, + ]) + }) + + it('should reorder only entries with links present in the changeset', () => { + // with links non present in the changeset + expect(sortEntries([itemWithNonExistentLinks, referencedItem, referencedItem1, nonReferencedItem])).to.deep.equal([ + itemWithNonExistentLinks, + referencedItem, + referencedItem1, + nonReferencedItem, + ]) + // With Both links present and not present + expect( + sortEntries([itemWithArrayLinks, referencedItem, referencedItem1, nonReferencedItem, itemWithNonExistentLinks]), + ).to.deep.equal([referencedItem, referencedItem1, nonReferencedItem, itemWithNonExistentLinks, itemWithArrayLinks]) + expect( + sortEntries([ + itemWithArrayLinks, + itemWithOneLink, + itemWithNonExistentLinks, + referencedItem, + referencedItem1, + nonReferencedItem, + ]), + ).to.deep.equal([ + itemWithNonExistentLinks, + referencedItem, + referencedItem1, + nonReferencedItem, + itemWithOneLink, + itemWithArrayLinks, + ]) + }) +}) From 052fc7b7a5fcbe4ea6c21cc27dac78106b94fbfe Mon Sep 17 00:00:00 2001 From: Cyberxon Date: Tue, 13 Aug 2024 10:12:10 +0200 Subject: [PATCH 2/3] fix: rename function --- .../tasks/create-add-entities-task.ts | 6 ++- src/engine/utils/sort-entries-by-reference.ts | 4 +- .../utils/sort-entries-by-reference.test.ts | 48 +++++++++++-------- 3 files changed, 34 insertions(+), 24 deletions(-) diff --git a/src/engine/apply-changeset/tasks/create-add-entities-task.ts b/src/engine/apply-changeset/tasks/create-add-entities-task.ts index cccfa66d..293b1b7b 100644 --- a/src/engine/apply-changeset/tasks/create-add-entities-task.ts +++ b/src/engine/apply-changeset/tasks/create-add-entities-task.ts @@ -6,7 +6,7 @@ import { ApplyChangesetContext } from '../types' import { pluralizeEntry } from '../../utils' import { isString } from 'lodash' import { publishEntity } from '../actions/publish-entity' -import sortEntries from '../../utils/sort-entries-by-reference' +import sortEntriesByReference from '../../utils/sort-entries-by-reference' export const createAddEntitiesTask = (): ListrTask => { return { @@ -14,7 +14,9 @@ export const createAddEntitiesTask = (): ListrTask => { task: async (context: ApplyChangesetContext, task) => { const { client, changeset, environmentId, logger, responseCollector } = context logger.info('Start createAddEntitiesTask') - const entries = sortEntries(changeset.items.filter((item) => item.changeType === 'add') as AddedChangesetItem[]) + const entries = sortEntriesByReference( + changeset.items.filter((item) => item.changeType === 'add') as AddedChangesetItem[], + ) const entityCount = entries.length task.title = `Adding ${entityCount} ${pluralizeEntry(entityCount)}` diff --git a/src/engine/utils/sort-entries-by-reference.ts b/src/engine/utils/sort-entries-by-reference.ts index 46172d67..663f3ff9 100644 --- a/src/engine/utils/sort-entries-by-reference.ts +++ b/src/engine/utils/sort-entries-by-reference.ts @@ -6,12 +6,14 @@ import { EntrySkeletonType } from 'contentful/dist/types/types/query' type LinksPerEntry = { index: number; linkIndexes: number[] } /** + * This function was inspired by the sortEntries function in contentful-import + * https://github.com/contentful/contentful-import/blob/028ba2cf1a0df9415da5f0608adb8a24e0428a98/lib/utils/sort-entries.ts * Given a list of entries, this function reorders them so that entries which * are linked from other entries always come first in the order. This ensures * that when we publish the newly added entries, we are not publishing entries * which contain links to other entries that haven't been published yet. */ -export default function sortEntries(entries: AddedChangesetItem[]): AddedChangesetItem[] { +export default function sortEntriesByReference(entries: AddedChangesetItem[]): AddedChangesetItem[] { const linkedEntries = getLinkedEntries(entries) const mergedLinkedEntries = mergeSort(linkedEntries, (a: LinksPerEntry) => { diff --git a/test/unit/engine/utils/sort-entries-by-reference.test.ts b/test/unit/engine/utils/sort-entries-by-reference.test.ts index af0db6b7..909a1dd1 100644 --- a/test/unit/engine/utils/sort-entries-by-reference.test.ts +++ b/test/unit/engine/utils/sort-entries-by-reference.test.ts @@ -1,5 +1,5 @@ import { AddedChangesetItem } from '../../../../src/engine/types' -import sortEntries from '../../../../src/engine/utils/sort-entries-by-reference' +import sortEntriesByReference from '../../../../src/engine/utils/sort-entries-by-reference' import { expect } from 'chai' import { createChangesetItemWithData } from '../../../../src/test/helpers/create-changeset-item-with-data' @@ -43,33 +43,36 @@ describe('sortEntriesByReference', () => { it('should order based on references if the linked entry is present in the changeset', () => { // one link - expect(sortEntries([itemWithOneLink, referencedItem, referencedItem1])).to.deep.equal([ + expect(sortEntriesByReference([itemWithOneLink, referencedItem, referencedItem1])).to.deep.equal([ referencedItem, referencedItem1, itemWithOneLink, ]) // multi link - expect(sortEntries([itemWithArrayLinks, referencedItem, referencedItem1])).to.deep.equal([ + expect(sortEntriesByReference([itemWithArrayLinks, referencedItem, referencedItem1])).to.deep.equal([ referencedItem, referencedItem1, itemWithArrayLinks, ]) // both - expect(sortEntries([itemWithArrayLinks, itemWithOneLink, referencedItem, referencedItem1])).to.deep.equal([ - referencedItem, - referencedItem1, - itemWithOneLink, - itemWithArrayLinks, - ]) + expect( + sortEntriesByReference([itemWithArrayLinks, itemWithOneLink, referencedItem, referencedItem1]), + ).to.deep.equal([referencedItem, referencedItem1, itemWithOneLink, itemWithArrayLinks]) }) it('should not reorder if the linked entry is not present in the changeset', () => { // one Link - expect(sortEntries([itemWithOneLink, nonReferencedItem])).to.deep.equal([itemWithOneLink, nonReferencedItem]) + expect(sortEntriesByReference([itemWithOneLink, nonReferencedItem])).to.deep.equal([ + itemWithOneLink, + nonReferencedItem, + ]) // Muli links - expect(sortEntries([itemWithArrayLinks, nonReferencedItem])).to.deep.equal([itemWithArrayLinks, nonReferencedItem]) + expect(sortEntriesByReference([itemWithArrayLinks, nonReferencedItem])).to.deep.equal([ + itemWithArrayLinks, + nonReferencedItem, + ]) //both - expect(sortEntries([itemWithArrayLinks, itemWithOneLink, nonReferencedItem])).to.deep.equal([ + expect(sortEntriesByReference([itemWithArrayLinks, itemWithOneLink, nonReferencedItem])).to.deep.equal([ itemWithArrayLinks, itemWithOneLink, nonReferencedItem, @@ -77,7 +80,7 @@ describe('sortEntriesByReference', () => { }) it('should not reorder there are no links in the changeset', () => { - expect(sortEntries([nonReferencedItem, referencedItem, referencedItem1])).to.deep.equal([ + expect(sortEntriesByReference([nonReferencedItem, referencedItem, referencedItem1])).to.deep.equal([ nonReferencedItem, referencedItem, referencedItem1, @@ -86,18 +89,21 @@ describe('sortEntriesByReference', () => { it('should reorder only entries with links present in the changeset', () => { // with links non present in the changeset - expect(sortEntries([itemWithNonExistentLinks, referencedItem, referencedItem1, nonReferencedItem])).to.deep.equal([ - itemWithNonExistentLinks, - referencedItem, - referencedItem1, - nonReferencedItem, - ]) + expect( + sortEntriesByReference([itemWithNonExistentLinks, referencedItem, referencedItem1, nonReferencedItem]), + ).to.deep.equal([itemWithNonExistentLinks, referencedItem, referencedItem1, nonReferencedItem]) // With Both links present and not present expect( - sortEntries([itemWithArrayLinks, referencedItem, referencedItem1, nonReferencedItem, itemWithNonExistentLinks]), + sortEntriesByReference([ + itemWithArrayLinks, + referencedItem, + referencedItem1, + nonReferencedItem, + itemWithNonExistentLinks, + ]), ).to.deep.equal([referencedItem, referencedItem1, nonReferencedItem, itemWithNonExistentLinks, itemWithArrayLinks]) expect( - sortEntries([ + sortEntriesByReference([ itemWithArrayLinks, itemWithOneLink, itemWithNonExistentLinks, From 6926fa235c74635f2cbc4c2c64796e070aa6691c Mon Sep 17 00:00:00 2001 From: Cyberxon Date: Wed, 14 Aug 2024 13:39:03 +0200 Subject: [PATCH 3/3] fix: use native js functions instead of lodash --- src/engine/utils/sort-entries-by-reference.ts | 47 ++++++++----------- 1 file changed, 19 insertions(+), 28 deletions(-) diff --git a/src/engine/utils/sort-entries-by-reference.ts b/src/engine/utils/sort-entries-by-reference.ts index 663f3ff9..e9c29ff9 100644 --- a/src/engine/utils/sort-entries-by-reference.ts +++ b/src/engine/utils/sort-entries-by-reference.ts @@ -1,9 +1,8 @@ -import { some, filter, map, flatten, values, get, has } from 'lodash' import { AddedChangesetItem } from '../types' -import { EntryField } from 'contentful' -import { EntrySkeletonType } from 'contentful/dist/types/types/query' +import { FieldsType } from 'contentful' type LinksPerEntry = { index: number; linkIndexes: number[] } +type CompareFunction = ((a: LinksPerEntry) => number) | ((a: LinksPerEntry, b: LinksPerEntry) => number) /** * This function was inspired by the sortEntries function in contentful-import @@ -20,11 +19,11 @@ export default function sortEntriesByReference(entries: AddedChangesetItem[]): A return hasLinkedIndexesInFront(a) }) - return map(mergedLinkedEntries, (linkInfo: LinksPerEntry) => entries[linkInfo.index]) + return mergedLinkedEntries.map((linkInfo: LinksPerEntry) => entries[linkInfo.index]) function hasLinkedIndexesInFront(item: LinksPerEntry): number { if (hasLinkedIndexes(item)) { - return some(item.linkIndexes, (index) => index > item.index) ? 1 : -1 + return item.linkIndexes.some((index) => index > item.index) ? 1 : -1 } return 0 } @@ -35,36 +34,35 @@ export default function sortEntriesByReference(entries: AddedChangesetItem[]): A } function getLinkedEntries(entries: AddedChangesetItem[]): LinksPerEntry[] { - return map(entries, function (entry: AddedChangesetItem) { - const entryIndex = entries.indexOf(entry) - - const rawLinks = map(entry.data.fields, (field) => { - field = values(field)[0] + return entries.map((entry: AddedChangesetItem, entryIndex: number) => { + const fieldsArray = Object.values(entry.data.fields as FieldsType) + const rawLinks = fieldsArray.map((field: FieldsType) => { + field = Object.values(field)[0] if (isEntryLink(field)) { return getFieldEntriesIndex(field, entries) - } else if (isEntityArray(field) && isEntryLink(field[0])) { - return map(field, (item: EntryField) => getFieldEntriesIndex(item, entries)) + } else if (isEntryArrayLink(field)) { + return field.map((item: FieldsType) => getFieldEntriesIndex(item, entries)) } }) return { index: entryIndex, - linkIndexes: filter(flatten(rawLinks), (index: number) => index >= 0) as number[], + linkIndexes: rawLinks.flat().filter((index: number) => index >= 0) as number[], } }) } -function getFieldEntriesIndex(field: EntryField, entries: AddedChangesetItem[]): number { - const id = get(field, 'sys.id') +function getFieldEntriesIndex(field: FieldsType, entries: AddedChangesetItem[]): number { + const { id } = field.sys return entries.findIndex((entry) => entry.data.sys.id === id) } -function isEntryLink(item: EntryField): boolean { - return get(item, 'sys.type') === 'Entry' || get(item, 'sys.linkType') === 'Entry' +function isEntryLink(item: FieldsType): boolean { + return item?.sys?.type === 'Entry' || item?.sys?.linkType === 'Entry' } -function isEntityArray(item: EntryField): boolean { - return Array.isArray(item) && item.length > 0 && has(item[0], 'sys') +function isEntryArrayLink(item: FieldsType): boolean { + return Array.isArray(item) && item.length > 0 && isEntryLink(item[0]) } /** @@ -73,10 +71,7 @@ function isEntityArray(item: EntryField): boolean { * Merge sort (http://en.wikipedia.org/wiki/Merge_sort) * @version 0.1.0 (2012/05/23) */ -function mergeSort( - arr: LinksPerEntry[], - compareFn: ((a: LinksPerEntry) => number) | ((a: LinksPerEntry, b: LinksPerEntry) => number), -): LinksPerEntry[] { +function mergeSort(arr: LinksPerEntry[], compareFn: CompareFunction): LinksPerEntry[] { if (arr.length < 2) return arr if (compareFn == null) compareFn = defaultCompare @@ -92,11 +87,7 @@ function defaultCompare(a: LinksPerEntry, b: LinksPerEntry): -1 | 1 | 0 { return a < b ? -1 : a > b ? 1 : 0 } -function merge( - left: LinksPerEntry[], - right: LinksPerEntry[], - compareFn: ((a: LinksPerEntry) => number) | ((a: LinksPerEntry, b: LinksPerEntry) => number), -): LinksPerEntry[] { +function merge(left: LinksPerEntry[], right: LinksPerEntry[], compareFn: CompareFunction): LinksPerEntry[] { const result: LinksPerEntry[] = [] while (left.length > 0 && right.length > 0) {