React on QQmlGrpcMetadata::dataChanged in QQmlGrpcChannelOptions

It's necessary not only to set the inner metadata when the setMetadata
is called, but also when QQmlGrpcMetadata::data is changed. Otherwise
the internal metadata value will not be in sync.

As drive-by change, fix the usage of channel options in
QGrpcHttp2Channel. Instead of storing the options duplicate in
QGrpcHttp2Channel, use the QAbstractGrpcHttp2Channel copy.
Make QAbstractGrpcHttp2Channel::channelOptions public.

TODO: QGrpcHttp2Channel doesn't consider the changes of ssl
configuration and serialization format when updating the
channelOptions, but it should react respectively or disallow setting
those at runtime.

Pick-to: 6.8
Change-Id: I40cf8476679f83a2925d77bcd1d89f043a0b6e67
Reviewed-by: Dennis Oberst <dennis.oberst@qt.io>

src/grpc/qabstractgrpcchannel.h

@@ -34,6 +34,9 @@ public:
     [[nodiscard]] virtual std::shared_ptr<QAbstractProtobufSerializer>
     serializer() const noexcept = 0;
 
+    [[nodiscard]] const QGrpcChannelOptions &channelOptions() const & noexcept;
+    void channelOptions() && = delete;
+
     void setChannelOptions(const QGrpcChannelOptions &options) noexcept;
     void setChannelOptions(QGrpcChannelOptions &&options) noexcept;
 
@@ -47,9 +50,6 @@ protected:
     virtual void clientStream(std::shared_ptr<QGrpcOperationContext> operationContext) = 0;
     virtual void bidiStream(std::shared_ptr<QGrpcOperationContext> operationContext) = 0;
 
-    [[nodiscard]] const QGrpcChannelOptions &channelOptions() const & noexcept;
-    void channelOptions() && = delete;
-
 private:
     std::shared_ptr<QGrpcCallReply> call(QLatin1StringView method, QLatin1StringView service,
                                          QByteArrayView arg,

src/grpc/qgrpchttp2channel.cpp

@@ -159,14 +159,14 @@ class QGrpcHttp2ChannelPrivate : public QObject
 {
     Q_OBJECT
 public:
-    explicit QGrpcHttp2ChannelPrivate(const QUrl &uri, const QGrpcChannelOptions &options);
+    explicit QGrpcHttp2ChannelPrivate(const QUrl &uri, QGrpcHttp2Channel *q);
     ~QGrpcHttp2ChannelPrivate() override;
 
     void processOperation(const std::shared_ptr<QGrpcOperationContext> &operationContext,
                           bool endStream = false);
     std::shared_ptr<QAbstractProtobufSerializer> serializer;
     QUrl hostUri;
-    QGrpcChannelOptions channelOptions;
+    QGrpcHttp2Channel *q_ptr = nullptr;
 
 private:
     Q_DISABLE_COPY_MOVE(QGrpcHttp2ChannelPrivate)
@@ -394,8 +394,8 @@ void QGrpcHttp2ChannelPrivate::Http2Handler::attachStream(QHttp2Stream *stream_)
     std::optional<std::chrono::milliseconds> deadline;
     if (channelOpPtr->callOptions().deadlineTimeout())
         deadline = channelOpPtr->callOptions().deadlineTimeout();
-    else if (parentChannel->channelOptions.deadlineTimeout())
+    else if (parentChannel->q_ptr->channelOptions().deadlineTimeout())
-        deadline = parentChannel->channelOptions.deadlineTimeout();
+        deadline = parentChannel->q_ptr->channelOptions().deadlineTimeout();
     if (deadline) {
         // We have an active stream and a deadline. It's time to start the timer.
         QObject::connect(&m_deadlineTimer, &QTimer::timeout, this,
@@ -428,7 +428,7 @@ void QGrpcHttp2ChannelPrivate::Http2Handler::prepareInitialRequest(QGrpcOperatio
                                                                    QGrpcHttp2ChannelPrivate
                                                                        *channel)
 {
-    auto &channelOptions = channel->channelOptions;
+    const auto &channelOptions = channel->q_ptr->channelOptions();
     QByteArray service{ operationContext->service().data(), operationContext->service().size() };
     QByteArray method{ operationContext->method().data(), operationContext->method().size() };
     m_initialHeaders = HPack::HttpHeader{
@@ -585,10 +585,10 @@ void QGrpcHttp2ChannelPrivate::Http2Handler::onDeadlineTimeout()
     }
 }
 
-QGrpcHttp2ChannelPrivate::QGrpcHttp2ChannelPrivate(const QUrl &uri,
+QGrpcHttp2ChannelPrivate::QGrpcHttp2ChannelPrivate(const QUrl &uri, QGrpcHttp2Channel *q)
-                                                   const QGrpcChannelOptions &options)
+    : hostUri(uri), q_ptr(q)
-    : hostUri(uri), channelOptions(options)
 {
+    auto channelOptions = q_ptr->channelOptions();
     auto formatSuffix = channelOptions.serializationFormat().suffix();
     const QByteArray defaultContentType = DefaultContentType.toByteArray();
     const QByteArray contentTypeFromOptions = !formatSuffix.isEmpty()
@@ -608,6 +608,7 @@ QGrpcHttp2ChannelPrivate::QGrpcHttp2ChannelPrivate(const QUrl &uri,
                                << it.value() << ". Using protobuf format as the default one.";
                 channelOptions.setSerializationFormat(SerializationFormat::Default);
             }
+            q_ptr->setChannelOptions(channelOptions);
         } else if (it.value() != contentTypeFromOptions) {
             warnAboutFormatConflict = true;
         } else {
@@ -652,13 +653,13 @@ QGrpcHttp2ChannelPrivate::QGrpcHttp2ChannelPrivate(const QUrl &uri,
         };
     } else
 #if QT_CONFIG(ssl)
-        if (hostUri.scheme() == "https"_L1 || options.sslConfiguration()) {
+        if (hostUri.scheme() == "https"_L1 || channelOptions.sslConfiguration()) {
         auto *sslSocket = initSocket<QSslSocket>();
         if (hostUri.port() < 0) {
             hostUri.setPort(443);
         }
 
-        if (const auto userSslConfig = options.sslConfiguration(); userSslConfig) {
+        if (const auto userSslConfig = channelOptions.sslConfiguration(); userSslConfig) {
             sslSocket->setSslConfiguration(*userSslConfig);
         } else {
             static const QByteArray h2NexProtocol = "h2"_ba;
@@ -824,8 +825,7 @@ void QGrpcHttp2ChannelPrivate::deleteHandler(Http2Handler *handler)
     Constructs QGrpcHttp2Channel with \a hostUri.
 */
 QGrpcHttp2Channel::QGrpcHttp2Channel(const QUrl &hostUri)
-    : QAbstractGrpcChannel(),
+    : QAbstractGrpcChannel(), d_ptr(std::make_unique<QGrpcHttp2ChannelPrivate>(hostUri, this))
-      d_ptr(std::make_unique<QGrpcHttp2ChannelPrivate>(hostUri, QGrpcChannelOptions{}))
 {
 }
 
@@ -834,7 +834,7 @@ QGrpcHttp2Channel::QGrpcHttp2Channel(const QUrl &hostUri)
 */
 QGrpcHttp2Channel::QGrpcHttp2Channel(const QUrl &hostUri, const QGrpcChannelOptions &options)
     : QAbstractGrpcChannel(options),
-      d_ptr(std::make_unique<QGrpcHttp2ChannelPrivate>(hostUri, options))
+      d_ptr(std::make_unique<QGrpcHttp2ChannelPrivate>(hostUri, this))
 {
 }
 
@@ -892,7 +892,7 @@ void QGrpcHttp2Channel::bidiStream(std::shared_ptr<QGrpcOperationContext> operat
 */
 std::shared_ptr<QAbstractProtobufSerializer> QGrpcHttp2Channel::serializer() const noexcept
 {
-    return d_ptr->channelOptions.serializationFormat().serializer();
+    return channelOptions().serializationFormat().serializer();
 }
 
 QT_END_NAMESPACE

src/grpcquick/qqmlgrpcchanneloptions.cpp

@@ -27,6 +27,7 @@ public:
 #if QT_CONFIG(ssl)
     QQmlSslConfiguration configuration;
 #endif // QT_CONFIG(ssl)
+    QMetaObject::Connection metadataUpdateConnection;
 };
 
 QQmlGrpcChannelOptionsPrivate::QQmlGrpcChannelOptionsPrivate() : QObjectPrivate()
@@ -68,12 +69,22 @@ void QQmlGrpcChannelOptions::setMetadata(QQmlGrpcMetadata *value)
 {
     Q_D(QQmlGrpcChannelOptions);
     if (d->metadata != value) {
+        if (d->metadataUpdateConnection) {
+            disconnect(d->metadataUpdateConnection);
+            d->metadataUpdateConnection = {};
+        }
+
         d->metadata = value;
-        if (d->metadata)
+        if (d->metadata) {
-            d->options.setMetadata(d->metadata->metadata());
+            const auto updateMetadata = [this]() {
-        else
+                Q_D(QQmlGrpcChannelOptions);
-            d->options.setMetadata({});
+                d->options.setMetadata(d->metadata->metadata());
-        emit metadataChanged();
+                emit metadataChanged();
+            };
+            d->metadataUpdateConnection = connect(d->metadata, &QQmlGrpcMetadata::dataChanged, this,
+                                                  updateMetadata);
+            updateMetadata();
+        }
     }
 }
 

tests/auto/grpcquick/client/unarycall/qml/tst_grpc_client_unarycall.qml

@@ -38,7 +38,10 @@ Item {
         return Qt.createQmlObject("import QtQuick; import QtGrpc; GrpcHttp2Channel { \
                                    hostUri: \"http://localhost:50051\"; \
                                    options: GrpcChannelOptions { \
-                                   deadlineTimeout: { 2000 } } }", root)
+                                   deadlineTimeout: { 2000 } \
+                                   metadata: GrpcMetadata {
+                                        data: ({ \"common-meta-data\": \"test-channel-metadata\" }) \
+                                   }}}", root)
     }
 
     function createGrpcChannelWithDeadlineItem() {
@@ -149,13 +152,16 @@ Item {
             var missingHeaders = Array()
             missingHeaders.push("user-name")
             missingHeaders.push("user-password")
+            missingHeaders.push("common-meta-data")
 
             for (var i = 0; i < unaryCallWithOptions.result.valuesData.length; i++) {
                 var md = unaryCallWithOptions.result.valuesData[i]
-                if (md.key == "user-name" && md.value == "localhost")
+                if (md.key === "user-name" && md.value === "localhost")
                     removeElementFromArray(missingHeaders, "user-name")
-                if (md.key == "user-password" && md.value == "qwerty")
+                if (md.key === "user-password" && md.value === "qwerty")
                     removeElementFromArray(missingHeaders, "user-password")
+                if (md.key === "common-meta-data" && md.value === "test-channel-metadata")
+                    removeElementFromArray(missingHeaders, "common-meta-data")
             }
 
             verify(missingHeaders.length === 0,