feat: Add dependency configuraiton for views #8240

[zaro]

Description of change

See #8240 for detailed description of the problem.

The PR adds dependsOn option to @view decorator, where dependencies can be listed. Also use these dependencies to order drop/create view correctly when generating migrations

Pull-Request Checklist

• Code is up-to-date with the master branch
• npm run lint passes with this change
• npm run test passes with this change
• This pull request links relevant issues as Fixes #0000
• There are new or updated unit tests validating the change
• Documentation has been updated to reflect this change
• The new commits follow conventions explained in CONTRIBUTING.md

[pleerock]

What if we explicitly define order of entities in entities section of the connection options:

entities: [
  ViewA,
  ViewB
]

doesn't it solve the problem?

[zaro]

@pleerock No, the order in which entities/views are listed in the configuration options is irrelevant. What defines it currently w/o this PR is actually the import order ( literally the order in which the decorators are called). For a simple cases like with two views it's easy to reorder imports and get almost correct result ( almost because even if the creation order is correct, the drop order is always incorrect as it needs to be the reverse or creation, and currently it's not reversed). But once the number of views grow, it become a game of whack a mole to reorder the imports in such a way that it will produce migration that executes at least the CREATE in the right order and the DROP statements still need to be reordered manually.

Then there is the problem where views depend on each other, we literally have the following

A uses B
B uses C
C uses D
D uses a Entity

Currently changing column on the entity will correctly trigger drop and recreate of D, but not the rest of the views, so the migration always fails because you can't drop D w/o dropping C, B, A first.
The PR solves this also.

[pleerock]

Okay... Then we might need to change current signature to factory function and design it as follows:

dependsOn: () => [ViewA]

To prevent possible circular reference errors.

[zaro]

Sure, No problem, will update the PR.

…

On Fri, Oct 22, 2021, 12:40 Umed Khudoiberdiev ***@***.***> wrote: Okay... Then we might need to change current signature to factory function and design it as follows: dependsOn: () => [ViewA] To prevent possible circular reference errors. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#8261 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABBDXUKRTF6KX5XSQK6GY3UIEWRLANCNFSM5FWPUNZQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[pleerock]

oh, I see you added "string entity name". If so, we might not need a function signature.

[pleerock]

Everything looks good, beside one small fact that code become a bit complex to review and understand, and it means worse for future maintenance. Can it be refactored to make it more simple and understandable?

[zaro]

@pleerock I added detailed comments on how dependencies are collected.
Also renamed some variables with obviously poorly chosen names :)

I hope this makes the intention more clear, because as algorithm, it's quite simple and I don't see what much can be changed. Maybe only collecting the dependencies in non recursive manner, but I don't believe it will become more readable.

[pleerock]

Okay, thank you for your efforts