doctrine / mongodb-odm

The Official PHP MongoDB ORM/ODM
https://www.doctrine-project.org/projects/doctrine-mongodb-odm/en/latest/
MIT License
1.09k stars 504 forks source link

[2.0]XML schema validation seems too strict #2095

Closed etshy closed 3 years ago

etshy commented 5 years ago

BC Break Report

Q A
BC Break yes
Version 2.0.1

Summary/Reproduce

I don't know if it's possible, but maybe the validation should be less strict ?

Previous behavior

There wasn't any error about that line in previous versions

Current behavior

throw XML schama validation exception during composer update

File

I paste here my whole xml mapping file, if that could help

<?xml version="1.0" encoding="UTF-8"?>
<doctrine-mongo-mapping xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
                        xmlns="http://doctrine-project.org/schemas/odm/doctrine-mongo-mapping"
                        xmlns:gedmo="http://gediminasm.org/schemas/orm/doctrine-extensions-mapping"
                        xsi:schemaLocation="http://doctrine-project.org/schemas/odm/doctrine-mongo-mapping
                http://doctrine-project.org/schemas/odm/doctrine-mongo-mapping.xsd">
    <document name="App\Model\Persistence\Series" repository-class="App\Model\ODM\Repository\SeriesRepository">
        <field name="name" type="string"/>
        <field name="additionalNames" type="string"/>
        <field name="slug" type="string">
            <gedmo:slug unique="true" fields="name" updatable="true"/>
        </field>
        <field name="author" type="string"/>
        <field name="artist" type="string"/>
        <field name="description" type="string"/>
        <field name="adult" type="bool"/>
        <field name="visible" type="bool"/>
        <field name="customTitle" type="string"/>

        <embed-one field="readerSettings" target-document="App\Model\Persistence\ReaderSettings">
        </embed-one>
        <reference-one field="image" target-document="App\Model\Persistence\Files\Image" store-as="ref">
        </reference-one>
        <reference-many field="chapters" target-document="App\Model\Persistence\Chapter" mapped-by="series"
                        strategy="atomicSetArray" store-as="ref">
        </reference-many>
        <reference-many field="teams" target-document="App\Model\Persistence\Team" mapped-by="teams"
                        strategy="atomicSetArray" store-as="ref">
            <sort>
                <sort field="name" order="asc"/>
            </sort>
        </reference-many>

    </document>
</doctrine-mongo-mapping>
alcaeus commented 5 years ago

That's not good. I'll take a look during the day. Thanks for providing a file I can test this with 👍

alcaeus commented 5 years ago

After taking a look, this is somewhat intended, with a few unintended consequences. We've decided to validate the mapping files against our schema, which makes sense as it catches errors in the schema definition that could otherwise lead to other errors when interpreting the mapping.

When adding the mappings for the Gedmo extension to the field mapping, the following validation errors occur (for the field mapping including a gedmo:slug element:

The mapping file /Users/alcaeus/Code/doctrine/mongodb-odm/tests/Doctrine/ODM/MongoDB/Tests/Mapping/xml/Doctrine.ODM.MongoDB.Tests.Mapping.MultipleNamespacesMappingDocument.dcm.xml is invalid: Line 12:0: Element '{http://doctrine-project.org/schemas/odm/doctrine-mongo-mapping}field': Character content is not allowed, because the content type is empty. Line 12:0: Element '{http://doctrine-project.org/schemas/odm/doctrine-mongo-mapping}field': Element content is not allowed, because the content type is empty.

Our schema doesn't allow adding any additional content in elements, which includes elements from other namespaces. To allow this, ORM used an xs:any element in every type, but this was removed for 3.0: https://github.com/doctrine/orm/pull/6943/commits/af0b0dbdc397b8c8fcc6cf4eb2e5ea0920dbd6bd. ODM never had such a mechanism, but since mapping files weren't validated against the schema, this was never caught.

My opinion is that these extra mappings should not just be somewhere in the ODM mappings. Instead, the extensions should be mapped in their own files to be independent from the main ODM mappings.

Alternatively, if one were to insist on having all these in one file, a separate XSD that extends the default ODM type definitions would be necessary. While it would be easy to allow configuring the schema file that is used for validation in the XML driver, I'm not sure how we could allow that and still verify that the mappings are in a format that we understand.

While you are using 1.x, you'll have to ignore these deprecations, as there isn't a lot you can do about it (apart from dropping the gedmo mappings from your ODM mapping files). I'm aware that this is a very hard BC break, and I'm happy to work with the Gedmo developers on finding a solution that works for the users, but I don't know what such a solution would look like.

alcaeus commented 5 years ago

Cross-referencing with https://github.com/Atlantic18/DoctrineExtensions/issues/2055.

alcaeus commented 3 years ago

Closing as this issue needs to be fixed upstream.