Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add integration tests for /metadata + fix relation deletion #8706

Merged
merged 11 commits into from
Nov 26, 2024
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Injectable } from '@nestjs/common';
import { InjectRepository } from '@nestjs/typeorm';

import { isDefined } from 'class-validator';
import isEmpty from 'lodash.isempty';
import { Repository } from 'typeorm';

import { FieldMetadataEntity } from 'src/engine/metadata-modules/field-metadata/field-metadata.entity';
Expand All @@ -19,6 +19,7 @@ import {
} from 'src/engine/metadata-modules/workspace-migration/workspace-migration.entity';
import { WorkspaceMigrationService } from 'src/engine/metadata-modules/workspace-migration/workspace-migration.service';
import { computeObjectTargetTable } from 'src/engine/utils/compute-object-target-table.util';
import { isDefined } from 'src/utils/is-defined';

@Injectable()
export class IndexMetadataService {
Expand All @@ -43,6 +44,10 @@ export class IndexMetadataService {
(fieldMetadata) => fieldMetadata.name as string,
);

if (isEmpty(columnNames)) {
throw new Error('Column names must not be empty');
}

const indexName = `IDX_${generateDeterministicIndexName([tableName, ...columnNames])}`;

let result: IndexMetadataEntity;
Expand Down Expand Up @@ -98,6 +103,44 @@ export class IndexMetadataService {
);
}

async deleteIndexMetadata(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: deleteIndexMetadata removes the metadata record but doesn't create a migration to drop the actual database index. This will leave orphaned indexes in the database. Need to add a call to workspaceMigrationService to create a DROP INDEX migration.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my case the index is deleted by the relation deletion.
But it could be misleading for future users that deleteIndexMetadata does not actually deletes the index in db; although in most cases I expect it should be deleted by the original action (delete or rename a column, etc.).

workspaceId: string,
objectMetadata: ObjectMetadataEntity,
fieldMetadataToIndex: Partial<FieldMetadataEntity>[],
) {
const tableName = computeObjectTargetTable(objectMetadata);

const columnNames: string[] = fieldMetadataToIndex.map(
(fieldMetadata) => fieldMetadata.name as string,
);

if (isEmpty(columnNames)) {
throw new Error('Column names must not be empty');
}

const indexName = `IDX_${generateDeterministicIndexName([tableName, ...columnNames])}`;

const indexMetadata = await this.indexMetadataRepository.findOne({
where: {
name: indexName,
objectMetadataId: objectMetadata.id,
workspaceId,
},
});

if (!indexMetadata) {
throw new Error(`Index metadata with name ${indexName} not found`);
}

try {
await this.indexMetadataRepository.delete(indexMetadata.id);
} catch (error) {
throw new Error(
`Failed to delete index metadata with name ${indexName} (error: ${error.message})`,
);
}
}

async createIndexCreationMigration(
workspaceId: string,
objectMetadata: ObjectMetadataEntity,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import { computeObjectTargetTable } from 'src/engine/utils/compute-object-target
import { WorkspaceCacheStorageService } from 'src/engine/workspace-cache-storage/workspace-cache-storage.service';
import { WorkspaceMigrationRunnerService } from 'src/engine/workspace-manager/workspace-migration-runner/workspace-migration-runner.service';
import { BASE_OBJECT_STANDARD_FIELD_IDS } from 'src/engine/workspace-manager/workspace-sync-metadata/constants/standard-field-ids';
import { isDefined } from 'src/utils/is-defined';

import {
RelationMetadataEntity,
Expand Down Expand Up @@ -137,22 +138,20 @@ export class RelationMetadataService extends TypeOrmQueryService<RelationMetadat
);
}

const deletedFieldMetadata = toObjectMetadata.fields.find(
const deletedAtFieldMetadata = toObjectMetadata.fields.find(
(fieldMetadata) =>
fieldMetadata.standardId === BASE_OBJECT_STANDARD_FIELD_IDS.deletedAt,
);

if (!deletedFieldMetadata) {
throw new RelationMetadataException(
`Deleted field metadata not found`,
RelationMetadataExceptionCode.RELATION_METADATA_NOT_FOUND,
);
}
this.throwIfDeletedAtFieldMetadataNotFound(deletedAtFieldMetadata);

await this.indexMetadataService.createIndexMetadata(
relationMetadataInput.workspaceId,
toObjectMetadata,
[foreignKeyFieldMetadata, deletedFieldMetadata],
[
foreignKeyFieldMetadata,
deletedAtFieldMetadata as FieldMetadataEntity<'default'>,
],
false,
false,
);
Expand Down Expand Up @@ -441,6 +440,24 @@ export class RelationMetadataService extends TypeOrmQueryService<RelationMetadat
columnName,
);

const deletedAtFieldMetadata = await this.fieldMetadataRepository.findOneBy(
{
objectMetadataId: relationMetadata.toObjectMetadataId,
name: 'deletedAt',
},
);

this.throwIfDeletedAtFieldMetadataNotFound(deletedAtFieldMetadata);

await this.indexMetadataService.deleteIndexMetadata(
workspaceId,
relationMetadata.toObjectMetadata,
[
foreignKeyFieldMetadata,
deletedAtFieldMetadata as FieldMetadataEntity<'default'>,
],
);

await this.workspaceMigrationRunnerService.executeMigrationFromPendingMigrations(
relationMetadata.workspaceId,
);
Expand Down Expand Up @@ -554,4 +571,15 @@ export class RelationMetadataService extends TypeOrmQueryService<RelationMetadat
],
);
}

private throwIfDeletedAtFieldMetadataNotFound(
deletedAtFieldMetadata?: FieldMetadataEntity<'default'> | null,
) {
if (!isDefined(deletedAtFieldMetadata)) {
throw new RelationMetadataException(
`Deleted field metadata not found`,
RelationMetadataExceptionCode.RELATION_METADATA_NOT_FOUND,
);
}
}
}
Loading
Loading