Merge branch 'master' into speedup_tests

.github/workflows/ci.yml

@@ -16,14 +16,16 @@ jobs:
             matrix:
                 php-version: [ '7.4', '8.2' ]
                 db-type: [ sqlite, mysql, pgsql, agnostic ]
-                symfony-version: [ '4-min', '4-max', '5-min', '5-max', '6-min', '6-max' ]
+                symfony-version: [ '5-min', '5-max', '6-min', '6-max', '7-min', '7-max']
                 exclude:
-                  - symfony-version: '4-min'
-                    php-version: '8.2'
                   - symfony-version: '6-min'
                     php-version: '7.4'
                   - symfony-version: '6-max'
                     php-version: '7.4'
+                  - symfony-version: '7-min'
+                    php-version: '7.4'
+                  - symfony-version: '7-max'
+                    php-version: '7.4'
         env:
             DB_NAME: 'propel_tests'
             DB_USER: 'propel'
@@ -125,6 +127,8 @@ jobs:
                   else
                     vendor/bin/phpunit -c tests/${{ matrix.db-type }}.phpunit.xml
                   fi
+              env:
+                SYMFONY_VERSION: ${{ matrix.symfony-version }}
 
             - name: Code Coverage Report
               if: success() && matrix.php-version == '7.4' && matrix.symfony-version == '5-max'
@@ -166,6 +170,8 @@ jobs:
               run: composer install --prefer-dist --no-interaction
 
             - name: PHPStan
+              env:
+                PHPSTAN: 1
               run: composer stan
 
             - name: Psalm

composer.json

@@ -18,13 +18,13 @@
     "require": {
         "php": ">=7.4",
         "psr/log": "^1.0 || ^2.0 || ^3.0",
-        "symfony/yaml": "^4.4.0 || ^5.0.0 || ^6.0.0",
-        "symfony/config": "^4.4.0 || ^5.0.0 || ^6.0.0",
-        "symfony/console": "^4.4.0 || ^5.0.0 || ^6.0.0",
-        "symfony/filesystem": "^4.4.0 || ^5.0.0 || ^6.0.0",
-        "symfony/finder": "^4.4.0 || ^5.0.0 || ^6.0.0",
-        "symfony/translation": "^4.4.0 || ^5.0.0 || ^6.0.0",
-        "symfony/validator": "^4.4.0 || ^5.0.0 || ^6.0.0"
+        "symfony/yaml": "^5.0.0 || ^6.0.0 || ^7.0.0",
+        "symfony/config": "^5.0.0 || ^6.0.0 || ^7.0.0",
+        "symfony/console": "^5.0.0 || ^6.0.0 || ^7.0.0",
+        "symfony/filesystem": "^5.0.0 || ^6.0.0 || ^7.0.0",
+        "symfony/finder": "^5.0.0 || ^6.0.0 || ^7.0.0",
+        "symfony/translation": "^5.0.0 || ^6.0.0 || ^7.0.0",
+        "symfony/validator": "^5.0.0 || ^6.0.0 || ^7.0.0"
     },
     "require-dev": {
         "ext-pdo": "*",

src/Propel/Generator/Behavior/Timestampable/TimestampableBehavior.php

@@ -106,7 +106,7 @@ public function preUpdate(AbstractOMBuilder $builder): string
                 : '\\Propel\\Runtime\\Util\\PropelDateTime::createHighPrecision()';
 
             return 'if ($this->isModified() && !$this->isColumnModified(' . $this->getColumnConstant('update_column', $builder) . ")) {
-    \$this->" . $this->getColumnSetter('update_column') . "(${valueSource});
+    \$this->" . $this->getColumnSetter('update_column') . "({$valueSource});
 }";
         }
 
@@ -131,7 +131,7 @@ public function preInsert(AbstractOMBuilder $builder): string
                 : '$highPrecision';
             $script .= "
 if (!\$this->isColumnModified(" . $this->getColumnConstant('create_column', $builder) . ")) {
-    \$this->" . $this->getColumnSetter('create_column') . "(${valueSource});
+    \$this->" . $this->getColumnSetter('create_column') . "({$valueSource});
 }";
         }
 
@@ -141,7 +141,7 @@ public function preInsert(AbstractOMBuilder $builder): string
                 : '$highPrecision';
             $script .= "
 if (!\$this->isColumnModified(" . $this->getColumnConstant('update_column', $builder) . ")) {
-    \$this->" . $this->getColumnSetter('update_column') . "(${valueSource});
+    \$this->" . $this->getColumnSetter('update_column') . "({$valueSource});
 }";
         }
 

src/Propel/Generator/Builder/Om/AbstractOMBuilder.php

@@ -686,34 +686,43 @@ public function getFKPhpNameAffix(ForeignKey $fk, bool $plural = false): string
      * @return string
      */
     protected function getCrossFKsPhpNameAffix(CrossForeignKeys $crossFKs, bool $plural = true): string
+    {
+        $baseName = $this->buildCombineCrossFKsPhpNameAffix($crossFKs, false);
+
+        $existingTable = $this->getDatabase()->getTableByPhpName($baseName);
+        $isNameCollision = $existingTable && $this->getTable()->isConnectedWithTable($existingTable);
+
+        return ($plural || $isNameCollision) ? $this->buildCombineCrossFKsPhpNameAffix($crossFKs, $plural, $isNameCollision) : $baseName;
+    }
+
+    /**
+     * @param \Propel\Generator\Model\CrossForeignKeys $crossFKs
+     * @param bool $plural
+     * @param bool $withPrefix
+     *
+     * @return string
+     */
+    protected function buildCombineCrossFKsPhpNameAffix(CrossForeignKeys $crossFKs, bool $plural = true, bool $withPrefix = false): string
     {
         $names = [];
+        if ($withPrefix) {
+            $names[] = 'Cross';
+        }
+        $fks = $crossFKs->getCrossForeignKeys();
+        $lastCrossFk = array_pop($fks);
+        $unclassifiedPrimaryKeys = $crossFKs->getUnclassifiedPrimaryKeys();
+        $lastIsPlural = $plural && !$unclassifiedPrimaryKeys;
 
-        if ($plural) {
-            if ($crossFKs->getUnclassifiedPrimaryKeys()) {
-                //we have a non fk as pk as well, so we need to make pluralisation on our own and can't
-                //rely on getFKPhpNameAffix's pluralisation
-                foreach ($crossFKs->getCrossForeignKeys() as $fk) {
-                    $names[] = $this->getFKPhpNameAffix($fk, false);
-                }
-            } else {
-                //we have only fks, so give us names with plural and return those
-                $lastIdx = count($crossFKs->getCrossForeignKeys()) - 1;
-                foreach ($crossFKs->getCrossForeignKeys() as $idx => $fk) {
-                    $needPlural = $idx === $lastIdx; //only last fk should be plural
-                    $names[] = $this->getFKPhpNameAffix($fk, $needPlural);
-                }
+        foreach ($fks as $fk) {
+            $names[] = $this->getFKPhpNameAffix($fk, false);
+        }
+        $names[] = $this->getFKPhpNameAffix($lastCrossFk, $lastIsPlural);
 
-                return implode('', $names);
-            }
-        } else {
-            // no plural, so $plural=false
-            foreach ($crossFKs->getCrossForeignKeys() as $fk) {
-                $names[] = $this->getFKPhpNameAffix($fk, false);
-            }
+        if (!$unclassifiedPrimaryKeys) {
+            return implode('', $names);
         }
 
-        foreach ($crossFKs->getUnclassifiedPrimaryKeys() as $pk) {
+        foreach ($unclassifiedPrimaryKeys as $pk) {
             $names[] = $pk->getPhpName();
         }
 

src/Propel/Generator/Command/MigrationUpCommand.php

@@ -44,7 +44,7 @@ protected function configure()
      *
      * @throws \Propel\Runtime\Exception\RuntimeException
      */
-    protected function execute(InputInterface $input, OutputInterface $output)
+    protected function execute(InputInterface $input, OutputInterface $output): int
     {
         $configOptions = [];
 

src/Propel/Generator/Model/CrossForeignKeys.php

@@ -132,7 +132,7 @@ public function getIncomingForeignKey(): ?ForeignKey
      */
     public function isAtLeastOneLocalPrimaryKeyNotCovered(ForeignKey $fk): bool
     {
-        $primaryKeys = $fk->getLocalPrimaryKeys();
+        $primaryKeys = $fk->getLocalColumnObjects();
         foreach ($primaryKeys as $primaryKey) {
             $covered = false;
             foreach ($this->getCrossForeignKeys() as $crossFK) {

src/Propel/Generator/Model/ForeignKey.php

@@ -874,7 +874,7 @@ public function isAtLeastOneLocalColumnRequired(): bool
      */
     public function isAtLeastOneLocalPrimaryKeyIsRequired(): bool
     {
-        foreach ($this->getLocalPrimaryKeys() as $pk) {
+        foreach ($this->getLocalColumnObjects() as $pk) {
             if ($pk->isNotNull() && !$pk->hasDefaultValue()) {
                 return true;
             }

src/Propel/Generator/Model/Table.php

@@ -2288,4 +2288,18 @@ public function getAdditionalModelClassImports(): ?array
 
         return null;
     }
+
+    /**
+     * Check if there is a FK rellation between the current table and the given
+     * table in either direction.
+     *
+     * @param \Propel\Generator\Model\Table $table
+     *
+     * @return bool
+     */
+    public function isConnectedWithTable(Table $table): bool
+    {
+        return $this->getForeignKeysReferencingTable($table->getName()) ||
+            $table->getForeignKeysReferencingTable($this->getName());
+    }
 }

src/Propel/Generator/Platform/PgsqlPlatform.php

@@ -10,6 +10,7 @@
 
 use Propel\Generator\Exception\EngineException;
 use Propel\Generator\Model\Column;
+use Propel\Generator\Model\ColumnDefaultValue;
 use Propel\Generator\Model\Database;
 use Propel\Generator\Model\Diff\ColumnDiff;
 use Propel\Generator\Model\Diff\TableDiff;
@@ -515,6 +516,16 @@ public function getColumnDDL(Column $col): string
             $ddl[] = $sqlType;
         }
 
+        if (
+            $col->getDefaultValue()
+            && $col->getDefaultValue()->isExpression()
+            && $col->getDefaultValue()->getValue() === 'CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP'
+        ) {
+            $col->setDefaultValue(
+                new ColumnDefaultValue('CURRENT_TIMESTAMP', ColumnDefaultValue::TYPE_EXPR),
+            );
+        }
+
         $default = $this->getColumnDefaultValueDDL($col);
         if ($default) {
             $ddl[] = $default;

src/Propel/Generator/Platform/SqlitePlatform.php

@@ -492,7 +492,7 @@ public function getColumnDDL(Column $col): string
         if (
             $col->getDefaultValue()
             && $col->getDefaultValue()->isExpression()
-            && $col->getDefaultValue()->getValue() === 'CURRENT_TIMESTAMP'
+            && in_array($col->getDefaultValue()->getValue(), ['CURRENT_TIMESTAMP', 'CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP'])
         ) {
             //sqlite use CURRENT_TIMESTAMP different than mysql/pgsql etc
             //we set it to the more common behavior

src/Propel/Generator/Platform/Util/AlterTableStatementMerger.php

@@ -9,6 +9,7 @@
 namespace Propel\Generator\Platform\Util;
 
 use Propel\Generator\Model\Table;
+use Propel\Generator\Util\SqlParser;
 
 /**
  * Merges several ALTER TABLE statements when creating migrations.
@@ -66,7 +67,10 @@ public function __construct(Table $table)
      */
     public function mergeStatements(string $sql): string
     {
-        $statements = explode(';', $sql);
+        $sqlParser = new SqlParser();
+        $sqlParser->setSQL($sql);
+        $statements = $sqlParser->explodeIntoStatements();
+
         $blocks = [];
         $currentBlock = [];
 

src/Propel/Generator/Reverse/MysqlSchemaParser.php

@@ -295,6 +295,9 @@ public function getColumnFromRow(array $row, Table $table): Column
             }
             if (in_array($default, ['CURRENT_TIMESTAMP', 'current_timestamp()'], true)) {
                 $default = 'CURRENT_TIMESTAMP';
+                if (strpos(strtolower($row['Extra']), 'on update current_timestamp') !== false) {
+                    $default = 'CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP';
+                }
                 $type = ColumnDefaultValue::TYPE_EXPR;
             } else {
                 $type = ColumnDefaultValue::TYPE_VALUE;

src/Propel/Generator/Util/SqlParser.php

@@ -232,14 +232,37 @@ public function explodeIntoStatements(): array
     public function getNextStatement(): string
     {
         $isAfterBackslash = false;
+        $isCommentLine = false;
         $isInString = false;
         $stringQuotes = '';
         $parsedString = '';
         $lowercaseString = ''; // helper variable for performance sake
         while ($this->pos <= $this->len) {
             $char = $this->sql[$this->pos] ?? '';
+
+            $newLine = !isset($this->sql[$this->pos - 1]) || $this->sql[$this->pos - 1] === PHP_EOL;
+
+            // Skip comments
+            if ($isCommentLine === true) {
+                $this->pos++;
+                if ($char === PHP_EOL) {
+                    $isCommentLine = false; // end of comments line
+                }
+
+                continue;
+            }
             // check flags for strings or escaper
             switch ($char) {
+                case '#':
+                    // detect comment line
+                    if ($newLine === true && $isCommentLine === false) {
+                        $this->pos++;
+                        $isCommentLine = true;
+
+                        continue 2;
+                    }
+
+                    break;
                 case '\\':
                     $isAfterBackslash = true;
 
@@ -295,8 +318,9 @@ public function getNextStatement(): string
                 // check for end of statement
                 if ($char . $nextChars == $this->delimiter) {
                     $this->pos += $i; // increase position
+                    $parsedTrimmedString = trim($parsedString);
 
-                    return trim($parsedString);
+                    return $parsedTrimmedString ?: $parsedString; // to check empty line to avoid stop parsing
                 }
                 // avoid using strtolower on the whole parsed string every time new character is added
                 // there is also no point in adding characters which are in the string

src/Propel/Runtime/Collection/CollectionIterator.php

@@ -248,8 +248,9 @@ public function append($value): void
     /**
      * @param int $flags Not used
      *
-     * @return bool
+     * @return true
      */
+    #[\ReturnTypeWillChange]
     public function asort(int $flags = SORT_REGULAR): bool
     {
         parent::asort();
@@ -261,8 +262,9 @@ public function asort(int $flags = SORT_REGULAR): bool
     /**
      * @param int $flags Not used
      *
-     * @return bool
+     * @return true
      */
+    #[\ReturnTypeWillChange]
     public function ksort(int $flags = SORT_REGULAR): bool
     {
         parent::ksort();
@@ -274,8 +276,9 @@ public function ksort(int $flags = SORT_REGULAR): bool
     /**
      * @param callable $callback
      *
-     * @return bool
+     * @return true
      */
+    #[\ReturnTypeWillChange]
     public function uasort($callback): bool
     {
         parent::uasort($callback);
@@ -287,8 +290,9 @@ public function uasort($callback): bool
     /**
      * @param callable $callback
      *
-     * @return bool
+     * @return true
      */
+    #[\ReturnTypeWillChange]
     public function uksort($callback): bool
     {
         parent::uksort($callback);
@@ -298,8 +302,9 @@ public function uksort($callback): bool
     }
 
     /**
-     * @return bool
+     * @return true
      */
+    #[\ReturnTypeWillChange]
     public function natsort(): bool
     {
         parent::natsort();
@@ -309,8 +314,9 @@ public function natsort(): bool
     }
 
     /**
-     * @return bool
+     * @return true
      */
+    #[\ReturnTypeWillChange]
     public function natcasesort(): bool
     {
         parent::natcasesort();

src/Propel/Runtime/Validator/Constraints/Date.php

@@ -12,11 +12,6 @@
 
 class Date extends SymfonyDateConstraint
 {
-    /**
-     * @var string
-     */
-    public $message = 'This value is not a valid date.';
-
     /**
      * @var string
      */

tests/Fixtures/bookstore/schema.xml

@@ -174,6 +174,7 @@
         <column name="enabled" type="BOOLEAN" default="true"/>
         <column name="not_enabled" type="BOOLEAN" default="false"/>
         <column name="created" type="TIMESTAMP" defaultExpr="CURRENT_TIMESTAMP"/>
+        <column name="updated" type="TIMESTAMP" defaultExpr="CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP"/>
         <column name="role_id" type="INTEGER" required="false" default="null"/>
         <column name="authenticator" type="VARCHAR" size="32" defaultExpr="'Password'"/>
         <foreign-key foreignTable="bookstore_employee" onDelete="cascade">