diff --git a/.github/workflows/apply-labels.yml b/.github/workflows/apply-labels.yml index 3e5f992d..931bccf1 100644 --- a/.github/workflows/apply-labels.yml +++ b/.github/workflows/apply-labels.yml @@ -14,6 +14,9 @@ name: 🏷️ Add labels jobs: label: + permissions: + contents: read + pull-requests: write uses: cycle/gh-actions/.github/workflows/apply-labels.yml@v4.0.0 with: os: ubuntu-latest diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 26377678..2ebbd5ed 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -54,12 +54,11 @@ jobs: strict: true - name: 🛠️ Setup PHP - uses: shivammathur/setup-php@2.30.2 + uses: shivammathur/setup-php@v2 with: php-version: ${{ matrix.php-version }} extensions: mbstring, pdo, pdo_sqlite ini-values: error_reporting=E_ALL - coverage: xdebug - name: 🛠️ Setup problem matchers run: | @@ -68,20 +67,10 @@ jobs: - name: 🤖 Validate composer.json and composer.lock run: composer validate --ansi --strict - - name: 🔍 Get composer cache directory - uses: cycle/gh-actions/actions/composer/get-cache-directory@v4.0.0 - - - name: ♻️ Restore cached dependencies installed with composer - uses: actions/cache@v4.0.2 - with: - path: ${{ env.COMPOSER_CACHE_DIR }} - key: php-${{ matrix.php-version }}-composer-${{ matrix.dependencies }}-${{ hashFiles('composer.lock') }} - restore-keys: php-${{ matrix.php-version }}-composer-${{ matrix.dependencies }}- - - - name: 📥 Install "${{ matrix.dependencies }}" dependencies - uses: cycle/gh-actions/actions/composer/install@v4.0.0 + - name: 📥 Install dependencies with composer + uses: ramsey/composer-install@v3 with: - dependencies: ${{ matrix.dependencies }} + dependency-versions: ${{ matrix.dependencies }} - name: 🔍 Run composer-normalize run: composer normalize --ansi --dry-run diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 91905d50..500cb4f9 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -16,55 +16,48 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - php-versions: ['8.0', '8.1', '8.2', '8.3'] + php-versions: ['8.0', '8.1', '8.2', '8.3', '8.4'] steps: - name: Install ODBC driver. run: | sudo curl https://packages.microsoft.com/config/ubuntu/$(lsb_release -rs)/prod.list | sudo tee /etc/apt/sources.list.d/mssql-release.list sudo ACCEPT_EULA=Y apt-get install -y msodbcsql18 - - name: Checkout + - name: 📦 Checkout uses: actions/checkout@v2 - - name: Setup DB services + - name: 🛠️ Setup DB services run: | cd tests docker compose up -d cd .. - - name: Setup PHP ${{ matrix.php-versions }} + - name: 🛠️ Setup PHP ${{ matrix.php-versions }} uses: shivammathur/setup-php@v2 with: php-version: ${{ matrix.php-versions }} coverage: pcov tools: pecl extensions: mbstring, pdo, pdo_sqlite, pdo_pgsql, pdo_sqlsrv, pdo_mysql - - name: Get Composer Cache Directory - id: composer-cache - run: echo "::set-output name=dir::$(composer config cache-files-dir)" - - name: Restore Composer Cache - uses: actions/cache@v2 - with: - path: ${{ steps.composer-cache.outputs.dir }} - key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.json') }} - restore-keys: ${{ runner.os }}-composer- - - name: Install dependencies with composer - if: matrix.php-versions != '8.3' - run: composer update --prefer-dist --no-interaction --no-progress --optimize-autoloader --ansi + - name: 🤖 Validate composer.json and composer.lock + run: composer validate --ansi --strict - - name: Install dependencies with composer php 8.3 - if: matrix.php-versions == '8.3' - run: composer update --ignore-platform-reqs --prefer-dist --no-interaction --no-progress --optimize-autoloader --ansi + - name: 📥 Install dependencies with composer + uses: ramsey/composer-install@v3 + with: + dependency-versions: "highest" - - name: Execute Tests + - name: 🚀 Execute Tests run: | vendor/bin/phpunit --coverage-clover=coverage.clover - - name: Upload coverage to Codecov + + - name: 🦆 Upload coverage to Codecov continue-on-error: true # if is fork uses: codecov/codecov-action@v1 with: token: ${{ secrets.CODECOV_TOKEN }} file: ./coverage.clover - - name: Upload coverage to Scrutinizer + + - name: 🦆 Upload coverage to Scrutinizer continue-on-error: true # if is fork uses: sudo-bot/action-scrutinizer@latest with: @@ -75,36 +68,27 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - php-versions: ['8.0', '8.1', '8.2', '8.3'] + php-versions: ['8.0', '8.1', '8.2', '8.3', '8.4'] steps: - - name: Checkout + - name: 📦 Checkout uses: actions/checkout@v2 - - name: Setup PHP ${{ matrix.php-versions }} + - name: 🛠️ Setup PHP ${{ matrix.php-versions }} uses: shivammathur/setup-php@v2 with: php-version: ${{ matrix.php-versions }} coverage: pcov tools: pecl extensions: mbstring, pdo, pdo_sqlite - - name: Get Composer Cache Directory - id: composer-cache - run: echo "::set-output name=dir::$(composer config cache-files-dir)" - - name: Restore Composer Cache - uses: actions/cache@v2 - with: - path: ${{ steps.composer-cache.outputs.dir }} - key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.json') }} - restore-keys: ${{ runner.os }}-composer- - - name: Install dependencies with composer - if: matrix.php-versions != '8.3' - run: composer update --prefer-dist --no-interaction --no-progress --optimize-autoloader --ansi + - name: 🤖 Validate composer.json and composer.lock + run: composer validate --ansi --strict - - name: Install dependencies with composer php 8.3 - if: matrix.php-versions == '8.3' - run: composer update --ignore-platform-reqs --prefer-dist --no-interaction --no-progress --optimize-autoloader --ansi + - name: 📥 Install dependencies with composer + uses: ramsey/composer-install@v3 + with: + dependency-versions: "highest" - - name: Execute Tests + - name: 🚀 Execute Tests env: DB: sqlite run: | diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 603162a6..4440ea0a 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -895,6 +895,7 @@ + comment]]> type]]> @@ -905,6 +906,7 @@ + @@ -1299,10 +1301,12 @@ + + @@ -1326,6 +1330,7 @@ + + + diff --git a/src/ColumnInterface.php b/src/ColumnInterface.php index b306ccb8..1fbe8284 100644 --- a/src/ColumnInterface.php +++ b/src/ColumnInterface.php @@ -13,6 +13,9 @@ /** * Represents table schema column abstraction. + * + * @method string getComment() Get column comment. + * An empty string will be returned if the feature is not supported by the driver. */ interface ColumnInterface { diff --git a/src/Driver/MySQL/Schema/MySQLColumn.php b/src/Driver/MySQL/Schema/MySQLColumn.php index bc530fb2..f9a3b084 100644 --- a/src/Driver/MySQL/Schema/MySQLColumn.php +++ b/src/Driver/MySQL/Schema/MySQLColumn.php @@ -32,11 +32,12 @@ * @method $this|AbstractColumn bigInteger(int $size, bool $unsigned = false, $zerofill = false) * @method $this|AbstractColumn unsigned(bool $value) * @method $this|AbstractColumn zerofill(bool $value) + * @method $this|AbstractColumn comment(string $value) */ class MySQLColumn extends AbstractColumn { /** - * Default timestamp expression (driver specific). + * Default timestamp expression (). */ public const DATETIME_NOW = 'CURRENT_TIMESTAMP'; @@ -177,6 +178,12 @@ class MySQLColumn extends AbstractColumn #[ColumnAttribute(self::INTEGER_TYPES)] protected bool $zerofill = false; + /** + * Column comment. + */ + #[ColumnAttribute] + protected string $comment = ''; + /** * @psalm-param non-empty-string $table */ @@ -185,6 +192,7 @@ public static function createInstance(string $table, array $schema, ?\DateTimeZo $column = new self($table, $schema['Field'], $timezone); $column->type = $schema['Type']; + $column->comment = $schema['Comment']; $column->nullable = \strtolower($schema['Null']) === 'yes'; $column->defaultValue = $schema['Default']; $column->autoIncrement = \stripos($schema['Extra'], 'auto_increment') !== false; @@ -288,8 +296,9 @@ public function sqlStatement(DriverInterface $driver): string $statement = parent::sqlStatement($driver); $this->defaultValue = $defaultValue; - if ($this->autoIncrement) { - return "{$statement} AUTO_INCREMENT"; + + if ($this->comment !== '') { + return "{$statement} COMMENT {$driver->quote($this->comment)}"; } return $statement; @@ -355,6 +364,11 @@ public function binary(int $size = 0): self return $this; } + public function getComment(): string + { + return $this->comment; + } + protected static function isEnum(AbstractColumn $column): bool { return $column->getAbstractType() === 'enum' || $column->getAbstractType() === 'set'; @@ -381,11 +395,12 @@ protected function formatDatetime( private function sqlStatementInteger(DriverInterface $driver): string { return \sprintf( - '%s %s(%s)%s%s%s%s%s', + '%s %s(%s)%s%s%s%s%s%s', $driver->identifier($this->name), $this->type, $this->size, $this->unsigned ? ' UNSIGNED' : '', + $this->comment !== '' ? " COMMENT {$driver->quote($this->comment)}" : '', $this->zerofill ? ' ZEROFILL' : '', $this->nullable ? ' NULL' : ' NOT NULL', $this->defaultValue !== null ? " DEFAULT {$this->quoteDefault($driver)}" : '', diff --git a/src/Driver/Postgres/PostgresHandler.php b/src/Driver/Postgres/PostgresHandler.php index 8015269a..5e9bfbc8 100644 --- a/src/Driver/Postgres/PostgresHandler.php +++ b/src/Driver/Postgres/PostgresHandler.php @@ -117,18 +117,21 @@ public function alterColumn( //Postgres columns should be altered using set of operations $operations = $column->alterOperations($this->driver, $initial); - if (empty($operations)) { - return; + if (\count($operations) > 0) { + //Postgres columns should be altered using set of operations + $query = \sprintf( + 'ALTER TABLE %s %s', + $this->identify($table), + \trim(\implode(', ', $operations), ', '), + ); + + $this->run($query); } - //Postgres columns should be altered using set of operations - $query = \sprintf( - 'ALTER TABLE %s %s', - $this->identify($table), - \trim(\implode(', ', $operations), ', '), - ); - - $this->run($query); + $operation = $column->commentOperation($this->driver, $initial); + if ($operation !== null) { + $this->run($operation); + } } public function enableForeignKeyConstraints(): void @@ -141,6 +144,26 @@ public function disableForeignKeyConstraints(): void $this->run('SET CONSTRAINTS ALL DEFERRED;'); } + public function createTable(AbstractTable $table): void + { + if (!$table instanceof PostgresTable) { + throw new SchemaException('Postgres handler can work only with Postgres table'); + } + + parent::createTable($table); + + foreach ($table->getColumns() as $column) { + $this->createComment($column); + } + } + + public function createComment(PostgresColumn $column): void + { + if ($column->getComment() !== '') { + $this->run($column->createComment($this->driver)); + } + } + /** * @psalm-param non-empty-string $statement */ diff --git a/src/Driver/Postgres/Schema/PostgresColumn.php b/src/Driver/Postgres/Schema/PostgresColumn.php index 6f6748b5..49bc1cc1 100644 --- a/src/Driver/Postgres/Schema/PostgresColumn.php +++ b/src/Driver/Postgres/Schema/PostgresColumn.php @@ -11,10 +11,10 @@ namespace Cycle\Database\Driver\Postgres\Schema; -use Cycle\Database\Driver\DriverInterface; -use Cycle\Database\Exception\SchemaException; use Cycle\Database\Injection\Fragment; use Cycle\Database\Schema\AbstractColumn; +use Cycle\Database\Driver\DriverInterface; +use Cycle\Database\Exception\SchemaException; use Cycle\Database\Schema\Attribute\ColumnAttribute; /** @@ -44,6 +44,7 @@ * @method $this smallSerial() * @method $this serial() * @method $this bigSerial() + * @method $this comment(string $value) */ class PostgresColumn extends AbstractColumn { @@ -289,6 +290,12 @@ class PostgresColumn extends AbstractColumn #[ColumnAttribute(['numeric'])] protected int $scale = 0; + /** + * Column comment. + */ + #[ColumnAttribute] + protected string $comment = ''; + /** * Internal field to determine if the serial is PK. */ @@ -317,6 +324,7 @@ public static function createInstance( }; $column->defaultValue = $schema['column_default']; + $column->comment = (string) $schema['description']; $column->nullable = $schema['is_nullable'] === 'YES'; if ( @@ -533,6 +541,19 @@ public function sqlStatement(DriverInterface $driver): string return $statement; } + /** + * @psalm-return non-empty-string|null + */ + public function commentOperation(DriverInterface $driver, PostgresColumn $initial): ?string + { + //Comment + if ($initial->comment !== $this->comment) { + return $this->createComment($driver); + } + + return null; + } + /** * Generate set of operations need to change column. */ @@ -616,6 +637,22 @@ public function compare(AbstractColumn $initial): bool ); } + public function getComment(): string + { + return $this->comment; + } + + /** + * @psalm-return non-empty-string + */ + public function createComment(DriverInterface $driver): string + { + $tableName = $driver->identifier($this->getTable()); + $identifier = $driver->identifier($this->getName()); + + return "COMMENT ON COLUMN {$tableName}.{$identifier} IS " . $driver->quote($this->comment); + } + protected static function isJson(AbstractColumn $column): bool { return $column->getAbstractType() === 'json' || $column->getAbstractType() === 'jsonb'; diff --git a/src/Driver/Postgres/Schema/PostgresTable.php b/src/Driver/Postgres/Schema/PostgresTable.php index da00c4be..676ec899 100644 --- a/src/Driver/Postgres/Schema/PostgresTable.php +++ b/src/Driver/Postgres/Schema/PostgresTable.php @@ -20,6 +20,8 @@ /** * @property PostgresDriver $driver + * + * @method PostgresColumn[] getColumns() */ class PostgresTable extends AbstractTable { @@ -93,12 +95,18 @@ protected function fetchColumns(): array )->fetchColumn(); $query = $this->driver->query( - 'SELECT * - FROM information_schema.columns - JOIN pg_type - ON (pg_type.typname = columns.udt_name) - WHERE table_schema = ? - AND table_name = ?', + 'SELECT columns.*, pg_type.*, pg_description.description + FROM information_schema.columns + JOIN pg_catalog.pg_type + ON (pg_type.typname = columns.udt_name) + JOIN pg_catalog.pg_statio_all_tables + ON (pg_statio_all_tables.relname = columns.table_name + AND pg_statio_all_tables.schemaname = columns.table_schema) + LEFT JOIN pg_catalog.pg_description + ON (pg_description.objoid = pg_statio_all_tables.relid + AND pg_description.objsubid = columns.ordinal_position) + WHERE columns.table_schema = ? + AND columns.table_name = ?', [$tableSchema, $tableName], ); diff --git a/src/Schema/AbstractColumn.php b/src/Schema/AbstractColumn.php index 5d3f2347..a9078841 100644 --- a/src/Schema/AbstractColumn.php +++ b/src/Schema/AbstractColumn.php @@ -785,4 +785,13 @@ protected function formatDatetime( default => $value, }; } + + /** + * Get column comment. + * An empty string will be returned if the feature is not supported by the driver. + */ + public function getComment(): string + { + return ''; + } } diff --git a/tests/Database/Functional/Driver/Common/BaseTest.php b/tests/Database/Functional/Driver/Common/BaseTest.php index f7b40549..27929de9 100644 --- a/tests/Database/Functional/Driver/Common/BaseTest.php +++ b/tests/Database/Functional/Driver/Common/BaseTest.php @@ -41,6 +41,7 @@ public function setUp(): void public function tearDown(): void { + $this->disableProfiling(); $this->dropDatabase($this->database); } diff --git a/tests/Database/Functional/Driver/Common/Connection/BaseConnectionTest.php b/tests/Database/Functional/Driver/Common/Connection/BaseConnectionTest.php index a2c145f6..76e901a4 100644 --- a/tests/Database/Functional/Driver/Common/Connection/BaseConnectionTest.php +++ b/tests/Database/Functional/Driver/Common/Connection/BaseConnectionTest.php @@ -9,7 +9,9 @@ use Cycle\Database\Tests\Stub\Driver\MysqlWrapDriver; use Cycle\Database\Tests\Stub\Driver\PostgresWrapDriver; use Cycle\Database\Tests\Stub\Driver\SQLiteWrapDriver; +use Cycle\Database\Tests\Utils\DontGenerateAttribute; +#[DontGenerateAttribute] abstract class BaseConnectionTest extends BaseTest { public function setUp(): void diff --git a/tests/Database/Functional/Driver/Common/Schema/CommentTest.php b/tests/Database/Functional/Driver/Common/Schema/CommentTest.php new file mode 100644 index 00000000..963065c1 --- /dev/null +++ b/tests/Database/Functional/Driver/Common/Schema/CommentTest.php @@ -0,0 +1,125 @@ +schema('table'); + $this->assertFalse($schema->exists()); + + $column = $schema->string('target'); + $column->comment('foo'); + + $schema->save(); + + $schema = $this->schema('table'); + $this->assertTrue($schema->exists()); + + $column2 = $schema->column('target'); + $this->assertTrue($column2->compare($column)); + self::assertSame('foo', $column2->getComment()); + } + + public function testChangeComment(): void + { + $schema = $this->schema('table'); + $this->assertFalse($schema->exists()); + + $column = $schema->string('target'); + $column->comment('foo'); + + $schema->save(); + + $schema = $this->schema('table'); + $this->assertTrue($schema->exists()); + + $column2 = $schema->column('target'); + $column2->comment('bar'); + + $schema->save(); + + $this->assertTrue($schema->column('target')->compare($column2)); + self::assertSame('bar', $schema->column('target')->getComment()); + } + + public function testChangeCommentToEmpty(): void + { + $schema = $this->schema('table'); + $column = $schema->string('target'); + $column->comment('foo'); + + $schema->save(); + + $schema = $this->schema('table'); + $this->assertTrue($schema->exists()); + + $column2 = $schema->column('target'); + self::assertSame('foo', $column2->getComment()); + + $column2->comment(''); + + $schema->save(); + + $schema = $this->schema('table'); + $column3 = $schema->column('target'); + self::assertSame('', $column3->getComment()); + } + + public function testCommentWithAutoIncrement(): void + { + $schema = $this->schema('table'); + $column = $schema->primary('target'); + $column->comment('foo'); + + $schema->save(); + + $schema = $this->schema('table'); + $this->assertTrue($schema->exists()); + + $column2 = $schema->column('target'); + $this->assertTrue($column2->compare($column)); + self::assertSame('foo', $column2->getComment()); + } + + public function testSQLInjection(): void + { + $schema = $this->schema('table'); + $column = $schema->string('target'); + $column->comment('f"o\'o`'); + + $schema->save(); + + $schema = $this->schema('table'); + $this->assertTrue($schema->exists()); + + $column2 = $schema->column('target'); + $this->assertTrue($column2->compare($column)); + self::assertSame('f"o\'o`', $column2->getComment()); + } + + public function testSQLInjectionIntegers(): void + { + $schema = $this->schema('table'); + $column = $schema->primary('target'); + $column->comment('f"o\'o`'); + + $schema->save(); + + $schema = $this->schema('table'); + $this->assertTrue($schema->exists()); + + $column2 = $schema->column('target'); + $this->assertTrue($column2->compare($column)); + self::assertSame('f"o\'o`', $column2->getComment()); + } +} diff --git a/tests/Database/Functional/Driver/MySQL/Schema/CommentTest.php b/tests/Database/Functional/Driver/MySQL/Schema/CommentTest.php new file mode 100644 index 00000000..4a0f0c5d --- /dev/null +++ b/tests/Database/Functional/Driver/MySQL/Schema/CommentTest.php @@ -0,0 +1,17 @@ + [__DIR__ . '/Database/Functional/Driver/Common'], @@ -56,7 +57,9 @@ continue; } - echo "Found {$class->getName()}\n"; + if ($class->getAttributes(DontGenerateAttribute::class) !== []) { + continue; + } $path = \str_replace( [\str_replace('\\', '/', __DIR__), 'Database/Functional/Driver/Common/'], @@ -64,10 +67,15 @@ \str_replace('\\', '/', $class->getFileName()), ); - $path = \ltrim($path, '/'); + $path = ltrim($path, '/'); foreach ($databases as $driver => $details) { - $filename = \sprintf('%s%s', $details['directory'], $path); + $filename = $details['directory'] . $path; + if (\file_exists($filename)) { + continue; + } + echo "Processing $filename\n"; + $dir = \pathinfo($filename, PATHINFO_DIRNAME); $namespace = \str_replace(