MLE-26420 Added DocumentWriteSetFilter #1867
Conversation
|
Copyright Validation Results ⏭️ Skipped (Excluded) Files
✅ Valid Files
✅ All files have valid copyright headers! |
rjrudin
force-pushed
the
feature/26420-just-filter
branch
from
eef2d02 to
c947055
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces a new DocumentWriteSetFilter interface that allows users to filter or modify document batches before they are written to MarkLogic. The filter can inspect batch context (batch number, database client, temporal collection) and return a modified or new DocumentWriteSet, or return null/empty to skip the write entirely.
Key changes:
- Added
DocumentWriteSetFilterinterface with aContextthat provides batch information - Integrated filter support into
WriteBatcherandBatchWriterto apply filters before writing - Reorganized test structure to enable testing of protected methods
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
DocumentWriteSetFilter.java |
New interface defining the filter contract and context |
WriteBatcher.java |
Added withDocumentWriteSetFilter() method to configure filters |
WriteBatcherImpl.java |
Stores filter and passes it to BatchWriter instances |
BatchWriter.java |
Applies filter before writing and handles empty results |
BatchWriteSet.java |
Implements DocumentWriteSetFilter.Context interface |
RemoveAllDocumentsFilterTest.java |
Test verifying filter behavior when all documents are removed |
WriteBatcherTemplate.java |
Helper class for reducing boilerplate in tests |
WriteNakedPropertiesTest.java |
Moved to different package for access to protected methods |
IncrementalWriteTest.java |
Removed (functionality moved to WriteBatcherTemplate) |
AbstractClientTest.java |
Added client cleanup to prevent resource leaks |
RetryIOExceptionInterceptor.java |
Extended exception handling to include MarkLogicIOException |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| documentWriteSet = filter.apply(batchWriteSet); | ||
| if (documentWriteSet == null || documentWriteSet.isEmpty()) { | ||
| logger.debug("Filter returned empty write set for batch {}, skipping write", batchWriteSet.getBatchNumber()); | ||
| closeAllHandles(); |
There was a problem hiding this comment.
When the filter returns an empty DocumentWriteSet, closeAllHandles() is called on the original batchWriteSet.getDocumentWriteSet() handles, but these handles are not necessarily the same as those in the filtered documentWriteSet. If the filter created a new DocumentWriteSet, the original handles from batchWriteSet won't be closed. Consider closing handles from both the original and filtered sets, or ensure handles are properly managed when filters return new DocumentWriteSets.
| private final ServerTransform transform; | ||
| private final String temporalCollection; | ||
|
|
||
| // Can be overridden after creation |
There was a problem hiding this comment.
The comment 'Can be overridden after creation' is misleading as this is a private field, not an overridable method. Consider revising to 'Can be replaced after creation via updateWithFilteredDocumentWriteSet()' to more accurately describe the intended usage pattern.
| // Can be overridden after creation | |
| // Can be replaced after creation via updateWithFilteredDocumentWriteSet() |
| import java.util.function.Consumer; | ||
|
|
||
| // Experimenting with a template that gets rid of some annoying DMSDK boilerplate. | ||
| record WriteBatcherTemplate(DatabaseClient databaseClient) { |
There was a problem hiding this comment.
The WriteBatcherTemplate record has package-private visibility but is used across different test packages (e.g., RemoveAllDocumentsFilterTest). Consider making it public or moving it to a more accessible location to support its intended cross-package usage.
| record WriteBatcherTemplate(DatabaseClient databaseClient) { | |
| public record WriteBatcherTemplate(DatabaseClient databaseClient) { |
Started making tests under "com.marklogic.client.datamovement" as well so that protected methods can be unit-tested.
rjrudin
force-pushed
the
feature/26420-just-filter
branch
from
c947055 to
a285cd5
Compare
Started making tests under "com.marklogic.client.datamovement" as well so that protected methods can be unit-tested.