Skip to content

Commit

Permalink
MXS-3729 Clean up Msg/OpMsgCommand further
Browse files Browse the repository at this point in the history 
Some code had to be added back, now that nosql::Msg:s are
moved around. It's not possible to take a reference to
a member in an object that is moved, and expect that
reference to be valid after the move. Thus there has
to be two constructors etc.
  • Loading branch information
Johan Wikman committed Aug 23, 2021
1 parent f590306 commit 46a1415
Show file tree
Hide file tree
Showing 8 changed files with 152 additions and 87 deletions.
3 changes: 2 additions & 1 deletion server/modules/protocol/NoSQL/commands/maxscale.hh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ public:
DocumentArguments arguments;
unique_ptr<OpMsgCommand> sCommand;

sCommand = OpMsgCommand::get(&m_database, m_pRequest, m_req, command, arguments);
nosql::Msg req(m_req);
sCommand = OpMsgCommand::get(&m_database, m_pRequest, std::move(req), command, arguments);

try
{
Expand Down
50 changes: 42 additions & 8 deletions server/modules/protocol/NoSQL/commands/query_and_write_operation.hh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,21 @@ public:
OrderedCommand(const std::string& name,
Database* pDatabase,
GWBUF* pRequest,
const nosql::Msg& req,
nosql::Msg&& req,
const std::string& array_key)
: MultiCommand(name, pDatabase, pRequest, std::move(req))
, m_key(array_key)
{
}

OrderedCommand(const std::string& name,
Database* pDatabase,
GWBUF* pRequest,
nosql::Msg&& req,
const bsoncxx::document::view& doc,
const DocumentArguments& arguments,
const std::string& array_key)
: MultiCommand(name, pDatabase, pRequest, req, doc, arguments)
: MultiCommand(name, pDatabase, pRequest, std::move(req), doc, arguments)
, m_key(array_key)
{
}
Expand Down Expand Up @@ -280,10 +290,18 @@ public:
Delete(const std::string& name,
Database* pDatabase,
GWBUF* pRequest,
const nosql::Msg& req,
nosql::Msg&& req)
: OrderedCommand(name, pDatabase, pRequest, std::move(req), key::DELETES)
{
}

Delete(const std::string& name,
Database* pDatabase,
GWBUF* pRequest,
nosql::Msg&& req,
const bsoncxx::document::view& doc,
const DocumentArguments& arguments)
: OrderedCommand(name, pDatabase, pRequest, req, doc, arguments, key::DELETES)
: OrderedCommand(name, pDatabase, pRequest, std::move(req), doc, arguments, key::DELETES)
{
}

Expand Down Expand Up @@ -573,10 +591,18 @@ public:
Insert(const std::string& name,
Database* pDatabase,
GWBUF* pRequest,
const nosql::Msg& req,
nosql::Msg&& req)
: OrderedCommand(name, pDatabase, pRequest, std::move(req), key::DOCUMENTS)
{
}

Insert(const std::string& name,
Database* pDatabase,
GWBUF* pRequest,
nosql::Msg&& req,
const bsoncxx::document::view& doc,
const DocumentArguments& arguments)
: OrderedCommand(name, pDatabase, pRequest, req, doc, arguments, key::DOCUMENTS)
: OrderedCommand(name, pDatabase, pRequest, std::move(req), doc, arguments, key::DOCUMENTS)
{
}

Expand Down Expand Up @@ -1190,10 +1216,18 @@ public:
Update(const std::string& name,
Database* pDatabase,
GWBUF* pRequest,
const nosql::Msg& req,
nosql::Msg&& req)
: OrderedCommand(name, pDatabase, pRequest, std::move(req), key::UPDATES)
{
}

Update(const std::string& name,
Database* pDatabase,
GWBUF* pRequest,
nosql::Msg&& req,
const bsoncxx::document::view& doc,
const DocumentArguments& arguments)
: OrderedCommand(name, pDatabase, pRequest, req, doc, arguments, key::UPDATES)
: OrderedCommand(name, pDatabase, pRequest, std::move(req), doc, arguments, key::UPDATES)
{
}

Expand Down
4 changes: 2 additions & 2 deletions server/modules/protocol/NoSQL/nosql.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2410,7 +2410,7 @@ GWBUF* nosql::NoSQL::handle_query(GWBUF* pRequest, nosql::Query&& req)
return pResponse;
}

GWBUF* nosql::NoSQL::handle_msg(GWBUF* pRequest, const nosql::Msg& req)
GWBUF* nosql::NoSQL::handle_msg(GWBUF* pRequest, nosql::Msg&& req)
{
MXB_INFO("Request(MSG): %s", bsoncxx::to_json(req.document()).c_str());

Expand All @@ -2431,7 +2431,7 @@ GWBUF* nosql::NoSQL::handle_msg(GWBUF* pRequest, const nosql::Msg& req)
mxb_assert(!m_sDatabase.get());
m_sDatabase = std::move(Database::create(name, &m_context, &m_config));

pResponse = m_sDatabase->handle_msg(pRequest, req);
pResponse = m_sDatabase->handle_msg(pRequest, std::move(req));

if (pResponse)
{
Expand Down
14 changes: 6 additions & 8 deletions server/modules/protocol/NoSQL/nosql.hh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,9 @@ class Packet
{
public:
Packet(const Packet&) = default;
Packet(Packet&& rhs) = default;
Packet& operator = (const Packet&) = default;
Packet& operator = (Packet&&) = default;

Packet(const uint8_t* pData, const uint8_t* pEnd)
: m_pEnd(pEnd)
Expand Down Expand Up @@ -675,6 +677,7 @@ class Query final : public Packet
{
public:
Query(const Packet& packet);
Query(Query&& rhs) = default;

uint32_t flags() const
{
Expand Down Expand Up @@ -711,9 +714,6 @@ public:
return m_fields;
}

Query(const Query&) = default;
Query& operator = (const Query&) = default;

std::ostream& out(std::ostream& o) const override
{
Packet::out(o);
Expand Down Expand Up @@ -805,9 +805,8 @@ public:
using DocumentArguments = std::unordered_map<std::string, DocumentVector>;

Msg(const Packet& packet);

Msg(const Msg&) = default;
Msg& operator = (const Msg&) = default;
Msg(const Msg& rhs) = default;
Msg(Msg&& rhs) = default;

bool checksum_present() const
{
Expand Down Expand Up @@ -976,8 +975,7 @@ private:
GWBUF* handle_insert(GWBUF* pRequest, nosql::Insert&& req);
GWBUF* handle_update(GWBUF* pRequest, nosql::Update&& req);
GWBUF* handle_query(GWBUF* pRequest, nosql::Query&& req);

GWBUF* handle_msg(GWBUF* pRequest, const nosql::Msg& req);
GWBUF* handle_msg(GWBUF* pRequest, nosql::Msg&& req);

State m_state { READY };
Context m_context;
Expand Down
87 changes: 59 additions & 28 deletions server/modules/protocol/NoSQL/nosqlcommand.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,57 +95,82 @@ class Unknown : public nosql::ImmediateCommand
using namespace nosql;

template<class ConcreteCommand>
unique_ptr<OpMsgCommand> create_command(const string& name,
Database* pDatabase,
GWBUF* pRequest,
const Msg& msg,
const bsoncxx::document::view& doc,
const OpMsgCommand::DocumentArguments& arguments)
unique_ptr<OpMsgCommand> create_default_command(const string& name,
Database* pDatabase,
GWBUF* pRequest,
Msg&& msg)
{
unique_ptr<ConcreteCommand> sCommand;

sCommand.reset(new ConcreteCommand(name, pDatabase, pRequest, msg, doc, arguments));
sCommand.reset(new ConcreteCommand(name, pDatabase, pRequest, std::move(msg)));

return sCommand;
}

using CreatorFunction = unique_ptr<OpMsgCommand> (*)(const string& name,
Database* pDatabase,
GWBUF* pRequest,
const Msg& msg,
const bsoncxx::document::view& doc,
const OpMsgCommand::DocumentArguments& arguments);
template<class ConcreteCommand>
unique_ptr<OpMsgCommand> create_diagnose_command(const string& name,
Database* pDatabase,
GWBUF* pRequest,
Msg&& msg,
const bsoncxx::document::view& doc,
const OpMsgCommand::DocumentArguments& arguments)
{
unique_ptr<ConcreteCommand> sCommand;

sCommand.reset(new ConcreteCommand(name, pDatabase, pRequest, std::move(msg), doc, arguments));

return sCommand;
}

using CreateDefaultFunction = unique_ptr<OpMsgCommand> (*)(const string& name,
Database* pDatabase,
GWBUF* pRequest,
Msg&& msg);

using CreateDiagnoseFunction = unique_ptr<OpMsgCommand> (*)(const string& name,
Database* pDatabase,
GWBUF* pRequest,
Msg&& msg,
const bsoncxx::document::view& doc,
const OpMsgCommand::DocumentArguments& arguments);

struct CommandInfo
{
CommandInfo()
: zKey(nullptr)
, zHelp(nullptr)
, create(nullptr)
, create_default(nullptr)
, create_diagnose(nullptr)
, is_admin(false)
{
}

CommandInfo(const char* zKey, const char* zHelp, CreatorFunction create, bool is_admin)
CommandInfo(const char* zKey, const char* zHelp,
CreateDefaultFunction create_default,
CreateDiagnoseFunction create_diagnose,
bool is_admin)
: zKey(zKey)
, zHelp(zHelp)
, create(create)
, create_default(create_default)
, create_diagnose(create_diagnose)
, is_admin(is_admin)
{
}

const char* zKey;
const char* zHelp;
CreatorFunction create;
bool is_admin;
const char* zKey;
const char* zHelp;
CreateDefaultFunction create_default;
CreateDiagnoseFunction create_diagnose;
bool is_admin;
};

template<class ConcreteCommand>
CommandInfo create_info()
{
return CommandInfo(ConcreteCommand::KEY,
ConcreteCommand::HELP,
&create_command<ConcreteCommand>,
&create_default_command<ConcreteCommand>,
&create_diagnose_command<ConcreteCommand>,
command::IsAdmin<ConcreteCommand>::is_admin);
}

Expand Down Expand Up @@ -891,10 +916,11 @@ pair<string, CommandInfo> get_info(const bsoncxx::document::view& doc)
}
}

if (!info.create)
if (!info.create_default)
{
name = "unknown";
info.create = &create_command<Unknown>;
info.create_default = &create_default_command<Unknown>;
info.create_diagnose = &create_diagnose_command<Unknown>;
info.is_admin = false;
}

Expand All @@ -906,24 +932,29 @@ pair<string, CommandInfo> get_info(const bsoncxx::document::view& doc)
//static
unique_ptr<OpMsgCommand> OpMsgCommand::get(nosql::Database* pDatabase,
GWBUF* pRequest,
const nosql::Msg& msg)
nosql::Msg&& msg)
{
return get(pDatabase, pRequest, msg, msg.document(), msg.arguments());
auto p = get_info(msg.document());

const string& name = p.first;
CreateDefaultFunction create = p.second.create_default;

return create(name, pDatabase, pRequest, std::move(msg));
}

//static
unique_ptr<OpMsgCommand> OpMsgCommand::get(nosql::Database* pDatabase,
GWBUF* pRequest,
const nosql::Msg& msg,
nosql::Msg&& msg,
const bsoncxx::document::view& doc,
const DocumentArguments& arguments)
{
auto p = get_info(doc);

const string& name = p.first;
CreatorFunction create = p.second.create;
CreateDiagnoseFunction create = p.second.create_diagnose;

return create(name, pDatabase, pRequest, msg, doc, arguments);
return create(name, pDatabase, pRequest, std::move(msg), doc, arguments);
}

GWBUF* OpMsgCommand::create_empty_response() const
Expand Down
20 changes: 16 additions & 4 deletions server/modules/protocol/NoSQL/nosqlcommand.hh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -282,24 +282,36 @@ public:
OpMsgCommand(const std::string& name,
Database* pDatabase,
GWBUF* pRequest,
const nosql::Msg& req,
nosql::Msg&& req)
: Command(pDatabase, pRequest, req.request_id(), response_kind(req))
, m_name(name)
, m_req(std::move(req))
, m_doc(m_req.document())
, m_arguments(m_req.arguments())
{
}

OpMsgCommand(const std::string& name,
Database* pDatabase,
GWBUF* pRequest,
nosql::Msg&& req,
const bsoncxx::document::view& doc,
const DocumentArguments& arguments)
: Command(pDatabase, pRequest, req.request_id(), response_kind(req))
, m_name(name)
, m_req(req)
, m_req(std::move(req))
, m_doc(doc)
, m_arguments(arguments)
{
}

static std::unique_ptr<OpMsgCommand> get(nosql::Database* pDatabase,
GWBUF* pRequest,
const nosql::Msg& req);
nosql::Msg&& req);

static std::unique_ptr<OpMsgCommand> get(nosql::Database* pDatabase,
GWBUF* pRequest,
const nosql::Msg& req,
nosql::Msg&& req,
const bsoncxx::document::view& doc,
const DocumentArguments& arguments);

Expand Down

0 comments on commit 46a1415

Please sign in to comment.