Skip to content

JAVA-2673#1950

Open
strogiyotec wants to merge 4 commits intomongodb:mainfrom
strogiyotec:JAVA-2673
Open

JAVA-2673#1950
strogiyotec wants to merge 4 commits intomongodb:mainfrom
strogiyotec:JAVA-2673

Conversation

@strogiyotec
Copy link
Copy Markdown
Contributor

@strogiyotec strogiyotec commented Apr 29, 2026

JAVA-2673
I added a new interface for ReadThenWrite so that we don't have to cast read operation to write operation

@strogiyotec strogiyotec requested a review from rozza April 29, 2026 00:08
@strogiyotec strogiyotec requested a review from a team as a code owner April 29, 2026 00:08
@strogiyotec strogiyotec changed the title JAVA-2673 JAVA-2673 DRAFT PLEASE DO NOT REVIEW Apr 29, 2026
*
* <p>This class is not part of the public API and may be removed or changed at any time</p>
*/
public interface AsyncWriteThenReadOperationCursor<T> {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this is a new interface that will be used by OperationExecutorImpl
It adds a new override to execute that accepts this interface instead of ReadOperation
All methods here were copied from ReadOperation interface

Copy link
Copy Markdown
Contributor

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 introduces a new internal “write-then-read cursor” operation shape and routes non-inline mapReduce execution through it, avoiding binding downcasts and separating inline vs non-inline execution paths.

Changes:

  • Add AsyncWriteThenReadOperationCursor (driver-core) and a corresponding OperationExecutor#execute overload.
  • Update reactive-streams execution plumbing (MongoOperationPublisher, OperationExecutorImpl, VoidWriteOperationThenCursorReadOperation) to support write-then-read cursor operations.
  • Change MapReducePublisherImpl so non-inline mapReduce uses the write-then-read path, and add a unit test asserting inline mapReduce stays on the read-operation path.

Reviewed changes

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

Show a summary per file
File Description
driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/OperationExecutor.java Adds executor overload for write-then-read cursor operations.
driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/OperationExecutorImpl.java Implements new execute path for write-then-read cursor operations.
driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/MongoOperationPublisher.java Adds helper to create Monos for write-then-read cursor operations.
driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/VoidWriteOperationThenCursorReadOperation.java Converts to the new write-then-read cursor operation interface using AsyncReadWriteBinding.
driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/MapReducePublisherImpl.java Routes non-inline mapReduce cursor reads through the new write-then-read path.
driver-core/src/main/com/mongodb/internal/operation/AsyncWriteThenReadOperationCursor.java Introduces new internal operation interface for write-then-read returning a cursor.
driver-reactive-streams/src/test/unit/com/mongodb/reactivestreams/client/internal/TestOperationExecutor.java Extends test executor to record/return write-then-read operations.
driver-reactive-streams/src/test/unit/com/mongodb/reactivestreams/client/internal/TestHelper.java Updates Mockito stubbing for overloaded execute methods.
driver-reactive-streams/src/test/unit/com/mongodb/reactivestreams/client/internal/MapReducePublisherImplTest.java Adds inline routing assertion test (and related assertions/import changes).

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

Comment on lines +184 to +190
@Override
public <T> Mono<AsyncBatchCursor<T>> execute(final AsyncWriteThenReadOperationCursor<T> operation, final ReadConcern readConcern,
@Nullable final ClientSession session) {
isTrue("open", !mongoClient.getCluster().isClosed());
notNull("operation", operation);
notNull("readConcern", readConcern);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I added notifyOperationInitiated call , also implementationf of notifyOperationInitiated now also checked instance of AsyncWriteThenRead

Comment on lines +211 to +216
public Mono<BatchCursor<T>> batchCursor(final int initialBatchSize) {
if (inline) {
return super.batchCursor(initialBatchSize);
}
return writeThenReadBatchCursor(initialBatchSize);
}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added a unit test here


import com.mongodb.reactivestreams.client.MapReducePublisher;


Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

removed empty line

This comment was marked as resolved.

@strogiyotec strogiyotec changed the title JAVA-2673 DRAFT PLEASE DO NOT REVIEW JAVA-2673 Apr 29, 2026
if (inline) {
return super.first();
}
return writeThenReadBatchCursor(1)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this number 1 comes from BatchCursorPublisher

Copy link
Copy Markdown
Contributor Author

@strogiyotec strogiyotec left a comment

Choose a reason for hiding this comment

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

clarifing comments

Copy link
Copy Markdown
Member

@rozza rozza left a comment

Choose a reason for hiding this comment

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

I think both async and sync paths should follow the same logic.

I also think that as this is for the deprecated MapReduce functionality I'm not sure its worth changing eg: I can live with the code smell.

Update: Seems Aggregation with $out also has this code smell.

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