doctrine / orm

Doctrine Object Relational Mapper (ORM)
https://www.doctrine-project.org/projects/orm.html
MIT License
9.93k stars 2.51k forks source link

Allow custom XSD schemas #11123

Open theofidry opened 9 months ago

theofidry commented 9 months ago

Feature Request

Q A
New Feature yes
RFC no
BC Break no

Summary

Currently when using the XML driver, the source of the XSD is hardcoded.

However usually you define the schema location within your XML files:

<?xml version="1.0" encoding="UTF-8" ?>
<doctrine-mapping xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
                  xmlns="http://doctrine-project.org/schemas/orm/doctrine-mapping"
                  xsi:schemaLocation="http://doctrine-project.org/schemas/orm/doctrine-mapping ../../../../vendor/doctrine/orm/doctrine-mapping.xsd">

I am not sure when it changed within Doctrine but I remember at some point the schema being missing something or out of sync with the version we were using so at some point we switched to the local XSD file.

After discussing with @greg0ire about how we are using some custom extensions, we found that it would be really nice if the XSD file used would be the one specified in the document (and use the current installed vendor one as a fallback).

I tried to upgrade to Doctrine3 on my project but after of couple of hours it turned unfruitful unfortunately. There is really a lot of dependencies to manage and the last blocking one I could see was Sentry (I still need to put more time to investigate why).

greg0ire commented 9 months ago

The value of xsi:schemaLocation can be obtained like so: $theDoc->documentElement->attributes->getNamedItem('schemaLocation')->value

It consists in a whitespace-separated list of whitespace-separated key value :roll_eyes: :facepalm:

namespace1 file1.xsd
namespace2 file2.xsd

Not sure if we need to support multiple namespaces, and more details on this at https://www.w3.org/TR/xmlschema-1/#schema-loc

IMO, we should only:

beberlei commented 8 months ago

@theofidry looking at doctrine-mapping.xsd, it allows ##other for attributes and sub-elements for it look like all elements. So you can add more attributes and elements, but they must be in a different namespace. Is that not working?

See for example:

https://github.com/doctrine/orm/blob/a8632aca8fca88444328737137d0daf6520ef70b/doctrine-mapping.xsd#L77-L83

greg0ire commented 8 months ago

@beberlei @jwage after reading https://github.com/doctrine/mongodb-odm/pull/2607 and https://github.com/doctrine/orm/issues/11117#issuecomment-1894492234, I think the issue is that even if our schema allows elements from another namespace, when you add another such element, it has to be valid according to XSD file of that other namespace (which isn't provided or even providable ATM).

mbabker commented 8 months ago

I think the issue is that even if our schema allows elements from another namespace, when you add another such element, it has to be valid according to XSD file of that other namespace (which isn't provided or even providable ATM).

That's pretty much it. I accidentally stumbled on this SO answer while checking to see if there was anything special to make xmllint validate against multiple schemas (which, now that I look at the doctrine-extensions repo's lint command, it makes sense why that stub import file is in the repo).

malarzm commented 8 months ago

I believe we'd need to have the schema configurable by users then and have them produce XSD files combining other files?

mbabker commented 8 months ago

https://github.com/doctrine/orm/issues/11123#issuecomment-1857682590 sounds good at face value as far as reading in the schema location for an XML document. But, anything that's file-based for reusable packages providing an XML schema gets tricky to deal with (i.e. FOSUserBundle's User schema) as the path to the Doctrine mapping XSD is going to be different based on whether it's the root project or if it's installed into another application.

Admittedly XML/XSD isn't really my thing, I've just spent way too much time generating XML files lately to help the doctrine-extensions repo break away from needing annotation support or using YAML mapping in tests 😅

greg0ire commented 8 months ago

Maybe a solution would be to allow them to provide a list of XSD files to combine, instead of the path to a combining XSD file. I see that in the DOMDocument API, there is a method called schemaValidateSource, and it takes a string instead of a file. This means that instead of asking users to produce XSD combining our XSD + e.g. Gedmo's XSD, we could do that combination ourselves at runtime. That way, we don't have to provide one combining XSD file or another depending on where each package is installed. Providing the path to each file is the responsibility of the user.

beberlei commented 8 months ago

Ah providing the extra schemas is necessary, yes that makes sense. We could write some code to extract them from the document, but I believe maybe the easiest fix is just to allow disabling schema validation on the XmlDriver for people that want to extend it.

Maybe based on a variable ?string $schema passed to the driver, if its null, dont validate, otherwise use passed schema. So yeah, with that knowledge allowing a custom xsd schema may be the easiest way around these problems.

greg0ire commented 8 months ago

Note that disabling the XSD validation is not possible on 3, but possible on 2, so if it comes down to that, we can revert https://github.com/doctrine/orm/pull/10768 and change the default value so that XSD validation is enabled by default (on 3, it is disabled by default). What's not great with that is that it's a global setting. So maybe the best would be to come up with a way to do that via the document as well. One way to do that would be to disable validation if the xsi:schemaLocation attribute cannot be found in the document.

oleg-andreyev commented 6 months ago

Current workaround is add "decorator" and override validate method: https://github.com/doctrine/orm/issues/11349#issuecomment-1989465309

but "extension" must import original XSD also.

We can't have an array of XSD because each time XSD validation will be failing due to missing things, so it must be somehow merged and gedmo-extension does good thing:

<?xml version="1.0" encoding="UTF-8"?>
<schema elementFormDefault="qualified" xmlns="http://www.w3.org/2001/XMLSchema">
    <import namespace="http://doctrine-project.org/schemas/orm/doctrine-mapping" schemaLocation="./vendor/doctrine/orm/doctrine-mapping.xsd"/>
    <import namespace="http://gediminasm.org/schemas/orm/doctrine-extensions-mapping" schemaLocation="./schemas/orm/doctrine-extensions-mapping-3-0.xsd"/>
</schema>
greg0ire commented 6 months ago

@oleg-andreyev another workaround is to disable the validation entirely, we re-added that possibility on 3.x

oleg-andreyev commented 6 months ago

@greg0ire imo disabling validation is not a good idea, I personally like how XML is validated and it saves money and time if something is wrong with mapping.

greg0ire commented 6 months ago

I'm glad you like it! :heart:

We can't have an array of XSD because each time XSD validation will be failing due to missing things, so it must be somehow merged and gedmo-extension does good thing:

I'm not 100% sure what you are talking about here, but maybe what I suggest in https://github.com/doctrine/orm/issues/11123#issuecomment-1895867870 addresses that point?

stof commented 3 months ago

For reference, symfony/dependency-injection solved this case of combining schemas in Symfony 2.0 (in 2010) to handle the case of XML configuration file providing the semantic configuration of bundles: https://github.com/symfony/symfony/blob/2cb470e338ff6b04d17903a9ce7c130b7e333dbd/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php#L675-L753

The part that will need to be different is the way to register the location of XSD schemas for other namespaces (the DI component expects bundles to provide a path to their local copy of the schema, overriding the location provided in schemaLocation).