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

Fix all deprecations #1634

Merged
merged 10 commits into from
Mar 22, 2023
Merged

Fix all deprecations #1634

merged 10 commits into from
Mar 22, 2023

Conversation

dmaicher
Copy link
Contributor

@dmaicher dmaicher commented Feb 28, 2023 •
edited
Loading

All deprecations are fixed now 😊

@dmaicher dmaicher added this to the 2.9.0 milestone Feb 28, 2023
@dmaicher
Copy link
Contributor Author

@greg0ire I might need some help to figure out how to resolve this one

  1x: Using XML mapping driver with XSD validation disabled is deprecated and will not be supported in Doctrine ORM 3.0. (XmlDriver.php:65 called by SimplifiedXmlDriver.php:23, https://github.com/doctrine/orm/pull/6728, package doctrine/orm)
    1x in ContainerTest::testContainer from Doctrine\Bundle\DoctrineBundle\Tests

I had a look at doctrine/orm#6728 and figured out that we seem to be using https://github.com/doctrine/orm/blob/2.14.x/lib/Doctrine/ORM/Mapping/Driver/SimplifiedXmlDriver.php#L23

So I cannot really get rid of this deprecation easily, or?

@dmaicher dmaicher force-pushed the fix_deprecations branch 2 times, most recently from 54b0acd to f5756ac Compare February 28, 2023 13:38
@stof
Copy link
Member

stof commented Feb 28, 2023

Well, looks like the ORM forgot to update the constructor of the SimplifiedXmlDriver to expose the boolean argument added in the parent

@greg0ire
Copy link
Member

@Kern046 can you please take a look at this simplified xml driver stuff? I'm AFK now and for a few more days I think.

dmaicher
dmaicher commented Feb 28, 2023
View reviewed changes
DependencyInjection/Compiler/CacheCompatibilityPass.php Outdated
@@ -59,7 +59,7 @@ private function updateSecondLevelCache(ContainerBuilder $container, Definition
$factoryDefinition = $methodCall[1][0];
assert($factoryDefinition instanceof Definition);
$aliasId = (string) $factoryDefinition->getArgument(1);
$this->wrapIfNecessary($container, $aliasId, (string) $container->getAlias($aliasId), false);
$this->wrapIfNecessary($container, $aliasId, (string) $container->getAlias($aliasId), true);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

second level also supports PSR6 caches since ORM 2.11 (which we require). See doctrine/orm#9322

@dmaicher dmaicher mentioned this pull request Mar 13, 2023
Merged
@ostrolucky ostrolucky reopened this Mar 14, 2023
@dmaicher dmaicher marked this pull request as ready for review March 14, 2023 10:27
@dmaicher dmaicher marked this pull request as draft March 14, 2023 11:39
@dmaicher dmaicher marked this pull request as ready for review March 14, 2023 12:03
ostrolucky
ostrolucky reviewed Mar 14, 2023
View reviewed changes
DependencyInjection/DoctrineExtension.php
// enable validation
$args[] = true;

$xmlDriverDef->setArguments($args);
Copy link
Member

Choose a reason for hiding this comment

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

@stof can we move this to symfony/doctrine-bridge instead? I mean that bridge is the one that specifies constructor args there. I am just not sure if symfony folks are ok to enable strict xml validation with symfony 6.3. If not, we will do it here but end result will be same.

ostrolucky
ostrolucky approved these changes Mar 19, 2023
View reviewed changes
Copy link
Member

@ostrolucky ostrolucky left a comment

Choose a reason for hiding this comment

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

Let's be pragmatic, bite a bullet and merge this as is instead of moving code around

@ostrolucky ostrolucky changed the title fix deprecations Fix all deprecations Mar 22, 2023
@ostrolucky ostrolucky merged commit cae13b0 into doctrine:2.9.x Mar 22, 2023
@dmaicher dmaicher deleted the fix_deprecations branch March 22, 2023 09:54
@phansys phansys mentioned this pull request Apr 14, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stof stof stof left review comments

@ostrolucky ostrolucky ostrolucky approved these changes

Assignees
No one assigned
Labels
Improvement
Projects
None yet
Milestone
2.9.0
Development

Successfully merging this pull request may close these issues.
4 participants
@dmaicher @stof @greg0ire @ostrolucky