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

Cannot read Document via its DocumentDescriptor #1729

Open
ralfhergert opened this issue Dec 4, 2024 · 6 comments
Open

Cannot read Document via its DocumentDescriptor #1729

ralfhergert opened this issue Dec 4, 2024 · 6 comments

Comments

@ralfhergert
Copy link

Hi @rjrudin,

I'm not sure whether this is expected behaviour or whether this is actually a bug:

(Using makrlogic-java-client in version 7.0.0)
Retrievung the DocumentDescriptor of an existing document works as expected:

final DocumentDescriptor documentDescriptor = xmlDocumentManager.exists(docUri);

This DocumentDescriptor has the version of the document set.

Problem is this DocumentDescriptor cannot be used to read the document. Calling:

xmlDocumentManager.read(documentDescriptor, new JAXBHandle<>(jaxbContext, docClass));

will just return a null handle.

This is strange as the write call works just fine when using this DocumentDescriptor:

xmlDocumentManager.write(documentDescriptor, writeHandle);
@rjrudin
Copy link
Contributor

rjrudin commented Dec 5, 2024

I tried this as a quick test:

@XmlRootElement
public static class TestClass {
	public String letter;
}

@Test
void jaxbTest() throws Exception {
	String uri = "/a.xml";
	DatabaseClient client = DatabaseClientFactory.newClient("localhost", 8003,
		new DatabaseClientFactory.DigestAuthContext("admin", "admin"));

	XMLDocumentManager xmlMgr = client.newXMLDocumentManager();
	DocumentDescriptor desc = xmlMgr.exists(uri);
	System.out.println(desc.getUri());

	TestClass output = xmlMgr.read(desc, new JAXBHandle<>(JAXBContext.newInstance(TestClass.class), TestClass.class)).get();
	System.out.println(output.letter);
}

And I used the following in qconsole to insert a test document:

xdmp:document-insert("/a.xml", <testClass><letter>a</letter></testClass>)

That produced the following output:

/a.xml
a

Does that look like the same as your test?

@ralfhergert
Copy link
Author

Yes, that is exactly, how I want to use it. Could you verify that the version is set on your descriptor?

Assertions.assertNotEquals(-1, desc.getVersion(), "version should not be unknown");

Because in my tests the read attempt works fine as long as the version number is -1 (aka "unknown"), but as soon as the version number is a real timestamp the read call no longer gets the document from the database.

Here is the code I'm using. It is the same as yours except the null check on the descriptor and some generics.

private final JAXBContext jaxbContext;
private final Class<DocType> docClass;

/** happens inside constructor*/
this.jaxbContext = JAXBContextHelper.newContext(docClass);
DatabaseClientFactory.getHandleRegistry().register(JAXBHandle.newFactory(docClass));

public DocType read(String docUri) {
    final XMLDocumentManager manager = databaseClient.newXMLDocumentManager();

    final DocumentDescriptor documentDescriptor = manager.exists(docUri);
    if (documentDescriptor == null) {
        throw new NotFoundException("did not find document " + docUri);
    }
    final DocType doc = manager.read(documentDescriptor, new JAXBHandle<>(jaxbContext, docClass)).get();
    [...]

and I run straight into a NullPointerException:

java.lang.NullPointerException: Cannot invoke "com.marklogic.client.io.JAXBHandle.get()" because the return value of "com.marklogic.client.document.XMLDocumentManager.read(com.marklogic.client.document.DocumentDescriptor, com.marklogic.client.io.marker.AbstractReadHandle)" is null

@rjrudin
Copy link
Contributor

rjrudin commented Dec 9, 2024

Yup, I just reproduced this by adding this file named rest-properties.json to src/main/ml-modules in my test app:

{
  "update-policy": "VERSION_OPTIONAL"
}

That results in a non-negative-1 version and I get the same error. Digging into it further.

@rjrudin
Copy link
Contributor

rjrudin commented Dec 9, 2024

@ralfhergert I think this is working as designed, per the example at https://docs.marklogic.com/guide/rest-dev/documents#id_37703 .

Note step 4 - the document is retrieved with the versionId that matches the one in the database for the document, and so nothing is returned.

For the Java Client, that means the Handle object is set to null, and calling get() on it throws the NPE.

I can see an argument for having the Handle object be non-null and get() returning null instead, but the current behavior has likely been in place for many years and changing it now would be considered a breaking change.

@ralfhergert
Copy link
Author

Thanks for analyzing this issue deeper, @rjrudin . And thanks for the hint towards /src/main/ml-modules/rest-properties.xml file. Yes, here we have the update-policy set to VERSION_OPTIONAL. (I guess to get the CPF and Alerting running?!)

Concerning the REST-API and step 4: I agree, here it is obviously a "read only if modified" call, and the HTTP-304-response is an expected result.

But from the perspective of the XmlDocumentManager this behaviour is not obvious. For instance the JavaDoc for <T extends R> T read(DocumentDescriptor desc, T contentHandle) (perma link) states

<T extends R> T read(DocumentDescriptor desc, T contentHandle)

Reads the document content from the database in the representations provided by the handles

To call read(), an application must authenticate as rest-reader, rest-writer, or rest-admin.

@param ...

My disconnect is, that nothing here in the JavaDoc explains, that the version number in the descriptor suddenly may have an impact, and I might get a null-handle back. Up until now I would say this "only if modified" is not a well established concept in this client. Or is it?

I could imagine different solutions:

  1. JavaDoc could be enhanced explaining this aspect;
  2. Instead of creating a null-handle, throwning a NotModifiedException could make more sense (this would better fit into the scheme set by the ResourceNotFoundException); (On the other hand this "only if modified" is not a well established theme in the client (I would say)... so maybe ...)
  3. None of the above: just ignore the version-number in the description when preparing the read-request and deliver the document anyway. (this would indeed be a breaking change.)

@rjrudin
Copy link
Contributor

rjrudin commented Dec 10, 2024

@ralfhergert I think you hit the nail on the head with approach number 1. A colleague and I are looking at this, and it strikes us both that the following two places in the Javadocs (which of course would appear in most IDEs) should make the possibility of a null return value very clear:

  1. Each read method in DocumentManager that accepts a DocumentDescriptor.
  2. DocumentDescriptor itself should make it more clear that a primary use case for it is content versioning, which means reading with a DD may return null.

We're tracking this internally via ticket MLE-18751 . Thanks for the feedback!

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