Skip to content

Conversation

@rozza
Copy link
Member

@rozza rozza commented Oct 14, 2025

Ensuring collection write concern options are ignored when inside a transaction.

JAVA-5684

…ons are ignored when inside a transaction.

JAVA-5684
@rozza rozza requested a review from a team as a code owner October 14, 2025 09:38
@rozza rozza requested review from Copilot, katcharov and nhachicha and removed request for a team and katcharov October 14, 2025 09:38

This comment was marked as outdated.


@DisplayName("Options Inside Transaction Prose Tests. 1. Write concern not inherited from collection object inside transaction")
@Test
void testWriteConcernInheritance() {
Copy link
Member Author

Choose a reason for hiding this comment

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

Having to use map / flatMap to pass the session into each Mono is painful. But appears to be the correct way to ensure that the session use is correctly ordered.

import static com.mongodb.ClusterFixture.isSharded;
import static org.junit.Assert.assertTrue;
import static org.junit.Assume.assumeTrue;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
Copy link
Member Author

Choose a reason for hiding this comment

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

I took the opportunity to modernize the test to junit5

session.startTransaction();
addresses.add(collection.find(session, Document.parse("{}")));
try (MongoCursor<Document> cursor = collection.find(session, Document.parse("{}")).cursor()) {
addresses.add(cursor.getServerAddress());
Copy link
Member Author

Choose a reason for hiding this comment

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

The test wasn't to spec as it didn't actually check the server addresses, as it does in the prose test spec.

session = client.startSession();
public void testNewTransactionUnpinsSession() {
collection.insertOne(Document.parse("{}"));
try (ClientSession session = client.startSession()) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Prefer try with resources (this code was from java 6 days).

return isSharded();
@DisplayName("Options Inside Transaction Prose Tests. 1. Write concern not inherited from collection object inside transaction")
@Test
void testWriteConcernInheritance() {
Copy link
Member Author

Choose a reason for hiding this comment

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

The new prose test.

@rozza rozza requested a review from Copilot October 14, 2025 09:52

This comment was marked as outdated.

}

@Override
public Publisher<Void> commitTransaction() {
Copy link
Member Author

Choose a reason for hiding this comment

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

Writing the reactive streams prose test identified a bug. Validation was done when the publisher was created and not when the publisher was being used.

This complicated the usage of transactions, a fact that was hidden by the lack of prose tests! This fix is in the remit of JAVA-5895


@BeforeEach
public void setUp() {
assumeTrue(isSharded());
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we also check for server version as per the spec?

Suggested change
assumeTrue(isSharded());
assumeTrue(isSharded());
assumeTrue(serverVersionAtLeast(4, 2));

.applyConnectionString(multiMongosConnectionString);

client = MongoClients.create(MongoClientSettings.builder(builder.build())
.applyToSocketSettings(builder1 -> builder1.readTimeout(5, TimeUnit.SECONDS))
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need the SocketSettings?

collection.insertOne(Document.parse("{}"));
session = client.startSession();
public void testNewTransactionUnpinsSession() {
collection.insertOne(Document.parse("{}"));
Copy link
Contributor

Choose a reason for hiding this comment

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

we can add assumeTrue(serverVersionAtLeast(4, 2)); // spec requires 4.1.6

try {
collection.insertOne(Document.parse("{}"));
session = client.startSession();
collection.insertOne(Document.parse("{}"));
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto (version check)

try (ClientSession session = client.startSession()) {
session.startTransaction();
collection.insertOne(session, Document.parse("{ _id : 1 }"));
session.commitTransaction();
Copy link
Contributor

Choose a reason for hiding this comment

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

The spec test doesn't commit the transaction, an oversight? (same for the previous test)

@DisplayName("Options Inside Transaction Prose Tests. 1. Write concern not inherited from collection object inside transaction")
@Test
void testWriteConcernInheritance() {
assumeTrue(serverVersionAtLeast(4, 4));
Copy link
Contributor

Choose a reason for hiding this comment

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

Why 4.4 only? Isn't this supposed to run against

transactions require a 4.0+ server when non-sharded and 4.2+ when sharded

void testWriteConcernInheritance() {
assumeTrue(serverVersionAtLeast(4, 4));
try (ClientSession session = client.startSession()) {
MongoCollection<Document> wcCollection = collection.withWriteConcern(new WriteConcern(0));
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
MongoCollection<Document> wcCollection = collection.withWriteConcern(new WriteConcern(0));
MongoCollection<Document> wcCollection = collection.withWriteConcern(WriteConcern.UNACKNOWLEDGED);

try (ClientSession session = client.startSession()) {
MongoCollection<Document> wcCollection = collection.withWriteConcern(new WriteConcern(0));

assertDoesNotThrow(() -> {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure at which point this test asserts that the write concern is not inherited ?

+ "and normal server selection is performed for the next operation.")
@Test
void testNewTransactionUnpinsSession() {
abort("There is no ability to get the server address with the reactive api");
Copy link
Contributor

Choose a reason for hiding this comment

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

Why can't we call ?

client.getClusterDescription().getServerDescriptions()

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