Skip to content

Commit

Permalink
fix(MongoDB) use constants instead of typed enum in OpMsgMessage (poc…
Browse files Browse the repository at this point in the history 
…oproject#4822)

* fix(MongoDB): Use constants for payload types instead of enum to avoid improper binary serialisation with some compilers (int instead of unsigned char).

* enh(MongoDB): Minor code improvements to use string literals.
  • Loading branch information
@matejk
matejk authored Dec 17, 2024
1 parent b380b57 commit 3f76ad6
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 48 deletions.
6 changes: 0 additions & 6 deletions MongoDB/include/Poco/MongoDB/OpMsgMessage.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,12 +139,6 @@ class MongoDB_API OpMsgMessage: public Message

private:

enum PayloadType : UInt8
{
PAYLOAD_TYPE_0 = 0,
PAYLOAD_TYPE_1 = 1
};

std::string _databaseName;
std::string _collectionName;
UInt32 _flags { MSG_FLAGS_DEFAULT };
Expand Down
86 changes: 44 additions & 42 deletions MongoDB/src/OpMsgMessage.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,47 +23,49 @@ namespace Poco {
namespace MongoDB {

// Query and write
const std::string OpMsgMessage::CMD_INSERT { "insert" };
const std::string OpMsgMessage::CMD_DELETE { "delete" };
const std::string OpMsgMessage::CMD_UPDATE { "update" };
const std::string OpMsgMessage::CMD_FIND { "find" };
const std::string OpMsgMessage::CMD_FIND_AND_MODIFY { "findAndModify" };
const std::string OpMsgMessage::CMD_GET_MORE { "getMore" };
const std::string OpMsgMessage::CMD_INSERT { "insert"s };
const std::string OpMsgMessage::CMD_DELETE { "delete"s };
const std::string OpMsgMessage::CMD_UPDATE { "update"s };
const std::string OpMsgMessage::CMD_FIND { "find"s };
const std::string OpMsgMessage::CMD_FIND_AND_MODIFY { "findAndModify"s };
const std::string OpMsgMessage::CMD_GET_MORE { "getMore"s };

// Aggregation
const std::string OpMsgMessage::CMD_AGGREGATE { "aggregate" };
const std::string OpMsgMessage::CMD_COUNT { "count" };
const std::string OpMsgMessage::CMD_DISTINCT { "distinct" };
const std::string OpMsgMessage::CMD_MAP_REDUCE { "mapReduce" };
const std::string OpMsgMessage::CMD_AGGREGATE { "aggregate"s };
const std::string OpMsgMessage::CMD_COUNT { "count"s };
const std::string OpMsgMessage::CMD_DISTINCT { "distinct"s };
const std::string OpMsgMessage::CMD_MAP_REDUCE { "mapReduce"s };

// Replication and administration
const std::string OpMsgMessage::CMD_HELLO { "hello" };
const std::string OpMsgMessage::CMD_REPL_SET_GET_STATUS { "replSetGetStatus" };
const std::string OpMsgMessage::CMD_REPL_SET_GET_CONFIG { "replSetGetConfig" };

const std::string OpMsgMessage::CMD_CREATE { "create" };
const std::string OpMsgMessage::CMD_CREATE_INDEXES { "createIndexes" };
const std::string OpMsgMessage::CMD_DROP { "drop" };
const std::string OpMsgMessage::CMD_DROP_DATABASE { "dropDatabase" };
const std::string OpMsgMessage::CMD_KILL_CURSORS { "killCursors" };
const std::string OpMsgMessage::CMD_LIST_DATABASES { "listDatabases" };
const std::string OpMsgMessage::CMD_LIST_INDEXES { "listIndexes" };
const std::string OpMsgMessage::CMD_HELLO { "hello"s };
const std::string OpMsgMessage::CMD_REPL_SET_GET_STATUS { "replSetGetStatus"s };
const std::string OpMsgMessage::CMD_REPL_SET_GET_CONFIG { "replSetGetConfig"s };

const std::string OpMsgMessage::CMD_CREATE { "create"s };
const std::string OpMsgMessage::CMD_CREATE_INDEXES { "createIndexes"s };
const std::string OpMsgMessage::CMD_DROP { "drop"s };
const std::string OpMsgMessage::CMD_DROP_DATABASE { "dropDatabase"s };
const std::string OpMsgMessage::CMD_KILL_CURSORS { "killCursors"s };
const std::string OpMsgMessage::CMD_LIST_DATABASES { "listDatabases"s };
const std::string OpMsgMessage::CMD_LIST_INDEXES { "listIndexes"s };

// Diagnostic
const std::string OpMsgMessage::CMD_BUILD_INFO { "buildInfo" };
const std::string OpMsgMessage::CMD_COLL_STATS { "collStats" };
const std::string OpMsgMessage::CMD_DB_STATS { "dbStats" };
const std::string OpMsgMessage::CMD_HOST_INFO { "hostInfo" };
const std::string OpMsgMessage::CMD_BUILD_INFO { "buildInfo"s };
const std::string OpMsgMessage::CMD_COLL_STATS { "collStats"s };
const std::string OpMsgMessage::CMD_DB_STATS { "dbStats"s };
const std::string OpMsgMessage::CMD_HOST_INFO { "hostInfo"s };


static const std::string& commandIdentifier(const std::string& command);
/// Commands have different names for the payload that is sent in a separate section


static const std::string keyCursor {"cursor"};
static const std::string keyFirstBatch {"firstBatch"};
static const std::string keyNextBatch {"nextBatch"};
static const std::string keyCursor { "cursor"s };
static const std::string keyFirstBatch { "firstBatch"s };
static const std::string keyNextBatch { "nextBatch"s };

constexpr static Poco::UInt8 PAYLOAD_TYPE_0 { 0 };
constexpr static Poco::UInt8 PAYLOAD_TYPE_1 { 1 };

OpMsgMessage::OpMsgMessage() :
Message(MessageHeader::OP_MSG)
Expand Down Expand Up @@ -112,7 +114,7 @@ void OpMsgMessage::setCommandName(const std::string& command)
{
_body.add(_commandName, _collectionName);
}
_body.add("$db", _databaseName);
_body.add("$db"s, _databaseName);
}


Expand All @@ -123,11 +125,11 @@ void OpMsgMessage::setCursor(Poco::Int64 cursorID, Poco::Int32 batchSize)

// IMPORTANT: Command name must be first
_body.add(_commandName, cursorID);
_body.add("$db", _databaseName);
_body.add("collection", _collectionName);
_body.add("$db"s, _databaseName);
_body.add("collection"s, _collectionName);
if (batchSize > 0)
{
_body.add("batchSize", batchSize);
_body.add("batchSize"s, batchSize);
}
}

Expand All @@ -146,7 +148,7 @@ void OpMsgMessage::setAcknowledgedRequest(bool ack)

_acknowledged = ack;

auto writeConcern = _body.get<Document::Ptr>("writeConcern", nullptr);
auto writeConcern = _body.get<Document::Ptr>("writeConcern"s, nullptr);
if (writeConcern)
writeConcern->remove("w");

Expand All @@ -158,9 +160,9 @@ void OpMsgMessage::setAcknowledgedRequest(bool ack)
{
_flags = _flags | MSG_MORE_TO_COME;
if (!writeConcern)
_body.addNewDocument("writeConcern").add("w", 0);
_body.addNewDocument("writeConcern"s).add("w"s, 0);
else
writeConcern->add("w", 0);
writeConcern->add("w"s, 0);
}

}
Expand Down Expand Up @@ -205,9 +207,9 @@ const Document::Vector& OpMsgMessage::documents() const
bool OpMsgMessage::responseOk() const
{
Poco::Int64 ok {false};
if (_body.exists("ok"))
if (_body.exists("ok"s))
{
ok = _body.getInteger("ok");
ok = _body.getInteger("ok"s);
}
return (ok != 0);
}
Expand Down Expand Up @@ -387,13 +389,13 @@ const std::string& commandIdentifier(const std::string& command)
{
// Names of identifiers for commands that send bulk documents in the request
// The identifier is set in the section type 1.
static std::map<std::string, std::string> identifiers {
{ OpMsgMessage::CMD_INSERT, "documents" },
{ OpMsgMessage::CMD_DELETE, "deletes" },
{ OpMsgMessage::CMD_UPDATE, "updates" },
static const std::map<std::string, std::string> identifiers {
{ OpMsgMessage::CMD_INSERT, "documents"s },
{ OpMsgMessage::CMD_DELETE, "deletes"s },
{ OpMsgMessage::CMD_UPDATE, "updates"s },

// Not sure if create index can send document section
{ OpMsgMessage::CMD_CREATE_INDEXES, "indexes" }
{ OpMsgMessage::CMD_CREATE_INDEXES, "indexes"s }
};

const auto i = identifiers.find(command);
Expand Down

0 comments on commit 3f76ad6

Please sign in to comment.