Store migrations inside bundles and collect them from installed bundles?

[watari]

Can this logic be implemented in scope of the bundle? It can be very handy in some cases.
Similar question was aroused in doctrine/DoctrineMigrationsBundle

[caciobanu]

From my point of view this is not a good idea. Letting bundles control the schema of your DB is a dangerous path to walk. As some people said in doctrine/DoctrineMigrationsBundle#152, migrations are tied to your application and specific to it.

[watari]

I know that this is dangerous and should be used with caution. I try to explain my reasons about why I propose this approach.
This approach will be very useful in modular applications that is build on reusable modules or just build in modular way. In this apps logic divided by modules that know only about its data schema and contain logic only for work with data related to this schemes. In this case idea of "migrations per bundle" will be very useful because migrations will be placed together with rest of domain logic and can be managed centralized for all apps.

[watari]

@caciobanu , can you, please, comment my explanation of approach?

[caciobanu]

I understand the need for a feature like this but the security risks are huge from my point of view.

@redthor, @trq what do you guys think ?

[redthor]

For me I think it could be good. I have thought of having this system before.

To be honest I was not able to follow the wholle argument in doctrine/DoctrineMigrationsBundle#152 very well.

For us our application is made up of many bundles. We use it to keep things separate. Having migrations per bundle makes a lot of sense. But I have not investigated the solution.

The security issue is true for bundles, and open source software in general, regardless of database access. Someone could write a bundle that sends a curl request for example. This is a good article about that sort of thing: https://hackernoon.com/im-harvesting-credit-card-numbers-and-passwords-from-your-site-here-s-how-9a8cb347c5b5

Besides, when I integrate a bundle I will use it first in development. The bundle will come with instructions. It will say 'run the migrations'. It should not be a problem to run in development, and I will know what it does.

If I stop using the bundle then the bundle README could say: "Use this mongodb syntax to remove database tables created by this bundle" or something else similar.

We can help with security by adding something like --include-bundles so that the developer has to explicitly opt in.

[caciobanu]

Security risks will still be there but I was also thinking about the behavior to be opt-in through configuration to give this feature a try. Maybe some some warnings from commands would be great :)

[redthor]

@watari - do you have an implementation in mind? Could you send in a PR?

I presume we would need a container compilation step that:

• enumerated bundles
• looked for a certain directory structure and/or certain file name (e.g. YYYYMMDDHHMMSS) in each bundle.

[watari]

@redthor, I have some ideas about implementation.
About configuration:

• each bundle should be explicitly indicated in configuration.
• migrations directory and namespace for each of bundles will be assumed as bundle namespace + Migrations/MongoDB by default but it can be configured for each bundle separately.
• migrations order is defined independently for each bundle.

Example of configs:

mongo_db_migrations:
    collection_name: "migration_versions"
    database_name: '%env(MONGODB_DB)%'
    dir_name: "%kernel.root_dir%/../src//Migrations/MongoDB"
    script_dir_name: "%kernel.root_dir%/bin"
    name: "MongoDB Migrations"
    namespace: "App\\Migrations\\MongoDB"
    bundles:
        \ImportControlBundle\ImportControlBundle: # Case when we want to override default configs
            directory: "Migrations"
            namespace: "Migrations"
        \ProductBundle\ProductBundle: # Case when we want to use default configs.
            defaults: true

About params dir_name and namespace. In this case they must points to migrations in kernel bundle.
If configs is OK for you, then I investigate more your extension and provide PR for this issue.

[nuxwin]

Good evening,

I'm also searching for that feature. The main (cons) argument in doctrine/DoctrineMigrationsBundle#152 was: Database schema belong to application, not to bundles...

Short answer
That not always true for a true modular application. Here we are talking about modulare application, not simple bundle providing libraries which can make development easier...

Long answer
We are providing a shared hosting management control panel for Linux OS which make possible to manage various services such as Apache2, Postfix, Bind9 and so on... So basically, our application compose several OPTIONAL modules (zf3 modules in our case), each of them providing a well-known application domain. For instance, we have a module Ftp which make possible to manage Ftp accounts. That module comes with its own entities (FtpUser, FtpGroup...) and must provide its own database table schema. In term of doctrine migrations, we would want be able to:

1. Up the migrations of the module being installed
2. Down the migration of the module being uninstalled
3. ...

Those task could be trigggered automatically through composer plugin or whatever...

Of course, operating in such way (modular) impose a stric policy: Migrations from a specific module MUST never do any change on a table schema provided by other modules.

Right now, it is not possible to process this way and that is a bit sad.

From my point of view, each module should be able to configure specific namespace for its migration and target them through CLI:

./vendor/bin/doctrine-module migrations:migrate --namespace modulename
...

[redthor]

@watari the config looks good but just a couple of suggestions.

Instead of \ImportControlBundle\ImportControlBundle should we use the bundle alias? E.g. import_control. That way the namespace can change and the config stays the same. I feel like using the bundle alias is a little more symfony-like too.

With the \ImportControlBundle\ImportControlBundle example you have a directory key but I presume that it would be dir_name just as with the standard config?

@nuxwin I agree that I don't think the argument against having this feature is strong enough. I can see why you would want it. @watari is suggesting (I think) that if the config file contains the bundle then the migrations bundle will process it. You are suggesting that there needs to be a command line argument for each bundle explicitly. I added that the use would need to say --include-bundles and it would just include all of them.

Perhaps we say this:

--include-bundle=<bundle.alias>   // This will run for only one bundle `alias`, repeat for multiple bundles
--include-bundles   // This will process all configured bundles

If you want to run it for two bundles:

--include-bundle=<bundle.alias1> --include-bundle=<bundle.alias2>

You cannot supply --include-bundles and --include-bundle=<bundle.alias> together, that will throw an error.

Lastly, I am working at refactoring how we create the configuration for migrations. There will be no change to the config file, just how the Configuration class is created. See here: doesntmattr/mongodb-migrations#34 and here: doesntmattr/mongodb-migrations#38

I will finish more updates the next week and then that may be a better place to start this new feature.

[watari]

@redthor , @caciobanu , I forked your extensions and modified logic for bundles support.
doesntmattrmongodb-migrations-bundle
doesntmattr/mongodb-migrations
Can you check them and say your opinion?
At the current moment only logic here. I checked it manually. If everything is OK, then I will update tests, check code against your metrics and create pull requests to original repositories. I want to know if I on the right way or not.

Shortly about changes.
mongodb-migrations repository

• There is one BC brake - new field "prefix" isadded to database migration record. This field is used by new logic in bundle extension for managing all migrations. It store bundle alias or default stub for app applications. We can ship with new release command that will fix user existing migrations.

• Firstly, I have in mind another implementation and added interfaces for Configuration entity and Configuration builder but then I changed my mind and they is unused now. We can keep them to make extending more flexible or remove them.

• Also I added param and return types in some places and declare_strict statements.

mongodb-migrations-bundle repository

• I added BundleAwareTrait to extension and move there logic related to bundles specification.

• Service container configs for bundle was changed (services.xml). Autowiring and autoconfiguration was enabled.

• I implemented options support as we agreed for migrate command and add bundle option. This option indicate that current action must be applied only to specified bundle. It is also used by generate, version, status, execute commands (by all commands that perform actions on specific bundle). If it ommited, then action will be applied to app bundle.

• For bundles support I rewrited logic of migrate command by analogy with parent. But now here is no parent call.

• For status command I now perform call of parent for each bundle. This approach allow me to reuse parent logic, but not very nice. Maybe I should rewrite it in other way?

If all will be ok, then I maybe refactor some other parts of libs code that I meet on my way.

[buffcode]

Any chance something like this will be implemented / merged? As of Symfony 4 everything should be a bundle and we have a huge customer app where business logic and data models reside in their respective bundles. Each bundle is responsible for it's own data and migration.

Including by namespace or path for security is fine and IMHO sufficient. You would need to register the extra bundles within Symfony anyway before they could be detected, so it's unlikely to happen that a 3rd party bundle suddenly ships a malicious code somewhere in it's dependencies.