Merge pull request #19743 from totten/master-upg

release#16 - Allow omission of empty upgrade steps

CRM/Upgrade/Form.php

@@ -88,9 +88,11 @@ class CRM_Upgrade_Form extends CRM_Core_Form {
   }
 
   /**
-   * @param $version
+   * @param string $version
+   *   Ex: '5.22' or '5.22.3'
    *
-   * @return mixed
+   * @return CRM_Upgrade_Incremental_Base
+   *   Ex: CRM_Upgrade_Incremental_php_FiveTwentyTwo
    */
   public static function &incrementalPhpObject($version) {
     static $incrementalPhpObject = [];
@@ -105,6 +107,29 @@ class CRM_Upgrade_Form extends CRM_Core_Form {
     return $incrementalPhpObject[$versionName];
   }
 
+  /**
+   * @return array
+   *   ex: ['5.13', '5.14', '5.15']
+   */
+  public static function incrementalPhpObjectVersions() {
+    $versions = [];
+
+    $phpDir = implode(DIRECTORY_SEPARATOR, [dirname(__FILE__), 'Incremental', 'php']);
+    $phpFiles = glob("$phpDir/*.php");
+    foreach ($phpFiles as $phpFile) {
+      $phpWord = substr(basename($phpFile), 0, -4);
+      if (CRM_Utils_EnglishNumber::isNumeric($phpWord)) {
+        /** @var \CRM_Upgrade_Incremental_Base $instance */
+        $className = 'CRM_Upgrade_Incremental_php_' . $phpWord;
+        $instance = new $className();
+        $versions[] = $instance->getMajorMinor();
+      }
+    }
+
+    usort($versions, 'version_compare');
+    return $versions;
+  }
+
   /**
    * @param $version
    * @param $release
@@ -287,23 +312,18 @@ SET    version = '$version'
   }
 
   /**
+   * Get a list of all patch-versions that appear in upgrade steps, whether
+   * as *.mysql.tpl or as *.php.
+   *
    * @return array
    * @throws Exception
    */
   public function getRevisionSequence() {
     $revList = [];
-    $sqlDir = implode(DIRECTORY_SEPARATOR,
-      [dirname(__FILE__), 'Incremental', 'sql']
-    );
-    $sqlFiles = scandir($sqlDir);
 
-    $sqlFilePattern = '/^((\d{1,2}\.\d{1,2})\.(\d{1,2}\.)?(\d{1,2}|\w{4,7}))\.(my)?sql(\.tpl)?$/i';
-    foreach ($sqlFiles as $file) {
-      if (preg_match($sqlFilePattern, $file, $matches)) {
-        if (!in_array($matches[1], $revList)) {
-          $revList[] = $matches[1];
-        }
-      }
+    foreach (self::incrementalPhpObjectVersions() as $majorMinor) {
+      $phpUpgrader = self::incrementalPhpObject($majorMinor);
+      $revList = array_merge($revList, array_values($phpUpgrader->getRevisionSequence()));
     }
 
     usort($revList, 'version_compare');
@@ -516,6 +536,12 @@ SET    version = '$version'
     $queue->createItem($task);
 
     $revisions = $upgrade->getRevisionSequence();
+    $maxRevision = empty($revisions) ? NULL : end($revisions);
+    reset($revisions);
+    if (version_compare($latestVer, $maxRevision, '<')) {
+      throw new CRM_Core_Exception("Malformed upgrade sequence.  The incremental update $maxRevision exceeds target version $latestVer");
+    }
+
     foreach ($revisions as $rev) {
       // proceed only if $currentVer < $rev
       if (version_compare($currentVer, $rev) < 0) {
@@ -548,6 +574,16 @@ SET    version = '$version'
       }
     }
 
+    // It's possible that xml/version.xml points to a version that doesn't have any concrete revision steps.
+    if (!in_array($latestVer, $revisions)) {
+      $task = new CRM_Queue_Task(
+        ['CRM_Upgrade_Form', 'doIncrementalUpgradeFinish'],
+        [$rev, $latestVer, $latestVer, $postUpgradeMessageFile],
+        "Finish Upgrade DB to $latestVer"
+      );
+      $queue->createItem($task);
+    }
+
     return $queue;
   }
 
@@ -733,7 +769,12 @@ SET    version = '$version'
   }
 
   /**
-   * Perform an incremental version update.
+   * Mark an incremental update as finished.
+   *
+   * This method may be called in two cases:
+   *
+   * - After performing each incremental update (`X.X.X.mysql.tpl` or `upgrade_X_X_X()`)
+   * - If needed, one more time at the end of the upgrade for the final version-number.
    *
    * @param CRM_Queue_TaskContext $ctx
    * @param string $rev

CRM/Upgrade/Incremental/Base.php

@@ -17,6 +17,57 @@ use Civi\Core\SettingsBag;
 class CRM_Upgrade_Incremental_Base {
   const BATCH_SIZE = 5000;
 
+  /**
+   * @var string|null
+   */
+  protected $majorMinor;
+
+  /**
+   * Get the major and minor version for this class (based on English-style class name).
+   *
+   * @return string
+   *   Ex: '5.34' or '4.7'
+   */
+  public function getMajorMinor() {
+    if (!$this->majorMinor) {
+      $className = explode('_', static::CLASS);
+      $numbers = preg_split("/([[:upper:]][[:lower:]]+)/", array_pop($className), -1, PREG_SPLIT_DELIM_CAPTURE | PREG_SPLIT_NO_EMPTY);
+      $major = CRM_Utils_EnglishNumber::toInt(array_shift($numbers));
+      $minor = CRM_Utils_EnglishNumber::toInt(implode('', $numbers));
+      $this->majorMinor = $major . '.' . $minor;
+    }
+    return $this->majorMinor;
+  }
+
+  /**
+   * Get a list of revisions (PATCH releases) related to this class.
+   *
+   * @return array
+   *   Ex: ['4.5.6', '4.5.7']
+   * @throws \ReflectionException
+   */
+  public function getRevisionSequence() {
+    $revList = [];
+
+    $sqlGlob = implode(DIRECTORY_SEPARATOR, [dirname(__FILE__), 'sql', $this->getMajorMinor() . '.*.mysql.tpl']);
+    $sqlFiles = glob($sqlGlob);;
+    foreach ($sqlFiles as $file) {
+      $revList[] = str_replace('.mysql.tpl', '', basename($file));
+    }
+
+    $c = new ReflectionClass(static::class);
+    foreach ($c->getMethods() as $method) {
+      /** @var \ReflectionMethod $method */
+      if (preg_match(';^upgrade_([0-9_alphabeta]+)$;', $method->getName(), $m)) {
+        $revList[] = str_replace('_', '.', $m[1]);
+      }
+    }
+
+    $revList = array_unique($revList);
+    usort($revList, 'version_compare');
+    return $revList;
+  }
+
   /**
    * Verify DB state.
    *

CRM/Upgrade/Incremental/php/FiveThirtySeven.php

@@ -16,8 +16,8 @@ class CRM_Upgrade_Incremental_php_FiveThirtySeven extends CRM_Upgrade_Incrementa
   /**
    * Compute any messages which should be displayed beforeupgrade.
    *
-   * Note: This function is called iteratively for each upcoming
-   * revision to the database.
+   * Note: This function is called iteratively for each incremental upgrade step.
+   * There must be a concrete step (eg 'X.Y.Z.mysql.tpl' or 'upgrade_X_Y_Z()').
    *
    * @param string $preUpgradeMessage
    * @param string $rev
@@ -34,6 +34,9 @@ class CRM_Upgrade_Incremental_php_FiveThirtySeven extends CRM_Upgrade_Incrementa
   /**
    * Compute any messages which should be displayed after upgrade.
    *
+   * Note: This function is called iteratively for each incremental upgrade step.
+   * There must be a concrete step (eg 'X.Y.Z.mysql.tpl' or 'upgrade_X_Y_Z()').
+   *
    * @param string $postUpgradeMessage
    *   alterable.
    * @param string $rev

CRM/Upgrade/Incremental/php/FiveZero.php

@@ -47,15 +47,6 @@ class CRM_Upgrade_Incremental_php_FiveZero extends CRM_Upgrade_Incremental_Base
     //}
   }
 
-  /**
-   * Upgrade function.
-   *
-   * @param string $rev
-   */
-  public function upgrade_5_0_0($rev) {
-    $this->addTask(ts('Upgrade DB to %1: SQL', [1 => $rev]), 'runSql', $rev);
-  }
-
   /*
    * Important! All upgrade functions MUST add a 'runSql' task.
    * Uncomment and use the following template for a new upgrade version

CRM/Upgrade/Incremental/php/Template.php

@@ -22,8 +22,8 @@ class CRM_Upgrade_Incremental_php_<?php echo $camelNumber; ?> extends CRM_Upgrad
   /**
    * Compute any messages which should be displayed beforeupgrade.
    *
-   * Note: This function is called iteratively for each upcoming
-   * revision to the database.
+   * Note: This function is called iteratively for each incremental upgrade step.
+   * There must be a concrete step (eg 'X.Y.Z.mysql.tpl' or 'upgrade_X_Y_Z()').
    *
    * @param string $preUpgradeMessage
    * @param string $rev
@@ -40,6 +40,9 @@ class CRM_Upgrade_Incremental_php_<?php echo $camelNumber; ?> extends CRM_Upgrad
   /**
    * Compute any messages which should be displayed after upgrade.
    *
+   * Note: This function is called iteratively for each incremental upgrade step.
+   * There must be a concrete step (eg 'X.Y.Z.mysql.tpl' or 'upgrade_X_Y_Z()').
+   *
    * @param string $postUpgradeMessage
    *   alterable.
    * @param string $rev

CRM/Utils/EnglishNumber.php

@@ -122,4 +122,60 @@ class CRM_Utils_EnglishNumber {
     }
   }
 
+  /**
+   * Convert an English-style number to an int.
+   *
+   * @param string $english
+   *   Ex: 'TwentyTwo' or 'forty-four'
+   *
+   * @return int
+   *   22 or 44
+   */
+  public static function toInt(string $english) {
+    $intBuf = 0;
+    $strBuf = strtolower(str_replace('-', '', $english));
+
+    foreach (self::$intervalsOfTen as $num => $name) {
+      if (CRM_Utils_String::startsWith($strBuf, strtolower($name))) {
+        $intBuf += 10 * $num;
+        $strBuf = substr($strBuf, strlen($name));
+        break;
+      }
+    }
+    foreach (array_reverse(self::$lowNumbers, TRUE) as $num => $name) {
+      if (CRM_Utils_String::startsWith($strBuf, strtolower($name))) {
+        $intBuf += $num;
+        $strBuf = substr($strBuf, strlen($name));
+        break;
+      }
+    }
+
+    if (!empty($strBuf)) {
+      throw new InvalidArgumentException("Failed to parse english number: $strBuf");
+    }
+
+    return $intBuf;
+  }
+
+  /**
+   * Determine if a string looks like
+   *
+   * @param string $english
+   *
+   * @return bool
+   */
+  public static function isNumeric(string $english): bool {
+    static $pat;
+    if (empty($pat)) {
+      $words = array_map(
+        function($w) {
+          return preg_quote(strtolower($w));
+        },
+        array_merge(array_values(self::$lowNumbers), array_values(self::$intervalsOfTen))
+      );
+      $pat = '/^(\-|' . implode('|', $words) . ')+$/';
+    }
+    return (bool) preg_match($pat, strtolower($english));
+  }
+
 }

tests/phpunit/CRM/Utils/EnglishNumberTest.php

+<?php
+
+/**
+ * Class CRM_Utils_EnglishNumberTest
+ *
+ * @group headless
+ */
+class CRM_Utils_EnglishNumberTest extends CiviUnitTestCase {
+
+  public function setUp() {
+    parent::setUp();
+    $this->useTransaction();
+  }
+
+  public function testRoundTrip() {
+    for ($i = 0; $i < 100; $i++) {
+      $camel = CRM_Utils_EnglishNumber::toCamelCase($i);
+      $camelToInt = CRM_Utils_EnglishNumber::toInt($camel);
+      $this->assertEquals($camelToInt, $i);
+
+      $hyphen = CRM_Utils_EnglishNumber::toHyphen($i);
+      $hyphenToInt = CRM_Utils_EnglishNumber::toInt($hyphen);
+      $this->assertEquals($hyphenToInt, $i);
+    }
+  }
+
+  public function testCamel() {
+    $this->assertEquals('Seven', CRM_Utils_EnglishNumber::toCamelCase(7));
+    $this->assertEquals('Nineteen', CRM_Utils_EnglishNumber::toCamelCase(19));
+    $this->assertEquals('Thirty', CRM_Utils_EnglishNumber::toCamelCase(30));
+    $this->assertEquals('FiftyFour', CRM_Utils_EnglishNumber::toCamelCase(54));
+    $this->assertEquals('NinetyNine', CRM_Utils_EnglishNumber::toCamelCase(99));
+  }
+
+  public function testHyphen() {
+    $this->assertEquals('seven', CRM_Utils_EnglishNumber::toHyphen(7));
+    $this->assertEquals('nineteen', CRM_Utils_EnglishNumber::toHyphen(19));
+    $this->assertEquals('thirty', CRM_Utils_EnglishNumber::toHyphen(30));
+    $this->assertEquals('fifty-four', CRM_Utils_EnglishNumber::toHyphen(54));
+    $this->assertEquals('ninety-nine', CRM_Utils_EnglishNumber::toHyphen(99));
+  }
+
+  public function testIsNumeric() {
+    $assertNumeric = function($expectBool, $string) {
+      $this->assertEquals($expectBool, CRM_Utils_EnglishNumber::isNumeric($string), "isNumeric($string) should return " . (int) $expectBool);
+    };
+    $assertNumeric(TRUE, 'FiveThirtyEight');
+    $assertNumeric(TRUE, 'Seventeen');
+    $assertNumeric(TRUE, 'four-one-one');
+    $assertNumeric(FALSE, 'Eleventy');
+    $assertNumeric(FALSE, 'Bazillions');
+  }
+
+}

tools/bin/scripts/set-version.php

@@ -25,22 +25,15 @@ if (!isVersionValid($oldVersion)) {
   fatal("failed to read old version from \"xml/version.xml\"\n");
 }
 
-$newVersion = @$argv[1];
+/** @var string $newVersion */
+/** @var bool $doCommit */
+/** @var bool $doSql */
+extract(parseArgs($argv));
+
 if (!isVersionValid($newVersion)) {
   fatal("failed to read new version\n");
 }
 
-switch (@$argv[2]) {
-  case '--commit':
-    $doCommit = 1;
-    break;
-  case '--no-commit':
-    $doCommit = 0;
-    break;
-  default:
-    fatal("Must specify --commit or --no-commit\n");
-}
-
 /* *********************************************************************** */
 /* Main */
 
@@ -56,9 +49,12 @@ $phpFile = initFile("CRM/Upgrade/Incremental/php/{$verName}.php", function () us
   return ob_get_clean();
 });
 
-$sqlFile = initFile("CRM/Upgrade/Incremental/sql/{$newVersion}.mysql.tpl", function () use ($newVersion) {
-  return "{* file to handle db changes in $newVersion during upgrade *}\n";
-});
+// It is typical for `*.alpha` to need SQL file -- and for `*.beta1` and `*.0` to NOT need a SQL file.
+if ($doSql === TRUE || ($doSql === 'auto' && preg_match(';alpha;', $newVersion))) {
+  $sqlFile = initFile("CRM/Upgrade/Incremental/sql/{$newVersion}.mysql.tpl", function () use ($newVersion) {
+    return "{* file to handle db changes in $newVersion during upgrade *}\n";
+  });
+}
 
 updateFile("xml/version.xml", function ($content) use ($newVersion, $oldVersion) {
   return str_replace($oldVersion, $newVersion, $content);
@@ -79,9 +75,13 @@ updateFile("sql/test_data_second_domain.mysql", function ($content) use ($newVer
 });
 
 if ($doCommit) {
-  $files = "xml/version.xml sql/civicrm_generated.mysql sql/test_data_second_domain.mysql " . escapeshellarg($phpFile) . ' ' . escapeshellarg($sqlFile);
-  passthru("git add $files");
-  passthru("git commit $files -m " . escapeshellarg("Set version to $newVersion"));
+  $files = array_filter(
+    ['xml/version.xml', 'sql/civicrm_generated.mysql', 'sql/test_data_second_domain.mysql', $phpFile, @$sqlFile],
+    'file_exists'
+  );
+  $filesEsc = implode(' ', array_map('escapeshellarg', $files));
+  passthru("git add $filesEsc");
+  passthru("git commit $filesEsc -m " . escapeshellarg("Set version to $newVersion"));
 }
 
 /* *********************************************************************** */
@@ -131,7 +131,7 @@ function initFile($file, $callback) {
  *   Ex: 'FiveTen'.
  */
 function makeVerName($version) {
-  list ($a, $b) = explode('.', $version);
+  [$a, $b] = explode('.', $version);
   require_once 'CRM/Utils/EnglishNumber.php';
   return CRM_Utils_EnglishNumber::toCamelCase($a) . CRM_Utils_EnglishNumber::toCamelCase($b);
 }
@@ -145,8 +145,67 @@ function isVersionValid($v) {
  */
 function fatal($error) {
   echo $error;
-  echo "usage: set-version.php <new-version> [--commit|--no-commit]\n";
-  echo "  With --commit, any changes will be committed automatically the current git branch.\n";
-  echo "  With --no-commit, any changes will be left uncommitted.\n";
+  echo "usage: set-version.php <new-version> [--sql|--no-sql] [--commit|--no-commit]\n";
+  echo "  --sql        A placeholder *.sql file will be created.\n";
+  echo "  --no-sql     A placeholder *.sql file will not be created.\n";
+  echo "  --commit     Any changes will be committed automatically the current git branch.\n";
+  echo "  --no-commit  Any changes will be left uncommitted.\n";
+  echo "\n";
+  echo "If the SQL style is not specified, it will decide automatically. (Alpha versions get SQL files.)\n";
+  echo "\n";
+  echo "You MUST indicate whether to commit.\n";
   exit(1);
 }
+
+/**
+* @param array $argv
+ *  Ex: ['myscript.php', '--no-commit', '5.6.7']
+ * @return array
+ *  Ex: ['scriptFile' => 'myscript.php', 'doCommit' => FALSE, 'newVersion' => '5.6.7']
+ */
+function parseArgs($argv) {
+  $parsed = [];
+  $parsed['doSql'] = 'auto';
+  $positions = ['scriptFile', 'newVersion'];
+  $positional = [];
+
+  foreach ($argv as $arg) {
+    switch ($arg) {
+      case '--commit':
+        $parsed['doCommit'] = TRUE;
+        break;
+
+      case '--no-commit':
+        $parsed['doCommit'] = FALSE;
+        break;
+
+      case '--sql':
+        $parsed['doSql'] = TRUE;
+        break;
+
+      case '--no-sql':
+        $parsed['doSql'] = FALSE;
+        break;
+
+      default:
+        if ($arg[0] !== '-') {
+          $positional[] = $arg;
+        }
+        else {
+          fatal("Unrecognized argument: $arg\n");
+        }
+        break;
+    }
+  }
+
+  foreach ($positional as $offset => $value) {
+    $name = $positions[$offset] ?? "unknown_$offset";
+    $parsed[$name] = $value;
+  }
+
+  if (!isset($parsed['doCommit'])) {
+    fatal("Must specify --commit or --no-commit\n");
+  }
+
+  return $parsed;
+}