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.

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

Allow custom XSD schemas #11123

Open
theofidry opened this issue Dec 15, 2023 · 14 comments
Open

Allow custom XSD schemas #11123

theofidry opened this issue Dec 15, 2023 · 14 comments

Comments

@theofidry
Copy link

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
Copy link
Member

greg0ire commented Dec 15, 2023

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 🙄 🤦

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:

  • deprecate using a url as an xsd file
  • deprecate using a path to a file that does not exist
  • if the file exists, use it (hopefully that's not a big breaking change)

@beberlei
Copy link
Member

beberlei commented Jan 15, 2024

@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:

orm/doctrine-mapping.xsd

Lines 77 to 83 in a8632ac

<xs:complexType name="lifecycle-callbacks">
<xs:choice minOccurs="0" maxOccurs="unbounded">
<xs:element name="lifecycle-callback" type="orm:lifecycle-callback" minOccurs="1" maxOccurs="unbounded" />
<xs:any minOccurs="0" maxOccurs="unbounded" namespace="##other"/>
</xs:choice>
<xs:anyAttribute namespace="##other"/>
</xs:complexType>

@greg0ire
Copy link
Member

@beberlei @jwage after reading doctrine/mongodb-odm#2607 and #11117 (comment), 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
Copy link
Contributor

mbabker commented Jan 16, 2024

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
Copy link
Member

malarzm commented Jan 16, 2024

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

@mbabker
Copy link
Contributor

mbabker commented Jan 16, 2024

#11123 (comment) 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
Copy link
Member

greg0ire commented Jan 17, 2024

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
Copy link
Member

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
Copy link
Member

greg0ire commented Jan 17, 2024

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 #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
Copy link
Contributor

oleg-andreyev commented Mar 12, 2024

Current workaround is add "decorator" and override validate method:
#11349 (comment)

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
Copy link
Member

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

@oleg-andreyev
Copy link
Contributor

@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
Copy link
Member

I'm glad you like it! ❤️

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 #11123 (comment) addresses that point?

@stof
Copy link
Member

stof commented Jun 25, 2024

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants