Error in XML mapping (gedmo) afer upgrade to 2.9

[eisberg]

BC Break Report

Q	A
BC Break	yes
Version	2.9

Summary

i get error after upgrade this version.

libxml error: Element '{http://gediminasm.org/schemas/orm/doctrine-extensions-mapping}timestampable': No matching global element declaration available, but demanded by the strict wildcard.
in src/Resources/config/doctrine/UserGroupRelation.orm.xml at line 21

Problem in PR: doctrine/orm#10579

see comments in issue: doctrine/orm#10552

[dmaicher]

you are using DoctrineBundle, right? Can you check if this is fixed if you revert back to v2.8.3 for DoctrineBundle? I'm guessing you are currently on the latest version v2.9.0?

[eisberg]

That's right, in DoctrineBundle v2.8.3 everything is ok

[dmaicher]

Its caused by enabling strict XSD validation by default on DoctrineBundle v2.9.0.

We decided to not make it configurable for now by adding yet another config option. Can this issue be resolved instead by making the XML valid for this XSD validation?

cc @ostrolucky

[alexander-schranz]

We @sulu are running into the same problem. Here is our

Can this issue be resolved instead by making the XML valid for this XSD validation?

I think the validation is failing because gedmo extension requires additional xml tags.

Here is our XML: https://github.com/sulu/sulu/blob/08a40b7de9c1b3f9a6c42edc928ee8e4ff9ea97c/src/Sulu/Bundle/MediaBundle/Resources/config/doctrine/Collection.orm.xml#L5

It follows the gedmo docs and:

xmlns:gedmo="http://gediminasm.org/schemas/orm/doctrine-extensions-mapping"

Is added but still seems failing.

As already mention the enabled validation here seems be the problem:

https://github.com/dmaicher/DoctrineBundle/blob/14b66ef3bef181f663d07a1d475bc67ed8a4ce56/DependencyInjection/DoctrineExtension.php#L1158-L1159

[greg0ire]

Transferring to the correct repository.

Also, not sure if this is going to fix your issue @alexander-schranz but I'd recommend you use vendor/doctrine/orm/doctrine-mapping.xsd instead of http://doctrine-project.org/schemas/orm/doctrine-mapping.xsd. That way you are validating your mapping against the right version of the schema.

[jakubtobiasz]

Same problem in @Sylius 🙋🏼‍♂️.

[greg0ire]

@jakubtobiasz noted, but please, let's not pile up +1s here, and focus on getting to a solution instead. If you want to +1, you have reactions.

A way forward could be to disable XSD validation in a patch release of the doctrine bundle, then expose a configuration node in the next minor release, and deprecate not setting it to true.

[ostrolucky]

Gedmo has wrong schema, it should be fixed there. Adding config option that we have to keep around for 2 major releases just because of deprecations is precisely what we wanted to avoid.

[jakubtobiasz]

@ostrolucky I understand your move from a maintainer perspective, but from a user perspective it's a troublemaker. In should be introduced with a switch or postponed until the next major release IMO. For now, it'd be great to have an option to disable it.

[stof]

DoctrineBundle should not force users to enable the XSD validation if they are not compatible with it yet. Otherwise, it effectively turns the ORM deprecation into a BC break.

The fact that the doctrine extensions need to change the way to configure their mapping is reported at doctrine-extensions/DoctrineExtensions#2318 already, but there is not yet an alternative.

[ostrolucky]

You wouldn't complain if symfony validation constraint validator is extended to cover some edge case, would you? BC break is matter of opinion. Every bug fix is a BC break for people relying on that bug and invalid schema looks like a bug to me, we just opted to stop supporting that to make people fix those. Link above where it was reported already in 2021 that gedmo is relying on invalid schema and nothing was done about it is a proof it won't be fixed until it's necessary. Anyways if someone works on such option in bundle I will merge it, but wouldn't it be easier to fix gedmo?

[stof]

@ostrolucky the ORM introduces this change as a opt-in precisely because it is known that this change has a BC impact. the way the bundle handled it is to force the new behavior in a minor version of the bundle, effectively ignoring the decision of the ORM maintainers to make it opt-in for BC reasons. gedmo being a widely used package impacted by the ORM change was a known thing, which is why it was opt-in. And before ORM 2.10, the ORM XSD was explicitly allowing to have nodes from different namespaces in those places, so the gedmo decision (done at the time of ORM 2.0) cannot be considered a bug in gedmo (they rely on a feature that has been deprecated 10 years after they started using it).

And even if Gedmo is fixed, this won't magically solve things: projects will need to migrate to the new way of configuring gedmo before enabling the ORM validation (and AFAIK, nobody is working yet on implementing such new configuration format in gedmo).

[ostrolucky]

There are several options:

• Gedmo can define standalone xml schema. That's easier than having to change to new way of configuring gedmo.
• ORM enables strict validation by default, but in 2.x turns schema errors into deprecation only. Then we can stop enabling it in bundle and have green deprecation CI build at the same time
• Related code is moved to doctrine-bridge like we were asking in https://github.com/doctrine/DoctrineBundle/pull/1634/files#r1135594578, this way deprecation will not be direct for bundle, also achieving green build
• We ignore this particular deprecation via baseline. But then users will not have an option to deal with the deprecation.
• Setting for bundle which flips between these two is added. Least favorite option for me because not only we have to count with more possible combinations of execution paths or having to have some option that we already know we will have to remove, but also setting will stop working in ORM 3, then we will have yet more options around that don't do anything.

[stof]

@ostrolucky Defining a standalone XML schema in gedmo implies that the gedmo config should be in a separate XML file than the ORM mapping. That's precisely what requires changing the way gedmo is configured. And this will take time and will require projects using gedmo to migrate to the new configuration system using separate files.

Moving the code to the bridge will not solve the fact that without exposing a configuration setting, you still force users to use the new ORM behavior which is a BC break.

• Least favorite option for me because not only we have to count with more possible combinations of execution paths or having to have some option that we already know we will have to remove, but also setting will stop working in ORM 3, then we will have yet more options around that don't do anything.

Well, this is a way that is regularly used in Symfony's core bundles when we need projects to opt in for a new behavior.
We introduce the new setting (which is a then passed directly to the driver constructor so there is no combinations to take into account) and we deprecate setting it to false (or not setting it as the default would be false). Then, in the next major version of the bundle, we remove that setting (and if DoctrineBundle 2.x adds support for ORM 3.0, it can trigger an exception instead of reporting a deprecation for false values when ORM 3.0 is used).

• ORM enables strict validation by default, but in 2.x turns schema errors into deprecation only. Then we can stop enabling it in bundle and have green deprecation CI build at the same time

This option could work, but I'm not sure how feasible it is for the ORM. That's an interesting idea as it would report deprecations only for projects that have an invalid XML configuration according to the new XSD.

[ostrolucky]

Well, this is a way that is regularly used in Symfony's core bundles when we need projects to opt in for a new behavior.
We introduce the new setting (which is a then passed directly to the driver constructor so there is no combinations to take into account) and we deprecate setting it to false (or not setting it as the default would be false). Then, in the next major version of the bundle, we remove that setting (and if DoctrineBundle 2.x adds support for ORM 3.0, it can trigger an exception instead of reporting a deprecation for false values when ORM 3.0 is used)

I don't think Symfony ends up being in situation where you have config options related to external libraries that don't do anything anymore (because those options don't exist in newer versions of those libraries but both versions still need to be supported). Anyways I'm well aware of how Symfony does it, doctrine-bundle is not required to follow same policies though. But like I said, if someone creates MR for this, it's fair enough.

[eisberg]

I propose a solution:

1. In Doctrine\ORM\Mapping\Driver\XmlDriver:

// Doctrine\ORM\Mapping\Driver\XmlDriver:


private function validateMapping(string $file): void
    {
        if (!$this->isXsdValidationEnabled) {
            return;
        }

        $backedUpErrorSetting = libxml_use_internal_errors(true);

        try {
            $document = new DOMDocument();
            $document->load($file);
            // if (! $document->schemaValidate(__DIR__ . '/../../../../../doctrine-mapping.xsd')) {
            //     throw MappingException::fromLibXmlErrors(libxml_get_errors());
            // }

            $this->validateDocument($document); // <<<  instead of the code above
        } finally {
            libxml_clear_errors();
            libxml_use_internal_errors($backedUpErrorSetting);
        }
    }

    private function validateDocument(DOMDocument $document): void
    {
        $allSchemas = $this->extractAllSchemas($document);
        $allSchemas['http://doctrine-project.org/schemas/orm/doctrine-mapping'] = __DIR__ . '/../../../../../doctrine-mapping.xsd';

        // Create a combined schema
        $imports = [];
        foreach ($allSchemas as $namespace => $location) {
            $imports[] = sprintf('<xsd:import namespace="%s" schemaLocation="%s" />', $namespace, $location);
        }

        $combinedSchema = '
            <xsd:schema
                xmlns:xsd="http://www.w3.org/2001/XMLSchema"
                elementFormDefault="qualified">
                <xsd:import namespace="http://www.w3.org/XML/1998/namespace" />
                ' . implode(PHP_EOL, $imports) . '
            </xsd:schema>
        ';

        $this->safeSchemaValidateSource($document, $combinedSchema);
    }

    private function safeSchemaValidateSource(DOMDocument $document, string $schema): void
    {
        $initialErrorsFlag = libxml_use_internal_errors(true);
        if (!$document->schemaValidateSource($schema)) {
            throw MappingException::fromLibXmlErrors(libxml_get_errors());
        }

        libxml_clear_errors();

        // Return the use internal errors to what it was.
        libxml_use_internal_errors($initialErrorsFlag);
    }
protected  function extractAllSchemas(\DOMDocument $document): array
    {
        $xpath = new \DOMXPath($document);
        $schemaLocations = $xpath->query('//*[@xsi:schemaLocation]');


        // "namespace1 location1 namespace2 location2"
        $locationRegex = '/'
            . '(?P<namespace>[^\s]+)'
            . '\s+'
            . '(?P<location>[^\s]+)'
            . '/';

        $allSchemas = [];
        foreach ($schemaLocations as $element) {
            $schemaLocation = $element->getAttribute('xsi:schemaLocation');
            if (preg_match_all($locationRegex, $schemaLocation, $matches)) {
                $schemas = array_combine($matches['namespace'], $matches['location']);
                $allSchemas = array_merge($allSchemas, $schemas);
            } else {
                throw new Exception(
                    'Unable to parse the value of schemaLocation. Expected "namespace1 xsd1 namespace2 xsd2" '
                    . 'but found "' . $schemaLocation . '"'
                );
            }
        }

        return array_unique($allSchemas);
    }

2. In ORM XML (for example):

<?xml version="1.0" encoding="utf-8"?>
<doctrine-mapping xmlns="http://doctrine-project.org/schemas/orm/doctrine-mapping"
                  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
                  xsi:schemaLocation="http://doctrine-project.org/schemas/orm/doctrine-mapping
                    https://www.doctrine-project.org/schemas/orm/doctrine-mapping.xsd
                    http://gediminasm.org/schemas/orm/doctrine-extensions-mapping
                    http://gediminasm.org/schemas/orm/doctrine-extensions-mapping"
                  xmlns:gedmo="http://gediminasm.org/schemas/orm/doctrine-extensions-mapping">
    <entity

A solution similar to what Symfony does in Symfony\Component\DependencyInjection\Loader\XmlFileLoader

[stof]

@eisberg that's not a solution, given that the goal of ORM maintainers is to stop allowing to add external nodes in XML mapping files (otherwise, they would not have removed support for using external nodes in the ORM XSD.

What we need here is a proper deprecation path, not a new feature that does the opposite of that deprecation.

[greg0ire]

I'll reiterate what I think is the best course of action:

1. release a patch disabling validation;
2. release a minor allowing to enable it, and deprecating not doing so;
3. make the knob a no-op in next major;
4. remove the knob in next next major.

Having many unused nodes is a big deal because we don't do major releases often. That's what should IMO change.

[greg0ire]

By the way, I just realized that the best value for this validation setting is probably %kernel.debug%, which means it should stay available forever, thus removing one of the things that makes you uncomfortable @ostrolucky (having a no-op setting): we don't need to check this in production, do we?

So new plan:

1. release a patch disabling validation;
2. tweak the ORM to trigger a deprecation when the setting is not configured explicitely as opposed to set to true (or removing the deprecation entirely, I don't know);
3. release a minor allowing to enable it, and deprecating not setting it to %kernel.debug%.

[dmaicher]

I propose to revert it with a patch release first: #1644

Then we can think about how to tackle it properly