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.

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

[6.x] Fix Migrations out of order with multiple path with certain filenames #29996

Merged
merged 1 commit into from Sep 17, 2019

Conversation

@brunogaspar
Copy link
Contributor

commented Sep 14, 2019

For some reason, with the paths & files i'll show below, the migrations are returned out of order when the same migrations are in the vendor/ directory (from a package) and also when they are on database/migrations/ (if those same migrations are published using the vendor:publish command).

After some code sniffing, the culprit so far is the sortBy here.

Steps To Reproduce:

The code below is to pretty much simulate what the getMigrationFiles() method does without the last part which is not necessary here, since the response in the end will be wrong too.

So this can be placed anywhere on the migrator class.

$files = [
    // Path from a package
    "/vendor/billing/resources/migrations/2019_08_08_100000_billing_rename_table_one.php",
    "/vendor/billing/resources/migrations/2019_08_08_200000_billing_rename_table_two.php",
    "/vendor/billing/resources/migrations/2019_08_08_300000_billing_rename_table_three.php",
    "/vendor/billing/resources/migrations/2019_08_08_400000_billing_rename_table_four.php",
    "/vendor/billing/resources/migrations/2019_08_08_500000_billing_rename_table_five.php",
    "/vendor/billing/resources/migrations/2019_08_08_600000_billing_rename_table_six.php",

    "/vendor/billing/resources/migrations/2019_08_09_100000_billing_create_table_one.php",
    "/vendor/billing/resources/migrations/2019_08_09_200000_billing_create_table_two.php",
    "/vendor/billing/resources/migrations/2019_08_09_300000_billing_create_table_three.php",
    "/vendor/billing/resources/migrations/2019_08_09_400000_billing_create_table_four.php",
    "/vendor/billing/resources/migrations/2019_08_09_500000_billing_create_table_five.php",
    "/vendor/billing/resources/migrations/2019_08_09_600000_billing_create_table_six.php",
    "/vendor/billing/resources/migrations/2019_08_09_700000_billing_create_table_seven.php",
    "/vendor/billing/resources/migrations/2019_08_09_800000_billing_create_table_eight.php",
    "/vendor/billing/resources/migrations/2019_08_09_900000_billing_create_table_nine.php",

    // Path from the app
    "/database/migrations/2019_08_08_100000_billing_rename_table_one.php",
    "/database/migrations/2019_08_08_200000_billing_rename_table_two.php",
    "/database/migrations/2019_08_08_300000_billing_rename_table_three.php",
    "/database/migrations/2019_08_08_400000_billing_rename_table_four.php",
    "/database/migrations/2019_08_08_500000_billing_rename_table_five.php",
    "/database/migrations/2019_08_08_600000_billing_rename_table_six.php",

    "/database/migrations/2019_08_09_100000_billing_create_table_one.php",
    "/database/migrations/2019_08_09_200000_billing_create_table_two.php",
    "/database/migrations/2019_08_09_300000_billing_create_table_three.php",
    "/database/migrations/2019_08_09_400000_billing_create_table_four.php",
    "/database/migrations/2019_08_09_500000_billing_create_table_five.php",
    "/database/migrations/2019_08_09_600000_billing_create_table_six.php",
    "/database/migrations/2019_08_09_700000_billing_create_table_seven.php",
    "/database/migrations/2019_08_09_800000_billing_create_table_eight.php",
    "/database/migrations/2019_08_09_900000_billing_create_table_nine.php",
];

$files = collect($files)->filter()->sortBy(function ($file) {
    return $this->getMigrationName($file);
})->dd();

When this is ran, it will spit something like this:

Illuminate\Support\Collection^ {#683
  #items: array:30 [
    0 => "/vendor/billing/resources/migrations/2019_08_08_100000_billing_rename_table_one.php"
    15 => "/database/migrations/2019_08_08_100000_billing_rename_table_one.php"

    16 => "/database/migrations/2019_08_08_200000_billing_rename_table_two.php"
    1 => "/vendor/billing/resources/migrations/2019_08_08_200000_billing_rename_table_two.php"

    17 => "/database/migrations/2019_08_08_300000_billing_rename_table_three.php"
    2 => "/vendor/billing/resources/migrations/2019_08_08_300000_billing_rename_table_three.php"

    3 => "/vendor/billing/resources/migrations/2019_08_08_400000_billing_rename_table_four.php"
    18 => "/database/migrations/2019_08_08_400000_billing_rename_table_four.php"

    19 => "/database/migrations/2019_08_08_500000_billing_rename_table_five.php"
    4 => "/vendor/billing/resources/migrations/2019_08_08_500000_billing_rename_table_five.php"

    5 => "/vendor/billing/resources/migrations/2019_08_08_600000_billing_rename_table_six.php"
    20 => "/database/migrations/2019_08_08_600000_billing_rename_table_six.php"

    21 => "/database/migrations/2019_08_09_100000_billing_create_table_one.php"
    6 => "/vendor/billing/resources/migrations/2019_08_09_100000_billing_create_table_one.php"

    22 => "/database/migrations/2019_08_09_200000_billing_create_table_two.php"
    7 => "/vendor/billing/resources/migrations/2019_08_09_200000_billing_create_table_two.php"

    8 => "/vendor/billing/resources/migrations/2019_08_09_300000_billing_create_table_three.php"
    23 => "/database/migrations/2019_08_09_300000_billing_create_table_three.php"

    9 => "/vendor/billing/resources/migrations/2019_08_09_400000_billing_create_table_four.php"
    24 => "/database/migrations/2019_08_09_400000_billing_create_table_four.php"

    10 => "/vendor/billing/resources/migrations/2019_08_09_500000_billing_create_table_five.php"
    25 => "/database/migrations/2019_08_09_500000_billing_create_table_five.php"

    11 => "/vendor/billing/resources/migrations/2019_08_09_600000_billing_create_table_six.php"
    26 => "/database/migrations/2019_08_09_600000_billing_create_table_six.php"

    12 => "/vendor/billing/resources/migrations/2019_08_09_700000_billing_create_table_seven.php"
    27 => "/database/migrations/2019_08_09_700000_billing_create_table_seven.php"

    13 => "/vendor/billing/resources/migrations/2019_08_09_800000_billing_create_table_eight.php"
    28 => "/database/migrations/2019_08_09_800000_billing_create_table_eight.php"

    14 => "/vendor/billing/resources/migrations/2019_08_09_900000_billing_create_table_nine.php"
    29 => "/database/migrations/2019_08_09_900000_billing_create_table_nine.php"

  ]
}

Note: I've added a few line breaks between the files combination so it's easier to follow.

As you can see, some of the migrations are out of order starting from the index 16 and 1 as the vendor one is coming after the one from the app, which is wrong.

Note: There's a whole lot more explanation for this on #29964 too that would be difficult to add to the description here.

I've added those empty migration "stubs" to make ensure i didn't disrupt the other tests and not cause the class already defined on path exception in some cases.

Fixes: #29964

Signed-off-by: Bruno Gaspar <brunofgaspar1@gmail.com>
@brunogaspar brunogaspar changed the title [6.x] Migrations out of order with multiple path with certain filenames [6.x] Fix Migrations out of order with multiple path with certain filenames Sep 17, 2019
@taylorotwell taylorotwell merged commit 355b969 into laravel:6.x Sep 17, 2019
2 checks passed
2 checks passed
continuous-integration/styleci/pr The analysis has passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@brunogaspar brunogaspar deleted the brunogaspar:fix/29964-migrations-out-of-order branch Sep 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.