Address API removals from upstream #9885

greg0ire · 2022-07-07T15:51:39Z

doctrine/dbal#5477 has removed 2 things:

array and object types
custom schema options

When I introduced OrmFunctionalTestCase::createSchemaForModels(), I made several wrong assumptions: - the call is always wrapped in a try / catch; - that try / catch is always typed with "Exception". Because of that, I missed, many, many occurrences of SchemaTool::createSchema(). I recently noticed that contributors kept using the SchemaTool::createSchema() and figured not everything had been migrated. Migrating some of them did not result in something far better until I also introduced similar methods for SchemaTool::getUpdateSchemaSql() and SchemaTool::getSchemaFromMetadata().

Migrate more usages of SchemaTool::createSchema()

*FlushEventArgs classes should extend ManagerEventArgs from doctrine/persistence to be able to use ManagerEventArgs for any persistance implementation. OnClearEventArgs should extend from OnClearEventArgs from doctrine/persistence.

If ObjectHydrator faces an empty row to an uninitialized collection, it initializes it, to prevent it from querying again (DDC-1526). However, if that row is the first but not the only in the collection, the next rows will be ignored, as the collection will be considered "existing", and "existing" collections are only replaced if REFRESH hint is present. To prevent it, we defer initialization to the end of the hydration. Fixes doctrineGH-9807

[doctrineGH-9807] Fix: initialize potentially empty collections at the hydration complete

Change parent classes in some events

Fix typo in AbstractQuery

greg0ire · 2022-07-07T21:52:29Z

The removal of custom schema options seems to break the code introduced in #9655 for some reason…

* 2.12.x: PHPStan 1.8.0 (doctrine#9887) Fix typo in AbstractQuery ObjectHydrator: defer initialization of potentially empty collections Migrate more usages of SchemaTool::createSchema() preUpdate: Add restriction that changed field needs to be in computed changeset (doctrine#9871)

greg0ire · 2022-07-09T11:11:46Z

@ruudk @morozov maybe you can help with this failure: https://github.com/doctrine/orm/runs/7242015946?check_suite_focus=true#step:7:79

I don't understand what's happening, but I think it must be somehow related to custom schema options. The original PR of @ruudk relied on them: #9636 , but not the second one: #9655

greg0ire · 2022-07-09T11:12:45Z

tests/Doctrine/Tests/OrmFunctionalTestCase.php

+    final protected function assertQueryLog(string ...$expectedQueries): void
+    {
+        self::assertSame($expectedQueries, array_column($this->getQueryLog()->queries, 'sql'));
+    }


Introducing that method allows to compare the 2 query logs rather than getting an exception because there is no seventh query.

greg0ire · 2022-07-09T11:29:35Z

but not the second one: #9655

Custom options are not mentioned in the code, but they are mentioned in the summary of that PR though 🤔

UPD: I think this line is throwing away the extra options:

orm/lib/Doctrine/ORM/Tools/SchemaTool.php

Line 799 in 291765e

    
           $options                        = array_intersect_key($mappingOptions, array_flip(self::KNOWN_COLUMN_OPTIONS));

Before my changes, there was a separation into known options and custom ones: https://github.com/doctrine/orm/blob/2.12.x/lib/Doctrine/ORM/Tools/SchemaTool.php#L799-L800

Removing that breaks the test suite because of how enum types are implemented, but that might be the way to go, I'll have to investigate why that separation was needed in the first place.

The issue with the test suite is related to #9382

Now it blows up with The "enumType" column option is not supported., thrown here: https://github.com/doctrine/dbal/blob/2485d0a49dd17d05f54b68bb020eced4242388c8/src/Schema/Column.php#L83-L85

@morozov , what's the upgrade path for this?

UPD: nevermind, I think I've found what needed to be done.

greg0ire · 2022-07-09T15:50:42Z

The build is green, but I'm not setting this as ready for review yet: there are things I can backport, and other things that might be incorrect.

SerializationModel has a field with type array, and another with type object. Both types are deprecated.

It contains fields with deprecated types.

…ype-deprecation Address array object type deprecation

Merge 2.12.x up into 2.13.x

greg0ire · 2022-07-10T21:43:54Z

lib/Doctrine/ORM/Tools/SchemaTool.php

-        $options                        = array_intersect_key($mappingOptions, array_flip(self::KNOWN_COLUMN_OPTIONS));
-        $options['customSchemaOptions'] = array_diff_key($mappingOptions, $options);
+        $options                    = array_intersect_key($mappingOptions, array_flip(self::KNOWN_COLUMN_OPTIONS));
+        $options['platformOptions'] = array_diff_key($mappingOptions, $options);


I don't know if I should backport this as well… Column::getCustomSchemaOptions() will not return the same thing as previously, not sure how acceptable that is.

I'd leave it as is in older branches. This API is supported up until DBAL 4.0, so there's no reason to break anything prior to supporting it.

Custom schema options have been removed in doctrine/dbal 4

greg0ire · 2022-07-12T19:00:36Z

I've backported what was reasonable to backport; this is now the combination of a merge-up and fixes for the build (which are in separate commits)

ThomasLandauer and others added 9 commits June 28, 2022 23:44

preUpdate: Add restriction that changed field needs to be in computed…

0ef08c5

… changeset (doctrine#9871)

Merge pull request doctrine#9874 from greg0ire/migrate-to-csfm

3295ccf

Migrate more usages of SchemaTool::createSchema()

Change parent classes

eea5339

*FlushEventArgs classes should extend ManagerEventArgs from doctrine/persistence to be able to use ManagerEventArgs for any persistance implementation. OnClearEventArgs should extend from OnClearEventArgs from doctrine/persistence.

Merge pull request doctrine#9870 from popov-a-e/doctrineGH-9807

83c1ad2

[doctrineGH-9807] Fix: initialize potentially empty collections at the hydration complete

Merge pull request doctrine#9876 from franmomu/extend_event_manager_args

68405f3

Change parent classes in some events

Fix typo in AbstractQuery

306b5f9

Merge pull request doctrine#9880 from CarlSchwan/patch-1

f79ec43

Fix typo in AbstractQuery

greg0ire mentioned this pull request Jul 7, 2022

Migrate more code to PHP8 #9851

Merged

greg0ire force-pushed the fix-build branch 3 times, most recently from 9495bb8 to 8c43b3d Compare July 7, 2022 21:37

derrabus added 2 commits July 8, 2022 13:53

PHPStan 1.8.0 (doctrine#9887)

291765e

greg0ire commented Jul 9, 2022

View reviewed changes

greg0ire force-pushed the fix-build branch from 8c43b3d to 54dc8ea Compare July 9, 2022 15:23

greg0ire mentioned this pull request Jul 9, 2022

Introduce assertQueryLogTail() for better tests #9890

Closed

greg0ire force-pushed the fix-build branch 2 times, most recently from c2caf0a to 725ecc2 Compare July 9, 2022 18:08

greg0ire added 4 commits July 10, 2022 11:22

Test invalid mapping file with boolean model

baf6a39

SerializationModel has a field with type array, and another with type object. Both types are deprecated.

Use ::class notation

d6dcfbd

Document what we test in test method name

7ed0db0

Remove SerializationModel from generic model set

00989d6

It contains fields with deprecated types.

greg0ire mentioned this pull request Jul 10, 2022

Address array object type deprecation #9892

Merged

Merge pull request doctrine#9892 from greg0ire/address-array-object-t…

ab4844b

…ype-deprecation Address array object type deprecation

greg0ire added 2 commits July 10, 2022 23:07

Merge remote-tracking branch 'origin/2.12.x' into 2.13.x

4a62b66

Merge pull request doctrine#9893 from greg0ire/2.13.x

48b4f63

Merge 2.12.x up into 2.13.x

greg0ire commented Jul 10, 2022

View reviewed changes

greg0ire force-pushed the fix-build branch 2 times, most recently from 27fa390 to 3bd0427 Compare July 12, 2022 18:14

greg0ire and others added 4 commits July 12, 2022 20:24

Merge remote-tracking branch 'origin/2.13.x' into fix-build

db9c43f

Rely on platform options

68dc5fe

Custom schema options have been removed in doctrine/dbal 4

Address removal/deprecation of ArrayType/ObjectType

1fa1e70

Ignore errors caused by nullable assets filter

1ac9d52

greg0ire force-pushed the fix-build branch from 3bd0427 to 1ac9d52 Compare July 12, 2022 18:24

greg0ire requested review from derrabus and SenseException July 12, 2022 19:00

greg0ire marked this pull request as ready for review July 12, 2022 20:38

greg0ire requested a review from morozov July 12, 2022 20:38

morozov approved these changes Jul 13, 2022

View reviewed changes

greg0ire merged commit 990a1fe into doctrine:3.0.x Jul 13, 2022

greg0ire deleted the fix-build branch July 13, 2022 10:10

derrabus added this to the 3.0.0 milestone Jul 25, 2022

derrabus added the Improvement label Jul 25, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Address API removals from upstream #9885

Address API removals from upstream #9885

greg0ire commented Jul 7, 2022 •

edited

Loading

greg0ire commented Jul 7, 2022

greg0ire commented Jul 9, 2022

greg0ire Jul 9, 2022

greg0ire commented Jul 9, 2022 •

edited

Loading

greg0ire commented Jul 9, 2022

greg0ire Jul 10, 2022

morozov Jul 10, 2022 •

edited

Loading

greg0ire commented Jul 12, 2022

Address API removals from upstream #9885

Address API removals from upstream #9885

Conversation

greg0ire commented Jul 7, 2022 • edited Loading

greg0ire commented Jul 7, 2022

greg0ire commented Jul 9, 2022

greg0ire Jul 9, 2022

Choose a reason for hiding this comment

greg0ire commented Jul 9, 2022 • edited Loading

greg0ire commented Jul 9, 2022

greg0ire Jul 10, 2022

Choose a reason for hiding this comment

morozov Jul 10, 2022 • edited Loading

Choose a reason for hiding this comment

greg0ire commented Jul 12, 2022

greg0ire commented Jul 7, 2022 •

edited

Loading

greg0ire commented Jul 9, 2022 •

edited

Loading

morozov Jul 10, 2022 •

edited

Loading