magento-hackathon / developer-paradise-2016

2 stars 3 forks source link

Proper Database Migrations #9

Open gsomoza opened 8 years ago

gsomoza commented 8 years ago

Just an idea, not sure how useful this will be or not, but here's the situation as I see it:

Problem

Magento2 offers the ability to add setup / schema upgrade scripts to plugins, but the problem is that its very simple. Several core components are doing something like this:

class UpgradeData implements UpgradeDataInterface
{
    public function upgrade(ModuleDataSetupInterface $setup, ModuleContextInterface $context)
    {
        $setup->startSetup();
        if ($context->getVersion()
            && version_compare($context->getVersion(), '2.0.1') < 0
        ) {
            // upgrade logic here
            // (dozens of lines of code)
        }

        if (version_compare($context->getVersion(), '2.0.2') < 0) {
            // more upgrade logic here
            // (dozens of lines of code)
        }

        if (version_compare($context->getVersion(), '2.0.3') < 0) {
            // so on and so forth....
        }

        // .... after a while this will become ridiculous

        $setup->endSetup();
    }
}

Problem 1: Extensions with long shelf-life will soon have huge monolithic upgrade files. Which will make them hard to maintain in the long term (even medium-term). This may or may not be an issue depending on the degree of OCD you have.

Problem 2: the never-ending version checks are not only horrible to look at, but they're also not necessarily the best way to approach upgrades: developers might run into situations where they'll have to bump their component's MINOR or PATCH numbers "just" to trigger a schema upgrade, which couples another concern (data/schema management) to the concept of semantically versioning a module, and semver wasn't really meant for juggling so many different concerns.

Problem 3: there's currently no mechanism whatsoever to enable component "uninstalls" (thanks @peterjaap).

Solution

Following up with Issue#2 above, I'd argue its just better to version data/schema separately using timestamps (as shown in the example in the next section below), which is the method that has been widely adopted in virtually every popular migration framework inside and outside PHP.

With this in mind, I think we can easily integrate with a generic migration tool that would take care of removing those "ifs" from the upgrade classes, and separating the semantic concerns of code vs database. The result would be similar to what's being done with projects like Doctrine Migrations or Phinx, where each upgrade script is contained in its own class under a timestamped name. And each of those classes can be injected with its own dependencies.

In other words, something like this:

- Module root
|-- Setup
  |-- Schema
    |-- v20160425002128_InstallSchema.php
    |-- v20160425002139_AddTable.php
  |-- Data
    |-- v20160503173952_InstallData.php
    |-- v20160603173953_AddDefaultForAdminOption.php
    |-- etc, etc, etc.

The module might even eliminate the need of defining the "UpgradeData" or "UpgradeSchema" classes, taking the migration process a step further by allowing developers to just create new migration classes for each atomic transformation they want to apply to the schema or data.

Implementation

There are several migration libraries that could be used to power the mechanism for this project, but to me there are two that stand out because they're "generic" and can be easily integrated with Magento 2:

PHP Mig has been around for a while and has been well-received among custom application developers (custom ZF2, Symfony, etc). But it has several disadvantages, among which lack of official support for PHP 7 is a big one. Another disadvantage is that it has several bugs and limitations that Baleen doesn't.

Baleen is a brand-new tool that's quite stable but hasn't committed to a v1 yet (but so neither did Composer until a couple of months ago!). Its the result of a collaboration between myself and Mike Simonson, the maintainer of the wildly popular Doctrine Migrations project. Baleen is incredibly flexible, and has almost none of the "bigger" limitations of other current migration frameworks. It also has 100% unit test coverage for the core mechanisms and a decent amount of coverage for everything else. But since its brand-new, its bound to have bugs that we're not aware of yet. Having said that: they should be easily fixable because things are very flexibly designed in Baleen.

As the main author Baleen I'm of course a bit biased, but I can testify for its effectiveness because I already used it with a client's project to create a migration tool for Pimcore (a less-known framework), and was able to create a fully-featured migration tool within a few hours of work.

It will take longer for a single developer to do this with Magento though, but as a team we might be able to get something ready within the time-constraints of the hackathon. Fair warning: embarking on this journey will be TOUGH and a junior developer will find it incredibly challenging.

Result

The result should be an easy-to-use migration framework. Component developers should be able to use the Magento CLI tool to create new migration files for their modules (e.g. by specifying module name). Each of those files should be instantiated through the Object Manager in order to benefit from its constructor injection capabilities.

philwinkle commented 8 years ago

The pleasure of working with a system like Baleen should be in the tooling provided by the framework. If there is no tooling (like Laravel's artisan provides) to create migration scripts for a model/module then it really doesn't matter what "system" we use - it still requires us to have to manually create files, rev versions.

I would lend a 👍 to the suggestion that we could use an already-existing package to accomplish this, and possibly MIG, but whatever the tool is that we use it has to be integrated fully with bin/magento.

gsomoza commented 8 years ago

Phil! Are you at the conference? Thanks for weighing in on this.

Whatever we choose, you're right: end-users would never have to even know which library is powering the migrations mechanism, they'll just issue commands like bin/magento migrations:create and bin/magento migrations:migrate

If anyone knows another framework that could be interesting we should definitely consider that as well.

peterjaap commented 8 years ago

Also; uninstall mechanism.

philwinkle commented 8 years ago

@gsomoza no I'm not. But this is a hot button issue for my team right now. We're debating whether or not to roll our own because the current iteration is not acceptable for large builds.

gsomoza commented 8 years ago

Yes, it is. Good progress today: we created this PR (still needs a fix for a test. UPDATE: not anymore, just waiting for Travis to get fixed) cause otherwise it was impossible to hook into the ./bin/magento setup:upgrade process (for example).

IvanChepurnyi commented 8 years ago

Why not to join this project? https://github.com/EcomDev/magento-migrations/blob/develop/docs/USAGE.md

gsomoza commented 8 years ago

We didn't know about it back then (even though we did search for something like this). Is it working? Do you plan on hooking to Magento's setup to integrate this?

IvanChepurnyi commented 8 years ago

It is an RFC for public API.

gsomoza commented 8 years ago

Oh, I see. One of the things we discussed during the hackathon was how exactly to hook up to Magento's application life-cycle. That's how we ended up creating that PR.

Do you have any thoughts / alternative ideas on how to do that at this point or is it too early to discuss that?

IvanChepurnyi commented 8 years ago

There is a need to add PR to core itself with separation of setup via separate command by utilizing core api.

gsomoza commented 8 years ago

Yes, I think the PR that we submitted could help with that part - although it could probably use your review and insights!