From 6cf8e05703eb60e64f826b206ae802ad68be09dd Mon Sep 17 00:00:00 2001 From: Alexey Edelev Date: Sun, 25 Aug 2024 18:39:02 +0200 Subject: [PATCH] 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 --- src/grpc/qabstractgrpcchannel.h | 6 ++-- src/grpc/qgrpchttp2channel.cpp | 28 +++++++++---------- src/grpcquick/qqmlgrpcchanneloptions.cpp | 21 ++++++++++---- .../qml/tst_grpc_client_unarycall.qml | 12 ++++++-- 4 files changed, 42 insertions(+), 25 deletions(-) diff --git a/src/grpc/qabstractgrpcchannel.h b/src/grpc/qabstractgrpcchannel.h index 06b973a9..60315d0e 100644 --- a/src/grpc/qabstractgrpcchannel.h +++ b/src/grpc/qabstractgrpcchannel.h @@ -34,6 +34,9 @@ public: [[nodiscard]] virtual std::shared_ptr 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 operationContext) = 0; virtual void bidiStream(std::shared_ptr operationContext) = 0; - [[nodiscard]] const QGrpcChannelOptions &channelOptions() const & noexcept; - void channelOptions() && = delete; - private: std::shared_ptr call(QLatin1StringView method, QLatin1StringView service, QByteArrayView arg, diff --git a/src/grpc/qgrpchttp2channel.cpp b/src/grpc/qgrpchttp2channel.cpp index 630bc4bc..e866bb61 100644 --- a/src/grpc/qgrpchttp2channel.cpp +++ b/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 &operationContext, bool endStream = false); std::shared_ptr 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 deadline; if (channelOpPtr->callOptions().deadlineTimeout()) deadline = channelOpPtr->callOptions().deadlineTimeout(); - else if (parentChannel->channelOptions.deadlineTimeout()) - deadline = parentChannel->channelOptions.deadlineTimeout(); + else if (parentChannel->q_ptr->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, - const QGrpcChannelOptions &options) - : hostUri(uri), channelOptions(options) +QGrpcHttp2ChannelPrivate::QGrpcHttp2ChannelPrivate(const QUrl &uri, QGrpcHttp2Channel *q) + : hostUri(uri), q_ptr(q) { + 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(); 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(), - d_ptr(std::make_unique(hostUri, QGrpcChannelOptions{})) + : QAbstractGrpcChannel(), d_ptr(std::make_unique(hostUri, this)) { } @@ -834,7 +834,7 @@ QGrpcHttp2Channel::QGrpcHttp2Channel(const QUrl &hostUri) */ QGrpcHttp2Channel::QGrpcHttp2Channel(const QUrl &hostUri, const QGrpcChannelOptions &options) : QAbstractGrpcChannel(options), - d_ptr(std::make_unique(hostUri, options)) + d_ptr(std::make_unique(hostUri, this)) { } @@ -892,7 +892,7 @@ void QGrpcHttp2Channel::bidiStream(std::shared_ptr operat */ std::shared_ptr QGrpcHttp2Channel::serializer() const noexcept { - return d_ptr->channelOptions.serializationFormat().serializer(); + return channelOptions().serializationFormat().serializer(); } QT_END_NAMESPACE diff --git a/src/grpcquick/qqmlgrpcchanneloptions.cpp b/src/grpcquick/qqmlgrpcchanneloptions.cpp index afd3bbdd..44052a05 100644 --- a/src/grpcquick/qqmlgrpcchanneloptions.cpp +++ b/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) - d->options.setMetadata(d->metadata->metadata()); - else - d->options.setMetadata({}); - emit metadataChanged(); + if (d->metadata) { + const auto updateMetadata = [this]() { + Q_D(QQmlGrpcChannelOptions); + d->options.setMetadata(d->metadata->metadata()); + emit metadataChanged(); + }; + d->metadataUpdateConnection = connect(d->metadata, &QQmlGrpcMetadata::dataChanged, this, + updateMetadata); + updateMetadata(); + } } } diff --git a/tests/auto/grpcquick/client/unarycall/qml/tst_grpc_client_unarycall.qml b/tests/auto/grpcquick/client/unarycall/qml/tst_grpc_client_unarycall.qml index 7002b235..33b5201e 100644 --- a/tests/auto/grpcquick/client/unarycall/qml/tst_grpc_client_unarycall.qml +++ b/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,