4
\$\begingroup\$

Context

As many of you may know I have a library that allows C++ objects to be converted into JSON/YAML/BSON automatically with a single declaration (see previous code reviews).

I am now (trying) using this to connect and send data to Mongo without the developer having to write any specific code to convert their C++ objects into BSON.

I also have a wrapper around a Socket that makes it behave like a std::iostream so it can simply be used with operator<< and operator>>. This Socket wrapper, when used in my Service Library (Nissa), allows thread (Co-Routine) switching when a read/write would block, thus making it very efficient.

Usage Pattern

I want to insert objects into Mongo like this:

struct Person
{
     std::string    name;
     int            age;
};
ThorsAnvil_MakeTraits(Person, name, age);

int main()
{
     MongoConnection    connection(DB, USER, PASSWORD); // Creates a connection
                                                        // It wraps a Socket stream described
                                                        // above but also sends all the
                                                        // handshaking and authentication required
                                                        // by Mongo.
                                                        // But you can think of it like a stream.

    

     Person  person{"Loki", 12};
     using ThorsAnvil::DB::Mongo::make_CmdDB_Insert;

     // This line below is what I am interested in.
     // I will show you more usage below and how I though about implementing it
     // with the code. Hoping for 
     connection << make_CmdDB_Insert(DB, "Collection", QueryOptions{}, person);

     CmdDB_Reply  reply;
     connection >> reply;
 }

Usage of Insert

I am using Insert as an example here. But there are several commands each with their own unique options. So I want to get some feedback on this idea before I go an implement everything for all the commands.

The Insert has the following extra optional parameters: Full Details

  • ordered
    Default: True
    Inserts are ordered.
    Method: unordered() => turns this off
  • writeConcern
    An object that defines how the object is written.
    Method: setWriteConcern(int w = 1, bool j = false, std::time_t wtimeout = 0)
  • bypassDocumentValidation
    Default: false
    Validates the object being written to the connection.
    Method: byPass() => turn off validation (i.e. set param to true)
  • comment
    A string that will be written to different places in Mongo.
    Method: setComment(std::string&& c) add a comment

Normally the default values are fine, but I do want the user to be able to modify them if required. The thing is that if they are not explicitly set, then I don't want to send the value over the wire (as there is a cost to convert them and write them to the socket). So I thought I could set it up like this:

If I try putting all the options in the constructor then it will lead to a proliferation of constructor methods. Which seems hard. So I want to try using option methods that return a reference to the object so they can be chained; like this:

connection << make_CmdDB_Insert(DB, "Collection", QueryOptions{}, person)
              .unordered()
              .setComment("Hi there");
              // etc Just add a call for each option you want to set.
              // You should be able to use any combination of options
              // in any order.
              //
              // The reason to do this is to avoid an explosion in constructors
              // with different parameters in different orders.
              //
              // With Only 4 options there are 8 different constructors needed.
              // With other commands (like Find) the number of options is 16
              // Which would lead to thousands of constructors.

Implementation

Since this is code review. Here is my implementation.

Note: There is an underlying layer that is not provided here (for brevity). I will be positing all the code for a full review when it is done.

CmdDB_Insert.h
#ifndef THORSANVIL_DB_MONGO_CMD_DB_INSERT_H
#define THORSANVIL_DB_MONGO_CMD_DB_INSERT_H

// https://docs.mongodb.com/manual/reference/command/insert/#dbcmd.insert

#include "CmdDB.h"

namespace ThorsAnvil::DB::Mongo
{

template<typename Document>
struct Insert
{
    public:
        Insert(std::string const& collection, Document const& doc);

        void unordered();
        void byPass();
        void setWrieConcern(int w = 1, bool j = false, std::time_t wtimeout = 0);
        void setComment(std::string&& c);
    private:
        // Allow Serialization code access to private members.
        friend class ThorsAnvil::Serialize::Traits<Insert>;
        friend class ThorsAnvil::Serialize::Filter<Insert>;

        // What fields will not be serialized.
        // Serialized if not in the filter or in the filter and value is true.
        std::map<std::string, bool> filter  = {{"ordered", false}, {"writeConcern", false}, {"bypassDocumentValidation", false}, {"comment", false}};

        // Members that will be sent on the wire in BSON to mongo server.
        std::string                 insert;
        std::vector<Document>       documents;
        bool                        ordered                  = true;
        WriteConcern                writeConcern;
        bool                        bypassDocumentValidation = false;
        std::string                 comment;
};

// Wrapper around the CmdDB_Query object.
// This inherits from Op_Query:
//     This handles the low level OP_QUERY object that contains the header and query flags.
//
// The CmdDB_Query
// This handles writing to the underlying connection object.
// Insert is the command it is writing (as defined above)
// Document: Is the user defined object that we are writing.
template<typename Document>
using CmdDB_Insert      = CmdDB_Query<Insert<Document>>;

template<typename Document>
CmdDB_Insert<Document>
make_CmdDB_Insert(std::string const& db, std::string const& collection, QueryOptions&& options, Document const& doc)
{
    using Document = typename std::iterator_traits<I>::value_type;
    return CmdDB_Insert<Document>(db, collection, std::move(options), doc);
}

}

// Tells the serializing code to filter the member based on the member filter.
ThorsAnvil_Template_MakeFilter(1, ThorsAnvil::DB::Mongo::Insert,        filter);
// Tells the serialization code what to send across the wire in BSON.
ThorsAnvil_Template_MakeTrait(1,  ThorsAnvil::DB::Mongo::Insert,        insert, documents, ordered, writeConcern, bypassDocumentValidation, comment);

#include "CmdDB_Insert.tpp"
#endif
CmdDB_Insert.tpp
#ifndef THORSANVIL_DB_MONGO_CMD_DB_INSERT_TPP
#define THORSANVIL_DB_MONGO_CMD_DB_INSERT_TPP

#ifndef THORSANVIL_DB_MONGO_CMD_DB_INSERT_H
#error  "This should only be included from CmdDB_Insert.h"
#endif

namespace ThorsAnvil::DB::Mongo
{


template<typename Document>
Insert<Document>::Insert(std::string const& collection, Document const& doc)
    : insert(collection)
    , documents(1, doc)
{}

template<typename Document>
void Insert<Document>::unordered()
{
    ordered = false;
    filter["ordered"] = true;
}

template<typename Document>
void Insert<Document>::byPass()
{
    bypassDocumentValidation = false;
    filter["bypassDocumentValidation"] = true;
}

template<typename Document>
void Insert<Document>::setWrieConcern(int w, bool j, std::time_t wtimeout)
{
    writeConcern    = WriteConcern{w, j, wtimeout};
    filter["writeConcern"]  = true;
}

template<typename Document>
void Insert<Document>::setComment(std::string&& c)
{
    comment = c;
    filter["comment"] = true;
}

}

#endif
CmdDB.h
#ifndef THORSANVIL_DB_MONGO_CMD_DB_H
#define THORSANVIL_DB_MONGO_CMD_DB_H

// https://docs.mongodb.com/manual/reference/command/nav-crud/

#include "Op_Query.h"
#include "Op_Reply.h"
#include "ThorSerialize/SerUtil.h"

namespace ThorsAnvil::DB::Mongo
{

enum class ReadConcernLevel {local, available, majority, linearizable};

struct Collation
{
    std::string     locale;
    bool            caseLevel;
    std::string     caseFirst;
    int             strength;
    bool            numericOrdering;
    std::string     alternate;
    std::string     maxVariable;
    bool            backwards;
};

struct WriteErrors
{
    std::size_t                 index;
    int                         code;
    std::string                 errmsg;
};

struct WriteConcernError
{
    int                         code;
    std::string                 errmsg;
};

struct CmdReply
{
    double                      ok;
    std::size_t                 n;
    std::string                 errmsg;
    std::string                 codeName;
    int                         code;
    std::vector<WriteErrors>    writeErrors;
};

class CmdDB_Reply: public Op_Reply<CmdReply>
{
    public:
        bool replyCount() const;
        virtual bool isOk() const override;
        virtual std::string getHRErrorMessage() const override;
    protected:
        friend std::ostream& operator<<(std::ostream& stream, HumanReadable<CmdDB_Reply> const& reply);
        std::ostream& printHR(std::ostream& stream) const {return Op_Reply::printHR(stream);}
};


struct WriteConcern
{
    WriteConcern(int w = 1, bool j = false, std::time_t wtimeout = 0);

    int             w;
    bool            j;
    std::time_t     wtimeout;
};

template<typename Action>
class CmdDB_Query: public Op_Query<Action>
{
    public:
        template<typename... Args>
        CmdDB_Query(std::string const& db, std::string const& collection, QueryOptions&& options, Args&&... args);

        // Insert
        CmdDB_Query& byPass();

        // Options
        // All these options are implemented for this object
        // in the CmdDB.tpp file.
        //
        // But if you make a call to a method that is not implemented
        // in your action object then you will get a compile time error.
        // i.e.  If you used .addHint("A hint") on an Insert Action
        //       this will fail to compile as this will call .addHint()
        //       on the actio but this is not implements on Insert
        //       but is implemented on Find document.


        // Insert & Delete
        CmdDB_Query& unordered();
        CmdDB_Query& setWrieConcern(int w = 1, bool j = false, std::time_t wtimeout = 0);

        // Insert & Find
        CmdDB_Query& setComment(std::string&& c);

        // Find
        CmdDB_Query& addFileds(std::initializer_list<std::string> const& fieldNames);
        CmdDB_Query& addHint(std::string&& hint);
        CmdDB_Query& setSkip(std::size_t val);
        CmdDB_Query& setLimit(std::size_t val);
        CmdDB_Query& setBatchSize(std::size_t val);
        CmdDB_Query& singleBatch();
        CmdDB_Query& setMaxTimeout(std::size_t val);
        CmdDB_Query& addReadConcern(ReadConcernLevel val);
        CmdDB_Query& addMax(std::string const& field, int val);
        CmdDB_Query& addMin(std::string const& field, int val);
        CmdDB_Query& justKeys();
        CmdDB_Query& showId();
        CmdDB_Query& tailableCursor();
        CmdDB_Query& tailedCursorAwait();
        CmdDB_Query& setNoCursorTimeout();
        CmdDB_Query& setAllowPartialResults();
        CmdDB_Query& useDisk();

        friend std::ostream& operator<<(std::ostream& stream, HumanReadable<CmdDB_Query> const& data);
        friend std::ostream& operator<<(std::ostream& stream, CmdDB_Query const& data) {return data.print(stream);}
};


}

ThorsAnvil_MakeEnum(ThorsAnvil::DB::Mongo::ReadConcernLevel,    local, available, majority, linearizable);
ThorsAnvil_MakeTrait(ThorsAnvil::DB::Mongo::Collation,          locale, caseLevel, strength, numericOrdering, alternate, maxVariable, backwards);
ThorsAnvil_MakeTrait(ThorsAnvil::DB::Mongo::WriteConcern,       w, j, wtimeout);
ThorsAnvil_MakeTrait(ThorsAnvil::DB::Mongo::WriteErrors,        index, code, errmsg);
ThorsAnvil_MakeTrait(ThorsAnvil::DB::Mongo::WriteConcernError,  code, errmsg);
ThorsAnvil_MakeTrait(ThorsAnvil::DB::Mongo::CmdReply,           ok, n, writeErrors, writeConcernError, errmsg, codeName, code);

#include "CmdDB.tpp"

#endif
CmdDB.tpp
#ifndef THORSANVIL_DB_MONGO_CMD_DB_TPP
#define THORSANVIL_DB_MONGO_CMD_DB_TPP

#ifndef THORSANVIL_DB_MONGO_CMD_DB_H
#error  "This should only be included from CmdDB.h"
#endif

namespace ThorsAnvil::DB::Mongo
{

template<typename Action>
template<typename... Args>
CmdDB_Query<Action>::CmdDB_Query(std::string const& db, std::string const& collection, QueryOptions&& options, Args&&... args)
    : Op_Query<Action>(db + ".$cmd", std::forward<QueryOptions>(options), 1, 0, collection, std::forward<Args>(args)...)
{}

template<typename Action>
CmdDB_Query<Action>& CmdDB_Query<Action>::unordered()
{
    this->getQuery().unordered();
    return *this;
}

template<typename Action>
CmdDB_Query<Action>& CmdDB_Query<Action>::byPass()
{
    this->getQuery().byPass();
    return *this;
}

template<typename Action>
CmdDB_Query<Action>& CmdDB_Query<Action>::setComment(std::string&& val)
{
    this->getQuery().setComment(std::forward<std::string>(val));
    return *this;
}
// etc.
// These all look like
// 1: Call getQuery() which gets the `Insert` object.
// 2: Call the setComment() or appropriate option method on action object.
// 3: return *this to allow chaining.

}
#endif

If you are interested:

\$\endgroup\$
7
  • 1
    \$\begingroup\$ Please edit your question so that the title describes the purpose of the code, rather than its mechanism. We really need to understand the motivational context to give good reviews. Thanks! \$\endgroup\$ – Toby Speight Jan 19 at 21:29
  • \$\begingroup\$ Much better, thanks. \$\endgroup\$ – Mast Jan 20 at 5:25
  • \$\begingroup\$ @Toby It may not look like it at first glance, but I think the title actually explains the purpose of the code, not the concerns. The code is an attempt to reduce the constructor count. The rest of the project is much bigger. \$\endgroup\$ – Mast Jan 20 at 5:28
  • \$\begingroup\$ So this is a program that tries to reduce constructor count? I disagree; it seems to be some other kind of program/library that has a reduced constructor count. I'll see if I can come up with something more appropriate. \$\endgroup\$ – Toby Speight Jan 20 at 11:08
  • \$\begingroup\$ I've edited - does the new title accurately represent this code? \$\endgroup\$ – Toby Speight Jan 20 at 11:12
3
\$\begingroup\$

Is it more efficient for the user of this API?

It looks nice for a single insert operation, but what if I need to insert many things, all with the same options? I would not want to have to repeat the .unordered().setComment(...).anotheroption(...) each time. There are several alternative ways to handle this:

Create stream manipulator objects

Just like the STL's input/output manipulators, you could make something for your connection object, so that you would write something like:

connection << unordered()
           << setComment("Hi there")
           << make_CmdDB_Insert(DB, "Collection", QueryOptions{}, person)
           << make_CmdDB_Insert(...);

Move options into their own object

Instead of having struct Insert hold both the documents to be inserted and the options of how to insert those, split the latter of into its own struct that can be manipulated as before, but is passed to make_CmdDb_Insert() as a parameter:

auto options = InsertOptions().unordered().setComment("Hi there");
connection << make_CmdDB_Insert(..., options)
           << ...
           << make_CmdDB_Insert(..., options);

But since you mentioned C++20: instead of having to write lots of member functions, you could use designated initializers instead, like so:

struct InsertOptions {
    bool                        ordered                  = true;
    WriteConcern                writeConcern;
    bool                        bypassDocumentValidation = false;
    std::string                 comment;
};

...

InsertOptions options = {
    .ordered = false,
    .comment = "Hi there",
};

connection << make_CmdDB_Insert(..., options)
           << ...
           << make_CmdDB_Insert(..., options);

And you can still do it as a one-liner if necessary:

connection << make_CmdDB_Insert(..., {.ordered = false, .comment = "Hi there"});

Why use operator<<?

For iostreams, operator<< and operator>> kind of make sense. They accept any kind of object but each operator always perform the same action on that object. But on a database, you want to perform different types of actions: insert, delete, query, and so on. The object is the data you want to insert/delete/query. However, you are mixing the object and the action together just to be able to use a generic operator<<. And there's quite some template gymnastics involved in making that happen.

As a user, I don't want to know about connections at all. I want to have an object that represents the MongoDB instance I am connected to, and then perform actions on that instance. So I think the following would just be more natural and easier to implement:

MongoInstance my_mongo(DB, USER, PASS);
Person person{"Mayura", 16};
my_mongo.insert("Collection", {}, person, {.ordered = false, ...});

The insert() member function should then of course be a template so it can accept any type of serializable object as the third parameter.

Make it easy to have options depend on variables

Your API has a problem if you don't know up front whether you want to have the operations to be ordered or unordered for example. Here is an example of the problem:

void somefunc(..., bool unordered) {
    if (unordered) {
        connection << make_CmdDB_Insert(...).unordered();
    } else {
        connection << make_CmdDB_Insert(...);
    }
}

This will explode when you have multiple options that you want to conditionally apply. Of course you could work around that a bit and keep it linear:

void somefunc(..., bool unordered) {
    auto insert_command = make_CmdDB.Insert(...);
    if (unordered)
        insert_command.unordered();
    connection << insert_command;
}

But much better would be to have a way to avoid conditional statements. I would have every option take a parameter, and of course you can give it a default value so you can still use it as above, but also allow:

void somefunc(..., bool unordered) {
    connection << make_CmdDB_Insert(...).unordered(unordered);
}
\$\endgroup\$
3
  • \$\begingroup\$ What is the name of this feature: {.ordered = false, ...} what version of C++ is it available in? \$\endgroup\$ – Martin York Jan 20 at 22:10
  • \$\begingroup\$ designated initializers, available in C++20 (although GCC and Clang support it even if you specify -std=c++11 it seems). The only drawback is that you have to specify the options in the correct order. \$\endgroup\$ – G. Sliepen Jan 20 at 22:31
  • \$\begingroup\$ Thanks for the excellent write up. I am seriously questioning the Builder pattern design now. I will re-think how this is going to work. \$\endgroup\$ – Martin York Jan 21 at 16:57

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.