Refactor migration runner within transaction (#12941)
Modifying the data-model can sometimes fail in the middle of your operation, due to the way we handle both metadata update and schema migration separately, a field can be created while the associated column creation failed (same for object/table and such). This is also an issue because WorkspaceMigrations are then stored as FAILED can never really recovered by themselves so the schema is broken and we can't update the models anymore. This PR adds a executeMigrationFromPendingMigrationsWithinTransaction method where we can (and must) pass a queryRunner executing a transaction, which should come from the metadata services so that if anything during metadata update OR schema update fails, it rolls back everything (this also mean a workspaceMigration should never stay in a failed state now). This also fixes some issues with migration not running in the correct order due to having the same timestamp and having to do some weird logic to fix that. This is a first step and fix before working on a much more reliable solution in the upcoming weeks where we will refactor the way we interact with the data model. --------- Co-authored-by: Charles Bochet <charlesBochet@users.noreply.github.com>
This commit is contained in:
+63
-28
@@ -3,7 +3,7 @@ import { InjectRepository } from '@nestjs/typeorm';
|
||||
|
||||
import isEmpty from 'lodash.isempty';
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
import { Repository } from 'typeorm';
|
||||
import { QueryRunner, Repository } from 'typeorm';
|
||||
|
||||
import { FieldMetadataEntity } from 'src/engine/metadata-modules/field-metadata/field-metadata.entity';
|
||||
import {
|
||||
@@ -30,15 +30,25 @@ export class IndexMetadataService {
|
||||
private readonly workspaceMigrationService: WorkspaceMigrationService,
|
||||
) {}
|
||||
|
||||
async createIndexMetadata(
|
||||
workspaceId: string,
|
||||
objectMetadata: ObjectMetadataEntity,
|
||||
fieldMetadataToIndex: Partial<FieldMetadataEntity>[],
|
||||
isUnique: boolean,
|
||||
isCustom: boolean,
|
||||
indexType?: IndexType,
|
||||
indexWhereClause?: string,
|
||||
) {
|
||||
async createIndexMetadata({
|
||||
workspaceId,
|
||||
objectMetadata,
|
||||
fieldMetadataToIndex,
|
||||
isUnique,
|
||||
isCustom,
|
||||
indexType,
|
||||
indexWhereClause,
|
||||
queryRunner,
|
||||
}: {
|
||||
workspaceId: string;
|
||||
objectMetadata: ObjectMetadataEntity;
|
||||
fieldMetadataToIndex: Partial<FieldMetadataEntity>[];
|
||||
isUnique: boolean;
|
||||
isCustom: boolean;
|
||||
indexType?: IndexType;
|
||||
indexWhereClause?: string;
|
||||
queryRunner?: QueryRunner;
|
||||
}) {
|
||||
const tableName = computeObjectTargetTable(objectMetadata);
|
||||
|
||||
const columnNames: string[] = fieldMetadataToIndex.map(
|
||||
@@ -53,7 +63,11 @@ export class IndexMetadataService {
|
||||
|
||||
let result: IndexMetadataEntity;
|
||||
|
||||
const existingIndex = await this.indexMetadataRepository.findOne({
|
||||
const indexMetadataRepository = queryRunner
|
||||
? queryRunner.manager.getRepository(IndexMetadataEntity)
|
||||
: this.indexMetadataRepository;
|
||||
|
||||
const existingIndex = await indexMetadataRepository.findOne({
|
||||
where: {
|
||||
name: indexName,
|
||||
workspaceId,
|
||||
@@ -68,7 +82,7 @@ export class IndexMetadataService {
|
||||
}
|
||||
|
||||
try {
|
||||
result = await this.indexMetadataRepository.save({
|
||||
result = await indexMetadataRepository.save({
|
||||
name: indexName,
|
||||
indexFieldMetadatas: fieldMetadataToIndex.map(
|
||||
(fieldMetadata, index) => ({
|
||||
@@ -93,15 +107,15 @@ export class IndexMetadataService {
|
||||
);
|
||||
}
|
||||
|
||||
await this.createIndexCreationMigration(
|
||||
await this.createIndexCreationMigration({
|
||||
workspaceId,
|
||||
objectMetadata,
|
||||
fieldMetadataToIndex,
|
||||
isUnique,
|
||||
isCustom,
|
||||
indexType,
|
||||
indexWhereClause,
|
||||
);
|
||||
queryRunner,
|
||||
});
|
||||
}
|
||||
|
||||
async recomputeIndexMetadataForObject(
|
||||
@@ -110,8 +124,13 @@ export class IndexMetadataService {
|
||||
ObjectMetadataEntity,
|
||||
'nameSingular' | 'isCustom' | 'id'
|
||||
>,
|
||||
queryRunner?: QueryRunner,
|
||||
) {
|
||||
const indexesToRecompute = await this.indexMetadataRepository.find({
|
||||
const indexMetadataRepository = queryRunner
|
||||
? queryRunner.manager.getRepository(IndexMetadataEntity)
|
||||
: this.indexMetadataRepository;
|
||||
|
||||
const indexesToRecompute = await indexMetadataRepository.find({
|
||||
where: {
|
||||
objectMetadataId: updatedObjectMetadata.id,
|
||||
workspaceId,
|
||||
@@ -142,7 +161,7 @@ export class IndexMetadataService {
|
||||
...columnNames,
|
||||
])}`;
|
||||
|
||||
await this.indexMetadataRepository.update(index.id, {
|
||||
await indexMetadataRepository.update(index.id, {
|
||||
name: newIndexName,
|
||||
});
|
||||
|
||||
@@ -160,6 +179,7 @@ export class IndexMetadataService {
|
||||
workspaceId: string,
|
||||
objectMetadata: ObjectMetadataEntity,
|
||||
fieldMetadataToIndex: Partial<FieldMetadataEntity>[],
|
||||
queryRunner?: QueryRunner,
|
||||
) {
|
||||
const tableName = computeObjectTargetTable(objectMetadata);
|
||||
|
||||
@@ -173,7 +193,11 @@ export class IndexMetadataService {
|
||||
|
||||
const indexName = `IDX_${generateDeterministicIndexName([tableName, ...columnNames])}`;
|
||||
|
||||
const indexMetadata = await this.indexMetadataRepository.findOne({
|
||||
const indexMetadataRepository = queryRunner
|
||||
? queryRunner.manager.getRepository(IndexMetadataEntity)
|
||||
: this.indexMetadataRepository;
|
||||
|
||||
const indexMetadata = await indexMetadataRepository.findOne({
|
||||
where: {
|
||||
name: indexName,
|
||||
objectMetadataId: objectMetadata.id,
|
||||
@@ -186,7 +210,7 @@ export class IndexMetadataService {
|
||||
}
|
||||
|
||||
try {
|
||||
await this.indexMetadataRepository.delete(indexMetadata.id);
|
||||
await indexMetadataRepository.delete(indexMetadata.id);
|
||||
} catch (error) {
|
||||
throw new Error(
|
||||
`Failed to delete index metadata with name ${indexName} (error: ${error.message})`,
|
||||
@@ -194,15 +218,23 @@ export class IndexMetadataService {
|
||||
}
|
||||
}
|
||||
|
||||
async createIndexCreationMigration(
|
||||
workspaceId: string,
|
||||
objectMetadata: ObjectMetadataEntity,
|
||||
fieldMetadataToIndex: Partial<FieldMetadataEntity>[],
|
||||
isUnique: boolean,
|
||||
isCustom: boolean,
|
||||
indexType?: IndexType,
|
||||
indexWhereClause?: string,
|
||||
) {
|
||||
async createIndexCreationMigration({
|
||||
workspaceId,
|
||||
objectMetadata,
|
||||
fieldMetadataToIndex,
|
||||
isUnique,
|
||||
indexType,
|
||||
indexWhereClause,
|
||||
queryRunner,
|
||||
}: {
|
||||
workspaceId: string;
|
||||
objectMetadata: ObjectMetadataEntity;
|
||||
fieldMetadataToIndex: Partial<FieldMetadataEntity>[];
|
||||
isUnique: boolean;
|
||||
indexType?: IndexType;
|
||||
indexWhereClause?: string;
|
||||
queryRunner?: QueryRunner;
|
||||
}) {
|
||||
const tableName = computeObjectTargetTable(objectMetadata);
|
||||
|
||||
const columnNames: string[] = fieldMetadataToIndex.map(
|
||||
@@ -230,6 +262,7 @@ export class IndexMetadataService {
|
||||
generateMigrationName(`create-${objectMetadata.nameSingular}-index`),
|
||||
workspaceId,
|
||||
[migration],
|
||||
queryRunner,
|
||||
);
|
||||
}
|
||||
|
||||
@@ -244,6 +277,7 @@ export class IndexMetadataService {
|
||||
previousName: string;
|
||||
newName: string;
|
||||
}[],
|
||||
queryRunner?: QueryRunner,
|
||||
) {
|
||||
for (const recomputedIndex of recomputedIndexes) {
|
||||
const { previousName, newName, indexMetadata } = recomputedIndex;
|
||||
@@ -283,6 +317,7 @@ export class IndexMetadataService {
|
||||
generateMigrationName(`update-${objectMetadata.nameSingular}-index`),
|
||||
workspaceId,
|
||||
[migration],
|
||||
queryRunner,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user