fix: update dependencies for dbal v3#1172
Merged
tikhanovichA merged 9 commits intodevelopfrom Feb 26, 2026
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughMigrate code to Doctrine DBAL 3 patterns (use Result and Exception types, replace fetch/fetchAll with fetchAssociative/fetchAllAssociative, use executeQuery), add Flysystem local adapter constant, tighten several method signatures, and add test guards/teardown resilience; require PHP ^8.0. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
tikhanovichA
requested review from
a team,
Karol-Stelmaczonek,
KirylHatalski,
bartlomiejmarszal and
pnal
and removed request for
a team
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
common/uri/class.MicrotimeUriProvider.php (1)
59-72:⚠️ Potential issue | 🟠 MajorReset
$uriExisteach iteration to prevent sticky true state.Line 69 sets
$uriExist = trueon collision, but it is never reset inside the loop. After one collision, the loop can become non-terminating.Proposed fix
$uriExist = false; do { + $uriExist = false; list($usec, $sec) = explode(" ", microtime()); $uri = $modelUri . 'i' . (str_replace(".", "", $sec . "" . $usec)); $sqlResult = $dbWrapper->query( "SELECT COUNT(subject) AS num FROM statements WHERE subject = '" . $uri . "'" ); if (($row = $sqlResult->fetchAssociative()) !== false) { $found = (int)$row['num']; if ($found > 0) { $uriExist = true; } } } while ($uriExist);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@common/uri/class.MicrotimeUriProvider.php` around lines 59 - 72, The loop in MicrotimeUriProvider uses $uriExist to control repetition but never resets it after a collision, causing it to remain true forever; fix by reinitializing $uriExist = false at the start of each do iteration (or derive $uriExist solely from the current query result) so that each generated $uri is re-checked correctly in the block that builds $uri, runs the SELECT COUNT(...), and inspects $row['num'].
🧹 Nitpick comments (4)
core/kernel/uri/class.DatabaseSerialUriProvider.php (1)
75-77: UsefetchOne()for single-value retrieval instead offetchAssociative()withcurrent().Line 77 uses
current($row)on an associative array, introducing implicit dependence on key order. The codebase already usesfetchOne()for scalar value retrieval (e.g., in class.DbWrapper.php and class.Class.php). Refactor to use this more explicit and order-independent API:$returnValue = (string)($sth->fetchOne() ?? '');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/kernel/uri/class.DatabaseSerialUriProvider.php` around lines 75 - 77, In DatabaseSerialUriProvider, replace the scalar retrieval using $sth->fetchAssociative() and current($row) with the explicit single-value API $sth->fetchOne() to avoid reliance on associative key order; change the assignment to set $returnValue from the result of $sth->fetchOne() (casting to string and defaulting to empty string if null) instead of using fetchAssociative()/current().test/unit/extension/ComposerInfoTest.php (1)
44-49: GuardsetAccessible(true)for backward compatibility with PHP <8.1.
ReflectionProperty::setAccessible(true)became a no-op in PHP 8.1.0 and will be deprecated in PHP 8.5+. Wrap it with a version check to support PHP 8.0 and earlier without introducing deprecation warnings on newer versions.Proposed change
foreach (['availablePackages', 'jsons', 'locks'] as $propName) { if ($ref->hasProperty($propName)) { $prop = $ref->getProperty($propName); - $prop->setAccessible(true); + if (PHP_VERSION_ID < 80100) { + $prop->setAccessible(true); + } $prop->setValue(null); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/unit/extension/ComposerInfoTest.php` around lines 44 - 49, The test calls ReflectionProperty::setAccessible(true) for properties 'availablePackages','jsons','locks' via a ReflectionClass in ComposerInfoTest; guard these calls with a PHP version check (e.g. only call setAccessible when PHP_VERSION_ID < 80100) so the setAccessible(true) invocation is skipped on PHP 8.1+ and avoids deprecation/no-op warnings — wrap the $prop->setAccessible(true) call inside that conditional in the loop that uses $ref->getProperty(...) so behavior remains the same on older PHP while silent on newer versions.common/oatbox/filesystem/FileSystemService.php (1)
45-47: Reuse the new local-adapter constant to remove duplication.You introduced
FLYSYSTEM_LOCAL_ADAPTER, butregisterLocalFileSystem()still hardcodesLocalFilesystemAdapter::class. Reusing the constant keeps adapter wiring centralized.Proposed refactor
- 'class' => LocalFilesystemAdapter::class, + 'class' => self::FLYSYSTEM_LOCAL_ADAPTER,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@common/oatbox/filesystem/FileSystemService.php` around lines 45 - 47, registerLocalFileSystem() still hardcodes LocalFilesystemAdapter::class instead of using the newly added FLYSYSTEM_LOCAL_ADAPTER constant; update registerLocalFileSystem() to reference self::FLYSYSTEM_LOCAL_ADAPTER (or static::FLYSYSTEM_LOCAL_ADAPTER if appropriate) when wiring the adapter so the adapter class is centralized via the constant and duplication is removed.core/kernel/persistence/smoothsql/class.Class.php (1)
111-112: Simplify the existence check and remove the unused loop variable.Line 118 assigns
$rowwithout using it. A direct fetch check is clearer and avoids static-analysis noise.Proposed refactor
- while (($row = $result->fetchAssociative()) !== false) { - $returnValue = true; - break; - } + $returnValue = $result->fetchAssociative() !== false;Also applies to: 118-120
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/kernel/persistence/smoothsql/class.Class.php` around lines 111 - 112, The existence check uses an unused loop variable ($row) and can be simplified: when calling $this->getPersistence()->query($query, [...]) store the Result in $result and then directly call $result->fetch() (or check truthiness of the fetch) instead of iterating or assigning $row; remove the unnecessary foreach/loop and unused $row variable in the method in class.Class.php to make the check concise and silence static-analysis warnings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@core/kernel/uri/MicrotimeRandUriProvider.php`:
- Around line 78-82: The code in MicrotimeRandUriProvider.php interpolates $uri
into the SQL for the statements lookup; change the query to a
parameterized/prepared one (use the persistence layer's parameter binding, e.g.,
executeQuery/prepare + execute with params) instead of string concatenation so
the subject value is bound as a parameter when calling getPersistence()->query
(or prepare) and then fetchAssociative() as before; update the block where
$sqlResult is created and where $uri is referenced to use the bound parameter.
In `@core/resource/ResourceCollection.php`:
- Around line 146-155: The code uses $results->rowCount() to set
$this->endOfClass which is unreliable for SELECTs; change the logic to fetch the
rows first (use the result of $results->fetchAllAssociative()), compute the
count with count($rows) and set $this->endOfClass = $count < $this->cacheSize,
then iterate over $rows to push into $this->resources and update $this->lastId
(replace references to $results->fetchAllAssociative() in the foreach with the
fetched $rows variable).
---
Outside diff comments:
In `@common/uri/class.MicrotimeUriProvider.php`:
- Around line 59-72: The loop in MicrotimeUriProvider uses $uriExist to control
repetition but never resets it after a collision, causing it to remain true
forever; fix by reinitializing $uriExist = false at the start of each do
iteration (or derive $uriExist solely from the current query result) so that
each generated $uri is re-checked correctly in the block that builds $uri, runs
the SELECT COUNT(...), and inspects $row['num'].
---
Nitpick comments:
In `@common/oatbox/filesystem/FileSystemService.php`:
- Around line 45-47: registerLocalFileSystem() still hardcodes
LocalFilesystemAdapter::class instead of using the newly added
FLYSYSTEM_LOCAL_ADAPTER constant; update registerLocalFileSystem() to reference
self::FLYSYSTEM_LOCAL_ADAPTER (or static::FLYSYSTEM_LOCAL_ADAPTER if
appropriate) when wiring the adapter so the adapter class is centralized via the
constant and duplication is removed.
In `@core/kernel/persistence/smoothsql/class.Class.php`:
- Around line 111-112: The existence check uses an unused loop variable ($row)
and can be simplified: when calling $this->getPersistence()->query($query,
[...]) store the Result in $result and then directly call $result->fetch() (or
check truthiness of the fetch) instead of iterating or assigning $row; remove
the unnecessary foreach/loop and unused $row variable in the method in
class.Class.php to make the check concise and silence static-analysis warnings.
In `@core/kernel/uri/class.DatabaseSerialUriProvider.php`:
- Around line 75-77: In DatabaseSerialUriProvider, replace the scalar retrieval
using $sth->fetchAssociative() and current($row) with the explicit single-value
API $sth->fetchOne() to avoid reliance on associative key order; change the
assignment to set $returnValue from the result of $sth->fetchOne() (casting to
string and defaulting to empty string if null) instead of using
fetchAssociative()/current().
In `@test/unit/extension/ComposerInfoTest.php`:
- Around line 44-49: The test calls ReflectionProperty::setAccessible(true) for
properties 'availablePackages','jsons','locks' via a ReflectionClass in
ComposerInfoTest; guard these calls with a PHP version check (e.g. only call
setAccessible when PHP_VERSION_ID < 80100) so the setAccessible(true) invocation
is skipped on PHP 8.1+ and avoids deprecation/no-op warnings — wrap the
$prop->setAccessible(true) call inside that conditional in the loop that uses
$ref->getProperty(...) so behavior remains the same on older PHP while silent on
newer versions.
ℹ️ Review info
Configuration used: Path: https://raw.githubusercontent.com/oat-sa/tao-code-quality/main/coderabbit/php/authoring/v1/.coderabbit.yaml (via .coderabbit.yaml)
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (24)
common/oatbox/filesystem/FileSystemService.phpcommon/persistence/class.SqlKvDriver.phpcommon/persistence/sql/class.Platform.phpcommon/persistence/sql/class.QueryIterator.phpcommon/persistence/sql/dbal/class.Driver.phpcommon/uri/class.MicrotimeUriProvider.phpcore/kernel/api/class.ModelExporter.phpcore/kernel/classes/class.DbWrapper.phpcore/kernel/impl/class.ApiModelOO.phpcore/kernel/persistence/smoothsql/class.Class.phpcore/kernel/persistence/smoothsql/class.Resource.phpcore/kernel/persistence/smoothsql/search/GateWay.phpcore/kernel/persistence/smoothsql/search/TaoResultSet.phpcore/kernel/uri/MicrotimeRandUriProvider.phpcore/kernel/uri/class.DatabaseSerialUriProvider.phpcore/resource/ResourceCollection.phptest/integration/RdfExportTest.phptest/integration/common/persistence/sql/UpdateMultipleTestAbstract.phptest/integration/common/persistence/sql/dbal/DriverTest.phptest/integration/helpers/FileSerializerMigrationHelperTest.phptest/integration/model/persistence/starsql/ClassTest.phptest/unit/core/DependencyInjection/ContainerBuilderTest.phptest/unit/extension/ComposerInfoTest.phptest/unit/model/persistence/smoothsql/SmoothModelIteratorTest.php
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/resource/ResourceCollection.php (1)
1-1:⚠️ Potential issue | 🟡 MinorAdd
declare(strict_types=1)— coding guideline violation.The file does not declare strict types.
Proposed fix
<?php + +declare(strict_types=1); + /** * This program is free software; ...As per coding guidelines: "Use strict types
declare(strict_types=1);in all PHP files."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/resource/ResourceCollection.php` at line 1, Add a strict types declaration to this PHP file by inserting declare(strict_types=1); immediately after the opening <?php tag in ResourceCollection.php so the file follows the coding guideline; ensure the declaration appears before any namespace, use statements, or class definitions (e.g., class ResourceCollection) so strict typing is applied across the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@core/resource/ResourceCollection.php`:
- Around line 201-204: The code uses the PHP 8.0 `mixed` return type in
ResourceCollection::current() and ResourceCollection::key(), so update
composer.json to declare a PHP >=8.0 requirement (e.g. add "php": "^8.0" to the
"require" section) so installations on older PHP (7.4) won't cause parse errors;
after updating composer.json, run composer update to refresh the lockfile.
---
Outside diff comments:
In `@core/resource/ResourceCollection.php`:
- Line 1: Add a strict types declaration to this PHP file by inserting
declare(strict_types=1); immediately after the opening <?php tag in
ResourceCollection.php so the file follows the coding guideline; ensure the
declaration appears before any namespace, use statements, or class definitions
(e.g., class ResourceCollection) so strict typing is applied across the file.
ℹ️ Review info
Configuration used: Path: https://raw.githubusercontent.com/oat-sa/tao-code-quality/main/coderabbit/php/authoring/v1/.coderabbit.yaml (via .coderabbit.yaml)
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
core/resource/ResourceCollection.php
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
composer.json (1)
98-98:⚠️ Potential issue | 🟠 MajorRemove
oat\\generis\\test\\namespace fromautoloadsection and correctautoload-devpath.The namespace
oat\\generis\\test\\is currently declared in bothautoload(line 98) andautoload-dev(line 109). Theautoloadsection exposes the test namespace in all environments, including production, which violates the principle of least privilege. Theautoload-deventry points to"generis/test/", which does not exist; test files are located in"test/".Remove the test namespace from
autoloadentirely. Updateautoload-devto map"oat\\generis\\test\\"to"test/"to correctly target the directory containing test classes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@composer.json` at line 98, Remove the test namespace mapping from the autoload section so production code does not expose tests: delete the "oat\\generis\\test\\" entry under "autoload". Then correct the autoload-dev mapping by adding or updating the "oat\\generis\\test\\" key to point to "test/" (not "generis/test/") under "autoload-dev" so test classes are only autoloaded in dev environments.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@composer.json`:
- Line 57: Update the PHP platform constraint in composer.json from "php":
"^8.0" to "php": "^8.1" so it matches Symfony 6.4 requirements; modify the "php"
entry in composer.json and then regenerate the lockfile (run composer update or
composer update --lock) to resolve dependencies against PHP 8.1, ensuring
packages like symfony/lock, symfony/cache, symfony/dependency-injection and
symfony/config can be installed.
---
Outside diff comments:
In `@composer.json`:
- Line 98: Remove the test namespace mapping from the autoload section so
production code does not expose tests: delete the "oat\\generis\\test\\" entry
under "autoload". Then correct the autoload-dev mapping by adding or updating
the "oat\\generis\\test\\" key to point to "test/" (not "generis/test/") under
"autoload-dev" so test classes are only autoloaded in dev environments.
ℹ️ Review info
Configuration used: Path: https://raw.githubusercontent.com/oat-sa/tao-code-quality/main/coderabbit/php/authoring/v1/.coderabbit.yaml (via .coderabbit.yaml)
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
composer.json
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
common/persistence/class.SqlPersistence.php (1)
1-1: Missingdeclare(strict_types=1);.Per coding guidelines, all PHP files must use
declare(strict_types=1);. This is a pre-existing omission, but given that this PR tightens the method signature with explicit types, adding strict types would be consistent with the direction of these changes.Proposed fix
<?php + +declare(strict_types=1);As per coding guidelines, "Use strict types
declare(strict_types=1);in all PHP files."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@common/persistence/class.SqlPersistence.php` at line 1, Add the strict types declaration to this PHP file by inserting declare(strict_types=1); immediately after the opening <?php tag in class.SqlPersistence.php so the SqlPersistence class and its newly tightened method signatures are validated under strict typing rules; ensure there is a blank line or newline after the declaration to keep file formatting consistent.core/kernel/api/class.ModelFactory.php (1)
1-1: Missingdeclare(strict_types=1);.The file does not declare strict types. As per coding guidelines, "Use strict types
declare(strict_types=1);in all PHP files."♻️ Proposed fix
<?php + +declare(strict_types=1); + /** * This program is free software; ...🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/kernel/api/class.ModelFactory.php` at line 1, Add a strict types declaration to this PHP file by inserting declare(strict_types=1); immediately after the opening <?php tag in core/kernel/api/class.ModelFactory.php so the file (and the ModelFactory class) enforces strict typing per project guidelines.scripts/tools/CleanUpOrphanFiles.php (1)
1-1: Adddeclare(strict_types=1);to the file.The file is missing the strict types declaration required by coding guidelines for all PHP files. As per coding guidelines: "Use strict types
declare(strict_types=1);in all PHP files."♻️ Proposed fix
<?php + +declare(strict_types=1); + /** * This program is free software; ...🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/tools/CleanUpOrphanFiles.php` at line 1, Add the strict types declaration at the top of the PHP file immediately after the opening PHP tag (<?php) by inserting declare(strict_types=1); so the file uses strict typing; ensure it's the first statement following the PHP open tag and before any other code or comments to comply with the project's coding guidelines.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@common/oatbox/log/logger/TaoMonolog.php`:
- Around line 154-155: Validate $handlerOptions before using $handlerOptions[0]:
ensure it's an array and that an element at numeric index 0 exists and is a
string; if not, extract the first string value from the array (e.g., via
reset(array_values($handlerOptions)) or a short foreach) or throw a clear
configuration error. Replace the direct access $path = $handlerOptions[0]; with
a guarded assignment that checks isset($handlerOptions[0]) &&
is_string($handlerOptions[0]) and otherwise finds a viable string value from
$handlerOptions, then proceed with the existing strpos(...) and subsequent file
checks using that validated $path in TaoMonolog.php.
In `@common/persistence/class.SqlPersistence.php`:
- Around line 107-112: Update the method signatures for
common_persistence_sql_Driver::query() and
common_persistence_sql_dbal_Driver::query() to match the SqlPersistence::query()
signature by adding the scalar and return type declarations: change the
$statement parameter to be type-hinted as string and add the return type
declaration : Result to the method signature; ensure the implementations (in
class common_persistence_sql_dbal_Driver and the interface
common_persistence_sql_Driver) and any PHPDoc remain consistent with the new
signature to satisfy PSR-12.
---
Nitpick comments:
In `@common/persistence/class.SqlPersistence.php`:
- Line 1: Add the strict types declaration to this PHP file by inserting
declare(strict_types=1); immediately after the opening <?php tag in
class.SqlPersistence.php so the SqlPersistence class and its newly tightened
method signatures are validated under strict typing rules; ensure there is a
blank line or newline after the declaration to keep file formatting consistent.
In `@core/kernel/api/class.ModelFactory.php`:
- Line 1: Add a strict types declaration to this PHP file by inserting
declare(strict_types=1); immediately after the opening <?php tag in
core/kernel/api/class.ModelFactory.php so the file (and the ModelFactory class)
enforces strict typing per project guidelines.
In `@scripts/tools/CleanUpOrphanFiles.php`:
- Line 1: Add the strict types declaration at the top of the PHP file
immediately after the opening PHP tag (<?php) by inserting
declare(strict_types=1); so the file uses strict typing; ensure it's the first
statement following the PHP open tag and before any other code or comments to
comply with the project's coding guidelines.
ℹ️ Review info
Configuration used: Path: https://raw.githubusercontent.com/oat-sa/tao-code-quality/main/coderabbit/php/authoring/v1/.coderabbit.yaml (via .coderabbit.yaml)
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
common/oatbox/log/logger/TaoMonolog.phpcommon/persistence/class.SqlPersistence.phpcore/kernel/api/class.ModelFactory.phpcore/kernel/persistence/smoothsql/class.Resource.phpscripts/tools/CleanUpOrphanFiles.php
🚧 Files skipped from review as they are similar to previous changes (1)
- core/kernel/persistence/smoothsql/class.Resource.php
Karol-Stelmaczonek
approved these changes
Feb 26, 2026
Co-authored-by: Karol-Stelmaczonek <118974926+Karol-Stelmaczonek@users.noreply.github.com> Signed-off-by: Aleksej Tikhanovich <tihanovich.a@gmail.com>
Version❕ Some commits are not using the conventional commits formats. They will be ignored in version management.
There are 0 BREAKING CHANGE, 5 features, 3 fixes |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Update dbal dependencies AUT-4457
Summary by CodeRabbit
Refactor
Bug Fixes
Tests
Chore