Skip to content

Remove dead code in ReplyHeader #1231

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 6, 2023
Merged

Remove dead code in ReplyHeader #1231

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion driver-core/src/main/com/mongodb/internal/connection/InternalStreamConnection.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -546,7 +546,7 @@ private <T> void sendCommandMessageAsync(final int messageId, final Decoder<T> d
}

private <T> T getCommandResult(final Decoder<T> decoder, final ResponseBuffers responseBuffers, final int messageId) {
T result = new ReplyMessage<>(responseBuffers, decoder, messageId).getDocuments().get(0);
T result = new ReplyMessage<>(responseBuffers, decoder, messageId).getDocument();
MongoException writeConcernBasedError = createSpecialWriteConcernException(responseBuffers, description.getServerAddress());
if (writeConcernBasedError != null) {
throw new MongoWriteConcernWithResponseException(writeConcernBasedError, result);
Expand Down
108 changes: 13 additions & 95 deletions driver-core/src/main/com/mongodb/internal/connection/ReplyHeader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,17 +40,10 @@ public final class ReplyHeader {
*/
public static final int TOTAL_REPLY_HEADER_LENGTH = REPLY_HEADER_LENGTH + MESSAGE_HEADER_LENGTH;

private static final int CURSOR_NOT_FOUND_RESPONSE_FLAG = 1;
private static final int QUERY_FAILURE_RESPONSE_FLAG = 2;

private final int messageLength;
private final int requestId;
private final int responseTo;
private final int responseFlags;
private final long cursorId;
private final int startingFrom;
private final int numberReturned;
private final int opMsgFlagBits;
private final boolean hasMoreToCome;

ReplyHeader(final ByteBuf header, final MessageHeader messageHeader) {
this(messageHeader.getMessageLength(), messageHeader.getOpCode(), messageHeader, header);
Expand All @@ -66,27 +59,23 @@ private ReplyHeader(final int messageLength, final int opCode, final MessageHead
this.requestId = messageHeader.getRequestId();
this.responseTo = messageHeader.getResponseTo();
if (opCode == OP_MSG.getValue()) {
responseFlags = 0;
cursorId = 0;
startingFrom = 0;
numberReturned = 1;

opMsgFlagBits = header.getInt();
header.get(); // ignore payload type
int flagBits = header.getInt();
hasMoreToCome = (flagBits & (1 << 1)) != 0;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No need to store all the flag bigs since we only ever examine this one.

header.get(); // ignored payload type
} else if (opCode == OP_REPLY.getValue()) {
if (messageLength < TOTAL_REPLY_HEADER_LENGTH) {
throw new MongoInternalException(format("The reply message length %d is less than the mimimum message length %d",
throw new MongoInternalException(format("The reply message length %d is less than the minimum message length %d",
messageLength, TOTAL_REPLY_HEADER_LENGTH));
}
hasMoreToCome = false;

responseFlags = header.getInt();
cursorId = header.getLong();
startingFrom = header.getInt();
numberReturned = header.getInt();
opMsgFlagBits = 0;
header.getInt(); // ignored responseFlags
header.getLong(); // ignored cursorId
header.getInt(); // ignored startingFrom
int numberReturned = header.getInt();

if (numberReturned < 0) {
throw new MongoInternalException(format("The reply message number of returned documents, %d, is less than 0",
if (numberReturned != 1) {
throw new MongoInternalException(format("The reply message number of returned documents, %d, is expected to be 1",
numberReturned));
}
} else {
Expand Down Expand Up @@ -123,78 +112,7 @@ public int getResponseTo() {
return responseTo;
}

/**
* Gets additional information about the response.
* <ul>
* <li>0 - <i>CursorNotFound</i>: Set when getMore is called but the cursor id is not valid at the server. Returned with zero
* results.</li>
* <li>1 - <i>QueryFailure</i>: Set when query failed. Results consist of one document containing an "$err" field describing the
* failure.
* <li>2 - <i>ShardConfigStale</i>: Drivers should ignore this. Only mongos will ever see this set, in which case,
* it needs to update config from the server.
* <li>3 - <i>AwaitCapable</i>: Set when the server supports the AwaitData Query option. If it doesn't,
* a client should sleep a little between getMore's of a Tailable cursor. Mongod version 1.6 supports AwaitData and thus always
* sets AwaitCapable.
* <li>4-31 - <i>Reserved</i>: Ignore
* </ul>
*
* @return bit vector - see details above
*/
public int getResponseFlags() {
return responseFlags;
}

/**
* Gets the cursor ID that this response is a part of. If there are no more documents to fetch from the server, the cursor ID will be 0.
* This cursor ID must be used in any messages used to get more data, and also must be closed by the client when no longer needed.
*
* @return cursor ID to use if the client needs to fetch more from the server
*/
public long getCursorId() {
return cursorId;
}

/**
* Returns the position in the cursor that is the start point of this reply.
*
* @return where in the cursor this reply is starting
*/
public int getStartingFrom() {
return startingFrom;
}

/**
* Gets the number of documents to expect in the body of this reply.
*
* @return number of documents in the reply
*/
public int getNumberReturned() {
return numberReturned;
}

/**
* Gets whether this query was performed with a cursor ID that was not valid on the server.
*
* @return true if this reply indicates the request to get more data was performed with a cursor ID that's not valid on the server
*/
public boolean isCursorNotFound() {
return (responseFlags & CURSOR_NOT_FOUND_RESPONSE_FLAG) == CURSOR_NOT_FOUND_RESPONSE_FLAG;
}

/**
* Gets whether the query failed or not.
*
* @return true if this reply indicates the query failed.
*/
public boolean isQueryFailure() {
return (responseFlags & QUERY_FAILURE_RESPONSE_FLAG) == QUERY_FAILURE_RESPONSE_FLAG;
}

public int getOpMsgFlagBits() {
return opMsgFlagBits;
}

public boolean hasMoreToCome() {
return (opMsgFlagBits & (1 << 1)) != 0;
return hasMoreToCome;
}
}
53 changes: 12 additions & 41 deletions driver-core/src/main/com/mongodb/internal/connection/ReplyMessage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,6 @@
import org.bson.io.BsonInput;
import org.bson.io.ByteBufferBsonInput;

import java.util.ArrayList;
import java.util.List;

import static java.lang.String.format;

/**
Expand All @@ -35,50 +32,24 @@
*/
public class ReplyMessage<T> {

private final ReplyHeader replyHeader;
private final List<T> documents;
private final T document;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In practice this is always a list of exactly 1, so get rid of the list


public ReplyMessage(final ResponseBuffers responseBuffers, final Decoder<T> decoder, final long requestId) {
this(responseBuffers.getReplyHeader(), requestId);

if (replyHeader.getNumberReturned() > 0) {
try (BsonInput bsonInput = new ByteBufferBsonInput(responseBuffers.getBodyByteBuffer().duplicate())) {
while (documents.size() < replyHeader.getNumberReturned()) {
try (BsonBinaryReader reader = new BsonBinaryReader(bsonInput)) {
documents.add(decoder.decode(reader, DecoderContext.builder().build()));
}
}
} finally {
responseBuffers.reset();
}
}
}

ReplyMessage(final ReplyHeader replyHeader, final long requestId) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This constructor was only used by a test, so removed it and refactored the test to use the remaining constructor.

if (requestId != replyHeader.getResponseTo()) {
if (requestId != responseBuffers.getReplyHeader().getResponseTo()) {
throw new MongoInternalException(format("The responseTo (%d) in the response does not match the requestId (%d) in the "
+ "request", replyHeader.getResponseTo(), requestId));
+ "request", responseBuffers.getReplyHeader().getResponseTo(), requestId));
}
this.replyHeader = replyHeader;

documents = new ArrayList<>(replyHeader.getNumberReturned());
}

/**
* Gets the reply header.
*
* @return the reply header
*/
public ReplyHeader getReplyHeader() {
return replyHeader;
try (BsonInput bsonInput = new ByteBufferBsonInput(responseBuffers.getBodyByteBuffer().duplicate())) {
try (BsonBinaryReader reader = new BsonBinaryReader(bsonInput)) {
document = decoder.decode(reader, DecoderContext.builder().build());
}
} finally {
responseBuffers.reset();
}
}

/**
* Gets the documents.
*
* @return the documents
*/
public List<T> getDocuments() {
return documents;
public T getDocument() {
return document;
}
}
2 changes: 1 addition & 1 deletion driver-core/src/main/com/mongodb/internal/connection/ResponseBuffers.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public ReplyHeader getReplyHeader() {
<T extends BsonDocument> T getResponseDocument(final int messageId, final Decoder<T> decoder) {
ReplyMessage<T> replyMessage = new ReplyMessage<>(this, decoder, messageId);
reset();
return replyMessage.getDocuments().get(0);
return replyMessage.getDocument();
}

/**
Expand Down
22 changes: 5 additions & 17 deletions ...-core/src/test/functional/com/mongodb/internal/connection/ReplyHeaderSpecification.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class ReplyHeaderSpecification extends Specification {
writeInt(responseFlags)
writeLong(9000)
writeInt(4)
writeInt(30)
writeInt(1)
}
def byteBuf = outputBuffer.byteBuffers.get(0)

Expand All @@ -46,12 +46,6 @@ class ReplyHeaderSpecification extends Specification {
replyHeader.messageLength == 186
replyHeader.requestId == 45
replyHeader.responseTo == 23
replyHeader.responseFlags == responseFlags
replyHeader.cursorId == 9000
replyHeader.startingFrom == 4
replyHeader.numberReturned == 30
replyHeader.cursorNotFound == cursorNotFound
replyHeader.queryFailure == queryFailure

where:
responseFlags << [0, 1, 2, 3]
Expand All @@ -72,7 +66,7 @@ class ReplyHeaderSpecification extends Specification {
writeInt(responseFlags)
writeLong(9000)
writeInt(4)
writeInt(30)
writeInt(1)
}
def byteBuf = outputBuffer.byteBuffers.get(0)
def compressedHeader = new CompressedHeader(byteBuf, new MessageHeader(byteBuf, getDefaultMaxMessageSize()))
Expand All @@ -84,12 +78,6 @@ class ReplyHeaderSpecification extends Specification {
replyHeader.messageLength == 274
replyHeader.requestId == 45
replyHeader.responseTo == 23
replyHeader.responseFlags == responseFlags
replyHeader.cursorId == 9000
replyHeader.startingFrom == 4
replyHeader.numberReturned == 30
replyHeader.cursorNotFound == cursorNotFound
replyHeader.queryFailure == queryFailure

where:
responseFlags << [0, 1, 2, 3]
Expand Down Expand Up @@ -138,7 +126,7 @@ class ReplyHeaderSpecification extends Specification {

then:
def ex = thrown(MongoInternalException)
ex.getMessage() == 'The reply message length 35 is less than the mimimum message length 36'
ex.getMessage() == 'The reply message length 35 is less than the minimum message length 36'
}

def 'should throw MongoInternalException on message size > max message size'() {
Expand Down Expand Up @@ -182,7 +170,7 @@ class ReplyHeaderSpecification extends Specification {

then:
def ex = thrown(MongoInternalException)
ex.getMessage() == 'The reply message number of returned documents, -1, is less than 0'
ex.getMessage() == 'The reply message number of returned documents, -1, is expected to be 1'
}

def 'should throw MongoInternalException on num documents < 0 with compressed header'() {
Expand All @@ -208,6 +196,6 @@ class ReplyHeaderSpecification extends Specification {

then:
def ex = thrown(MongoInternalException)
ex.getMessage() == 'The reply message number of returned documents, -1, is less than 0'
ex.getMessage() == 'The reply message number of returned documents, -1, is expected to be 1'
}
}
20 changes: 4 additions & 16 deletions driver-core/src/test/unit/com/mongodb/internal/connection/CommandMessageSpecification.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import com.mongodb.internal.session.SessionContext
import com.mongodb.internal.validator.NoOpFieldNameValidator
import org.bson.BsonArray
import org.bson.BsonBinary
import org.bson.BsonBinaryReader
import org.bson.BsonDocument
import org.bson.BsonInt32
import org.bson.BsonMaximumSizeExceededException
Expand All @@ -37,10 +36,7 @@ import org.bson.BsonTimestamp
import org.bson.ByteBuf
import org.bson.ByteBufNIO
import org.bson.codecs.BsonDocumentCodec
import org.bson.codecs.DecoderContext
import org.bson.io.BasicOutputBuffer
import org.bson.io.BsonInput
import org.bson.io.ByteBufferBsonInput
import spock.lang.Specification

import java.nio.ByteBuffer
Expand All @@ -63,7 +59,7 @@ class CommandMessageSpecification extends Specification {
.serverType(serverType as ServerType)
.sessionSupported(true)
.build(),
responseExpected, exhaustAllowed, null, null, clusterConnectionMode, null)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No need to test this flag bit since the driver never uses it.

responseExpected, null, null, clusterConnectionMode, null)
def output = new BasicOutputBuffer()

when:
Expand All @@ -76,8 +72,7 @@ class CommandMessageSpecification extends Specification {
messageHeader.opCode == OpCode.OP_MSG.value
replyHeader.requestId < RequestMessage.currentGlobalId
replyHeader.responseTo == 0
((replyHeader.opMsgFlagBits & (1 << 16)) != 0) == exhaustAllowed
((replyHeader.opMsgFlagBits & (1 << 1)) == 0) == responseExpected
replyHeader.hasMoreToCome() != responseExpected

def expectedCommandDocument = command.clone()
.append('$db', new BsonString(namespace.databaseName))
Expand All @@ -97,7 +92,7 @@ class CommandMessageSpecification extends Specification {
getCommandDocument(byteBuf, replyHeader) == expectedCommandDocument

where:
[readPreference, serverType, clusterConnectionMode, sessionContext, responseExpected, exhaustAllowed] << [
[readPreference, serverType, clusterConnectionMode, sessionContext, responseExpected] << [
[ReadPreference.primary(), ReadPreference.secondary()],
[ServerType.REPLICA_SET_PRIMARY, ServerType.SHARD_ROUTER],
[ClusterConnectionMode.SINGLE, ClusterConnectionMode.MULTIPLE],
Expand Down Expand Up @@ -126,7 +121,6 @@ class CommandMessageSpecification extends Specification {
getReadConcern() >> ReadConcern.DEFAULT
}
],
[true, false],
[true, false]
].combinations()
}
Expand Down Expand Up @@ -372,12 +366,6 @@ class CommandMessageSpecification extends Specification {
}

private static BsonDocument getCommandDocument(ByteBufNIO byteBuf, ReplyHeader replyHeader) {
new ReplyMessage<BsonDocument>(new ResponseBuffers(replyHeader, byteBuf), new BsonDocumentCodec(), 0).documents.get(0)
}

private static BsonDocument getCommandDocument(ByteBufNIO byteBuf) {
BsonInput bsonInput = new ByteBufferBsonInput(byteBuf)
BsonBinaryReader reader = new BsonBinaryReader(bsonInput)
new BsonDocumentCodec().decode(reader, DecoderContext.builder().build())
new ReplyMessage<BsonDocument>(new ResponseBuffers(replyHeader, byteBuf), new BsonDocumentCodec(), 0).document
}
}
Loading