Unable To Create Strong Relations Through MySQL Connector

A-Corregidor commented 3 years ago

Additional information

Objective: As LB4 doesn't currently support setting strong foreign keys, I'm trying to set those on MySQL DB through loopback-connector-MySQL.

Theoretical Steps:

Set foreignKey config on each @model() (as explained here)
Create tables on migration in correct order to avoid setting foreignKey before the destination table is created (as explained here)

Real Steps:

Forked loopback-next repo
Create new datasource (db2), model (NotTodo), controller (NotTodoController) and repository (NotTodoRepository)
NotTodo model uses the new datasource (db2)
Add foreignKey config to NotTodo Model
Modified migrate.ts to set migration order

Problem: Above steps should be OK when working with a unique datasource. However, when having more than one datasource, there're some problems:

You can't set migration order because lb does not know which datasource is this model linked to, so it crashes
I've noticed that drop table migration could fail when rebuilding DB if some foreignKey were set. When executing a drop table migration (if models['Todo', NotTodo'] were assigned, and NotTodo had a todoId field), tables should be created in that order (first 'Todo', then 'NotTodo'), but drop table queries should be executed in reverse order (DROP TABLE IF EXISTS NotTodo; DROP TABLE IF EXISTS Todo) to avoid errors like "Cannot drop table 'Todo' referenced by a foreign key constraint 'todoId' on table 'notTodo'"

If migrateSchema() code is modified as suggested by #5769, then line 15 of migration.ts will succeed, but foreignKeys won't be created. Only 'alter' migration (npm run migrate, instead of -- --rebuild) will create the foreignKeys

Steps to reproduce

Download forked repo, change to branch "strong-relations-with-more-than-one-datasource" and make an "npm install" on the root
Go to examples/todo/src/migrate.ts and choose which migration wanna try:
- Line 14 is gonna pass, but It would fail if there were models whose foreignKey were to be assigned before their table was created
- Line 15 is gonna fail
Go to examples/todo and execute 'npm run migrate -- --rebuild' (drop table on migration) command inside example/todo

Current Behavior

Throws error: 'Cannot migrate database schema Error: Cannot migrate models not attached to this datasource: NotTodo'

Expected Behavior

Developers should be able to specify which models of which datasource should be migrated and its order, so LB4 could create the tables in correct order inside each datasource and then be able to create the foreign keys

Link to reproduction sandbox

FORK

achrinza commented 3 years ago

At a glance, I believe this would be fixed by #5769 - would you be able to confirm if this is the case?

A-Corregidor commented 3 years ago

At a glance, I believe this would be fixed by #5769 - would you be able to confirm if this is the case?

You're right, #5769 allows migration to continue instead of crashing, BUT it doesn't create the foreignKey INDEX on DB. With the same config (added to @model()), the alter table migration correctly create foreignKey.

However, I've noticed that drop table migration could fail when rebuilding DB if some foreignKey were set. When executing a drop table migration (if models['foo', 'bar'] were assigned, and bar had a fooId field), tables should be created in that order (first 'foo', then 'bar'), but drop table queries should be executed in reverse order (DROP TABLE IF EXISTS bar; DROP TABLE IF EXISTS foo) to avoid errors like "Cannot drop table 'foo' referenced by a foreign key constraint 'fooId' on table 'bar'"

Should I add this to the main question?

achrinza commented 3 years ago

Hi @A-Corregidor, apologies for the delayed reply; Yes, please do add any details that may be useful in identifying the problems and scope of the issues. This would be helpful in distinguishing between similar issues.

A-Corregidor commented 3 years ago

Hi @A-Corregidor, apologies for the delayed reply; Yes, please do add any details that may be useful in identifying the problems and scope of the issues. This would be helpful in distinguishing between similar issues.

Hi @achrinza , I've updated the main topic as we talked

dhmlau commented 3 years ago

@A-Corregidor, it'd be a feature to support strong relation, please see this epic: https://github.com/strongloop/loopback-next/issues/2331. @bajtos had done a spike in the past and created a list of to-dos in order to support it. Please continue the discussion over there. With that, I'd like to close this issue as duplicate. Thanks.

achrinza commented 3 years ago

Hi @dhmlau, I'd like to keep this issue open as it's with regards to migration "drop" mode not generating the necessary indexes, not so much about first-class strong relations support.

A-Corregidor commented 3 years ago

Hi @dhmlau, I'd like to keep this issue open as it's with regards to migration "drop" mode not generating the necessary indexes, not so much about first-class strong relations support.

Hi @achrinza , do you want me to update main topic (and the forked repo) to talk only about the migration "drop" mode?

emonddr commented 3 years ago

@A-Corregidor , there is a PR I haven't yet landed for the Todo example using mysql https://github.com/strongloop/loopback-next/pull/6642 . Perhaps it provides some extra info you may need. Particularly this file: docs/site/tutorials/todo-list/todo-list-tutorial-sqldb.md .

stale[bot] commented 3 years ago

This issue has been marked stale because it has not seen activity within six months. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository. This issue will be closed within 30 days of being stale.

stale[bot] commented 3 years ago

This issue has been closed due to continued inactivity. Thank you for your understanding. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository.

loopbackio / loopback-next