Implement splitting and encoding ops, nsInfo as separate OP_MSG sections, implement prose tests

[stIncMale]

The first four commits are well-organized to allow reviewing them one by one.

This PR depends on #1486.

The following test runners execute the unified and prose tests added in this PR:

• com.mongodb.client.CrudProseTest
• com.mongodb.client.AbstractClientSideOperationsTimeoutProseTest

JAVA-5529
JAVA-5610
JAVA-5695

[stIncMale]

@jyemin I incorporated the changes you proposed. See ea0633e, 9fe35e4.

However, I did not include jyemin@7d7f3eb, because as I understand it. CommandMessage.encodeMessageBodyWithMetadata in the main branch is already concerned with getSettings().getMaxDocumentSize(); BsonWriterHelper.getPayloadMessageSettings is concerned with DOCUMENT_HEADROOM; RequestMessage.addDocument creates a BsonBinaryWriter that checks that the document size is not greater than settings.getMaxDocumentSize() + DOCUMENT_HEADROOM. Given that, what is the purpose of moving similar validation checks to the operation layer specifically when it comes to DualMessageSequences?

[jyemin]

Given that, what is the purpose of moving similar validation checks to the operation layer specifically when it comes to DualMessageSequences?

I think the difference is that the checks you enumerated apply equally to all commands: no command document can be larger than maxDocumentSize + DOCUMENT_HEADROOM, period. Whereas for bulkWrite there is specific "business logic" that applies only to that command: the check applies to each document in the array, but only if it's a replace or insert, and only if write concern is unacknowledged. That's why it seemed more appropriate to consolidate that logic with the operation. Let me know your thoughts on this.

Also, wanted to check about jyemin@a2805a6, since you didn't mention that commit.

[jyemin]

Waiting for feedback from Valentin

[stIncMale]

@jyemin

Each time I look at this, I not only can't see a problem with the current approach, but even fail to convince myself that the proposed change makes things better overall.

for bulkWrite there is specific "business logic" that applies only to that command: the check applies to each document in the array, but only if it's a replace or insert

The logic that is aware of which documents are stored, and which ones are not, is in ClientBulkWriteOperation. Neither CommandMessage nor BsonWriterHelper are aware of it.

and only if write concern is unacknowledged. That's why it seemed more appropriate to consolidate that logic with the operation.

The logic that is aware of acknowledged writes and of what an application has requested, is in ClientBulkWriteOperation. CommandMessage knows only whether a response needs be requested from the server or not, and it's a reasonable thing to be aware of at this level of abstraction. CommandMessage treats different MessageSequences differently regardless of the document size validation, and treating them differently when it comes to making a decision on whether size validation should be done at all is no different (the logic is: if we have no way to learn about exceeded size from the server, we have to validate the size ourselves).

If for some reason we want CommandMessage to not treat different MessageSequences differently when it comes to size validation, we would have to move the logic of encoding the command document into an operation, like we do when encoding extra elements. This way ClientBulkWriteOperation would be able to decide whether to validate the size of the command document, which can exceed the limit, for example, because of a large comment or let.

[jyemin]

Latest commit LGTM.

Not sure if any more are coming in this PR so not approving the PR yet.

[stIncMale]

@vbabanin This PR is ready for your review.

[jyemin]

LGTM (again!)

[vbabanin]

Thanks for moving these to constants— makes the code easier to read!

[vbabanin]

Does this limitation apply only to client bulk write?

[stIncMale]

I am not sure if limitation is the right word here. But yes, this is unique to client bulk writes:

1. A mixed bulk write does not mix different kinds of operations in a batch because it is technically impossible there. As a result, we know in advance what kind of operations is in a batch, thus knowing whether it supports retries or not. That allows us to encode extraElements before encoding the PAYLOAD_TYPE_1_DOCUMENT_SEQUENCE section. Thus, when we are encoding the document sequence, we know exactly how much space we have left available before reaching MessageSettings.getMaxMessageSize.
2. A client bulk write may mix different kinds of operations in a batch, which means that we know whether the batch supports retries only after encoding its PAYLOAD_TYPE_1_DOCUMENT_SEQUENCE sections. That is, we may need to write something after writing those sections. That, in turn, means we can't know exactly how much space we have left when we encode document sequences. But whatever we write after writing the sequences, its size is bounded, and 1000 bytes is used in the spec as the value that is definitely not smaller than that bound.

[stIncMale]

Thank you, @vbabanin, all your refactoring suggestions were really good.

[vbabanin]

LGTM!