Merge pull request #36 from move-elevator/feat/slash-branch-names

feat: support feature branch names containing slashes

Author: konradmichalik

deployer/dev/task/sync.php

@@ -38,7 +38,8 @@
     if ($target !== 'prod' && $target !== 'stage') {
         $dbSyncToolSync = get('dev_db_sync_tool_default_sync');
 
-        $dbSyncToolOriginPath = str_replace('<feature>', $target, get('dev_db_sync_tool_origin_path'));
+        // the remote path carries the instance name, not the raw branch name
+        $dbSyncToolOriginPath = str_replace('<feature>', getFeatureName($target), get('dev_db_sync_tool_origin_path'));
         $additionalOptions = "--origin-path $dbSyncToolOriginPath";
     } else {
         $dbSyncToolSync = $target === 'prod' ? get('dev_db_sync_tool_prod_sync') : get('dev_db_sync_tool_default_sync');

deployer/feature/task/feature_init.php

@@ -55,6 +55,8 @@ function initFeature(?string $feature = null): ?string
             return $array[2];
         }, listFeatureInstances()));
     }
+    // branch names may contain path separators ("feature/ABC-12"), the instance name must stay flat
+    $feature = getFeatureName($feature);
     set('feature', $feature);
 
     if (isUrlShortener()) {

deployer/feature/task/feature_setup.php

@@ -67,7 +67,8 @@ function renderRemoteTemplates(): void
 {
     debug('Rendering remote template');
     $databaseName = DbUtility::getDatabaseManager()->getDatabaseName();
-    $feature = input()->getOption('feature');
+    // the normalized name, matching the instance directory and url segment
+    $feature = get('feature');
     $templates = get('feature_templates');
 
     if (!$templates) {
@@ -144,13 +145,3 @@ function uploadTemplate($localTemplate, $remoteTarget, $arguments): void {
     upload($temporaryFileName,get('deploy_path') . $remoteTarget);
     unlink($temporaryFileName);
 }
-
-/**
- * @param ?string $feature
- * @return array|string|string[]|null
- */
-function getFeatureName(?string $feature = null) {
-    $feature = $feature ?: input()->getOption('feature');
-
-    return preg_replace('/[^A-Za-z0-9\_\-.]/', '', $feature);
-}

deployer/feature/task/feature_stop.php

@@ -33,7 +33,7 @@
  */
 function deleteFeature(?string $feature = null, $needConfirmation = false): void
 {
-    $feature = $feature ?: input()->getOption('feature');
+    $feature = getFeatureName($feature);
 
     $filesRemoveCommand = "rm -rf " . get('deploy_path');
 

deployer/functions.php

@@ -3,6 +3,7 @@
 namespace Deployer;
 
 use Deployer\Exception\RunException;
+use MoveElevator\DeployerTools\Utility\FeatureUtility;
 use Symfony\Component\Console\Output\OutputInterface;
 
 /**
@@ -55,6 +56,26 @@ function featureRequested(): bool
     return null !== $feature && '' !== trim((string)$feature);
 }
 
+/**
+ * Normalize a feature identifier into the flat instance name used for the deploy
+ * directory, the public url segment, the url shortener symlink and the database name.
+ *
+ * Branch names may carry path separators ("feature/ABC-12"), which would otherwise
+ * nest the instance directory and break listing, cleanup and deletion. Identifiers
+ * that already consist of allowed characters only are returned unchanged.
+ *
+ * @param ?string $feature
+ * @return string
+ */
+function getFeatureName(?string $feature = null): string
+{
+    if (null === $feature || '' === trim($feature)) {
+        $feature = featureRequested() ? (string)input()->getOption('feature') : '';
+    }
+
+    return FeatureUtility::normalize($feature);
+}
+
 /**
  * Extend the deployer configuration with available environment variables (starting with "DEPLOYER_CONFIG_"):
  *

docs/FEATURE.md

@@ -55,6 +55,21 @@ The `feature:setup` command represent the initialization of a new feature branch
 $ vendor/bin/dep feature:setup stage --feature=TEST-01
 ```
 
+The `--feature=` value may be a full branch name. Path separators are replaced by a hyphen so the instance stays a single flat directory, url segment and database suffix:
+
+| `--feature=`         | instance name        |
+|----------------------|----------------------|
+| `TEST-01`            | `TEST-01`            |
+| `feature/TEST-01`    | `feature-TEST-01`    |
+| `bugfix/TEST-01`     | `bugfix-TEST-01`     |
+| `release/1.2.0`      | `release-1.2.0`      |
+
+Names that already consist of letters, digits, `_`, `-` and `.` are used unchanged, so existing instances and their databases stay reachable. Since the branch prefix is kept, `feature/TEST-01` and `bugfix/TEST-01` remain two separate instances. The same normalization is applied by `feature:cleanup` when it compares remote git branches with the deployed instances.
+
+A `--feature=` value that yields no usable name is rejected instead of falling back to the base instance, which the feature scaffolding would otherwise overwrite. That covers values without any allowed character as well as `.` and `..`.
+
+> Because the separator is replaced rather than encoded, `feature/TEST-01` and a branch literally named `feature-TEST-01` map to the same instance. Do not use both spellings for different branches in one project.
+
 The recipe already wires this task into the deploy flow, together with a `feature:init` before `deploy:info`. That ordering matters: `deploy:info` resolves `{{release_name}}` and deployer caches the result for the rest of the run, so the feature instance has to be known before it runs. Do not hook `feature:setup` any earlier yourself.
 
 > Upgrading: if your `deploy.php` carries a `before('deploy:info', 'feature:init')` (or an equivalent `feature:setup` hook) as a workaround for that ordering, remove it — the recipe registers it now and the hook would otherwise run twice.
@@ -75,7 +90,7 @@ This configuration defines the local template file as well as the remote target
 | `DEPLOYER_CONFIG_DATABASE_PORT` | default is `3306`, overwrite with deployer `set('database_port', '3306');`           |
 | `DEPLOYER_CONFIG_DATABASE_USER` | should be defined with `database_user` in the host configuration                     |
 | `DEPLOYER_CONFIG_DATABASE_NAME` | will be dynamically generated                                                        |
-| `DEPLOYER_CONFIG_FEATURE_NAME`  | will be provide with the `--feature=` command line argument                          |
+| `DEPLOYER_CONFIG_FEATURE_NAME`  | the normalized instance name derived from the `--feature=` command line argument     |
 | `DEPLOYER_CONFIG_FEATURE_URL`   | will be dynamically generated                                                        |
 | `DEPLOYER_CONFIG_FEATURE_PATH`  | will be dynamically generated                                                        |
 

src/Database/Manager/AbstractManager.php

@@ -5,6 +5,7 @@
 namespace MoveElevator\DeployerTools\Database\Manager;
 
 use MoveElevator\DeployerTools\Database\Exception\DatabaseException;
+use MoveElevator\DeployerTools\Utility\FeatureUtility;
 use MoveElevator\DeployerTools\Utility\VarUtility;
 
 use function Deployer\get;
@@ -41,16 +42,21 @@ public function run(string $command, bool $useDoubleQuotes = true): string
      */
     public function getDatabaseName(?string $feature = null): string
     {
-        $feature = $feature ?: input()->getOption('feature');
-        $project = get('project');
-        return substr($this->getFeatureName("{$project}--{$feature}"), 0, 63);
+        // both parts are normalized separately, so the "--" separator survives
+        $project = FeatureUtility::normalize((string) get('project'));
+        $feature = $this->getFeatureName($feature);
+
+        return substr($project . '--' . $feature, 0, 63);
     }
 
 
     public function getFeatureName(?string $feature = null): string
     {
-        $feature = $feature ?: input()->getOption('feature');
+        // only null and blank count as absent, "0" is a valid instance name
+        if (null === $feature || '' === trim($feature)) {
+            $feature = (string) input()->getOption('feature');
+        }
 
-        return preg_replace('/[^A-Za-z0-9\_\-.]/', '', (string) $feature);
+        return FeatureUtility::normalize($feature);
     }
 }

src/Database/Manager/Simple.php

@@ -11,7 +11,6 @@
 use function Deployer\set;
 use function Deployer\has;
 use function Deployer\run;
-use function Deployer\input;
 use function Deployer\upload;
 use function Deployer\runExtended;
 use function Deployer\test;
@@ -50,7 +49,7 @@ public function delete(string $feature): void
         debug('Deleting database');
         $this->ensureDatabasePoolExists();
         $this->initDatabaseConfiguration(feature: $feature);
-        $this->assignmentManager->removeAssignment($feature);
+        $this->assignmentManager->removeAssignment($this->getFeatureName($feature));
         $this->run($this->generateDropTablesQuery($this->getDatabaseName($feature)));
     }
 
@@ -78,8 +77,7 @@ public function exists(?string $feature = null): bool
 
     public function getDatabaseName(?string $feature = null): string
     {
-        $feature = $feature ?: input()->getOption('feature');
-        $databaseAssignment = $this->assignmentManager->getAssignment($feature);
+        $databaseAssignment = $this->assignmentManager->getAssignment($this->getFeatureName($feature));
 
         if (!$databaseAssignment) {
             return '';
@@ -115,7 +113,7 @@ private function initDatabaseConfiguration(?string $database = null, ?string $fe
     {
         $pool = get('database_pool');
         if (!$database) {
-            $database = $this->assignmentManager->getAssignment($feature ?: $this->getFeatureName());
+            $database = $this->assignmentManager->getAssignment($this->getFeatureName($feature));
         }
 
         if (!isset($pool[$database])) {

src/Utility/FeatureUtility.php

@@ -0,0 +1,45 @@
+<?php
+
+declare(strict_types=1);
+
+namespace MoveElevator\DeployerTools\Utility;
+
+/**
+ * Normalizes feature identifiers into a name that is safe to use as a directory,
+ * URL segment, symlink and database name suffix.
+ */
+final class FeatureUtility
+{
+    /**
+     * Turn a feature identifier (typically a git branch name) into a flat instance name.
+     *
+     * Path separators become hyphens instead of being dropped, so "feature/ABC-12" and
+     * "bugfix/ABC-12" stay distinct instances. Names that already consist of allowed
+     * characters only are returned unchanged, which keeps existing instances and their
+     * databases reachable.
+     *
+     * @throws \InvalidArgumentException if a non-blank identifier normalizes to an empty
+     *                                   name or to a relative path segment, either of which
+     *                                   would silently address the base instance
+     */
+    public static function normalize(?string $feature): string
+    {
+        $feature = trim((string) $feature);
+
+        if ('' === $feature) {
+            return '';
+        }
+
+        $normalized = str_replace(['/', '\\'], '-', $feature);
+        $normalized = (string) preg_replace('/[^A-Za-z0-9_\-.]/', '', $normalized);
+
+        // "" would resolve to the base instance, "." and ".." to it or its parent
+        if ('' === trim($normalized, '.')) {
+            throw new \InvalidArgumentException(
+                sprintf('The feature name "%s" does not yield a usable instance name.', $feature)
+            );
+        }
+
+        return $normalized;
+    }
+}