Fix: Error applying schemas with nested collection(s) (#13949)

* Fix nested collection creation when parent exists

* tests

* desc | comments

* typo

* minor tweak

Co-authored-by: Azri Kahar <42867097+azrikahar@users.noreply.github.com>
This commit is contained in:
Ian Pirro
2022-07-21 00:29:12 -07:00
committed by GitHub
parent 53168f75ad
commit 23912e5232
4 changed files with 747 additions and 6 deletions

View File

@@ -96,12 +96,38 @@ export async function applySnapshot(
}
};
// create top level collections (no group) first, then continue with nested collections recursively
await createCollections(
snapshotDiff.collections.filter(
({ diff }) => diff[0].kind === 'N' && (diff[0] as DiffNew<Collection>).rhs.meta?.group === null
)
);
// Finds all collections that need to be created
const filterCollectionsForCreation = ({ diff }: { collection: string; diff: Diff<Collection | undefined>[] }) => {
// Check new collections only
const isNewCollection = diff[0].kind === 'N';
if (!isNewCollection) return false;
// Create now if no group
const groupName = (diff[0] as DiffNew<Collection>).rhs.meta?.group;
if (!groupName) return true;
// Check if parent collection already exists in schema
const parentExists = current.collections.find((c) => c.collection === groupName) !== undefined;
// If this is a new collection and the parent collection doesn't exist in current schema ->
// Check if the parent collection will be created as part of applying this snapshot ->
// If yes -> this collection will be created recursively
// If not -> create now
// (ex.)
// TopLevelCollection - I exist in current schema
// NestedCollection - I exist in snapshotDiff as a new collection
// TheCurrentCollectionInIteration - I exist in snapshotDiff as a new collection but will be created as part of NestedCollection
const parentWillBeCreatedInThisApply =
snapshotDiff.collections.filter(({ collection, diff }) => diff[0].kind === 'N' && collection === groupName)
.length > 0;
// Has group, but parent is not new, parent is also not being created in this snapshot apply
if (parentExists && !parentWillBeCreatedInThisApply) return true;
return false;
};
// Create top level collections (no group, or highest level in existing group) first,
// then continue with nested collections recursively
await createCollections(snapshotDiff.collections.filter(filterCollectionsForCreation));
// delete top level collections (no group) first, then continue with nested collections recursively
await deleteCollections(

View File

@@ -273,3 +273,33 @@ export const userSchema = {
},
] as Relation[],
};
export const snapshotApplyTestSchema = {
collections: {
test_table: {
collection: 'test_table',
primary: 'id',
singleton: false,
note: 'test_table',
sortField: null,
accountability: 'all',
fields: {
id: {
field: 'id',
defaultValue: null,
nullable: false,
generated: false,
type: 'uuid',
dbType: 'uuid',
precision: null,
scale: null,
special: [],
note: null,
alias: false,
validation: null,
},
},
},
} as CollectionsOverview,
relations: [] as Relation[],
};

View File

@@ -0,0 +1,415 @@
import { Snapshot, SnapshotField } from '../../src/types';
export const testSnapshotBefore: Snapshot = {
version: 1,
directus: '0.0.0',
collections: [
{
collection: 'test_table',
meta: {
accountability: 'all',
collection: 'test_table',
group: null,
hidden: true,
icon: 'import_export',
item_duplication_fields: null,
note: null,
singleton: false,
translations: {},
},
schema: {
comment: null,
name: 'test_table',
schema: 'public',
},
},
],
fields: [
{
collection: 'test_table',
field: 'id',
meta: {
collection: 'test_table',
conditions: null,
display: null,
display_options: null,
field: 'id',
group: null,
hidden: true,
interface: null,
note: null,
options: null,
readonly: false,
required: false,
sort: null,
special: null,
translations: {},
validation: null,
validation_message: null,
width: 'full',
},
schema: {
comment: null,
data_type: 'uuid',
default_value: null,
foreign_key_column: null,
foreign_key_schema: null,
foreign_key_table: null,
generation_expression: null,
has_auto_increment: false,
is_generated: false,
is_nullable: false,
is_primary_key: true,
is_unique: true,
max_length: null,
name: 'id',
numeric_precision: null,
numeric_scale: null,
schema: 'public',
table: 'test_table',
},
type: 'uuid',
} as SnapshotField,
],
relations: [],
};
export const testSnapshotToApply: Snapshot = {
version: 1,
directus: '0.0.0',
collections: [
{
collection: 'test_table',
meta: {
accountability: 'all',
collection: 'test_table',
group: null,
hidden: true,
icon: 'import_export',
item_duplication_fields: null,
note: null,
singleton: false,
translations: {},
},
schema: {
comment: null,
name: 'test_table',
schema: 'public',
},
},
{
collection: 'test_table_2',
meta: {
accountability: 'all',
collection: 'test_table_2',
group: 'test_table',
hidden: true,
icon: 'import_export',
item_duplication_fields: null,
note: null,
singleton: false,
translations: {},
},
schema: {
comment: null,
name: 'test_table_2',
schema: 'public',
},
},
{
collection: 'test_table_3',
meta: {
accountability: 'all',
collection: 'test_table_3',
group: 'test_table_2',
hidden: true,
icon: 'import_export',
item_duplication_fields: null,
note: null,
singleton: false,
translations: {},
},
schema: {
comment: null,
name: 'test_table_3',
schema: 'public',
},
},
],
fields: [
{
collection: 'test_table',
field: 'id',
meta: {
collection: 'test_table',
conditions: null,
display: null,
display_options: null,
field: 'id',
group: null,
hidden: true,
interface: null,
note: null,
options: null,
readonly: false,
required: false,
sort: null,
special: null,
translations: {},
validation: null,
validation_message: null,
width: 'full',
},
schema: {
comment: null,
data_type: 'uuid',
default_value: null,
foreign_key_column: null,
foreign_key_schema: null,
foreign_key_table: null,
generation_expression: null,
has_auto_increment: false,
is_generated: false,
is_nullable: false,
is_primary_key: true,
is_unique: true,
max_length: null,
name: 'id',
numeric_precision: null,
numeric_scale: null,
schema: 'public',
table: 'test_table',
},
type: 'uuid',
} as SnapshotField,
{
collection: 'test_table_2',
field: 'id',
meta: {
collection: 'test_table_2',
conditions: null,
display: null,
display_options: null,
field: 'id',
group: null,
hidden: true,
interface: null,
note: null,
options: null,
readonly: false,
required: false,
sort: null,
special: null,
translations: {},
validation: null,
validation_message: null,
width: 'full',
},
schema: {
comment: null,
data_type: 'uuid',
default_value: null,
foreign_key_column: null,
foreign_key_schema: null,
foreign_key_table: null,
generation_expression: null,
has_auto_increment: false,
is_generated: false,
is_nullable: false,
is_primary_key: true,
is_unique: true,
max_length: null,
name: 'id',
numeric_precision: null,
numeric_scale: null,
schema: 'public',
table: 'test_table_2',
},
type: 'uuid',
} as SnapshotField,
{
collection: 'test_table_3',
field: 'id',
meta: {
collection: 'test_table_3',
conditions: null,
display: null,
display_options: null,
field: 'id',
group: null,
hidden: true,
interface: null,
note: null,
options: null,
readonly: false,
required: false,
sort: null,
special: null,
translations: {},
validation: null,
validation_message: null,
width: 'full',
},
schema: {
comment: null,
data_type: 'uuid',
default_value: null,
foreign_key_column: null,
foreign_key_schema: null,
foreign_key_table: null,
generation_expression: null,
has_auto_increment: false,
is_generated: false,
is_nullable: false,
is_primary_key: true,
is_unique: true,
max_length: null,
name: 'id',
numeric_precision: null,
numeric_scale: null,
schema: 'public',
table: 'test_table_3',
},
type: 'uuid',
} as SnapshotField,
],
relations: [],
};
export const testSnapshotToApplyNotNested: Snapshot = {
version: 1,
directus: '0.0.0',
collections: [
{
collection: 'test_table',
meta: {
accountability: 'all',
collection: 'test_table',
group: null,
hidden: true,
icon: 'import_export',
item_duplication_fields: null,
note: null,
singleton: false,
translations: {},
},
schema: {
comment: null,
name: 'test_table',
schema: 'public',
},
},
{
collection: 'test_table_2',
meta: {
accountability: 'all',
collection: 'test_table_2',
group: null,
hidden: true,
icon: 'import_export',
item_duplication_fields: null,
note: null,
singleton: false,
translations: {},
},
schema: {
comment: null,
name: 'test_table_2',
schema: 'public',
},
},
],
fields: [
{
collection: 'test_table',
field: 'id',
meta: {
collection: 'test_table',
conditions: null,
display: null,
display_options: null,
field: 'id',
group: null,
hidden: true,
interface: null,
note: null,
options: null,
readonly: false,
required: false,
sort: null,
special: null,
translations: {},
validation: null,
validation_message: null,
width: 'full',
},
schema: {
comment: null,
data_type: 'uuid',
default_value: null,
foreign_key_column: null,
foreign_key_schema: null,
foreign_key_table: null,
generation_expression: null,
has_auto_increment: false,
is_generated: false,
is_nullable: false,
is_primary_key: true,
is_unique: true,
max_length: null,
name: 'id',
numeric_precision: null,
numeric_scale: null,
schema: 'public',
table: 'test_table',
},
type: 'uuid',
} as SnapshotField,
{
collection: 'test_table_2',
field: 'id',
meta: {
collection: 'test_table_2',
conditions: null,
display: null,
display_options: null,
field: 'id',
group: null,
hidden: true,
interface: null,
note: null,
options: null,
readonly: false,
required: false,
sort: null,
special: null,
translations: {},
validation: null,
validation_message: null,
width: 'full',
},
schema: {
comment: null,
data_type: 'uuid',
default_value: null,
foreign_key_column: null,
foreign_key_schema: null,
foreign_key_table: null,
generation_expression: null,
has_auto_increment: false,
is_generated: false,
is_nullable: false,
is_primary_key: true,
is_unique: true,
max_length: null,
name: 'id',
numeric_precision: null,
numeric_scale: null,
schema: 'public',
table: 'test_table_2',
},
type: 'uuid',
} as SnapshotField,
],
relations: [],
};

View File

@@ -0,0 +1,270 @@
import knex, { Knex } from 'knex';
import { getTracker, MockClient, Tracker } from 'knex-mock-client';
import { snapshotApplyTestSchema } from '../__test-utils__/schemas';
import { applySnapshot } from '../../src/utils/apply-snapshot';
import { testSnapshotToApply, testSnapshotBefore, testSnapshotToApplyNotNested } from '../__test-utils__/snapshots';
import { CollectionsService, FieldsService } from '../../src/services';
import * as getSchema from '../../src/utils/get-schema';
jest.mock('../../src/database/index', () => {
return {
getDatabaseClient: jest.fn().mockReturnValue('postgres'),
};
});
jest.requireMock('../../src/database/index');
class Client_PG extends MockClient {}
describe('applySnapshot', () => {
let db: jest.Mocked<Knex>;
let tracker: Tracker;
beforeEach(() => {
db = knex({ client: Client_PG }) as jest.Mocked<Knex>;
tracker = getTracker();
});
afterEach(() => {
tracker.reset();
jest.clearAllMocks();
});
describe('Creating new collection(s)', () => {
it('Creates new top-level collection(s)', async () => {
const expected = {
collection: 'test_table_2',
meta: {
accountability: 'all',
collection: 'test_table_2',
group: null,
hidden: true,
icon: 'import_export',
item_duplication_fields: null,
note: null,
singleton: false,
translations: {},
},
schema: { comment: null, name: 'test_table_2', schema: 'public' },
fields: [
{
collection: 'test_table_2',
field: 'id',
meta: {
collection: 'test_table_2',
conditions: null,
display: null,
display_options: null,
field: 'id',
group: null,
hidden: true,
interface: null,
note: null,
options: null,
readonly: false,
required: false,
sort: null,
special: null,
translations: {},
validation: null,
validation_message: null,
width: 'full',
},
schema: {
comment: null,
data_type: 'uuid',
default_value: null,
foreign_key_column: null,
foreign_key_schema: null,
foreign_key_table: null,
generation_expression: null,
has_auto_increment: false,
is_generated: false,
is_nullable: false,
is_primary_key: true,
is_unique: true,
max_length: null,
name: 'id',
numeric_precision: null,
numeric_scale: null,
schema: 'public',
table: 'test_table_2',
},
type: 'uuid',
},
],
};
// Stop call to db later on in apply-snapshot
jest.spyOn(getSchema, 'getSchema').mockReturnValue(Promise.resolve(snapshotApplyTestSchema));
// We are not actually testing that createOne works, just that is is called correctly
const createOneCollectionSpy = jest
.spyOn(CollectionsService.prototype, 'createOne')
.mockImplementation(jest.fn());
const createFieldSpy = jest.spyOn(FieldsService.prototype, 'createField').mockImplementation(jest.fn());
await applySnapshot(testSnapshotToApplyNotNested, {
database: db,
current: testSnapshotBefore,
schema: snapshotApplyTestSchema,
});
expect(createOneCollectionSpy).toHaveBeenCalledTimes(1);
expect(createOneCollectionSpy).toHaveBeenCalledWith(expected);
// There should be no fields left to create
// they will get filtered in createCollections
expect(createFieldSpy).toHaveBeenCalledTimes(0);
});
it('Creates the highest-level nested collection(s) with existing parents and any children', async () => {
const expected = {
collection: 'test_table_2',
meta: {
accountability: 'all',
collection: 'test_table_2',
group: 'test_table',
hidden: true,
icon: 'import_export',
item_duplication_fields: null,
note: null,
singleton: false,
translations: {},
},
schema: { comment: null, name: 'test_table_2', schema: 'public' },
fields: [
{
collection: 'test_table_2',
field: 'id',
meta: {
collection: 'test_table_2',
conditions: null,
display: null,
display_options: null,
field: 'id',
group: null,
hidden: true,
interface: null,
note: null,
options: null,
readonly: false,
required: false,
sort: null,
special: null,
translations: {},
validation: null,
validation_message: null,
width: 'full',
},
schema: {
comment: null,
data_type: 'uuid',
default_value: null,
foreign_key_column: null,
foreign_key_schema: null,
foreign_key_table: null,
generation_expression: null,
has_auto_increment: false,
is_generated: false,
is_nullable: false,
is_primary_key: true,
is_unique: true,
max_length: null,
name: 'id',
numeric_precision: null,
numeric_scale: null,
schema: 'public',
table: 'test_table_2',
},
type: 'uuid',
},
],
};
const expected2 = {
collection: 'test_table_3',
fields: [
{
collection: 'test_table_3',
field: 'id',
meta: {
collection: 'test_table_3',
conditions: null,
display: null,
display_options: null,
field: 'id',
group: null,
hidden: true,
interface: null,
note: null,
options: null,
readonly: false,
required: false,
sort: null,
special: null,
translations: {},
validation: null,
validation_message: null,
width: 'full',
},
schema: {
comment: null,
data_type: 'uuid',
default_value: null,
foreign_key_column: null,
foreign_key_schema: null,
foreign_key_table: null,
generation_expression: null,
has_auto_increment: false,
is_generated: false,
is_nullable: false,
is_primary_key: true,
is_unique: true,
max_length: null,
name: 'id',
numeric_precision: null,
numeric_scale: null,
schema: 'public',
table: 'test_table_3',
},
type: 'uuid',
},
],
meta: {
accountability: 'all',
collection: 'test_table_3',
group: 'test_table_2',
hidden: true,
icon: 'import_export',
item_duplication_fields: null,
note: null,
singleton: false,
translations: {},
},
schema: { comment: null, name: 'test_table_3', schema: 'public' },
};
// Stop call to db later on in apply-snapshot
jest.spyOn(getSchema, 'getSchema').mockReturnValue(Promise.resolve(snapshotApplyTestSchema));
// We are not actually testing that createOne works, just that is is called correctly
const createOneCollectionSpy = jest
.spyOn(CollectionsService.prototype, 'createOne')
.mockImplementation(jest.fn());
const createFieldSpy = jest.spyOn(FieldsService.prototype, 'createField').mockImplementation(jest.fn());
await applySnapshot(testSnapshotToApply, {
database: db,
current: testSnapshotBefore,
schema: snapshotApplyTestSchema,
});
expect(createOneCollectionSpy).toHaveBeenCalledTimes(2);
expect(createOneCollectionSpy).toHaveBeenCalledWith(expected);
expect(createOneCollectionSpy).toHaveBeenCalledWith(expected2);
// There should be no fields left to create
// they will get filtered in createCollections
expect(createFieldSpy).toHaveBeenCalledTimes(0);
});
});
});