Refactor ColumnFactory (#368)

yiisoft · Nov 8, 2024 · 1364341 · 1364341
1 parent 4f85a22
commit 1364341
Show file tree

Hide file tree

Showing 7 changed files with 70 additions and 107 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,7 +11,7 @@
 - Enh #353: Update `bit` type according to main PR yiisoft/db#860 (@Tigrov) 
 - Enh #354: Refactor PHP type of `ColumnSchemaInterface` instances (@Tigrov)
 - Enh #356: Raise minimum PHP version to `^8.1` with minor refactoring (@Tigrov)
-- New #355: Implement `ColumnFactory` class (@Tigrov)
+- New #355, #368: Implement `ColumnFactory` class (@Tigrov)
 - Enh #359: Separate column type constants (@Tigrov)
 - Enh #359: Remove `Schema::TYPE_ARRAY` and `Schema::TYPE_STRUCTURED` constants (@Tigrov)
 - New #360: Realize `ColumnBuilder` class (@Tigrov)

diff --git a/src/Column/ColumnFactory.php b/src/Column/ColumnFactory.php
@@ -45,10 +45,9 @@ final class ColumnFactory extends AbstractColumnFactory
      * @link https://www.postgresql.org/docs/current/datatype.html#DATATYPE-TABLE
      *
      * @var string[]
-     *
-     * @psalm-suppress MissingClassConstType
+     * @psalm-var array<string, ColumnType::*>
      */
-    private const TYPE_MAP = [
+    protected const TYPE_MAP = [
         'bool' => ColumnType::BOOLEAN,
         'boolean' => ColumnType::BOOLEAN,
         'bit' => ColumnType::BIT,
@@ -115,46 +114,21 @@ final class ColumnFactory extends AbstractColumnFactory
         'jsonb' => ColumnType::JSON,
     ];
 
-    /**
-     * @psalm-param ColumnType::* $type
-     * @psalm-param ColumnInfo $info
-     * @psalm-suppress MoreSpecificImplementedParamType
-     * @psalm-suppress ArgumentTypeCoercion
-     * @psalm-suppress InvalidNamedArgument
-     * @psalm-suppress PossiblyInvalidArgument
-     */
-    public function fromType(string $type, array $info = []): ColumnSchemaInterface
+    protected function getColumnClass(string $type, array $info = []): string
     {
-        $dimension = $info['dimension'] ?? 0;
-        unset($info['dimension']);
-
-        if ($dimension > 0) {
-            $info['column'] ??= $this->fromType($type, $info);
-            return new ArrayColumnSchema(...$info, dimension: $dimension);
-        }
-
         return match ($type) {
-            ColumnType::BOOLEAN => new BooleanColumnSchema($type, ...$info),
-            ColumnType::BIT => new BitColumnSchema($type, ...$info),
-            ColumnType::TINYINT => new IntegerColumnSchema($type, ...$info),
-            ColumnType::SMALLINT => new IntegerColumnSchema($type, ...$info),
-            ColumnType::INTEGER => new IntegerColumnSchema($type, ...$info),
+            ColumnType::BOOLEAN => BooleanColumnSchema::class,
+            ColumnType::BIT => BitColumnSchema::class,
+            ColumnType::TINYINT => IntegerColumnSchema::class,
+            ColumnType::SMALLINT => IntegerColumnSchema::class,
+            ColumnType::INTEGER => IntegerColumnSchema::class,
             ColumnType::BIGINT => PHP_INT_SIZE !== 8
-                ? new BigIntColumnSchema($type, ...$info)
-                : new IntegerColumnSchema($type, ...$info),
-            ColumnType::BINARY => new BinaryColumnSchema($type, ...$info),
-            ColumnType::STRUCTURED => new StructuredColumnSchema($type, ...$info),
-            default => parent::fromType($type, $info),
+                ? BigIntColumnSchema::class
+                : IntegerColumnSchema::class,
+            ColumnType::BINARY => BinaryColumnSchema::class,
+            ColumnType::ARRAY => ArrayColumnSchema::class,
+            ColumnType::STRUCTURED => StructuredColumnSchema::class,
+            default => parent::getColumnClass($type, $info),
         };
     }
-
-    protected function getType(string $dbType, array $info = []): string
-    {
-        return self::TYPE_MAP[$dbType] ?? ColumnType::STRING;
-    }
-
-    protected function isDbType(string $dbType): bool
-    {
-        return isset(self::TYPE_MAP[$dbType]);
-    }
 }
diff --git a/src/Schema.php b/src/Schema.php
@@ -18,7 +18,6 @@
 use Yiisoft\Db\Exception\NotSupportedException;
 use Yiisoft\Db\Expression\Expression;
 use Yiisoft\Db\Helper\DbArrayHelper;
-use Yiisoft\Db\Pgsql\Column\ArrayColumnSchema;
 use Yiisoft\Db\Pgsql\Column\ColumnFactory;
 use Yiisoft\Db\Pgsql\Column\SequenceColumnSchemaInterface;
 use Yiisoft\Db\Schema\Builder\ColumnInterface;
@@ -44,8 +43,6 @@
  * Implements the PostgreSQL Server specific schema, supporting PostgreSQL Server version 9.6 and above.
  *
  * @psalm-type ColumnArray = array{
- *   table_schema: string,
- *   table_name: string,
  *   column_name: string,
  *   data_type: string,
  *   type_type: string|null,
@@ -60,7 +57,9 @@
  *   size: int|string|null,
  *   scale: int|string|null,
  *   contype: string|null,
- *   dimension: int|string
+ *   dimension: int|string,
+ *   schema: string,
+ *   table: string
  * }
  * @psalm-type ConstraintArray = array<
  *   array-key,
@@ -621,8 +620,6 @@ protected function findColumns(TableSchemaInterface $table): bool
 
         $sql = <<<SQL
         SELECT
-            d.nspname AS table_schema,
-            c.relname AS table_name,
             a.attname AS column_name,
             COALESCE(td.typname, tb.typname, t.typname) AS data_type,
             COALESCE(td.typtype, tb.typtype, t.typtype) AS type_type,
@@ -687,9 +684,12 @@ protected function findColumns(TableSchemaInterface $table): bool
             a.attnum;
         SQL;
 
+        $schemaName = $table->getSchemaName();
+        $tableName = $table->getName();
+
         $columns = $this->db->createCommand($sql, [
-            ':schemaName' => $table->getSchemaName(),
-            ':tableName' => $table->getName(),
+            ':schemaName' => $schemaName,
+            ':tableName' => $tableName,
         ])->queryAll();
 
         if (empty($columns)) {
@@ -698,9 +698,12 @@ protected function findColumns(TableSchemaInterface $table): bool
 
         /** @psalm-var ColumnArray $info */
         foreach ($columns as $info) {
-            /** @psalm-var ColumnArray $info */
             $info = array_change_key_case($info);
 
+            $info['schema'] = $schemaName;
+            $info['table'] = $tableName;
+
+            /** @psalm-var ColumnArray $info */
             $column = $this->loadColumnSchema($info);
 
             $table->column($info['column_name'], $column);
@@ -733,40 +736,43 @@ private function loadColumnSchema(array $info): ColumnSchemaInterface
             $dbType = $info['type_scheme'] . '.' . $dbType;
         }
 
-        $columns = [];
+        $columnInfo = [
+            'autoIncrement' => (bool) $info['is_autoinc'],
+            'comment' => $info['column_comment'],
+            'dbType' => $dbType,
+            'enumValues' => $info['enum_values'] !== null
+                ? explode(',', str_replace(["''"], ["'"], $info['enum_values']))
+                : null,
+            'name' => $info['column_name'],
+            'notNull' => !$info['is_nullable'],
+            'primaryKey' => $info['contype'] === 'p',
+            'scale' => $info['scale'] !== null ? (int) $info['scale'] : null,
+            'schema' => $info['schema'],
+            'size' => $info['size'] !== null ? (int) $info['size'] : null,
+            'table' => $info['table'],
+            'unique' => $info['contype'] === 'u',
+        ];
 
         if ($info['type_type'] === 'c') {
             $structured = $this->resolveTableName($dbType);
 
             if ($this->findColumns($structured)) {
-                $columns = $structured->getColumns();
+                $columnInfo['columns'] = $structured->getColumns();
             }
 
-            /** @psalm-suppress ArgumentTypeCoercion */
-            $column = $columnFactory
-                ->fromType(ColumnType::STRUCTURED, [
-                    'columns' => $columns,
-                    'dbType' => $dbType,
-                    'dimension' => (int) $info['dimension'],
-                ]);
+            $column = $columnFactory->fromType(ColumnType::STRUCTURED, $columnInfo);
         } else {
-            /** @psalm-suppress ArgumentTypeCoercion */
-            $column = $columnFactory
-                ->fromDbType($dbType, ['dimension' => (int) $info['dimension']]);
+            $column = $columnFactory->fromDbType($dbType, $columnInfo);
         }
 
-        /** @psalm-suppress DeprecatedMethod */
-        $column->name($info['column_name']);
-        $column->notNull(!$info['is_nullable']);
-        $column->autoIncrement((bool) $info['is_autoinc']);
-        $column->comment($info['column_comment']);
-        $column->enumValues($info['enum_values'] !== null
-            ? explode(',', str_replace(["''"], ["'"], $info['enum_values']))
-            : null);
-        $column->primaryKey($info['contype'] === 'p');
-        $column->unique($info['contype'] === 'u');
-        $column->scale($info['scale'] !== null ? (int) $info['scale'] : null);
-        $column->size($info['size'] !== null ? (int) $info['size'] : null);
+        $dimension = (int) $info['dimension'];
+
+        if ($dimension > 0) {
+            $columnInfo['column'] = $column;
+            $columnInfo['dimension'] = $dimension;
+
+            $column = $columnFactory->fromType(ColumnType::ARRAY, $columnInfo);
+        }
 
         /**
          * pg_get_serial_sequence() doesn't track DEFAULT value change.
@@ -783,12 +789,6 @@ private function loadColumnSchema(array $info): ColumnSchemaInterface
             } elseif ($info['sequence_name'] !== null) {
                 $column->sequenceName($this->resolveTableName($info['sequence_name'])->getFullName());
             }
-        } elseif ($column instanceof ArrayColumnSchema) {
-            /** @var ColumnSchemaInterface $arrayColumn */
-            $arrayColumn = $column->getColumn();
-            $arrayColumn->enumValues($column->getEnumValues());
-            $arrayColumn->scale($column->getScale());
-            $arrayColumn->size($column->getSize());
         }
 
         $column->defaultValue($this->normalizeDefaultValue($defaultValue, $column));
@@ -799,7 +799,9 @@ private function loadColumnSchema(array $info): ColumnSchemaInterface
 
             if (is_array($defaultValue)) {
                 foreach ($column->getColumns() as $structuredColumnName => $structuredColumn) {
-                    $structuredColumn->defaultValue($defaultValue[$structuredColumnName] ?? null);
+                    if (isset($defaultValue[$structuredColumnName])) {
+                        $structuredColumn->defaultValue($defaultValue[$structuredColumnName]);
+                    }
                 }
             }
         }

diff --git a/tests/ColumnFactoryTest.php b/tests/ColumnFactoryTest.php
@@ -4,6 +4,7 @@
 
 namespace Yiisoft\Db\Pgsql\Tests;
 
+use Yiisoft\Db\Constant\ColumnType;
 use Yiisoft\Db\Pgsql\Column\ArrayColumnSchema;
 use Yiisoft\Db\Pgsql\Tests\Support\TestTrait;
 use Yiisoft\Db\Tests\AbstractColumnFactoryTest;
@@ -23,8 +24,8 @@ public function testFromDbType(string $dbType, string $expectedType, string $exp
         $db = $this->getConnection();
         $columnFactory = $db->getSchema()->getColumnFactory();
 
-        // With dimension
-        $column = $columnFactory->fromDbType($dbType, ['dimension' => 1]);
+        // For array type
+        $column = $columnFactory->fromType(ColumnType::ARRAY, ['dbType' => $dbType]);
 
         $this->assertInstanceOf(ArrayColumnSchema::class, $column);
         $this->assertInstanceOf($expectedInstanceOf, $column->getColumn());
@@ -62,8 +63,8 @@ public function testFromType(string $type, string $expectedType, string $expecte
         $db = $this->getConnection();
         $columnFactory = $db->getSchema()->getColumnFactory();
 
-        // With dimension
-        $column = $columnFactory->fromType($type, ['dimension' => 1]);
+        // For array type
+        $column = $columnFactory->fromType(ColumnType::ARRAY, ['column' => $columnFactory->fromType($type)]);
 
         $this->assertInstanceOf(ArrayColumnSchema::class, $column);
         $this->assertInstanceOf($expectedInstanceOf, $column->getColumn());

diff --git a/tests/Provider/ColumnFactoryProvider.php b/tests/Provider/ColumnFactoryProvider.php
@@ -92,4 +92,12 @@ public static function definitions(): array
 
         return $definitions;
     }
+
+    public static function pseudoTypes(): array
+    {
+        $result = parent::pseudoTypes();
+        $result['ubigpk'][2] = IntegerColumnSchema::class;
+
+        return $result;
+    }
 }
diff --git a/tests/Provider/SchemaProvider.php b/tests/Provider/SchemaProvider.php
@@ -504,16 +504,6 @@ public static function columns(): array
         ];
     }
 
-    public static function columnsTypeChar(): array
-    {
-        $columnsTypeChar = parent::columnsTypeChar();
-
-        $columnsTypeChar[0][3] = 'bpchar';
-        $columnsTypeChar[1][3] = 'varchar';
-
-        return $columnsTypeChar;
-    }
-
     public static function constraints(): array
     {
         $constraints = parent::constraints();

diff --git a/tests/SchemaTest.php b/tests/SchemaTest.php
@@ -182,18 +182,6 @@ public function testGetSchemaNames(): void
         $db->close();
     }
 
-    /**
-     * @dataProvider \Yiisoft\Db\Pgsql\Tests\Provider\SchemaProvider::columnsTypeChar
-     */
-    public function testGetStringFieldsSize(
-        string $columnName,
-        string $columnType,
-        int|null $columnSize,
-        string $columnDbType
-    ): void {
-        parent::testGetStringFieldsSize($columnName, $columnType, $columnSize, $columnDbType);
-    }
-
     /**
      * @throws Exception
      * @throws InvalidConfigException