Skip to content

Commit 3f76ad6

Browse files
matejkmatejk
authored
fix(MongoDB) use constants instead of typed enum in OpMsgMessage (#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.
1 parent b380b57 commit 3f76ad6

2 files changed

Lines changed: 44 additions & 48 deletions

File tree

‎MongoDB/include/Poco/MongoDB/OpMsgMessage.h‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -139,12 +139,6 @@ class MongoDB_API OpMsgMessage: public Message
139139

140140
private:
141141

142-
enum PayloadType : UInt8
143-
{
144-
PAYLOAD_TYPE_0 = 0,
145-
PAYLOAD_TYPE_1 = 1
146-
};
147-
148142
std::string _databaseName;
149143
std::string _collectionName;
150144
UInt32 _flags { MSG_FLAGS_DEFAULT };

‎MongoDB/src/OpMsgMessage.cpp‎

Lines changed: 44 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -23,47 +23,49 @@ namespace Poco {
2323
namespace MongoDB {
2424

2525
// Query and write
26-
const std::string OpMsgMessage::CMD_INSERT { "insert" };
27-
const std::string OpMsgMessage::CMD_DELETE { "delete" };
28-
const std::string OpMsgMessage::CMD_UPDATE { "update" };
29-
const std::string OpMsgMessage::CMD_FIND { "find" };
30-
const std::string OpMsgMessage::CMD_FIND_AND_MODIFY { "findAndModify" };
31-
const std::string OpMsgMessage::CMD_GET_MORE { "getMore" };
26+
const std::string OpMsgMessage::CMD_INSERT { "insert"s };
27+
const std::string OpMsgMessage::CMD_DELETE { "delete"s };
28+
const std::string OpMsgMessage::CMD_UPDATE { "update"s };
29+
const std::string OpMsgMessage::CMD_FIND { "find"s };
30+
const std::string OpMsgMessage::CMD_FIND_AND_MODIFY { "findAndModify"s };
31+
const std::string OpMsgMessage::CMD_GET_MORE { "getMore"s };
3232

3333
// Aggregation
34-
const std::string OpMsgMessage::CMD_AGGREGATE { "aggregate" };
35-
const std::string OpMsgMessage::CMD_COUNT { "count" };
36-
const std::string OpMsgMessage::CMD_DISTINCT { "distinct" };
37-
const std::string OpMsgMessage::CMD_MAP_REDUCE { "mapReduce" };
34+
const std::string OpMsgMessage::CMD_AGGREGATE { "aggregate"s };
35+
const std::string OpMsgMessage::CMD_COUNT { "count"s };
36+
const std::string OpMsgMessage::CMD_DISTINCT { "distinct"s };
37+
const std::string OpMsgMessage::CMD_MAP_REDUCE { "mapReduce"s };
3838

3939
// Replication and administration
40-
const std::string OpMsgMessage::CMD_HELLO { "hello" };
41-
const std::string OpMsgMessage::CMD_REPL_SET_GET_STATUS { "replSetGetStatus" };
42-
const std::string OpMsgMessage::CMD_REPL_SET_GET_CONFIG { "replSetGetConfig" };
43-
44-
const std::string OpMsgMessage::CMD_CREATE { "create" };
45-
const std::string OpMsgMessage::CMD_CREATE_INDEXES { "createIndexes" };
46-
const std::string OpMsgMessage::CMD_DROP { "drop" };
47-
const std::string OpMsgMessage::CMD_DROP_DATABASE { "dropDatabase" };
48-
const std::string OpMsgMessage::CMD_KILL_CURSORS { "killCursors" };
49-
const std::string OpMsgMessage::CMD_LIST_DATABASES { "listDatabases" };
50-
const std::string OpMsgMessage::CMD_LIST_INDEXES { "listIndexes" };
40+
const std::string OpMsgMessage::CMD_HELLO { "hello"s };
41+
const std::string OpMsgMessage::CMD_REPL_SET_GET_STATUS { "replSetGetStatus"s };
42+
const std::string OpMsgMessage::CMD_REPL_SET_GET_CONFIG { "replSetGetConfig"s };
43+
44+
const std::string OpMsgMessage::CMD_CREATE { "create"s };
45+
const std::string OpMsgMessage::CMD_CREATE_INDEXES { "createIndexes"s };
46+
const std::string OpMsgMessage::CMD_DROP { "drop"s };
47+
const std::string OpMsgMessage::CMD_DROP_DATABASE { "dropDatabase"s };
48+
const std::string OpMsgMessage::CMD_KILL_CURSORS { "killCursors"s };
49+
const std::string OpMsgMessage::CMD_LIST_DATABASES { "listDatabases"s };
50+
const std::string OpMsgMessage::CMD_LIST_INDEXES { "listIndexes"s };
5151

5252
// Diagnostic
53-
const std::string OpMsgMessage::CMD_BUILD_INFO { "buildInfo" };
54-
const std::string OpMsgMessage::CMD_COLL_STATS { "collStats" };
55-
const std::string OpMsgMessage::CMD_DB_STATS { "dbStats" };
56-
const std::string OpMsgMessage::CMD_HOST_INFO { "hostInfo" };
53+
const std::string OpMsgMessage::CMD_BUILD_INFO { "buildInfo"s };
54+
const std::string OpMsgMessage::CMD_COLL_STATS { "collStats"s };
55+
const std::string OpMsgMessage::CMD_DB_STATS { "dbStats"s };
56+
const std::string OpMsgMessage::CMD_HOST_INFO { "hostInfo"s };
5757

5858

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

6262

63-
static const std::string keyCursor {"cursor"};
64-
static const std::string keyFirstBatch {"firstBatch"};
65-
static const std::string keyNextBatch {"nextBatch"};
63+
static const std::string keyCursor { "cursor"s };
64+
static const std::string keyFirstBatch { "firstBatch"s };
65+
static const std::string keyNextBatch { "nextBatch"s };
6666

67+
constexpr static Poco::UInt8 PAYLOAD_TYPE_0 { 0 };
68+
constexpr static Poco::UInt8 PAYLOAD_TYPE_1 { 1 };
6769

6870
OpMsgMessage::OpMsgMessage() :
6971
Message(MessageHeader::OP_MSG)
@@ -112,7 +114,7 @@ void OpMsgMessage::setCommandName(const std::string& command)
112114
{
113115
_body.add(_commandName, _collectionName);
114116
}
115-
_body.add("$db", _databaseName);
117+
_body.add("$db"s, _databaseName);
116118
}
117119

118120

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

124126
// IMPORTANT: Command name must be first
125127
_body.add(_commandName, cursorID);
126-
_body.add("$db", _databaseName);
127-
_body.add("collection", _collectionName);
128+
_body.add("$db"s, _databaseName);
129+
_body.add("collection"s, _collectionName);
128130
if (batchSize > 0)
129131
{
130-
_body.add("batchSize", batchSize);
132+
_body.add("batchSize"s, batchSize);
131133
}
132134
}
133135

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

147149
_acknowledged = ack;
148150

149-
auto writeConcern = _body.get<Document::Ptr>("writeConcern", nullptr);
151+
auto writeConcern = _body.get<Document::Ptr>("writeConcern"s, nullptr);
150152
if (writeConcern)
151153
writeConcern->remove("w");
152154

@@ -158,9 +160,9 @@ void OpMsgMessage::setAcknowledgedRequest(bool ack)
158160
{
159161
_flags = _flags | MSG_MORE_TO_COME;
160162
if (!writeConcern)
161-
_body.addNewDocument("writeConcern").add("w", 0);
163+
_body.addNewDocument("writeConcern"s).add("w"s, 0);
162164
else
163-
writeConcern->add("w", 0);
165+
writeConcern->add("w"s, 0);
164166
}
165167

166168
}
@@ -205,9 +207,9 @@ const Document::Vector& OpMsgMessage::documents() const
205207
bool OpMsgMessage::responseOk() const
206208
{
207209
Poco::Int64 ok {false};
208-
if (_body.exists("ok"))
210+
if (_body.exists("ok"s))
209211
{
210-
ok = _body.getInteger("ok");
212+
ok = _body.getInteger("ok"s);
211213
}
212214
return (ok != 0);
213215
}
@@ -387,13 +389,13 @@ const std::string& commandIdentifier(const std::string& command)
387389
{
388390
// Names of identifiers for commands that send bulk documents in the request
389391
// The identifier is set in the section type 1.
390-
static std::map<std::string, std::string> identifiers {
391-
{ OpMsgMessage::CMD_INSERT, "documents" },
392-
{ OpMsgMessage::CMD_DELETE, "deletes" },
393-
{ OpMsgMessage::CMD_UPDATE, "updates" },
392+
static const std::map<std::string, std::string> identifiers {
393+
{ OpMsgMessage::CMD_INSERT, "documents"s },
394+
{ OpMsgMessage::CMD_DELETE, "deletes"s },
395+
{ OpMsgMessage::CMD_UPDATE, "updates"s },
394396

395397
// Not sure if create index can send document section
396-
{ OpMsgMessage::CMD_CREATE_INDEXES, "indexes" }
398+
{ OpMsgMessage::CMD_CREATE_INDEXES, "indexes"s }
397399
};
398400

399401
const auto i = identifiers.find(command);

0 commit comments

Comments
 (0)