Improve collision handling on multiple create calls (#2325)

Author: MasterOdin

src/Phinx/Console/Command/Create.php

@@ -167,10 +167,14 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 
         $path = realpath($path);
         $className = $input->getArgument('name');
+        $offset = 0;
+        do {
+            $timestamp = Util::getCurrentTimestamp($offset++);
+        } while (!Util::isUniqueTimestamp($path, $timestamp));
+
         if ($className === null) {
-            $currentTimestamp = Util::getCurrentTimestamp();
-            $className = 'V' . $currentTimestamp;
-            $fileName = $currentTimestamp . '.php';
+            $className = 'V' . $timestamp;
+            $fileName = '';
         } else {
             if (!Util::isValidPhinxClassName($className)) {
                 throw new InvalidArgumentException(sprintf(
@@ -179,9 +183,9 @@ protected function execute(InputInterface $input, OutputInterface $output): int
                 ));
             }
 
-            // Compute the file path
-            $fileName = Util::mapClassNameToFileName($className);
+            $fileName = Util::toSnakeCase($className);
         }
+        $fileName = $timestamp . $fileName . '.php';
 
         if (!Util::isUniqueMigrationClassName($className, $path)) {
             throw new InvalidArgumentException(sprintf(

src/Phinx/Util/Util.php

@@ -47,13 +47,28 @@ class Util
      *
      * @return string
      */
-    public static function getCurrentTimestamp(): string
+    public static function getCurrentTimestamp(?int $offset = null): string
     {
         $dt = new DateTime('now', new DateTimeZone('UTC'));
+        if ($offset) {
+            $dt->modify('+' . $offset . ' seconds');
+        }
 
         return $dt->format(static::DATE_FORMAT);
     }
 
+    /**
+     * Checks that the given timestamp is a unique prefix for any files in the given path.
+     *
+     * @param string $path Path to check
+     * @param string $timestamp Timestamp to check
+     * @return bool
+     */
+    public static function isUniqueTimestamp(string $path, string $timestamp): bool
+    {
+        return !count(static::glob($path . DIRECTORY_SEPARATOR . $timestamp . '*.php'));
+    }
+
     /**
      * Gets an array of all the existing migration class names.
      *
@@ -99,11 +114,27 @@ public static function getVersionFromFileName(string $fileName): int
         return $value;
     }
 
+    /**
+     * Given a string, convert it to snake_case.
+     *
+     * @param string $string String to convert
+     * @return string
+     */
+    public static function toSnakeCase(string $string): string
+    {
+        $snake = function ($matches) {
+            return '_' . strtolower($matches[0]);
+        };
+
+        return preg_replace_callback('/\d+|[A-Z]/', $snake, $string);
+    }
+
     /**
      * Turn migration names like 'CreateUserTable' into file names like
      * '12345678901234_create_user_table.php' or 'LimitResourceNamesTo30Chars' into
      * '12345678901234_limit_resource_names_to_30_chars.php'.
      *
+     * @deprecated Will be removed in 0.17.0
      * @param string $className Class Name
      * @return string
      */

tests/Phinx/Console/Command/CreateTest.php

@@ -9,6 +9,7 @@
 use Phinx\Console\Command\AbstractCommand;
 use Phinx\Console\Command\Create;
 use Phinx\Console\PhinxApplication;
+use Phinx\Util\Util;
 use Symfony\Component\Console\Input\ArrayInput;
 use Symfony\Component\Console\Output\StreamOutput;
 use Symfony\Component\Console\Tester\CommandTester;
@@ -561,6 +562,33 @@ public function testCreateMigrationWithUpDownStyleAsFlag(): void
         $this->assertStringNotContainsString('public function change()', $migrationContents);
     }
 
+    public function testCreateMigrationWithExistingTimestamp(): void
+    {
+        $application = new PhinxApplication();
+        $application->add(new Create());
+
+        /** @var Create $command */
+        $command = $application->find('create');
+
+        /** @var \Phinx\Migration\Manager $managerStub mock the manager class */
+        $managerStub = $this->getMockBuilder('\Phinx\Migration\Manager')
+            ->setConstructorArgs([$this->config, $this->input, $this->output])
+            ->getMock();
+
+        $command->setConfig($this->config);
+        $command->setManager($managerStub);
+
+        $commandTester = new CommandTester($command);
+        $commandTester->execute(['command' => $command->getName(), 'name' => 'Foo']);
+        $commandTester->execute(['command' => $command->getName(), 'name' => 'Bar']);
+
+        $files = array_map(fn ($file) => basename($file), Util::getFiles($this->config->getMigrationPaths()));
+        sort($files);
+        $timestamp = explode('_', $files[0])[0];
+        $secondTimestamp = (float)$timestamp + (str_ends_with($timestamp, '59') ? 41 : 1);
+        $this->assertEquals([$timestamp . '_foo.php', $secondTimestamp . '_bar.php'], $files);
+    }
+
     public function testCreateMigrationWithInvalidStyleFlagThrows(): void
     {
         $application = new PhinxApplication();

tests/Phinx/Util/UtilTest.php

@@ -51,6 +51,12 @@ public function testGetCurrentTimestamp()
         $this->assertLessThanOrEqual($expected + 2, $current);
     }
 
+    public function testIsUniqueTimestamp(): void
+    {
+        $this->assertFalse(Util::isUniqueTimestamp(__DIR__ . '/_files/migrations', '20120111235330'));
+        $this->assertTrue(Util::isUniqueTimestamp(__DIR__ . '/_files/migrations', '20120111235301'));
+    }
+
     public function testGetVersionFromFileName(): void
     {
         $this->assertSame(20221130101652, Util::getVersionFromFileName('20221130101652_test.php'));
@@ -62,7 +68,7 @@ public function testGetVersionFromFileNameErrorNoVersion(): void
         Util::getVersionFromFileName('foo.php');
     }
 
-    public function testGetVersionFromFileNameErrorZeroVersion(): VoidCommand
+    public function testGetVersionFromFileNameErrorZeroVersion(): void
     {
         $this->expectException(RuntimeException::class);
         Util::getVersionFromFileName('0_foo.php');