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.

Sign up for GitHub

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

Fixed migrations on clean / prod db schema #4591

Merged
merged 3 commits into from
Oct 7, 2024
Merged

Fixed migrations on clean / prod db schema #4591

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
aaed152
Fixed migrations on clean / prod db schema
valentinyanakiev Oct 4, 2024
a2e3300
Merge branch 'develop' into fix-migration-generate
valentinyanakiev Oct 4, 2024
23cfb52
Merge branch 'develop' into fix-migration-generate
techsmyth Oct 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 51 additions & 22 deletions src/migrations/1727872794564-cleanupRolesIndexes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,45 +1,74 @@
import { MigrationInterface, QueryRunner } from 'typeorm';
import { safelyDropFK } from './utils/safely-drop-foreignKey';
import { safelyDropIndex } from './utils/safely-drop-index';
import { safelyAddFK } from './utils/safely-add-foreignKey';

export class cleanupRolesIndexes1727872794564 implements MigrationInterface {
name = 'cleanupRolesIndexes1727872794564';

public async up(queryRunner: QueryRunner): Promise<void> {
// platform_invitation
// This is the new FK using the old index
await queryRunner.query(
`ALTER TABLE \`platform_invitation\` DROP FOREIGN KEY \`FK_562dce4a08bb214f08107b3631e\``
safelyDropFK(
queryRunner,
'platform_invitation',
'FK_562dce4a08bb214f08107b3631e'
);

// This is the old index that needs to be removed
await queryRunner.query(
`DROP INDEX \`FK_b3d3f3c2ce851d1059c4ed26ba2\` ON \`platform_invitation\`
`
safelyDropIndex(
queryRunner,
'platform_invitation',
'FK_b3d3f3c2ce851d1059c4ed26ba2'
);

// This will add the index and the FK back
await queryRunner.query(
`ALTER TABLE \`platform_invitation\` ADD CONSTRAINT \`FK_562dce4a08bb214f08107b3631e\` FOREIGN KEY (\`roleSetId\`) REFERENCES \`role_set\`(\`id\`) ON DELETE CASCADE ON UPDATE NO ACTION
`
safelyAddFK(
queryRunner,
'platform_invitation',
'FK_562dce4a08bb214f08107b3631e',
'roleSetId',
'role_set',
'id',
'CASCADE',
'NO ACTION'
);

// application
await queryRunner.query(
`ALTER TABLE \`application\` DROP FOREIGN KEY \`FK_8fb220ad1ac1f9c86ec39d134e4\``
);
await queryRunner.query(
`DROP INDEX \`FK_500cee6f635849f50e19c7e2b76\` ON \`application\``

safelyDropFK(queryRunner, 'application', 'FK_8fb220ad1ac1f9c86ec39d134e4');
safelyDropIndex(
queryRunner,
'application',
'FK_500cee6f635849f50e19c7e2b76'
);
await queryRunner.query(
`ALTER TABLE \`application\` ADD CONSTRAINT \`FK_8fb220ad1ac1f9c86ec39d134e4\` FOREIGN KEY (\`roleSetId\`) REFERENCES \`role_set\`(\`id\`) ON DELETE CASCADE ON UPDATE NO ACTION`
safelyAddFK(
queryRunner,
'application',
'FK_8fb220ad1ac1f9c86ec39d134e4',
'roleSetId',
'role_set',
'id',
'CASCADE',
'NO ACTION'
);

// invitation
await queryRunner.query(
`ALTER TABLE \`invitation\` DROP FOREIGN KEY \`FK_6a3b86c6db10582baae7058f5b9\``
safelyDropFK(queryRunner, 'invitation', 'FK_6a3b86c6db10582baae7058f5b9');
safelyDropIndex(
queryRunner,
'invitation',
'FK_339c1fe2a9c5caef5b982303fb0'
);
await queryRunner.query(
`DROP INDEX \`FK_339c1fe2a9c5caef5b982303fb0\` ON \`invitation\``
);
await queryRunner.query(
`ALTER TABLE \`invitation\` ADD CONSTRAINT \`FK_6a3b86c6db10582baae7058f5b9\` FOREIGN KEY (\`roleSetId\`) REFERENCES \`role_set\`(\`id\`) ON DELETE CASCADE ON UPDATE NO ACTION`
safelyAddFK(
queryRunner,
'invitation',
'FK_6a3b86c6db10582baae7058f5b9',
'roleSetId',
'role_set',
'id',
'CASCADE',
'NO ACTION'
);
}

Expand Down
103 changes: 103 additions & 0 deletions src/migrations/1728032163157-dropAllIndexesFKsRelations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
import { MigrationInterface, QueryRunner } from 'typeorm';

export class DropAllIndexesFKsRelations1728032163157
implements MigrationInterface
{
name = 'DropAllIndexesFKsRelations1728032163157';

public async up(queryRunner: QueryRunner): Promise<void> {
const dbName = 'alkemio'; // Your database name
ccanos marked this conversation as resolved.
Show resolved Hide resolved

// 1. Drop all foreign keys that can be deleted
const foreignKeysResult = await queryRunner.query(`
SELECT TABLE_NAME, CONSTRAINT_NAME
FROM information_schema.KEY_COLUMN_USAGE
WHERE CONSTRAINT_SCHEMA = '${dbName}'
AND REFERENCED_TABLE_NAME IS NOT NULL;
`);
Comment on lines +12 to +17
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Parameterize SQL queries to prevent SQL injection

Directly interpolating variables into SQL queries can lead to SQL injection vulnerabilities. Even if dbName is controlled, it's best practice to use parameterized queries.

[security]

Apply this diff to parameterize the query:

-    const foreignKeysResult = await queryRunner.query(`
-      SELECT TABLE_NAME, CONSTRAINT_NAME
-      FROM information_schema.KEY_COLUMN_USAGE
-      WHERE CONSTRAINT_SCHEMA = '${dbName}'
-        AND REFERENCED_TABLE_NAME IS NOT NULL;
-    `);
+    const foreignKeysResult = await queryRunner.query(
+      `SELECT TABLE_NAME, CONSTRAINT_NAME
+       FROM information_schema.KEY_COLUMN_USAGE
+       WHERE CONSTRAINT_SCHEMA = ?
+         AND REFERENCED_TABLE_NAME IS NOT NULL;`,
+      [dbName]
+    );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const foreignKeysResult = await queryRunner.query(`
SELECT TABLE_NAME, CONSTRAINT_NAME
FROM information_schema.KEY_COLUMN_USAGE
WHERE CONSTRAINT_SCHEMA = '${dbName}'
AND REFERENCED_TABLE_NAME IS NOT NULL;
`);
const foreignKeysResult = await queryRunner.query(
`SELECT TABLE_NAME, CONSTRAINT_NAME
FROM information_schema.KEY_COLUMN_USAGE
WHERE CONSTRAINT_SCHEMA = ?
AND REFERENCED_TABLE_NAME IS NOT NULL;`,
[dbName]
);


for (const fk of foreignKeysResult) {
try {
await queryRunner.query(
`ALTER TABLE \`${fk.TABLE_NAME}\` DROP FOREIGN KEY \`${fk.CONSTRAINT_NAME}\``
);
console.log(
`Dropped foreign key ${fk.CONSTRAINT_NAME} on table ${fk.TABLE_NAME}`
);
} catch (error) {
console.error(
`Could not drop foreign key ${fk.CONSTRAINT_NAME} on table ${fk.TABLE_NAME}:`,
error
);
continue;
}
techsmyth marked this conversation as resolved.
Show resolved Hide resolved
}

// 2. Drop all unique constraints that can be deleted
const uniqueConstraintsResult = await queryRunner.query(`
SELECT TABLE_NAME, CONSTRAINT_NAME
FROM information_schema.TABLE_CONSTRAINTS
WHERE CONSTRAINT_SCHEMA = '${dbName}'
AND CONSTRAINT_TYPE = 'UNIQUE';
`);
Comment on lines +37 to +42
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Parameterize SQL queries to prevent SQL injection

Avoid interpolating dbName directly into SQL queries. Use parameterized queries to enhance security.

[security]

Apply this diff:

-    const uniqueConstraintsResult = await queryRunner.query(`
-      SELECT TABLE_NAME, CONSTRAINT_NAME
-      FROM information_schema.TABLE_CONSTRAINTS
-      WHERE CONSTRAINT_SCHEMA = '${dbName}'
-        AND CONSTRAINT_TYPE = 'UNIQUE';
-    `);
+    const uniqueConstraintsResult = await queryRunner.query(
+      `SELECT TABLE_NAME, CONSTRAINT_NAME
+       FROM information_schema.TABLE_CONSTRAINTS
+       WHERE CONSTRAINT_SCHEMA = ?
+         AND CONSTRAINT_TYPE = 'UNIQUE';`,
+      [dbName]
+    );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const uniqueConstraintsResult = await queryRunner.query(`
SELECT TABLE_NAME, CONSTRAINT_NAME
FROM information_schema.TABLE_CONSTRAINTS
WHERE CONSTRAINT_SCHEMA = '${dbName}'
AND CONSTRAINT_TYPE = 'UNIQUE';
`);
const uniqueConstraintsResult = await queryRunner.query(
`SELECT TABLE_NAME, CONSTRAINT_NAME
FROM information_schema.TABLE_CONSTRAINTS
WHERE CONSTRAINT_SCHEMA = ?
AND CONSTRAINT_TYPE = 'UNIQUE';`,
[dbName]
);


for (const unique of uniqueConstraintsResult) {
try {
await queryRunner.query(
`ALTER TABLE \`${unique.TABLE_NAME}\` DROP INDEX \`${unique.CONSTRAINT_NAME}\``
);
console.log(
`Dropped unique constraint ${unique.CONSTRAINT_NAME} on table ${unique.TABLE_NAME}`
);
} catch (error) {
console.error(
`Could not drop unique constraint ${unique.CONSTRAINT_NAME} on table ${unique.TABLE_NAME}:`,
error
);
continue;
}
techsmyth marked this conversation as resolved.
Show resolved Hide resolved
}

// 3. Drop all regular indexes (except primary keys and indexes on AUTO_INCREMENT columns)
const indexesResult = await queryRunner.query(`
SELECT s.TABLE_NAME, s.INDEX_NAME, c.COLUMN_NAME, MAX(c.EXTRA) AS EXTRA
FROM information_schema.STATISTICS s
JOIN information_schema.COLUMNS c
ON s.TABLE_NAME = c.TABLE_NAME
AND s.TABLE_SCHEMA = c.TABLE_SCHEMA
AND s.COLUMN_NAME = c.COLUMN_NAME
WHERE s.TABLE_SCHEMA = '${dbName}'
AND s.INDEX_NAME != 'PRIMARY'
GROUP BY s.TABLE_NAME, s.INDEX_NAME, c.COLUMN_NAME
`);
techsmyth marked this conversation as resolved.
Show resolved Hide resolved

for (const index of indexesResult) {
if (index.EXTRA && index.EXTRA.includes('auto_increment')) {
techsmyth marked this conversation as resolved.
Show resolved Hide resolved
console.log(
`Skipping index ${index.INDEX_NAME} on table ${index.TABLE_NAME} (AUTO_INCREMENT column)`
);
continue;
}

// Try to drop the index
try {
await queryRunner.query(
`DROP INDEX \`${index.INDEX_NAME}\` ON \`${index.TABLE_NAME}\``
);
console.log(
`Dropped index ${index.INDEX_NAME} on table ${index.TABLE_NAME}`
);
} catch (error) {
console.error(
`Could not drop index ${index.INDEX_NAME} on table ${index.TABLE_NAME}:`,
error
);
continue;
}
Comment on lines +91 to +96
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove unnecessary continue statement in catch block

The continue statement at line 95 is unnecessary because it's at the end of the loop body.

Apply this diff:

          console.error(
            `Could not drop index ${index.INDEX_NAME} on table ${index.TABLE_NAME}:`,
            error
          );
-         continue;
        }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
console.error(
`Could not drop index ${index.INDEX_NAME} on table ${index.TABLE_NAME}:`,
error
);
continue;
}
console.error(
`Could not drop index ${index.INDEX_NAME} on table ${index.TABLE_NAME}:`,
error
);
}
🧰 Tools
🪛 Biome

[error] 95-95: Unnecessary continue statement

Unsafe fix: Delete the unnecessary continue statement

(lint/correctness/noUnnecessaryContinue)

}
}

public async down(queryRunner: QueryRunner): Promise<void> {
// Logic for down migration can be added here if needed
}
Comment on lines +100 to +102
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Implement the down method to make the migration reversible

Currently, the down method is empty, which means this migration cannot be reversed. Implementing the down method ensures that migrations are reversible and helps maintain database integrity.

Would you like assistance in implementing the down method to restore the dropped indexes, foreign keys, and unique constraints?

}
Loading