add the ability to invoke QML functions

[prototypicalpro]

This PR adds the ability to invoke arbitrary QML methods for testing purposes. For example:

TextArea {
  id: textArea
  function test(arg) {
    ...
    return {'myvalue': [3, true, 7.0]}
  }
}

s = xmlrpc.client.ServerProxy('http://localhost:9000')
# invoke a built-in method to change the selection area
s.invokeMethod('window/textArea', 'select', [200, 300])
# invoke a custom method
print(s.invokeMethod('window/textArea', 'test', ['myarg'])) # prints "{'myvalue': [3, True, 7.0]}"

I'm posting this PR as a draft to get more feedback on my approach before I continue. More specifically, I have written a lot of code devoted to massaging types (Value -> QVariant -> parameter type and vice versa) because QMetaObject::invokeMethod and QMetaMethod::invoke both require the parameter types to exactly match the QML interface. For instance when attempting to invoke TextArea.select(int, int):

QMetaObject::invoke(textArea, "select", Q_ARG(int, 100), Q_ARG(int, 200)); // ok
QMetaObject::invoke(textArea, "select", Q_ARG(QVariant, QVariant::fromValue(100)), Q_ARG(QVariant, QVariant::fromValue(200))); // fails
QMetaObject::invoke(textArea, "select" Q_ARG(QJSValue, ...), Q_ARG(QJSValue, ...)); // also fails

Because of this restriction my code must first search for a matching method interface, perform the numerous conversion steps to put the arguments in, and do the conversion again in reverse with the return value. I have implemented this and it works but is rather cumbersome: I'd appreciate ideas on how to improve this process. One lead I've investigated is finding a way to invoke QML functions JavaScript-style (with no conversions or type checks) using the QJSValue or QJSEngine API, but thus far I haven't found anything spix could use.

TODO before this PR is merges (in addition to any feedback):

• Qt5 support (currently only tested on Qt6 and there are some major differences with the meta typing systems).
• Tests
• Docs

[faaxm]

Thanks for looking into this! I generally like the idea of having a generic InvokeMethod call in spix. I guess it will be quite a challenge though to get this into a nice looking and generic implementation.

On Spix architecture

One mayor comment I have is about the general architecture, and I guess I should take some time to finally write that CONTRIBUTING.md now that more and more PRs are coming in...

One idea in spix is to keep its internals as generic as possible. That means it should be possible to extend it with any kind of frontend, or to remove any of the frontends without impact to the remaining code. E.g. if anyrpc was no longer cool, it should be possible to replace the anyrpc api with a gRPC one by just switching out the TestServer that is used.
Because of that, none of the internal classes should be dependant on any types provided by anyrpc. Even now, from the perspective of someone using the plain TestServer in a gtest, it would be odd if some of the arguments to a command have to be a anyrpc::Variant, rather than a std::variant.

I know it's an additional step, but I would prefer if everything from anyrpc was first converted to a std::variant and then, in the Scene/Qt/* classes, to a Qt object.

Another part of the idea of spix being as generic as possible is that it should support different backends. This is currently not used, and I think not a problem with your changes, but it should be possible to use spix for testing a wxwidgets app just by adding a new set of Scene/* classes.

On the whole invokeMethod "mess" 😉

From the top of my head, I also don't know of a non-cumbersome way to do this 😞.

I think as a first step it would help to move as much as possible of the "invokeMethod" related code into a separate file. This would then provide a clean API that QtItem can use to call methods without having to take care of the type conversion business.
At least this would move the "cumbersome" stuff into a separate part of the library.

I found one SO-post where a QVariant is used for the arguments and return type of invokeMethod, this would avoid the need for the special variant/conversion struct you have. Only some freestanding functions for converting a QVariant would be needed.

It seems to work without searching for the QMetaMethod:

QVariant returnedValue;
QVariant msg = "Hello from C++";
QMetaObject::invokeMethod(object, "myQmlFunction",
    Q_RETURN_ARG(QVariant, returnedValue),
    Q_ARG(QVariant, msg));

Have you looked into something like this already?

[faaxm]

not sure if I overlooked something, but those are no longer needed in the current version of the code, right?

[prototypicalpro]

Thanks for the detailed review!

I know it's an additional step, but I would prefer if everything from anyrpc was first converted to a std::variant and then, in the Scene/Qt/* classes, to a Qt object.

Sure, that makes sense to me. I initially stuck to anyrpc::Value since std::variant requires C++17, but if you'd prefer the abstraction layer and are good with bumping the C++ version I'd be happy to make that change. QVariant also has some integrations with std::variant so that should remove some conversion logic.

I found one SO-post where a QVariant is used for the arguments and return type of invokeMethod, this would avoid the need for the special variant/conversion struct you have. Only some freestanding functions for converting a QVariant would be needed.

I did look into this, and as you mentioned this method works only for functions that don't have type annotations:

function test(arg) {} // QVariant works
function test(arg: int) {} // QVariant doesn't work
function test(arg): int {} // QVariant doesn't work

This isn't really a problem for user-defined functions, but it becomes annoying when dealing with Qt-defined functions since almost all of them have type annotations: TextArea for example has type annotations for every method. A user-defined proxy method would fix this issue, but personally I'd rather not define a proxy method every time I want to call a Qt-defined QML function. I'll take a look at implementing the return type as std::variant and maybe it'll be good enough until Qt updates invokeMethod.

I think as a first step it would help to move as much as possible of the "invokeMethod" related code into a separate file. This would then provide a clean API that QtItem can use to call methods without having to take care of the type conversion business. At least this would move the "cumbersome" stuff into a separate part of the library.

For sure, I'll tackle this on my next pass.

[faaxm]

Sure, that makes sense to me. I initially stuck to anyrpc::Value since std::variant requires C++17, but if you'd prefer the abstraction layer and are good with bumping the C++ version I'd be happy to make that change. QVariant also has some integrations with std::variant so that should remove some conversion logic.

@prototypicalpro I think bumping the C++ version to 17 is fine 👍

I did look into this, and as you mentioned this method works only for functions that don't have type annotations:

Too bad. I was hoping there would already be some magic in place to map any type to the QVariant 😞

Thanks for looking into this!

[prototypicalpro]

I implemented an intermediary variant (spix::Variant) to preserve the abstraction layer between anyrpc, spix, and Qt: spix::Variant is a union of all the basic anyrpc types, including a map of itself and a list of itself. I was hoping that the conversions between the variant types would be a bit more elegant, but it turns out std::visit only supports std::variant and not classes inheriting from std::variant (at least until recently), so it looks like we're stuck with switch statements for now 😔.

In addition to the above:

• Moves some logic out of QtItem::invokeMethod. Replaced the return type tagged union with an std::variant.
• Refactored AnyRPCUtils: now split into AnyRPCUtils (type conversion stuff) and AnyRPCFunction (anyrpc registration stuff). Refactored callAndAssignAnyRpcResult to use if constexpr instead of SFINAE.
• Updates QMetaType stuff to be compatible with Qt5.
• Bumped to C++17.
• Added a log line when spix starts: "Spix server is enabled. Only use this in a safe environment." to take after the QML debugging warning (I can make this it's own PR if you want).

I think concept-wise this PR is good pending review. I'll work on adding docs, tests, and an example next week.

Edit: returning objects in Qt5 doesn't appear to be working, so I'll have to check that out as well.

[faaxm]

Nice! 🙂

[faaxm]

Totally makes sense to put your name here. Now that more and more people are contributing, I was thinking about changing all the copyright lines to a "(C) Spix Authors" and adding the names of contributors into a AUTHORS.txt or something like that.
Would you also be fine with that? Would be a separate PR once this is all merged that would be just about changing the copyright text.

[prototypicalpro]

Yup that sounds great!

[prototypicalpro]

I added tests and some text in the README, and with that I think this PR is good to go!

[faaxm]

Sorry for the delay... I have lots of stuff going on right now, but I will get to this PR eventually 😕

[faaxm]

Great work! Thank you for contributing this.