Plan - Code Metrics - Inspection of "Fix reversible migrations when dropping FK and ind..." - cakephp/phinx - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — master (#1523)

by José

created 2019-04-24 18:07 UTC

Plan A

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	421
Duplicated Lines	15.2 %

Coupling/Cohesion

Components	1
Dependencies	19

Importance

Changes

Metric	Value
wmc	42
lcom	1
cbo	19
dl	64
loc	421
rs	9.0399
c	0
b	0
f	0

14 Methods

Rating	Name	Duplication	Size	Complexity
A	__construct()	0	4	1
A	createPlan()	0	9	1
A	updatesSequence()	0	9	1
A	execute()	12	12	2
A	executeInverse()	12	12	2
A	resolveConflicts()	0	40	3
A	forgetTable()	0	12	3
A	remapContraintAndIndexConflicts()	0	27	3
B	forgetDropIndex()	0	37	6
A	gatherCreates()	0	33	4
A	gatherUpdates()	10	24	5
A	gatherTableMoves()	10	20	5
A	gatherIndexes()	10	23	3
A	gatherConstraints()	10	18	3

How to fix Duplicated Code Complexity

Duplicated Code

Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.

Common duplication problems, and corresponding solutions are:

If you have the same expression in different places: Extract expression to a method
If you have the same method in different sub-classes: Extract method, and pull up field to the parent class
If you have the same code in unrelated classes: Consider extracting the code to a new class, and injecting that class

Complex Class

Tip: Before tackling complexity, make sure that you eliminate any duplication first. This often can reduce the size of classes significantly.

Complex classes like Plan often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.

Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.

While breaking up the class, it is a good idea to analyze how other classes use Plan, and based on these observations, apply Extract Interface, too.

<?php
/**
 * Phinx
 *
 * (The MIT license)
 *
 * Permission is hereby granted, free of charge, to any person obtaining a copy
 * of this software and associated * documentation files (the "Software"), to
 * deal in the Software without restriction, including without limitation the
 * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
 * sell copies of the Software, and to permit persons to whom the Software is
 * furnished to do so, subject to the following conditions:
 *
 * The above copyright notice and this permission notice shall be included in
 * all copies or substantial portions of the Software.
 *
 * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
 * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
 * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
 * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
 * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
 * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
 * IN THE SOFTWARE.
 */
namespace Phinx\Db\Plan;

use ArrayObject;
use Phinx\Db\Action\AddColumn;
use Phinx\Db\Action\AddForeignKey;
use Phinx\Db\Action\AddIndex;
use Phinx\Db\Action\ChangeColumn;
use Phinx\Db\Action\ChangeComment;
use Phinx\Db\Action\ChangePrimaryKey;
use Phinx\Db\Action\CreateTable;
use Phinx\Db\Action\DropForeignKey;
use Phinx\Db\Action\DropIndex;
use Phinx\Db\Action\DropTable;
use Phinx\Db\Action\RemoveColumn;
use Phinx\Db\Action\RenameColumn;
use Phinx\Db\Action\RenameTable;
use Phinx\Db\Adapter\AdapterInterface;
use Phinx\Db\Plan\Solver\ActionSplitter;
use Phinx\Db\Table\Table;

/**
 * A Plan takes an Intent and transforms int into a sequence of
 * instructions that can be correctly executed by an AdapterInterface.
 *
 * The main focus of Plan is to arrange the actions in the most efficient
 * way possible for the database.
 */
class Plan
{

    /**
     * List of tables to be created
     *
     * @var \Phinx\Db\Plan\NewTable[]
     */
    protected $tableCreates = [];

    /**
     * List of table updates
     *
     * @var \Phinx\Db\Plan\AlterTable[]
     */
    protected $tableUpdates = [];

    /**
     * List of table removals or renames
     *
     * @var \Phinx\Db\Plan\AlterTable[]
     */
    protected $tableMoves = [];

    /**
     * List of index additions or removals
     *
     * @var \Phinx\Db\Plan\AlterTable[]
     */
    protected $indexes = [];

    /**
     * List of constraint additions or removals
     *
     * @var \Phinx\Db\Plan\AlterTable[]
     */
    protected $constraints = [];

    /**
     * Constructor
     *
     * @param Intent $intent All the actions that should be executed
     */
    public function __construct(Intent $intent)
    {
        $this->createPlan($intent->getActions());
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
    }

    /**
     * Parses the given Intent and creates the separate steps to execute
     *
     * @param Intent $actions The actions to use for the plan
     * @return void
     */
    protected function createPlan($actions)
    {
        $this->gatherCreates($actions);
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
        $this->gatherUpdates($actions);
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
        $this->gatherTableMoves($actions);
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
        $this->gatherIndexes($actions);
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
        $this->gatherConstraints($actions);
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
        $this->resolveConflicts();
    }

    /**
     * Returns a nested list of all the steps to execute
     *
     * @return AlterTable[][]
     */
    protected function updatesSequence()
    {
        return [
            $this->tableUpdates,
            $this->constraints,
            $this->indexes,
            $this->tableMoves,
        ];
    }

    /**
     * Executes this plan using the given AdapterInterface
     *
     * @param AdapterInterface $executor The executor object for the plan
     * @return void
     */
    public function execute(AdapterInterface $executor)

    {
        foreach ($this->tableCreates as $newTable) {
            $executor->createTable($newTable->getTable(), $newTable->getColumns(), $newTable->getIndexes());
        }

        collection($this->updatesSequence())
            ->unfold()
            ->each(function ($updates) use ($executor) {
                $executor->executeActions($updates->getTable(), $updates->getActions());
            });
    }

    /**
     * Executes the inverse plan (rollback the actions) with the given AdapterInterface:w
     *
     * @param AdapterInterface $executor The executor object for the plan
     * @return void
     */
    public function executeInverse(AdapterInterface $executor)

    {
        collection(array_reverse($this->updatesSequence()))
            ->unfold()
            ->each(function ($updates) use ($executor) {
                $executor->executeActions($updates->getTable(), $updates->getActions());
            });

        foreach ($this->tableCreates as $newTable) {
            $executor->createTable($newTable->getTable(), $newTable->getColumns(), $newTable->getIndexes());
        }
    }

    /**
     * Deletes certain actions from the plan if they are found to be conflicting or redundant.
     *
     * @return void
     */
    protected function resolveConflicts()
    {
        $moveActions = collection($this->tableMoves)
            ->unfold(function ($move) {
                return $move->getActions();
            });

        foreach ($moveActions as $action) {
            if ($action instanceof DropTable) {
                $this->tableUpdates = $this->forgetTable($action->getTable(), $this->tableUpdates);
                $this->constraints = $this->forgetTable($action->getTable(), $this->constraints);
                $this->indexes = $this->forgetTable($action->getTable(), $this->indexes);
            }
        }

        $this->tableUpdates = collection($this->tableUpdates)
            // Renaming a column and then changing the renamed column is something people do,
            // but it is a conflicting action. Luckily solving the conflict can be done by moving
            // the ChangeColumn action to another AlterTable
            ->unfold(new ActionSplitter(RenameColumn::class, ChangeColumn::class, function (RenameColumn $a, ChangeColumn $b) {
                return $a->getNewName() == $b->getColumnName();
            }))
            ->toList();

        $this->constraints = collection($this->constraints)
            ->map(function (AlterTable $alter) {
                // Dropping indexes used by foreign keys is a conflict, but one we can resolve
                // if the foreign key is also scheduled to be dropped. If we can find sucha a case,
                // we force the execution of the index drop after the foreign key is dropped.
                return $this->remapContraintAndIndexConflicts($alter);
            })
            // Changing constraint properties sometimes require dropping it and then
            // creating it again with the new stuff. Unfortunately, we have already bundled
            // everything together in as few AlterTable statements as we could, so we need to
            // resolve this conflict manually
            ->unfold(new ActionSplitter(DropForeignKey::class, AddForeignKey::class, function (DropForeignKey $a, AddForeignKey $b) {
                return $a->getForeignKey()->getColumns() === $b->getForeignKey()->getColumns();
            }))
            ->toList();
    }

    /**
     * Deletes all actions related to the given table and keeps the
     * rest
     *
     * @param Table $table The table to find in the list of actions
     * @param AlterTable[] $actions The actions to transform
     * @return AlterTable[] The list of actions without actions for the given table
     */
    protected function forgetTable(Table $table, $actions)
    {
        $result = [];
        foreach ($actions as $action) {
            if ($action->getTable()->getName() === $table->getName()) {
                continue;
            }
            $result[] = $action;
        }

        return $result;
    }

    /**
     * Finds all DropForeignKey actions in an AlterTable and moves
     * all conflicting DropIndex action in `$this->indexes` into the
     * given AlterTable.
     *
     * @param AlterTable $alter The collection of actions to inspect
     * @return AlterTable The updated AlterTable object. This function
     * has the side effect of changing the `$this->indexes` property.
     */
    protected function remapContraintAndIndexConflicts(AlterTable $alter)
    {
        $newAlter = new AlterTable($alter->getTable());
        collection($alter->getActions())
            ->unfold(function ($action) {
                if (!$action instanceof DropForeignKey) {
                    return [$action];
                }

                list($this->indexes, $dropIndexActions) = $this->forgetDropIndex(
                    $action->getTable(),
                    $action->getForeignKey()->getColumns(),
                    $this->indexes
                );

                if (!empty($dropIndexActions)) {
                    return array_merge([$action], $dropIndexActions);
                }

                return [$action];
            })
            ->each(function ($action) use ($newAlter) {
                $newAlter->addAction($action);
            });

        return $newAlter;
    }

    /**
     * Deletes any DropIndex actions for the given table and exact columns
     *
     * @param Table $table The table to find in the list of actions
     * @param string[] $columns The column names to match
     * @param AlterTable[] $actions The actions to transform
     * @return array A tuple containing the list of actions without actions for dropping the index
     * and a list of drop index actions that were removed.
     */
    protected function forgetDropIndex(Table $table, array $columns, array $actions)
    {
        $dropIndexActions = new ArrayObject();
        $indexes = collection($actions)
            ->map(function ($alter) use ($table, $columns, $dropIndexActions) {
                if ($alter->getTable()->getName() !== $table->getName()) {
                    return $alter;
                }

                $newAlter = new AlterTable($table);
                collection($alter->getActions())
                    ->map(function ($action) use ($columns) {
                        if (!$action instanceof DropIndex) {
                            return [$action, null];
                        }
                        if ($action->getIndex()->getColumns() === $columns) {
                            return [null, $action];
                        }

                        return [$action, null];
                    })
                    ->each(function ($tuple) use ($newAlter, $dropIndexActions) {
                        list($action, $dropIndex) = $tuple;
                        if ($action) {
                            $newAlter->addAction($action);
                        }
                        if ($dropIndex) {
                            $dropIndexActions->append($dropIndex);
                        }
                    });

                return $newAlter;
            })
            ->toArray();

        return [$indexes, $dropIndexActions->getArrayCopy()];
    }

    /**
     * Collects all table creation actions from the given intent
     *
     * @param \Phinx\Db\Action\Action[] $actions The actions to parse
     * @return void
     */
    protected function gatherCreates($actions)
    {
        collection($actions)
            ->filter(function ($action) {
                return $action instanceof CreateTable;
            })
            ->map(function ($action) {
                return [$action->getTable()->getName(), new NewTable($action->getTable())];
            })
            ->each(function ($step) {
                $this->tableCreates[$step[0]] = $step[1];
            });

        collection($actions)
            ->filter(function ($action) {
                return $action instanceof AddColumn
                    || $action instanceof AddIndex;
            })
            ->filter(function ($action) {
                return isset($this->tableCreates[$action->getTable()->getName()]);
            })
            ->each(function ($action) {
                $table = $action->getTable();

                if ($action instanceof AddColumn) {
                    $this->tableCreates[$table->getName()]->addColumn($action->getColumn());
                }

                if ($action instanceof AddIndex) {
                    $this->tableCreates[$table->getName()]->addIndex($action->getIndex());
                }
            });
    }

    /**
     * Collects all alter table actions from the given intent
     *
     * @param \Phinx\Db\Action\Action[] $actions The actions to parse
     * @return void
     */
    protected function gatherUpdates($actions)
    {
        collection($actions)
            ->filter(function ($action) {
                return $action instanceof AddColumn
                    || $action instanceof ChangeColumn
                    || $action instanceof RemoveColumn
                    || $action instanceof RenameColumn;
            })
            // We are only concerned with table changes
            ->reject(function ($action) {
                return isset($this->tableCreates[$action->getTable()->getName()]);
            })
            ->each(function ($action) {

                $table = $action->getTable();
                $name = $table->getName();

                if (!isset($this->tableUpdates[$name])) {
                    $this->tableUpdates[$name] = new AlterTable($table);
                }

                $this->tableUpdates[$name]->addAction($action);
            });
    }

    /**
     * Collects all alter table drop and renames from the given intent
     *
     * @param \Phinx\Db\Action\Action[] $actions The actions to parse
     * @return void
     */
    protected function gatherTableMoves($actions)
    {
        collection($actions)
            ->filter(function ($action) {
                return $action instanceof DropTable
                    || $action instanceof RenameTable
                    || $action instanceof ChangePrimaryKey
                    || $action instanceof ChangeComment;
            })
            ->each(function ($action) {

                $table = $action->getTable();
                $name = $table->getName();

                if (!isset($this->tableMoves[$name])) {
                    $this->tableMoves[$name] = new AlterTable($table);
                }

                $this->tableMoves[$name]->addAction($action);
            });
    }

    /**
     * Collects all index creation and drops from the given intent
     *
     * @param \Phinx\Db\Action\Action[] $actions The actions to parse
     * @return void
     */
    protected function gatherIndexes($actions)
    {
        collection($actions)
            ->filter(function ($action) {
                return $action instanceof AddIndex
                    || $action instanceof DropIndex;
            })
            ->reject(function ($action) {
                // Indexes for new tables are created inline
                // so we don't wan't them here too
                return isset($this->tableCreates[$action->getTable()->getName()]);
            })
            ->each(function ($action) {

                $table = $action->getTable();
                $name = $table->getName();

                if (!isset($this->indexes[$name])) {
                    $this->indexes[$name] = new AlterTable($table);
                }

                $this->indexes[$name]->addAction($action);
            });
    }

    /**
     * Collects all foreign key creation and drops from the given intent
     *
     * @param \Phinx\Db\Action\Action[] $actions The actions to parse
     * @return void
     */
    protected function gatherConstraints($actions)
    {
        collection($actions)
            ->filter(function ($action) {
                return $action instanceof AddForeignKey
                    || $action instanceof DropForeignKey;
            })
            ->each(function ($action) {

                $table = $action->getTable();
                $name = $table->getName();

                if (!isset($this->constraints[$name])) {
                    $this->constraints[$name] = new AlterTable($table);
                }

                $this->constraints[$name]->addAction($action);
            });
    }
}


cakephp / phinx