[phar-io#233] Separate user migration and internal migration

MacFJA · May 12, 2020 · 0851445 · 0851445
1 parent cc14a78
commit 0851445
Show file tree

Hide file tree

Showing 14 changed files with 79 additions and 48 deletions.
diff --git a/src/Factory.php b/src/Factory.php
@@ -491,7 +491,7 @@ private function getCompatibilityService(): CompatibilityService {
 
     private function getMigrationService(): MigrationService {
         return new MigrationService(
-            new MigrationFactory($this, $this->getEnvironment(), $this->getConsoleInput())
+            new MigrationFactory($this, $this->getConsoleInput())
         );
     }
 

diff --git a/src/autoload.php b/src/autoload.php
@@ -111,6 +111,7 @@ function($class) {
                 'phario\\phive\\installcontext' => '/commands/install/InstallContext.php',
                 'phario\\phive\\installedphar' => '/shared/phar/InstalledPhar.php',
                 'phario\\phive\\installservice' => '/services/phar/InstallService.php',
+                'phario\\phive\\internalfilemigration' => '/services/migration/InternalFileMigration.php',
                 'phario\\phive\\invalidhashexception' => '/shared/exceptions/InvalidHashException.php',
                 'phario\\phive\\ioexception' => '/shared/exceptions/IOException.php',
                 'phario\\phive\\jsondata' => '/shared/JsonData.php',
@@ -219,6 +220,7 @@ function($class) {
                 'phario\\phive\\url' => '/shared/Url.php',
                 'phario\\phive\\urlrepository' => '/shared/repository/UrlRepository.php',
                 'phario\\phive\\usedphar' => '/shared/phar/UsedPhar.php',
+                'phario\\phive\\userfilemigration' => '/services/migration/UserFileMigration.php',
                 'phario\\phive\\verificationfailedexception' => '/shared/exceptions/VerificationFailedException.php',
                 'phario\\phive\\verificationresult' => '/services/signature/VerificationResult.php',
                 'phario\\phive\\versioncommand' => '/commands/version/VersionCommand.php',

diff --git a/src/services/migration/FileMigration.php b/src/services/migration/FileMigration.php
@@ -4,15 +4,12 @@
 use PharIo\FileSystem\Filename;
 
 abstract class FileMigration implements Migration {
-    /** @var Cli\Input */
-    private $input;
     /** @var Filename */
     private $legacy;
     /** @var Filename */
     private $new;
 
-    public function __construct(Cli\Input $input, Filename $legacy, Filename $new) {
-        $this->input       = $input;
+    public function __construct(Filename $legacy, Filename $new) {
         $this->legacy      = $legacy;
         $this->new         = $new;
     }
@@ -32,25 +29,10 @@ public function migrate(): void {
 
         $this->doMigrate($this->legacy, $this->new);
 
-        if ($this->mustMigrate()) {
-            $this->legacy->delete();
-        } else {
-            $this->handleOldFile($this->getFileDescription(), $this->legacy);
-        }
+        $this->handleOldFile($this->legacy);
     }
 
     abstract protected function doMigrate(Filename $legacy, Filename $new): void;
 
-    abstract protected function getFileDescription(): string;
-
-    protected function handleOldFile(string $description, Filename $old): void {
-        $message = \sprintf('Migration of %s is almost finished. Do you want to keep the old file?', $description);
-
-        if ($this->input->confirm($message, true)) {
-            $newName = \basename($old->asString()) . '.backup';
-            $old->renameTo($newName);
-        } else {
-            $old->delete();
-        }
-    }
+    abstract protected function handleOldFile(Filename $old): void;
 }
diff --git a/src/services/migration/HomePharsXmlMigration.php b/src/services/migration/HomePharsXmlMigration.php
@@ -3,10 +3,9 @@
 
 use PharIo\FileSystem\Filename;
 
-class HomePharsXmlMigration extends FileMigration {
-    public function __construct(Config $config, Cli\Input $input) {
+class HomePharsXmlMigration extends InternalFileMigration {
+    public function __construct(Config $config) {
         parent::__construct(
-            $input,
             $config->getHomeDirectory()->file('phars.xml'),
             $config->getRegistry()
         );
@@ -20,6 +19,10 @@ public function getDescription(): string {
         return 'Change the name of the list of all installed Phars file.';
     }
 
+    public function isUserMigration(): bool {
+        return false;
+    }
+
     protected function doMigrate(Filename $legacy, Filename $new): void {
         $new->putContent($legacy->read()->getContent());
     }

diff --git a/src/services/migration/HomePhiveXmlMigration.php b/src/services/migration/HomePhiveXmlMigration.php
@@ -3,10 +3,9 @@
 
 use PharIo\FileSystem\Filename;
 
-class HomePhiveXmlMigration extends FileMigration {
-    public function __construct(Config $config, Cli\Input $input) {
+class HomePhiveXmlMigration extends InternalFileMigration {
+    public function __construct(Config $config) {
         parent::__construct(
-            $input,
             $config->getHomeDirectory()->file('phive.xml'),
             $config->getGlobalInstallation()
         );
@@ -20,6 +19,10 @@ public function getDescription(): string {
         return 'Change the name of globally installed Phars configuration file.';
     }
 
+    public function isUserMigration(): bool {
+        return false;
+    }
+
     protected function doMigrate(Filename $legacy, Filename $new): void {
         $new->putContent($legacy->read()->getContent());
     }

diff --git a/src/services/migration/InternalFileMigration.php b/src/services/migration/InternalFileMigration.php
@@ -0,0 +1,10 @@
+<?php declare(strict_types = 1);
+namespace PharIo\Phive;
+
+use PharIo\FileSystem\Filename;
+
+abstract class InternalFileMigration extends FileMigration {
+    protected function handleOldFile(Filename $old): void {
+        $old->delete();
+    }
+}
diff --git a/src/services/migration/Migration.php b/src/services/migration/Migration.php
@@ -13,6 +13,12 @@ public function canMigrate(): bool;
      * Return true if only the new state is allowed
      */
     public function mustMigrate(): bool;
+
+    /**
+     * Indicate if the migration is a user/project related migration or
+     * an Phive/internal migration.
+     */
+    public function isUserMigration(): bool;
     public function inError(): bool;
     public function getDescription(): string;
     public function migrate(): void;

diff --git a/src/services/migration/MigrationFactory.php b/src/services/migration/MigrationFactory.php
@@ -5,25 +5,22 @@ class MigrationFactory {
 
     /** @var Factory */
     private $factory;
-    /** @var Environment */
-    private $environment;
     /** @var Cli\Input */
     private $input;
 
-    public function __construct(Factory $factory, Environment $environment, Cli\Input $input) {
-        $this->factory     = $factory;
-        $this->environment = $environment;
-        $this->input       = $input;
+    public function __construct(Factory $factory, Cli\Input $input) {
+        $this->factory = $factory;
+        $this->input   = $input;
     }
 
     /**
      * @return Migration[]
      */
     public function getMigrations(): array {
         return [
-            new HomePharsXmlMigration($this->factory->getConfig(), $this->input),
-            new HomePhiveXmlMigration($this->factory->getConfig(), $this->input),
-            new ProjectPhiveXmlMigration($this->environment, $this->factory->getConfig(), $this->input),
+            new HomePharsXmlMigration($this->factory->getConfig()),
+            new HomePhiveXmlMigration($this->factory->getConfig()),
+            new ProjectPhiveXmlMigration($this->factory->getConfig(), $this->input),
         ];
     }
 }
diff --git a/src/services/migration/MigrationService.php b/src/services/migration/MigrationService.php
@@ -31,7 +31,7 @@ public function ensureFitness(): void {
      */
     public function getUserMigrations(): array {
         return \array_filter($this->factory->getMigrations(), function (Migration $migration) {
-            return !$migration->mustMigrate();
+            return $migration->isUserMigration();
         });
     }
     public function runAll(): int {

diff --git a/src/services/migration/ProjectPhiveXmlMigration.php b/src/services/migration/ProjectPhiveXmlMigration.php
@@ -3,11 +3,11 @@
 
 use PharIo\FileSystem\Filename;
 
-class ProjectPhiveXmlMigration extends FileMigration {
-    public function __construct(Environment $environment, Config $config, Cli\Input $input) {
+class ProjectPhiveXmlMigration extends UserFileMigration {
+    public function __construct(Config $config, Cli\Input $input) {
         parent::__construct(
             $input,
-            $environment->getWorkingDirectory()->file('phive.xml'),
+            $config->getWorkingDirectory()->file('phive.xml'),
             $config->getProjectInstallation()
         );
     }
@@ -20,6 +20,10 @@ public function getDescription(): string {
         return 'Move the \'phive.xml\' inside the new \'.phive/\' configuration directory.';
     }
 
+    public function isUserMigration(): bool {
+        return true;
+    }
+
     protected function doMigrate(Filename $legacy, Filename $new): void {
         $new->putContent($legacy->read()->getContent());
     }

diff --git a/src/services/migration/UserFileMigration.php b/src/services/migration/UserFileMigration.php
@@ -0,0 +1,27 @@
+<?php declare(strict_types = 1);
+namespace PharIo\Phive;
+
+use PharIo\FileSystem\Filename;
+
+abstract class UserFileMigration extends FileMigration {
+    /** @var Cli\Input */
+    private $input;
+
+    public function __construct(Cli\Input $input, Filename $legacy, Filename $new) {
+        $this->input       = $input;
+        parent::__construct($legacy, $new);
+    }
+
+    abstract protected function getFileDescription(): string;
+
+    protected function handleOldFile(Filename $old): void {
+        $message = \sprintf('Migration of %s is almost finished. Do you want to keep the old file?', $this->getFileDescription());
+
+        if ($this->input->confirm($message, true)) {
+            $newName = \basename($old->asString()) . '.backup';
+            $old->renameTo($newName);
+        } else {
+            $old->delete();
+        }
+    }
+}
diff --git a/tests/unit/services/migration/HomePharsXmlMigrationTest.php b/tests/unit/services/migration/HomePharsXmlMigrationTest.php
@@ -63,7 +63,7 @@ public function testMigrate(): void {
         $config = $this->createPartialMock(Config::class, ['getHomeDirectory']);
         $config->method('getHomeDirectory')->willReturn($directory);
 
-        $migration = new HomePharsXmlMigration($config, $this->getInputMock($this, false));
+        $migration = new HomePharsXmlMigration($config);
 
         $migration->migrate();
 
@@ -81,6 +81,6 @@ private function createMigration(array $existingFiles): HomePharsXmlMigration {
         $config = $this->createPartialMock(Config::class, ['getHomeDirectory']);
         $config->method('getHomeDirectory')->willReturn($this->getDirectoryWithFileMock($this, $existingFiles));
 
-        return new HomePharsXmlMigration($config, $this->getInputMock($this, true));
+        return new HomePharsXmlMigration($config);
     }
 }
diff --git a/tests/unit/services/migration/HomePhiveXmlMigrationTest.php b/tests/unit/services/migration/HomePhiveXmlMigrationTest.php
@@ -66,7 +66,7 @@ public function testMigrate(): void {
         $config = $this->createPartialMock(Config::class, ['getHomeDirectory']);
         $config->method('getHomeDirectory')->willReturn($directory);
 
-        $migration = new HomePhiveXmlMigration($config, $this->getInputMock($this, false));
+        $migration = new HomePhiveXmlMigration($config);
 
         $migration->migrate();
 
@@ -84,6 +84,6 @@ private function createMigration(array $existingFiles): HomePhiveXmlMigration {
         $config = $this->createPartialMock(Config::class, ['getHomeDirectory']);
         $config->method('getHomeDirectory')->willReturn($this->getDirectoryWithFileMock($this, $existingFiles));
 
-        return new HomePhiveXmlMigration($config, $this->getInputMock($this, true));
+        return new HomePhiveXmlMigration($config);
     }
 }
diff --git a/tests/unit/services/migration/ProjectPhiveXmlMigrationTest.php b/tests/unit/services/migration/ProjectPhiveXmlMigrationTest.php
@@ -64,7 +64,6 @@ public function testMigrate(): void {
         $environment->method('getWorkingDirectory')->willReturn($directory);
 
         $migration = new ProjectPhiveXmlMigration(
-            $environment,
             new Config($environment, $this->getOptionsMock($this)),
             $this->getInputMock($this, false)
         );
@@ -89,7 +88,6 @@ public function testMigrateRename(): void {
         $environment->method('getWorkingDirectory')->willReturn($directory);
 
         $migration = new ProjectPhiveXmlMigration(
-            $environment,
             new Config($environment, $this->getOptionsMock($this)),
             $this->getInputMock($this, true)
         );
@@ -121,7 +119,6 @@ private function createMigration(bool $haveLegacy, bool $haveNewFile, bool $acce
             ->willReturn($workingDirectory);
 
         return new ProjectPhiveXmlMigration(
-            $environment,
             new Config($environment, $this->getOptionsMock($this)),
             $this->getInputMock($this, $accepted)
         );