Skip to content

Commit

Permalink
fix(database): relation issues (#517)
Browse files Browse the repository at this point in the history
* refactor(database): modify relation filter queries for 1:1 1:n relations
fix(database): relation sync issues when there are missing models
feat(database): add synced bool in sql schemas

* fix(database): parser not correctly renaming relation fields
fix(database): many-to-many relations not syncing properly
fix(database): create objects that include relations was failing due to null constraints

* [CodeFactor] Apply fixes

---------

Co-authored-by: codefactor-io <support@codefactor.io>
  • Loading branch information
kkopanidis and code-factor committed Feb 15, 2023
1 parent c2ee7f5 commit e8f1cdc
Show file tree
Hide file tree
Showing 5 changed files with 86 additions and 45 deletions.
28 changes: 23 additions & 5 deletions modules/database/src/adapters/sequelize-adapter/SequelizeSchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export abstract class SequelizeSchema implements SchemaAdapter<ModelStatic<any>>
model: ModelStatic<any>;
fieldHash: string;
excludedFields: string[];
synced: boolean;
readonly idField;

protected constructor(
Expand Down Expand Up @@ -176,7 +177,7 @@ export abstract class SequelizeSchema implements SchemaAdapter<ModelStatic<any>>
if (this.associations) {
assocs = extractAssociationsFromObject(parsedQuery, this.associations);
}
const relationObjects = this.extractManyRelationsModification(parsedQuery);
const relationObjectsArray = this.extractManyRelationsModification(parsedQuery);
const t = await this.sequelize.transaction({ type: Transaction.TYPES.IMMEDIATE });
return this.model
.bulkCreate(parsedQuery, {
Expand All @@ -186,7 +187,7 @@ export abstract class SequelizeSchema implements SchemaAdapter<ModelStatic<any>>
.then(docs => {
return Promise.all(
docs.map((doc, index) =>
this.createWithPopulation(doc, relationObjects[index], t),
this.createWithPopulation(doc, relationObjectsArray[index], t),
),
);
})
Expand Down Expand Up @@ -233,6 +234,18 @@ export abstract class SequelizeSchema implements SchemaAdapter<ModelStatic<any>>
}
}
}
for (const relation in this.extractedRelations) {
if (!this.extractedRelations.hasOwnProperty(relation)) continue;
const value = this.extractedRelations[relation];
// many-to-many relations cannot be null
if (!Array.isArray(value)) continue;
const item = value[0];
const name = this.model.name + '_' + item.originalSchema.name;
promiseChain = promiseChain.then(() =>
this.sequelize.models[name].sync({ alter: { drop: false } }),
);
}
promiseChain = promiseChain.then(() => (this.synced = true));
return promiseChain;
}

Expand Down Expand Up @@ -340,9 +353,14 @@ export abstract class SequelizeSchema implements SchemaAdapter<ModelStatic<any>>
for (const target in parsedQuery) {
if (!parsedQuery.hasOwnProperty(target)) continue;
if (this.extractedRelations.hasOwnProperty(target)) {
// @ts-ignore
relationObjects[target] = parsedQuery[target];
delete parsedQuery[target];
if (Array.isArray(parsedQuery[target])) {
// @ts-ignore
relationObjects[target] = parsedQuery[target];
delete parsedQuery[target];
} else {
parsedQuery[target + 'Id'] = parsedQuery[target];
delete parsedQuery[target];
}
}
}
return relationObjects;
Expand Down
17 changes: 13 additions & 4 deletions modules/database/src/adapters/sequelize-adapter/parser/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -199,10 +199,15 @@ function handleRelation(
) {
const relationKey = key.indexOf('.') !== -1 ? key.split('.')[0] : key;
if (relations && relations[relationKey]) {
if (requiredRelations.indexOf(key) === -1) {
requiredRelations.push(key);
// many-to-many relations and querying of fields other than id
if (Array.isArray(relations[key]) || key.indexOf('.') !== -1) {
if (requiredRelations.indexOf(key) === -1) {
requiredRelations.push(key);
}
return { [`$${key}${key.indexOf('.') !== -1 ? '' : '._id'}$`]: value };
} else {
return { [`${key}Id`]: value };
}
return { [`$${key}${key.indexOf('.') !== -1 ? '' : '._id'}$`]: value };
}
}

Expand Down Expand Up @@ -286,6 +291,7 @@ function parseSelect(
if (!Array.isArray(relations[tmp])) {
// @ts-ignore
include.push([tmp + 'Id', tmp]);
exclude.push(tmp + 'Id');
} else {
include.push(tmp);
}
Expand Down Expand Up @@ -330,19 +336,22 @@ function parseSelect(
export function renameRelations(
population: string[],
relations: { [key: string]: SequelizeSchema | SequelizeSchema[] },
): { include: string[] } {
): { include: string[]; exclude: string[] } {
const include: string[] = [];
const exclude: string[] = [];

for (const relation in relations) {
if (population.indexOf(relation) !== -1) continue;
if (!Array.isArray(relations[relation])) {
// @ts-ignore
include.push([relation + 'Id', relation]);
exclude.push(relation + 'Id');
}
}

return {
include,
exclude,
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ export class PostgresAdapter extends SequelizeAdapter<PostgresSchema> {
const rel = Array.isArray(extractedRelations[relation])
? (extractedRelations[relation] as any[])[0]
: extractedRelations[relation];
if (!this.models[rel.model]) {
if (!this.models[rel.model] || !this.models[rel.model].synced) {
if (!pendingModels.includes(rel.model)) {
pendingModels.push(rel.model);
}
Expand All @@ -87,7 +87,7 @@ export class PostgresAdapter extends SequelizeAdapter<PostgresSchema> {
while (pendingModels.length > 0) {
await sleep(500);
pendingModels = pendingModels.filter(model => {
if (!this.models[model]) {
if (!this.models[model] || !this.models[model].synced) {
return true;
} else {
for (const schema in relatedSchemas) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ export class SQLAdapter extends SequelizeAdapter<SQLSchema> {
const rel = Array.isArray(extractedRelations[relation])
? (extractedRelations[relation] as any[])[0]
: extractedRelations[relation];
if (!this.models[rel.model]) {
if (!this.models[rel.model] || !this.models[rel.model].synced) {
if (!pendingModels.includes(rel.model)) {
pendingModels.push(rel.model);
}
Expand All @@ -113,7 +113,7 @@ export class SQLAdapter extends SequelizeAdapter<SQLSchema> {
while (pendingModels.length > 0) {
await sleep(500);
pendingModels = pendingModels.filter(model => {
if (!this.models[model]) {
if (!this.models[model] || !this.models[model].synced) {
return true;
} else {
for (const schema in relatedSchemas) {
Expand Down
78 changes: 46 additions & 32 deletions modules/database/src/adapters/sequelize-adapter/utils/schema.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { DataTypes, ModelStatic, Sequelize } from 'sequelize';
import { ConduitSchema, Indexable } from '@conduitplatform/grpc-sdk';
import { SequelizeSchema } from '../SequelizeSchema';
import assert from 'assert';

const deepdash = require('deepdash/standalone');

Expand All @@ -14,42 +13,57 @@ export const extractRelations = (
for (const relation in relations) {
if (relations.hasOwnProperty(relation)) {
const value = relations[relation];
// many-to-many relations cannot be null
if (Array.isArray(value)) {
const item = value[0];
model.belongsToMany(item.model, {
foreignKey: item.originalSchema.name,
// foreignKey: {
// name: item.originalSchema.name,
// allowNull: !((originalSchema.fields[relation] as any[])[0] as any).required,
// defaultValue: ((originalSchema.fields[relation] as any[])[0] as any).default,
// },
as: relation,
onUpdate: 'CASCADE',
onDelete: 'CASCADE',
through: model.name + '_' + item.originalSchema.name,
});
item.model.belongsToMany(model, {
foreignKey: name,
// foreignKey: {
// name,
// allowNull: !((originalSchema.fields[relation] as any[])[0] as any).required,
// defaultValue: ((originalSchema.fields[relation] as any[])[0] as any).default,
// },
as: relation,
through: model.name + '_' + item.originalSchema.name,
});
item.sync();
if (
item.model.associations[relation] &&
item.model.associations[relation].foreignKey === name
) {
model.belongsToMany(item.model, {
foreignKey: item.originalSchema.name,
as: relation,
onUpdate: 'CASCADE',
onDelete: 'SET NULL',
through: model.name + '_' + item.originalSchema.name,
});
continue;
} else if (
item.model.associations[relation] &&
item.model.associations[relation].foreignKey !== name
) {
throw new Error(
`Relation ${relation} already exists on ${item.model.name} with a different foreign key`,
);
} else {
model.belongsToMany(item.model, {
foreignKey: item.originalSchema.name,
as: relation,
onUpdate: 'CASCADE',
onDelete: 'SET NULL',
through: model.name + '_' + item.originalSchema.name,
});
item.model.belongsToMany(model, {
foreignKey: name,
as: relation,
through: model.name + '_' + item.originalSchema.name,
});
item.sync();
}
} else {
model.belongsTo(value.model, {
foreignKey: relation + 'Id',
// foreignKey: {
// name: relation + 'Id',
// allowNull: !(originalSchema.fields[relation] as any).required,
// defaultValue: (originalSchema.fields[relation] as any).default,
// },
foreignKey: {
name: relation + 'Id',
allowNull: !(originalSchema.fields[relation] as any).required,
defaultValue: (originalSchema.fields[relation] as any).default,
},
as: relation,
onUpdate: 'CASCADE',
onDelete: 'CASCADE',
onUpdate: (originalSchema.fields[relation] as any).required
? 'CASCADE'
: 'NO ACTION',
onDelete: (originalSchema.fields[relation] as any).required
? 'CASCADE'
: 'SET NULL',
});
}
}
Expand Down

0 comments on commit e8f1cdc

Please sign in to comment.