Skip to content

Conversation

@rjrudin
Copy link
Contributor

@rjrudin rjrudin commented Dec 31, 2025

No description provided.

Copilot AI review requested due to automatic review settings December 31, 2025 19:44
@github-actions
Copy link

github-actions bot commented Dec 31, 2025

Copyright Validation Results
Total: 4 | Passed: 4 | Failed: 0 | Skipped: 0 | at: 2025-12-31 19:50:35 UTC | commit: f5fbd74

✅ Valid Files

  • marklogic-client-api/src/main/java/com/marklogic/client/datamovement/filter/IncrementalWriteEvalFilter.java
  • marklogic-client-api/src/main/java/com/marklogic/client/datamovement/filter/IncrementalWriteFilter.java
  • marklogic-client-api/src/main/java/com/marklogic/client/datamovement/filter/IncrementalWriteOpticFilter.java
  • marklogic-client-api/src/test/java/com/marklogic/client/datamovement/filter/IncrementalWriteTest.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 PR enhances error handling for incremental writes by providing clearer error messages when a required range index is missing. The changes ensure that users receive helpful feedback when attempting to use the incremental write feature without proper database configuration.

Key Changes:

  • Added explicit error handling in both Optic and Eval-based incremental write filters to catch and enhance error messages when range index queries fail
  • Introduced comprehensive test coverage for missing range index scenarios
  • Refactored test helper methods to improve code organization and reusability

Reviewed changes

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

File Description
IncrementalWriteOpticFilter.java Wraps query execution in try-catch block to provide enhanced error message when range index is missing
IncrementalWriteEvalFilter.java Wraps eval script execution in try-catch block to provide enhanced error message when range index is missing
IncrementalWriteFilter.java Moves hash computation inline with content serialization (appears to be a refactoring)
IncrementalWriteTest.java Adds two new test cases for missing range index scenarios and refactors test helper methods for better reusability

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

Comment on lines 163 to 165
assertTrue(message.contains("Unable to query for existing incremental write hashes") && message.contains("XDMP-FIELDRIDXNOTFOUND"),
"When the user tries to use the incremental write feature without the required range index, we should " +
"fail with a clear error message. Actual message: " + message);
Copy link

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

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

The assertion message mentions 'clear error message' but the next test method uses 'helpful error message' for the same scenario. Consider using consistent language across both test methods to improve maintainability.

Copilot uses AI. Check for mistakes.
Comment on lines +217 to +221
DocumentPage page = mgr.search(Common.client.newQueryManager().newStructuredQueryBuilder().collection("incremental-test"), 1);
while (page.hasNext()) {
DocumentRecord doc = page.next();
DocumentMetadataHandle metadata = doc.getMetadata(new DocumentMetadataHandle());
assertTrue(metadata.getMetadataValues().containsKey("incrementalWriteHash"),
Copy link

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

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

The page size of 1 means this will iterate one document at a time from the server. For verifying 10 documents, consider using a larger page size (e.g., 10 or 100) to reduce the number of round trips to the server and improve test performance.

Suggested change
DocumentPage page = mgr.search(Common.client.newQueryManager().newStructuredQueryBuilder().collection("incremental-test"), 1);
while (page.hasNext()) {
DocumentRecord doc = page.next();
DocumentMetadataHandle metadata = doc.getMetadata(new DocumentMetadataHandle());
assertTrue(metadata.getMetadataValues().containsKey("incrementalWriteHash"),
mgr.setPageLength(10);
DocumentPage page = mgr.search(Common.client.newQueryManager().newStructuredQueryBuilder().collection("incremental-test"), 1);
while (page.hasNext()) {
DocumentRecord doc = page.next();
DocumentMetadataHandle metadata = doc.getMetadata(new DocumentMetadataHandle());

Copilot uses AI. Check for mistakes.
@rjrudin rjrudin force-pushed the feature/incremental-no-range-index branch from b60c77a to f5fbd74 Compare December 31, 2025 19:50
@rjrudin rjrudin merged commit 57a023c into develop Dec 31, 2025
3 checks passed
@rjrudin rjrudin deleted the feature/incremental-no-range-index branch December 31, 2025 20:52
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.

3 participants