Skip to content

Implementations of innerSerialize lack proper namespace #1537

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

Closed
ralfhergert opened this issue Mar 9, 2023 · 11 comments
Closed

Implementations of innerSerialize lack proper namespace #1537

ralfhergert opened this issue Mar 9, 2023 · 11 comments
Milestone

Comments

@ralfhergert
Copy link

(Concerning marklogic-java-client in 6.1.0) The StructuredQueryBuilder currently only allows to serialize a given StructuredQueryDefinition into a String. In cases in which the query needs to be embedded into another XML this makes it necessary to parse the query again.

Here I would kindly request the following feature: Could you please create a public accessible method like:
StructuredQueryBuilder#serialize(XMLStreamWriter) ?

This would vastly improve the possibilities for serialization. For instance org.jdom2.input.StAXStreamWriter could be used to directly serialize a query into a JDOM.

Meanwhile to get access to the method

public abstract void innerSerialize(XMLStreamWriter serializer) throws XMLStreamException;
we subclassed StructuredQueryBuilder.
By using org.jdom2.input.StAXStreamWriter we noticed, that the namespaces are wrong. What should look like

<query
    xmlns="http://marklogic.com/appservices/search"
    xmlns:xs="http://www.w3.org/2001/XMLSchema"
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
    <word-query>
        <field name="title"/><text>Test</text>
    </word-query>
</query>

was created as:

<query xmlns="http://marklogic.com/appservices/search"
    xmlns:search="http://marklogic.com/appservices/search">
    <word-query xmlns="">
        <field name="title"/><text>Test</text>
    </word-query>
</query>

Notice how the default namespace on word-query is reset.

If not mistaken I would believe this is due to the fact that the elements (like the word-query) are created with no namespaceUri. c.f.

this should be instead

serializer.writeStartElement(SEARCH_API_NS, "word-query");

Shouldn't it?

@rjrudin
Copy link
Contributor

rjrudin commented Mar 9, 2023

Hi @ralfhergert - to confirm, I think you are identifying an enhancement and a bug:

  1. Allow for serializing a query via an XMLStreamWriter.
  2. The existing serialization has a bug where query elements incorrectly have no namespace as opposed to the search namespace.

Is that correct?

For the first item - just to clarify, the existing serialize method is a member of AbstractStructuredQuery, not StructuredQueryBuilder. So I believe your request would be to add serialize(XMLStreamWriter) to AbstractStructuredQuery, right?

@ralfhergert
Copy link
Author

ralfhergert commented Mar 9, 2023

Hi @rjrudin. Yes my issue is both, a feature request and a bug issue. I think they are connected: as soon as a method like serialize(XMLStreamWriter) exists it gets substantial that the correct events are being produced.

Concering the class, since AbstractStructuredQuery is an internal class (not intended for public use). It might not be the right place, eighter. The right place could be the interface StructuredQueryDefinition. This interface also declares the parameter-less serialize-method.

@rjrudin
Copy link
Contributor

rjrudin commented Mar 9, 2023

Thanks @ralfhergert , you're right - StructuredQueryDefinition is the correct location.

We've captured both the enhancement request and the bug report. We'll let you know soon about either a 6.1.x bug release for the latter or a 6.2.0 for both.

@ralfhergert
Copy link
Author

ralfhergert commented Mar 28, 2023 via email

@rjrudin
Copy link
Contributor

rjrudin commented Mar 28, 2023

Hey @ralfhergert - we're planning our 6.2.0 release and looking to include this. Do you have a code snippet to share where you're using org.jdom2.input.StAXStreamWriter? I'm thinking that would be a good test case for us.

@rjrudin
Copy link
Contributor

rjrudin commented Mar 28, 2023

Ah - I see why innerSerialize won't currently work for the intended use case (which makes sense, since it's not public) - it's only invoked in the context of writeStructuredQueryImpl, which has a default namespace set.

@rjrudin
Copy link
Contributor

rjrudin commented Mar 28, 2023

@ralfhergert Take a look at the simple test I wrote in 4373679 . That produces the correct result, but it relies on the user doing the following:

  1. Must call factory.setProperty(XMLOutputFactory.IS_REPAIRING_NAMESPACES, true); in order for a default namespace to work correctly.
  2. Must set the default namespace to the Search namespace - serializer.setDefaultNamespace(SEARCH_NS);.
  3. And then the kicker - it appears necessary to wrap the serialized query in an element in the Search namespace - e.g. serializer.writeStartElement(SEARCH_NS, "theMarkLogicQuery");.

If you do that, then serialize(XMLStreamWriter) will work correctly.

I'm wondering if those requirements are feasible for you though - particularly the need for a wrapper element.

If not, I am thinking it would be necessary to modify the existing serialization implementation so that the Search namespace is always applied when an element is started. That's feasible, I just wanted to check on the above approach with you first.

@rjrudin
Copy link
Contributor

rjrudin commented Mar 28, 2023

Hmm - having said the above, it looks simple to just modify the few dozen places where the Java Client starts an element so that the Search namespace is always used. I can't think of any downsides to this. I did some testing of this with a default namespace, and STaX behaves the way you'd expect - i.e. if the default namespace is the Search namespace, it won't e.g. create a new namespace prefix or anything funky like that.

So I'm now thinking the right way to address this is:

  1. Expose serialize(XMLStreamWriter) in StructuredQueryDefinition.
  2. Ensure that the Search namespace is applied to each element regardless of how the above method is used.

@rjrudin
Copy link
Contributor

rjrudin commented Mar 28, 2023

@ralfhergert Sorry one more question - is your use case here to construct a combined query document - e.g. https://docs.marklogic.com/guide/rest-dev/search#id_84389 - using STaX? Or are you constructing some other kind of document?

@rjrudin rjrudin added this to the 6.2.0 milestone Mar 28, 2023
@rjrudin
Copy link
Contributor

rjrudin commented Mar 28, 2023

Resolved via #1543 . We'll release 6.2.0 soon.

@rjrudin rjrudin closed this as completed Mar 28, 2023
@rjrudin
Copy link
Contributor

rjrudin commented Apr 26, 2023

@ralfhergert 6.2.0 is now available, though we're still in the process of updating the Javadocs. But the method you're looking for is now there.

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

2 participants