Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Improvement]: Add support for Doctrine v4 #17003

Draft
wants to merge 41 commits into
base: 11.x
Choose a base branch
from
Draft

[Improvement]: Add support for Doctrine v4 #17003

wants to merge 41 commits into from

Conversation

kingjia90
Copy link
Contributor

@kingjia90 kingjia90 commented Apr 30, 2024 •
edited

Changes in this pull request

Resolves #13015

Still unsure if we can support without major refactoring, likely would be more replacing the deprecations
Draft: i am collecting the blocking problems under #13015 (comment)

@github-actions GitHub Actions
Copy link

Review Checklist

  • Target branch (11.2 for bug fixes, others 11.x)
  • Tests (if it's testable code, there should be a test for it - get help)
  • Docs (every functionality needs to be documented, see here)
  • Migration incl. install.sql (e.g. if the database schema changes, ...)
  • Upgrade notes (deprecations, important information, migration hints, ...)
  • Label
  • Milestone

@kingjia90 kingjia90 changed the title [Improvement]: Prepare pfor Doctrine v4 [Improvement]: Prepare for Doctrine v4 Apr 30, 2024
@kingjia90
replace hasPrimaryKey and getCustomSchemaOption
9145c45 
https://github.com/doctrine/dbal/blob/4.0.x/UPGRADE.md#bc-break-removed-table-methods
https://github.com/doctrine/dbal/blob/4.0.x/UPGRADE.md#bc-break-removed-support-for-custom-schema-options
@kingjia90
removed hasExplicitForeignKeyIndexes usage, not sure what was for, pr…
c0acf1a 
…obably wouldn't affect

https://github.com/doctrine/dbal/blob/4.0.x/UPGRADE.md#bc-break-removed-schema-methods-related-to-explicit-foreign-key-indexes
@kingjia90
replace wrappedConnection with NativeConnection
645eb62 
fix quote strings

https://github.com/doctrine/dbal/blob/4.0.x/UPGRADE.md#deprecate-accessors-for-the-native-connection-in-favor-of-getnativeconnection
@kingjia90
replace execute with executeQuery
912a6a6
@kingjia90
set maxtime as string to be quoted
128700c
@kingjia90
quote objectid as string
f9fc8cb
@kingjia90
Potential BC breaks that blocks the support to v4
6b69787
@kingjia90
remove serverInfoAwareConnection interface usages
d2946f3 
https://github.com/doctrine/dbal/blob/4.0.x/UPGRADE.md#bc-break-removed-driver-level-apis-that-dont-take-the-server-version-into-account
@kingjia90
fc layer getServerVersion()
bb0fa73 
it's a private method in dbal v3
@kingjia90
fc layer getServerVersion()
a21c4c6 
it's a private method in dbal v3
@kingjia90
should be covered already by the IF EXISTS
af8df16
@kingjia90
this quote seems unneccessary
fba5837
@kingjia90
try with regex?
f76e6d7
@kingjia90
catch exception when no select is there
fd30255
@kingjia90
use addSelect
c89f68b
@kingjia90
set dummy select to 1
d26e451
@kingjia90
test will pass?
08151d9
@kingjia90
_toString the expressionBuilder and condition in listingTest
f4d09ad
@kingjia90
refactor
c0e31a9
@kingjia90
Merge remote-tracking branch 'origin/doctrine4' into doctrine4
d731a33 
# Conflicts:
#	bundles/InstallBundle/src/Installer.php
#	lib/Tool/MaintenanceModeHelper.php
kingjia90
kingjia90 commented May 7, 2024
View reviewed changes
models/Element/Service.php
@@ -951,7 +958,7 @@ public static function addTreeFilterJoins(array $cv, Asset\Listing|DataObject\Li

$condition = $joinConfig['condition'];
$columns = $joinConfig['columns'];
$select->add('select', $columns, true);
$select->addSelect($columns);
Copy link
Contributor Author

@kingjia90 kingjia90 May 7, 2024 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: need to make sure $columns is a string, because addSelect accept only variantic string

@kingjia90 kingjia90 changed the title [Improvement]: Prepare for Doctrine v4 [Improvement]: Add support for Doctrine v4 May 21, 2024
@kingjia90 kingjia90 mentioned this pull request May 21, 2024
Merged
@kingjia90 kingjia90 changed the base branch from 11.x to prepare4doctrine4 May 21, 2024 08:33
@kingjia90 kingjia90 changed the base branch from prepare4doctrine4 to 11.x May 21, 2024 08:34
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented May 22, 2024

Quality Gate Passed Quality Gate passed

Issues
24 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
45.7% Duplication on New Code

See analysis details on SonarCloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Pimcore:ToDo
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Prepare for Doctrine v4
2 participants
@kingjia90 @wisconaut