fix(database): drop fk before dropping index

[innerdvations]

What does it do?

Changes the order of operations in syncSchema. In some databases (like mysql) it is impossible to drop an index on a foreign key unless you first drop the fk. The previous order of removal was: index -> fk -> column, I've changed it to fk -> column -> index

Why is it needed?

see above

more specifically, it also prevents the v4->v5 migration on mysql, because the long version of shortened db identifiers for fks cannot be dropped

How to test it?

NOTE: there's currently an issue with document id migrations on mysql as well preventing startup, which will prevent you testing this as a v4->v5 migration directly, but otherwise doing a v4->v5 migrations would be the best way to test this since that's what this PR is really aiming to solve

• Using mysql 8
• create your own fk index in the db that strapi won't recognize and will try to delete on startup (or try to migrate from Strapi 4 to Strapi 5)
• start strapi
  • without this PR, it will fail to start because of the error described in Migration errors between v5 beta.1 and beta.2 #20020
  • with this PR, the fk will be dropped and Strapi will start fine

Given the comments below, also check some other cases that might trigger a reverse problem. Try restarting strapi after the following cases:

• remove relation (foreign key) from a schema
• add foreign keys to a schema
• rename a relation in a schema
• change the type of a relation to something else while keeping the same name
• swap the names of two attributes, relation and non-relation (but if there's a problem confirm if it exists on v5/main or develop as well, I think there are already potentially some bugs around that area)

Manually tested:

• v4 -> v5 migration
• dropping relations in the CTB
• dropping components in the CTB
• editing relations in the CTB

with

• mysql
• sqlite
• postgres
• mariadb

Related issue(s)/PR(s)

Fixes #20020

DX-1374

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
contributor-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Apr 30, 2024 0:04am

[alexandrebodin]

I'm a bit surprised, Does this work everywhere else ? I seem to remember I had to drop the index on foreign keys before dropping them to avoid errors in the past 🤔

Otherwise code looks fine

[innerdvations]

I'm a bit surprised, Does this work everywhere else ? I seem to remember I had to drop the index on foreign keys before dropping them to avoid errors in the past 🤔

Otherwise code looks fine

Oh, that's odd, I hope there's not some dialectal difference where postgres requires it one way and mysql requires it the other. I will add some additional notes on QA on this.

[christiancp100]

QAed it and found this error renaming and changing the relations content type for the following content type. Previously was Address and changed to Country:

[innerdvations]

It turns out only mysql (and nobody else) drops the index along with the fk when you drop a fk.

So I added a check that for mysql only, we skip dropping the index if we already dropped a fk of the same name.

That may have been the error you faced a long time ago @alexandrebodin -- but I think this improvement makes it more stable, because this actually fixes an extremely rare bug (only happens when fk columns are renamed I think, and we don't offer graceful renaming yet) because mysql doesn't allow it the other way around either, where you drop the index before the fk. They must be done together.

fixed

[alexandrebodin]

Code looks good, I didn't QA

[jhoward1994]

I tested the FK dropping in mysql 8, it worked with this change upgrading from v5.0.0-beta.1 👍🏻

I did need to try starting the app twice after this change on the first start I got this error

[innerdvations]

I did need to try starting the app twice after this change on the first start I got this error

I wonder if that's an issue upgrading from 5.0.0-beta.1 instead of from 4.x. I've tried all 4 supported databases now going from 4.x -> this PR and also tried modifying and dropping relations, and didn't have any problem.