Skip to content

MLE-27078 More tests for this bug#1906

Merged
rjrudin merged 1 commit intodevelopfrom
feature/27078-fix
Feb 18, 2026
Merged

MLE-27078 More tests for this bug#1906
rjrudin merged 1 commit intodevelopfrom
feature/27078-fix

Conversation

@rjrudin
Copy link
Contributor

@rjrudin rjrudin commented Feb 18, 2026

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

Copilot AI review requested due to automatic review settings February 18, 2026 15:05
@github-actions
Copy link

github-actions bot commented Feb 18, 2026

Copyright Validation Results
Total: 5 | Passed: 2 | Failed: 0 | Skipped: 3 | at: 2026-02-18 18:46:27 UTC | commit: 31e1744

⏭️ Skipped (Excluded) Files

  • .env
  • Jenkinsfile
  • test-app/build.gradle

✅ Valid Files

  • marklogic-client-api/src/main/java/com/marklogic/client/impl/DocumentManagerImpl.java
  • marklogic-client-api/src/test/java/com/marklogic/client/test/document/ReadJsonAsBinaryTest.java

✅ All files have valid copyright headers!

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request fixes a bug (MLE-27078) in DocumentManagerImpl where a class field was being used to track whether metadata categories were modified, leading to incorrect behavior. The fix replaces the stateful tracking with a simple method that determines if the metadata categories differ from the default (ALL).

Changes:

  • Removed the stateful isProcessedMetadataModified boolean field and the overridden add/addAll methods in the processedMetadata HashSet
  • Introduced a new private method isMetadataCategoriesNotJustAll() to check if metadata categories have been modified from the default
  • Added comprehensive test coverage demonstrating the fix allows bulk reads after setting metadata to ALL

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
ReadJsonAsBinaryTest.java Adds new test methods to verify bulk read behavior with different metadata category configurations, including the case where setting to ALL should work correctly
DocumentManagerImpl.java Removes problematic stateful tracking and replaces it with a cleaner method-based approach; updates all references from processedMetadata to metadataCategories

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 52 to 56
Copy link

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The null check for metadataCategories is redundant. The field is instantiated as a new HashSet on line 58 and can never be null. The check should be removed or replaced with a check for an empty set if that's the intended behavior.

Copilot uses AI. Check for mistakes.
Comment on lines 52 to 63
Copy link

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent indentation: this file uses 2-space indentation throughout (see lines 44-45, 49-50, 74-77, 617-623), but tabs are used here. Replace tabs with 2 spaces to maintain consistency.

Copilot uses AI. Check for mistakes.
Comment on lines 116 to 118
Copy link

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent indentation: this file uses 2-space indentation throughout (see lines 44-45, 49-50, 74-77, 617-623), but tabs are used here. Replace tabs with 2 spaces to maintain consistency.

Copilot uses AI. Check for mistakes.
@rjrudin rjrudin force-pushed the feature/27078-fix branch 3 times, most recently from b82f981 to 06eb09b Compare February 18, 2026 16:24
@rjrudin rjrudin changed the title MLE-27078 One more fix for reading JSON as a binary MLE-27078 More tests for this bug Feb 18, 2026
Made a very confusing private field easier to understand in DocumentManagerImpl.
@rjrudin rjrudin merged commit 04aef10 into develop Feb 18, 2026
4 of 5 checks passed
@rjrudin rjrudin deleted the feature/27078-fix branch February 18, 2026 19:07
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

Successfully merging this pull request may close these issues.

2 participants

Comments