MLE-27078 One more fix for reading JSON as a binary

DocumentManagerImpl was needlessly using a class field (state = bad!) to track whether the user had modified the metadata categories to retrieve. So e.g. in the Spark connector, when we set "ALL" on the doc manager, the state variable was set to true - even though "ALL" is the default. That results in another way of triggering bug MLE-27191.

The class field is now gone in favor of a simple private method to determine if the user has modified the metadata categories to something besides just "ALL" - and it's more accurate now. One of the new tests passes as a result - i.e. doing a bulk read after setting metadataCategories to ALL.

Author: rjrudin

marklogic-client-api/src/main/java/com/marklogic/client/impl/DocumentManagerImpl.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010-2025 Progress Software Corporation and/or its subsidiaries or affiliates. All Rights Reserved.
+ * Copyright (c) 2010-2026 Progress Software Corporation and/or its subsidiaries or affiliates. All Rights Reserved.
  */
 package com.marklogic.client.impl;
 
@@ -43,30 +43,23 @@
 abstract class DocumentManagerImpl<R extends AbstractReadHandle, W extends AbstractWriteHandle>
   extends AbstractLoggingManager implements DocumentManager<R, W>,
   TemporalDocumentManager<R, W> {
+
   static final private long DEFAULT_PAGE_LENGTH = 50;
 
   static final private Logger logger = LoggerFactory
     .getLogger(DocumentManagerImpl.class);
 
-  private boolean isProcessedMetadataModified = false;
-  final private Set<Metadata> processedMetadata = new HashSet<Metadata>() {
-    @Override
-    public boolean add(Metadata e) {
-      isProcessedMetadataModified = true;
-      return super.add(e);
-    }
+	private boolean isMetadataCategoriesNotJustAll() {
+		return metadataCategories.size() != 1 ||
+			!Metadata.ALL.equals(metadataCategories.iterator().next());
+	}
 
-    @Override
-    public boolean addAll(Collection<? extends Metadata> c) {
-      isProcessedMetadataModified = true;
-      return super.addAll(c);
-    }
-  };
-  {
-    processedMetadata.add(Metadata.ALL);
-    // we need to know if the user modifies after us
-    isProcessedMetadataModified = false;
-  }
+	final private Set<Metadata> metadataCategories = new HashSet<>();
+
+	{
+		// Initialize the default metadata to retrieve on a read.
+		metadataCategories.add(Metadata.ALL);
+	}
 
   private RESTServices services;
   private Format contentFormat;
@@ -113,24 +106,25 @@ public void setContentFormat(Format format) {
   @Override
   public void setMetadataCategories(Set<Metadata> categories) {
     clearMetadataCategories();
-    processedMetadata.addAll(categories);
+    metadataCategories.addAll(categories);
   }
 
   @Override
   public void setMetadataCategories(Metadata... categories) {
     clearMetadataCategories();
-    for (Metadata category : categories)
-      processedMetadata.add(category);
+    for (Metadata category : categories) {
+		metadataCategories.add(category);
+	}
   }
 
   @Override
   public Set<Metadata> getMetadataCategories() {
-    return processedMetadata;
+    return metadataCategories;
   }
 
   @Override
   public void clearMetadataCategories() {
-    processedMetadata.clear();
+    metadataCategories.clear();
   }
 
   @Override
@@ -374,7 +368,7 @@ public <T extends R> T read(DocumentDescriptor desc,
       requestLogger,
       desc,
       transaction,
-      (metadataHandle != null) ? processedMetadata : null,
+      (metadataHandle != null) ? metadataCategories : null,
       mergeTransformParameters((transform != null) ? transform
         : getReadTransform(), extraParams), metadataHandle, contentHandle);
 
@@ -446,9 +440,8 @@ public DocumentPage read(long serverTimestamp, ServerTransform transform, Transa
       requestLogger,
       serverTimestamp,
       transaction,
-      // the default for bulk is no metadata, which differs from the normal
-      // default of ALL
-      (isProcessedMetadataModified || !withContent) ? processedMetadata : null,
+      // the default for bulk is no metadata, which differs from the normal default of ALL
+      (isMetadataCategoriesNotJustAll() || !withContent) ? metadataCategories : null,
       nonDocumentFormat,
       mergeTransformParameters((transform != null) ? transform : getReadTransform(), extraParams),
       withContent,
@@ -542,10 +535,8 @@ private DocumentPage search(SearchQueryDefinition querydef, long start,
       }
     }
 
-    // the default for bulk is no metadata, which differs from the normal
-    // default of ALL
-    Set<Metadata> metadata = isProcessedMetadataModified ? processedMetadata
-      : null;
+    // the default for bulk is no metadata, which differs from the normal default of ALL
+    Set<Metadata> metadata = isMetadataCategoriesNotJustAll() ? metadataCategories : null;
     return services.getBulkDocuments(requestLogger, serverTimestamp, querydef, start,
       getPageLength(), transaction, searchHandle, searchView, metadata,
       nonDocumentFormat, getReadTransform(), null, forestName);
@@ -943,7 +934,7 @@ protected TemporalDescriptor write(DocumentDescriptor desc, String temporalDocum
       requestLogger,
       desc,
       transaction,
-      (metadataHandle != null) ? processedMetadata : null,
+      (metadataHandle != null) ? metadataCategories : null,
       mergeTransformParameters((transform != null) ? transform
         : getWriteTransform(), extraParams), metadataHandle, contentHandle);
   }
@@ -1266,7 +1257,7 @@ protected DocumentDescriptorImpl create(DocumentUriTemplate template,
       requestLogger,
       template,
       transaction,
-      (metadataHandle != null) ? processedMetadata : null,
+      (metadataHandle != null) ? metadataCategories : null,
       mergeTransformParameters((transform != null) ? transform
         : getWriteTransform(), extraParams), metadataHandle, contentHandle);
   }
@@ -1326,7 +1317,7 @@ public void patch(DocumentDescriptor desc, DocumentPatchHandle patch,
     DocumentPatchHandleImpl builtPatch = (patch instanceof DocumentPatchHandleImpl) ? (DocumentPatchHandleImpl) patch
       : null;
     services.patchDocument(requestLogger, desc, transaction,
-      (builtPatch != null) ? builtPatch.getMetadata() : processedMetadata,
+      (builtPatch != null) ? builtPatch.getMetadata() : metadataCategories,
       (builtPatch != null) ? builtPatch.isOnContent() : true, patch);
   }
 
@@ -1360,7 +1351,7 @@ public void patch(String uri, String temporalDocumentURI,  String temporalCollec
     extraParams = addTemporalParams(extraParams, temporalCollection, temporalDocumentURI, null);
     DocumentPatchHandleImpl builtPatch = (patch instanceof DocumentPatchHandleImpl) ? (DocumentPatchHandleImpl) patch
       : null;
-    services.patchDocument(requestLogger, new DocumentDescriptorImpl(uri, true), transaction, (builtPatch != null) ? builtPatch.getMetadata() : processedMetadata,
+    services.patchDocument(requestLogger, new DocumentDescriptorImpl(uri, true), transaction, (builtPatch != null) ? builtPatch.getMetadata() : metadataCategories,
       (builtPatch != null) ? builtPatch.isOnContent() : true, extraParams, sourceDocumentURI, patch);
   }
 
@@ -1416,7 +1407,7 @@ public void writeDefaultMetadata(String uri, Transaction transaction)
       logger.info("Resetting metadata for {}", uri);
 
     services.deleteDocument(requestLogger,
-      new DocumentDescriptorImpl(uri, true), transaction, processedMetadata,
+      new DocumentDescriptorImpl(uri, true), transaction, metadataCategories,
       getWriteParams());
   }
 
@@ -1433,7 +1424,7 @@ public void writeDefaultMetadata(Transaction transaction, String... uris)
     if (uris.length == 0)
       throw new IllegalArgumentException(
               "Resetting document metadata with empty identifier list");
-    services.delete(requestLogger, transaction, processedMetadata, uris);
+    services.delete(requestLogger, transaction, metadataCategories, uris);
   }
 
   @Override

marklogic-client-api/src/test/java/com/marklogic/client/test/document/ReadJsonAsBinaryTest.java

@@ -17,34 +17,39 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
+@Disabled("See MLE-27191 for description of the server bug")
 class ReadJsonAsBinaryTest extends AbstractClientTest {
 
+	private static final String URI = "/a.json";
+
 	@Test
-	@Disabled("See MLE-27191 for description of the server bug")
-	void test() {
+	void genericDocumentManager() {
 		try (DatabaseClient client = Common.newClient()) {
-			final String uri = "/a.json";
-			writeJsonAsBinary(uri, client);
+			writeJsonAsBinary(URI, client);
 
 			GenericDocumentManager docManager = client.newDocumentManager();
+			// A bulk read works...
+			verifyBulkRead(docManager);
+			// But a request for a single URI does not!
+			verifySingleRead(docManager);
+		}
+	}
 
-			// A multipart request returns the correct document format.
-			try (DocumentPage page = docManager.read(uri)) {
-				DocumentRecord record = page.next();
-				assertEquals(uri, record.getUri());
-				assertEquals("application/json", record.getMimetype());
-				assertEquals(Format.BINARY, record.getFormat());
-			}
+	@Test
+	void managerWithModifiedMetadataCategories() {
+		try (DatabaseClient client = Common.newClient()) {
+			writeJsonAsBinary(URI, client);
 
-			// But a request for a single URI does not!
-			try (InputStreamHandle handle = docManager.read(uri, new InputStreamHandle())) {
-				assertEquals("application/json", handle.getMimetype());
-				assertEquals(Format.BINARY, handle.getFormat(), "Unfortunately due to MLE-27191, the " +
-					"format is JSON instead of binary. So this assertion will fail until that bug is fixed. " +
-					"For a Java Client user, the workaround is to use the read method above and get a DocumentPage " +
-					"back, as a multipart response seems to identify the correct headers.");
-			}
+			GenericDocumentManager docManager = client.newDocumentManager();
 
+			// Setting metadata categories to ALL should have no effect since that results in no metadata being
+			// retrieved on a bulk read.
+			docManager.setMetadataCategories(DocumentManager.Metadata.ALL);
+			verifyBulkRead(docManager);
+
+			// Setting metadata categories to anything besides ALL will cause the bug
+			docManager.setMetadataCategories(DocumentManager.Metadata.PERMISSIONS);
+			verifyBulkRead(docManager);
 		}
 	}
 
@@ -56,4 +61,23 @@ private void writeJsonAsBinary(String uri, DatabaseClient client) {
 		jsonDocManager.setWriteTransform(new ServerTransform("toBinary"));
 		jsonDocManager.write(uri, metadata, new JacksonHandle(new ObjectMapper().createObjectNode().put("a", 1)));
 	}
+
+	private void verifySingleRead(GenericDocumentManager docManager) {
+		try (InputStreamHandle handle = docManager.read(URI, new InputStreamHandle())) {
+			assertEquals("application/json", handle.getMimetype());
+			assertEquals(Format.BINARY, handle.getFormat(), "Unfortunately due to MLE-27191, the " +
+				"format is JSON instead of binary. So this assertion will fail until that bug is fixed. " +
+				"For a Java Client user, the workaround is to use the read method above and get a DocumentPage " +
+				"back, as a multipart response seems to identify the correct headers.");
+		}
+	}
+
+	private void verifyBulkRead(GenericDocumentManager docManager) {
+		try (DocumentPage page = docManager.read(URI)) {
+			DocumentRecord record = page.next();
+			assertEquals(URI, record.getUri());
+			assertEquals("application/json", record.getMimetype());
+			assertEquals(Format.BINARY, record.getFormat());
+		}
+	}
 }

test-app/build.gradle

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010-2025 Progress Software Corporation and/or its subsidiaries or affiliates. All Rights Reserved.
+ * Copyright (c) 2010-2026 Progress Software Corporation and/or its subsidiaries or affiliates. All Rights Reserved.
  */
 
 buildscript {
@@ -25,6 +25,11 @@ plugins {
 
 apply plugin: "com.marklogic.ml-gradle"
 
+// Sometimes, 3 x 20 isn't enough on Jenkins.
+mlWaitTillReady {
+	maxAttempts = 40
+}
+
 dependencies {
 	implementation "io.undertow:undertow-core:2.3.20.Final"
 	implementation "io.undertow:undertow-servlet:2.3.20.Final"