QVariantMap to message

[gmabey]

I have a situation where I'm producing message data much earlier than I want to start packing it into a protobuf message. In order to minimize the amount of code that #include's the generated .qpb.h file, I have a idea for passing large amounts of binary data around -- stashing it in a QVariantMap with a natural structure.

That is, I have some large-ish messages with binary data in them, and I'd like to formulate them into a structure with code that doesn't #include my .qpb.h file, without involving a step such as serializing them to a JSON string.

However, with the natural structures implied by JSON -- serializers such as
https://developers.google.com/protocol-buffers/docs/reference/cpp/google.protobuf.util.json_util#JsonStringToMessage.details
there are map and list structures that can be accommodated for within a QVariantMap. As if the message was serialized to JSON, then QJsonDocument was used to decode it first into a QJsonObject, and then that object was converted toVariantMap(). So, I'm not suggesting that QJsonDocument be used, only that a QVariantMap structured like it would produce be accepted as a constructor argument for the classes generated by qtprotobuf.

As I see it, this would effectively provide for a named-argument syntax for the constructor, alongside the many variations on the positional-argument system you currently have going, which don't seem to account for message elements being removed as the .proto file evolves.

So, for this kind of message:

message PlotVals {
    float xVal;
    float yVal;
    float zVal;
}

This kind of constructor could be used:

    PlotVals mess(QVariantMap{{"xVals", xs}, {"zVals", zs}});

This is a very pie-in-the-sky feature suggestion -- no timeline is envisioned or requested, it's just an idea for future development that I think would be cool and useful.

[semlanik]

I like this idea. Will check some capabilities, but I would prefer to avoid extra constuctor and make something like static methods for bidirectional coversion

namespace QtProtobuf {
template<typename T>
convert(QVariantMap, T) {
    ...
}

template<typename T>
convert(T, QVariantMap) {
    ...
}
}

[gmabey]

I think that the bidirectional thing you describe would be really great!

Just elaborating on this a bit ... maybe this is in addition to the convert() functions you describe. Actually, this next thought would have to be a static class method ...

Instead of a QVariantMap, a QVector<QPair<enum MessageClass::FIELD_NAMES, QVariant>> would be much tighter -- because the keys would have to be valid. Basically, I'm suggesting that #108 could be brought to bear on this "generalized assignment" approach. So, it would look a lot like the example I originally posted:

PlotVals mess = PlotVals::fromValueVector({{PlotVals::XVALS_FIELD_NUM, xs}, {PlotVals::YVALS_FIELD_NUM, zs}});

But, being able to go back and forth from JSON (er, it's parallel, in a QVariantMap) is really interesting, especially as nested messages come into play. For this idea, the following would have to be used, which is a bit ugly, but still workable

message PlotVals {
    float xVal = 1;
    float yVal = 2;
    float zVal = 3;
}
message MoreVals {
    PlotVals pVals = 1;
    float yetAnotherVal = 2;
}

using MoreVals;
MoreVals mv = MoreVals::fromValueVector({{YETANOTHERVAL_FIELD_NUM, 1.0f}, {PVALS_FIELD_NUM, QVariant::fromValue(PlotVals::fromValueVector({{PlotVals::ZVAL_FIELD_NUM, 2.f}})}});

But the corresponding JSON/QVariantMap would be very nice:

{
  "yetAnotherVal": 1.0,
  "pVals": { "zVal": 2.0 }
}

[semlanik]

Found out, that there is huge issue related to memory management when tried to implement this approach. Since properties in generated classes return raw pointers to internal fields, I have this pointers in output QVariantMap/QList containers. Same happens in backward conversion, need to allocate raw pointers to obects when add members to container. I completely decline this approach and need more time to solve it. Probably this could be resolved by using custom QVariant converters, but would like to postpone this issue for now.

[gmabey]

I have no complaint about you removing this from the v0.6.0 milestone but I'm glad to hear that you're still interested in this feature. I also think that I understand the significant challenges this approach brings.

I also suspect that if #180 were fully implemented, most of those issues would disappear.