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

XML config seems partially broken #303

Closed
Stratadox opened this issue Feb 17, 2020 · 2 comments
Closed

XML config seems partially broken #303

Stratadox opened this issue Feb 17, 2020 · 2 comments
Labels

Comments

@Stratadox
Copy link
Contributor

Stratadox commented Feb 17, 2020

Hey all,

This library looks like a great time saver! However it seems like the xml configuration does not work as advertised out of the box.

I made this xml configuration for my AccountOverview class:

<?xml version="1.0" encoding="UTF-8" ?>
<serializer>
    <class
        name="Stratadox\CardGame\ReadModel\Account\AccountOverview"
        xml-root-name="resource"
        xmlns:h="https://github.com/willdurand/Hateoas"
    >
        <property name="visitorId" exclude="true" />
        <h:relation rel="self">
            <h:href uri="test://foo" />
        </h:relation>
        <h:relation rel="proposals:open">
            <h:href uri="test://bar" />
        </h:relation>
    </class>
</serializer>

And the HatoasBuilder is (or, was) configured as follows:

       $this->serializer = HateoasBuilder::create()
            ->setXmlSerializer(new XmlHalSerializer())
            ->setJsonSerializer(new JsonHalSerializer())
            ->addMetadataDir(__DIR__ . '/../../src/Infrastructure/Rest/Config')
            ->build();

According to the readme:

The tag name of an embedded resource is inferred from the @XmlRoot annotation (xml_root_name in YAML, xml-root-name in XML) coming from the Serializer configuration.

Accordingly, I would have expected the serializer to produce a resource element with excluded visitor id and two links, something like:

<?xml version="1.0" encoding="UTF-8" ?>
<resource href="test://foo">
  <foo>bar</foo>
  <link rel="proposals:open" href="test://bar" />
</resource>

Instead I received a result element with (not excluded) visitor id. The two links however are present, indicating that the configuration file is not skipped.

<?xml version="1.0" encoding="UTF-8" ?>
<result href="test://foo">
  <foo>bar</foo>
  <visitor-id>xyz</visitor-id>
  <link rel="proposals:open" href="test://bar" />
</result>

Solution

Using annotations instead of a configuration file works and would be a potential solution, yet unacceptable in my project.

This situation puzzled me somewhat, so I went bughunting, and found that the Hatoas XML-, YAML- and annotation drivers are only concerned with Hateoas specific elements (a Good Thing™) and the JMS drivers manage the rest of the configuration reading.

Since the JMS drivers, XmlDriver included, all have xml-root-name support, yet only the Annotation seemed to be functioning, the digging continued: it turns out the DefaultDriverFactory produces only an AnnotationDriver by default, unless metadata directories are provided.

The underlying problem seems to be that the Hateoas builder does not communicate to the JMS builder that a configuration directory was provided.

I managed to work around the problem (while keeping my precious xml config) by pre-configuring the SerializerBuilder:

        $serializerBuilder = new SerializerBuilder();
        $serializerBuilder->addMetadataDir(__DIR__ . '/../../src/TheInfrastructure/Rest/Config');
        $this->serializer = HateoasBuilder::create($serializerBuilder)
            ->setXmlSerializer(new XmlHalSerializer())
            ->setJsonSerializer(new JsonHalSerializer())
            ->addMetadataDir(__DIR__ . '/../../src/Infrastructure/Rest/Config')
            ->build();

Having this working, I quickly found that in order to use expressions from within the xml configuration, I also needed to setExpressionEvaluator.
It's all working now, but configuring it all becomes a bit awkward:

        $serializerBuilder = new SerializerBuilder();
        $serializerBuilder
            ->addMetadataDir(__DIR__ . '/../../src/TheInfrastructure/Rest/Config')
            ->setExpressionEvaluator(new ExpressionEvaluator(
                new ExpressionLanguage(),
                ['someService' => $this->someService]
            ));
        $this->serializer = HateoasBuilder::create($serializerBuilder)
            ->setXmlSerializer(new XmlHalSerializer())
            ->setJsonSerializer(new JsonHalSerializer())
            ->addMetadataDir(__DIR__ . '/../../src/TheInfrastructure/Rest/Config')
            ->setExpressionContextVariable('someService', $this->someService)
            ->build();

It seems like relatively easy to fix, I'll try to get a PR in somewhere this week.

TL;DR

HATEOAS builder does not add the MetadataDir nor Expression context variables to the JMS driver, leading to XML and YAML config mostly not working as expected.

@goetas
Copy link
Collaborator

goetas commented Feb 18, 2020

I guess you should use xml-root-name="resource" xml-root-prefix="account" xml-root-namespace="something??".

The XML:

<account:resource href="test://foo">
  <foo>bar</foo>
  <link rel="proposals:open" href="test://bar" />
</account:resource>

is not valid, since the prefix account:resource must be bound to a namespace.

@Stratadox
Copy link
Contributor Author

Ah, apologies for the confusion! The issue isn't about namespaces per se, it's just that xml-root-name, xml-root-prefix and xml-root-namespace don't work (out of the box) when configured in a xml configuration file. I've removed the namespace detail from the initial post, as it's essentially unrelated to the issue: when I configure xml-root-name="resource", the element I end up with is also a <result>.

In the "real" version, the account:resource has a proper namespace applied, although probably implemented in a sub-optimal way.

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

No branches or pull requests

2 participants