Refactor: Move the migration script out of the request path - #10
Merged
Merged
Conversation
On every request the migration system tried a migration to ensure that the database is correct. However this problematic in terms of perfomance and control. As such the migration logic is now inside of an external script that has to be called outside of the main request path to migrate the SQLite database. This script can be found as bin/migrate. To ensure that the website does not work with an invalid schema the version is still checked in the request path and returns a 500 internal reponse when invalid to ensure no invalid queries.
As migrations can theoretically be incorrect, the CI should be able to see that they are not valid. As such a new script is added that checks the correctness of these migrations.
It seems like there was a small oversight and the validity check was inverted. This meant that every database version that is NOT the current, was accepted - which is not desirable.
Previously the sorting was based on lexicographic sort. However this led
to the problem that when there are migration files like:
V9.php and V_10.php
it would sort them in an incorrect order:
V10.php and V9.php
This would lead to an incorrect order of applications of the migrations
and an invalid database. As such the `natsort` function is now used.
There seems to be an incorrect error message when migrating that references an outdated class.
It seems like (It didn't occur to me from the documentation) that natsort does not change the actual order of the keys and as such does not create a list. As such PHPStan (correctly) didn't like the natsort.
It seems like the MIGRATION_NAMESPACE was only static and not a constant. This is completely valid; However this does not mean that it is semantically correct.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
On every request the migration system tried a migration to ensure that the database is correct.
However this problematic in terms of perfomance and control. As such the migration logic is now inside of an external script that has to be called outside of the main request path to migrate the SQLite database. This script can be found as bin/migrate.
To ensure that the website does not work with an invalid schema the version is still checked in the request path and returns a 500 internal reponse when invalid to ensure no invalid queries.