Supporting namespaces in configuration, Migrations and Seeds.

[andrey-mokhov]

Please consider my pull request, in which I made some enhancement.
The pull request fixes following issues:

• Move migrations under the Phinx\Migration namespace #168 Move migrations under the Phinx\Migration namespace
• Namespace loading failed #756 Namespace loading failed
• BUG: The same migration name can be created in multiple directories. #1054 BUG: The same migration name can be created in multiple directories. (if add namespace in configuration)

I implemented an idea of @rquadling (view comment).
So, support of namespaces looks like this:
php like:

return [
    'paths' => [
        'migrations' => [
            '/path/to/migration/without/namespace',
            'Foo' => '/path/to/migration/Foo',
        ],
        'seeds' => [
            '/path/to/seeds/without/namespace',
            'Baz' => '/path/to/seeds/Baz',
        ]
    ],
];

yaml like:

paths:
    migrations:
      0: ./db/migrations
      Foo\Bar: ./db/FooBar
    seeds:
      0: ./db/seeds
      Foo\Bar: ./db/seeds/Foo/Bar

json like:

{
    "paths": {
        "migrations": {
            "0": "./db/migrations",
            "Foo\\Bar": "./db/FooBar"
        }
    }
}

Migrations/Seeds contain namespace just at the beginning of the file.

Migrations/Seeds files without namespace are still working for backward compatibility.

For using namespaces in migrations/seeds make new folders and change configuration.

The solution doesn't fully compatible with PSR-4 (there are versions in file names).

I tryed to covered the code with the unit's tests as full as possible.

[mvrhov]

I don't agree with namespaced migrations as this prevents me to have them in different modules.

[andrey-mokhov]

@mvrhov, current solution doesn't prevent you use migrations in different modules. Example:

return [
    'paths' => [
        'migrations' => [
            './module_first/migrations',
            './module_second/migrations',
            // etc
            './module_other/migrations',
        ],
        'seeds' => [
            // ...
        ]
    ],
];

If you need Namespace you can add an item with string key to configuration. Example:

return [
    'paths' => [
        'migrations' => [
            './module_first/migrations',
            './module_second/migrations',
            // etc
            './module_other/migrations',
            'Foo\Bar' => './module_new/migrations',
        ],
        'seeds' => [
            // ...
        ]
    ],
];

Will work both new and old (without namespace) migrations.

[mvrhov]

Oh. Sorry. I've not reviewed the code. I've just read the PR description. If the old way works then I have no objections.

[rquadling]

Supply $namespace separate to $defineNamespace please.

[andrey-mokhov]

I'm not sure I understand your proposals.

$namespace variable can contain string like Foo\Bar

$defineNamespace is a result of concatenation: namespace Foo\Bar;

When namespace for path is defined in configuration, then result string injected in template (see ./src/Phinx/Migration/Migration.template.php.dist, ./src/Phinx/Seed/Seed.template.php.dist).

If namespace for path not found, then namespace definition in template is absent ($defineNamespace var is empty).

Please clarify your sentence.

Do you mean to create separate template for migrations with namespace? I suppose it would be more convenient to leave a single template.

Or maybe you suggest rename $defineNamespace var to $namespaceDefinition?

[rquadling]

Supply $namespace separate to $defineNamespace please.

[rquadling]

Simply add '$namespace' => $namespace,

Basically, I would need to see JUST the namespace in my template. This is independent to the $defineNamespace value.

[rquadling]

And $namespaceDefinition is a nicer name, so $namespace and $namespaceDefinition. I'd have use for both of these in my templates.

[andrey-mokhov]

Renamed $defineNamespace to $namespaceDefinition, added $namespace to replace part.

[andrey-mokhov]

@rquadling Do you think, i can add $namespaceBackslash?
$namespaceBackslash like Foo\Bar\

[rquadling]

@rquadling Do you think, i can add $namespaceBackslash?

What use case?

[andrey-mokhov]

hm, i refuse the idea (namespaceBackslash)

[andrey-mokhov]

Hello! Could you please give a cue about this PR. In case of any doubts, I will be happy to offer any assistance.

[rquadling]

Just need to review it and play with it a bit.

[JapSeyz]

@rquadling Any news on whether or not this is a suitable implementation? It looks solid and good to me.

[lorenzo]

@JapSeyz it only needs docs. Would you like to contribute them?

[JapSeyz]

@lorenzo Yeah sure, I'll take a look at it next weekend when I have some time. Where should I put the PR?

Also, in which regard do you need documentation? It seems fairly straight forward to tell the user that namespaces required/enabled? or are you asking for in-depth documentation on how it resolves the path when it's namespaced?

[lorenzo]

This looks like it requires configuration in the yaml file. That should be explained, with an example of a namespaced migration file.

You can open a pull request to the docs folder in this repository.

thanks for the help!

[rquadling]

It has to work with existing migrations that won't be namespaced. If you want to start using namespaces, what's the consequences for existing migrations? Having some documentation for migrating the migrations (that's a bit self referential!) would be useful. That could be in a suitable UPGRADE file. This is assuming that namespaced and non-namespaced migrations cannot be used simultaneously.

[JapSeyz]

@rquadling Alright, I'll check it out. I'll do a bit of testing and write down my findings a long with documentation for how to use the namespaced migrations.

[JapSeyz]

@rquadling would you happen to know anything about the below issue? Is it actually an issue, because it discourages the use of namespaced and non-namespaced migrations in the same project.

It doesn't seem as if it is possible to have the same classname in different namespaces because UTIL::isUniqueMigrationClassName() only checks for the classname without paying attention to whether or not there's a namespace. - Edit found tests for this, so I'm investigating further, it may be possible anyways, and I just missed it.
Okay, from what I can find, it works fine when running the migrations because of the namespaced array key at 00c1a67#diff-ea7a57d4f5d8c75d7e158edb08f5ef73R658
But when running the Create command, it calls the UTIL::isUniqueMigrationClassName() method, which then calls getExistingMigrationClassNames(), but this time without the namespaced keys, meaning that classnames will clash and make it return false.
(Unless the full namespaced path is written in the console, this just makes it impossible to create AddusersTable migration in the global namespace AFTER one has been created in another namespace)
For now I'll make a note to write the full namespaced path in the commandline

Edit: I have not tested this theory, it's just a discovery I made while looking through the commits.

[JapSeyz]

@lorenzo I have created a draft here: https://github.com/JapSeyz/phinx/blob/master/docs/namespaces.rst
Is that explanative enough, or does it need something more?

In truth, it's a very easy thing to switch to namespaces. The above comment does however highlight a potential trap for users who want's to use namespaced and non-namespaced migrations/seeders in the same project. This has been written into the documentation though.

I have not yet tested this, but I presume you shouldn't escape the backslash in the console command?
$ phinx create Foo\MyNewMigration

[andrey-mokhov]

Creating migration with command $ phinx create Foo\MyNewMigration must return an error. create command accepts argument --path for defining valid namespace for migration as path.

In case if configuration file defines several paths to directories where migrations stored, e.g.:

paths:
    migrations:
      0: ./db/migrations
      Foo\Bar: ./db/FooBar

command $ phinx create MyNewMigration cause script to ask user for valid path for migration to store. If user chose first option (no namespaces required) - command will create migration file in default location, otherwise migration file will be created with namespace in specified location.

Calling $ phinx create MyNewMigration --path ./db/FooBar would lead to creation of migration, without additional prompt, with namespace.

[lorenzo]

@JapSeyz I think the document is good, but misses some examples. When you define the paths for migrations, can you show an example of a file with its path that would be resolved by each of the configuration entries?

[JapSeyz]

@andrey-mokhov Perfect. The namespace thing did bother me. I'll reflect this in the documentation.

@lorenzo I'll add some more examples.

[JapSeyz]

I have added more examples and reflected the --path flag.

Let me know if it's acceptable, as I've found it really difficult to give examples of the path resolving as it's basically just saying what the different parts of the string mean.

[lorenzo]

@JapSeyz It looks great! I'll be merging this soon

[JapSeyz]

@lorenzo Sounds great! I've created a PR for the documentation so it's simply to merge it. Thanks for the quick responses

[lorenzo]

Thanks!

[ghost]

Is this going any further seen as the docs were merged?

I really need this as I have a lot of Seeds for different testing scenarios, and namespaces would organise them somewhat.