MLE-27078 More tests for this bug

Made a very confusing private field easier to understand in DocumentManagerImpl.

Author: rjrudin

.env

@@ -4,4 +4,4 @@ MARKLOGIC_LOGS_VOLUME=./docker/marklogic/logs
 MARKLOGIC_IMAGE=ml-docker-db-dev-tierpoint.bed-artifactory.bedford.progress.com/marklogic/marklogic-server-ubi:latest-12
 
 # Latest public release
-#MARKLOGIC_IMAGE=progressofficial/marklogic-db:latest
+# MARKLOGIC_IMAGE=progressofficial/marklogic-db:latest

Jenkinsfile

@@ -150,7 +150,8 @@ pipeline {
 				}
 			}
 			steps {
-				setupDockerMarkLogic("ml-docker-db-dev-tierpoint.bed-artifactory.bedford.progress.com/marklogic/marklogic-server-ubi:latest-12")
+				setupDockerMarkLogic("progressofficial/marklogic-db:latest")
+				
 				sh label: 'run marklogic-client-api tests', script: '''#!/bin/bash
 					export JAVA_HOME=$JAVA_HOME_DIR
 					export GRADLE_USER_HOME=$WORKSPACE/$GRADLE_DIR

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;
 
@@ -48,24 +48,29 @@ abstract class DocumentManagerImpl<R extends AbstractReadHandle, W extends Abstr
   static final private Logger logger = LoggerFactory
     .getLogger(DocumentManagerImpl.class);
 
-  private boolean isProcessedMetadataModified = false;
-  final private Set<Metadata> processedMetadata = new HashSet<Metadata>() {
+	// UGH. This state variable is used to track if the user modifies the metadataCategories set. That's because the
+	// default value of "ALL" means something different than if the user sets it to "ALL". Specifically, on a bulk read,
+	// the expectation is that no metadata is retrieved. So if the value is "ALL" and the user didn't set that, we
+	// don't get metadata. But if the value is "ALL" and the user did set that, we get all the metadata. UGH!
+	private boolean isMetadataCategoriesModified;
+
+  final private Set<Metadata> metadataCategories = new HashSet<>() {
     @Override
     public boolean add(Metadata e) {
-      isProcessedMetadataModified = true;
+      isMetadataCategoriesModified = true;
       return super.add(e);
     }
 
     @Override
     public boolean addAll(Collection<? extends Metadata> c) {
-      isProcessedMetadataModified = true;
+      isMetadataCategoriesModified = true;
       return super.addAll(c);
     }
   };
   {
-    processedMetadata.add(Metadata.ALL);
+    metadataCategories.add(Metadata.ALL);
     // we need to know if the user modifies after us
-    isProcessedMetadataModified = false;
+    isMetadataCategoriesModified = false;
   }
 
   private RESTServices services;
@@ -113,24 +118,24 @@ 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);
+      metadataCategories.add(category);
   }
 
   @Override
   public Set<Metadata> getMetadataCategories() {
-    return processedMetadata;
+    return metadataCategories;
   }
 
   @Override
   public void clearMetadataCategories() {
-    processedMetadata.clear();
+    metadataCategories.clear();
   }
 
   @Override
@@ -374,7 +379,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);
 
@@ -448,7 +453,7 @@ public DocumentPage read(long serverTimestamp, ServerTransform transform, Transa
       transaction,
       // the default for bulk is no metadata, which differs from the normal
       // default of ALL
-      (isProcessedMetadataModified || !withContent) ? processedMetadata : null,
+      (isMetadataCategoriesModified || !withContent) ? metadataCategories : null,
       nonDocumentFormat,
       mergeTransformParameters((transform != null) ? transform : getReadTransform(), extraParams),
       withContent,
@@ -544,7 +549,7 @@ 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
+    Set<Metadata> metadata = isMetadataCategoriesModified ? metadataCategories
       : null;
     return services.getBulkDocuments(requestLogger, serverTimestamp, querydef, start,
       getPageLength(), transaction, searchHandle, searchView, metadata,
@@ -943,7 +948,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 +1271,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 +1331,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 +1365,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 +1421,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 +1438,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,37 @@
 
 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();
 
+			// With a default manager, metadata shouldn't be read by default, so this should work.
+			verifyBulkRead(docManager);
+
+			// Setting metadata categories to anything will cause the bug.
+			docManager.setMetadataCategories(DocumentManager.Metadata.PERMISSIONS);
+			verifyBulkRead(docManager);
 		}
 	}
 
@@ -56,4 +59,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"