From d70fd4b305fd8d97db8331d3991a801cc2589967 Mon Sep 17 00:00:00 2001 From: Dennis Oberst Date: Tue, 16 Jul 2024 13:23:06 +0200 Subject: [PATCH] QGrpcCallOptions: transform into implicitly shared value class As this class is copyable and really looks like a implicitly shared value class transform it into such. Also provide the missing testcase to this class. Task-number: QTBUG-123625 Pick-to: 6.8 Change-Id: Ibdd24155a04b0ba8a622998cbc482428965e2288 Reviewed-by: Alexey Edelev --- src/grpc/qgrpccalloptions.cpp | 70 ++++++---- src/grpc/qgrpccalloptions.h | 22 ++- tests/auto/grpc/CMakeLists.txt | 1 + .../auto/grpc/qgrpccalloptions/CMakeLists.txt | 17 +++ .../qgrpccalloptions/tst_qgrpccalloptions.cpp | 129 ++++++++++++++++++ 5 files changed, 205 insertions(+), 34 deletions(-) create mode 100644 tests/auto/grpc/qgrpccalloptions/CMakeLists.txt create mode 100644 tests/auto/grpc/qgrpccalloptions/tst_qgrpccalloptions.cpp diff --git a/src/grpc/qgrpccalloptions.cpp b/src/grpc/qgrpccalloptions.cpp index 683df3ee..6dab2af0 100644 --- a/src/grpc/qgrpccalloptions.cpp +++ b/src/grpc/qgrpccalloptions.cpp @@ -1,10 +1,10 @@ // Copyright (C) 2023 The Qt Company Ltd. // SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only -#include #include #include +#include QT_BEGIN_NAMESPACE @@ -20,22 +20,19 @@ using namespace Qt::StringLiterals; that are used by gRPC channels to communicate with the services. */ -class QGrpcCallOptionsPrivate +class QGrpcCallOptionsPrivate : public QSharedData { public: std::optional deadline; QGrpcMetadata metadata; }; -static void dPtrDeleter(QGrpcCallOptionsPrivate *ptr) -{ - delete ptr; -} +QT_DEFINE_QESDP_SPECIALIZATION_DTOR(QGrpcCallOptionsPrivate) /*! Constructs an empty QGrpcCallOptions object. */ -QGrpcCallOptions::QGrpcCallOptions() : dPtr(new QGrpcCallOptionsPrivate(), dPtrDeleter) +QGrpcCallOptions::QGrpcCallOptions() : d_ptr(new QGrpcCallOptionsPrivate()) { } @@ -47,21 +44,13 @@ QGrpcCallOptions::~QGrpcCallOptions() = default; /*! Construct a copy of QGrpcCallOptions with \a other object. */ -QGrpcCallOptions::QGrpcCallOptions(const QGrpcCallOptions &other) - : dPtr(new QGrpcCallOptionsPrivate(*other.dPtr), dPtrDeleter) -{ -} +QGrpcCallOptions::QGrpcCallOptions(const QGrpcCallOptions &other) = default; /*! Assigns \a other to this QGrpcCallOptions and returns a reference to this QGrpcCallOptions. */ -QGrpcCallOptions &QGrpcCallOptions::operator=(const QGrpcCallOptions &other) -{ - if (this != &other) - *dPtr = *other.dPtr; - return *this; -} +QGrpcCallOptions &QGrpcCallOptions::operator=(const QGrpcCallOptions &other) = default; /*! \fn QGrpcCallOptions::QGrpcCallOptions(QGrpcCallOptions &&other) noexcept @@ -82,6 +71,15 @@ QGrpcCallOptions &QGrpcCallOptions::operator=(const QGrpcCallOptions &other) value. */ +/*! + \since 6.8 + Constructs a new QVariant object from this QGrpcCallOptions. +*/ +QGrpcCallOptions::operator QVariant() const +{ + return QVariant::fromValue(*this); +} + /*! \since 6.8 \fn void QGrpcCallOptions::swap(QGrpcCallOptions &other) noexcept @@ -93,7 +91,11 @@ QGrpcCallOptions &QGrpcCallOptions::operator=(const QGrpcCallOptions &other) */ QGrpcCallOptions &QGrpcCallOptions::setDeadline(QGrpcDuration deadline) { - dPtr->deadline = deadline; + if (d_ptr->deadline == deadline) + return *this; + d_ptr.detach(); + Q_D(QGrpcCallOptions); + d->deadline = deadline; return *this; } @@ -105,7 +107,11 @@ QGrpcCallOptions &QGrpcCallOptions::setDeadline(QGrpcDuration deadline) */ QGrpcCallOptions &QGrpcCallOptions::setMetadata(const QGrpcMetadata &metadata) { - dPtr->metadata = metadata; + if (d_ptr->metadata == metadata) + return *this; + d_ptr.detach(); + Q_D(QGrpcCallOptions); + d->metadata = metadata; return *this; } @@ -114,9 +120,13 @@ QGrpcCallOptions &QGrpcCallOptions::setMetadata(const QGrpcMetadata &metadata) \sa setMetadata() */ -QGrpcCallOptions &QGrpcCallOptions::setMetadata(QGrpcMetadata &&metadata) noexcept +QGrpcCallOptions &QGrpcCallOptions::setMetadata(QGrpcMetadata &&metadata) { - dPtr->metadata = std::move(metadata); + if (d_ptr->metadata == metadata) + return *this; + d_ptr.detach(); + Q_D(QGrpcCallOptions); + d->metadata = std::move(metadata); return *this; } @@ -131,7 +141,8 @@ QGrpcCallOptions &QGrpcCallOptions::setMetadata(QGrpcMetadata &&metadata) noexce */ std::optional QGrpcCallOptions::deadline() const noexcept { - return dPtr->deadline; + Q_D(const QGrpcCallOptions); + return d->deadline; } /*! @@ -144,12 +155,16 @@ std::optional QGrpcCallOptions::deadline() const noexcept */ const QGrpcMetadata &QGrpcCallOptions::metadata() const & noexcept { - return dPtr->metadata; + Q_D(const QGrpcCallOptions); + return d->metadata; } -QGrpcMetadata QGrpcCallOptions::metadata() && noexcept +QGrpcMetadata QGrpcCallOptions::metadata() && { - return std::move(dPtr->metadata); + Q_D(QGrpcCallOptions); + if (d->ref.loadRelaxed() != 1) // return copy if shared + return { d->metadata }; + return std::move(d->metadata); } #ifndef QT_NO_DEBUG_STREAM @@ -160,9 +175,8 @@ QGrpcMetadata QGrpcCallOptions::metadata() && noexcept */ QDebug operator<<(QDebug debug, const QGrpcCallOptions &callOpts) { - QDebugStateSaver save(debug); - debug.nospace(); - debug.noquote(); + const QDebugStateSaver save(debug); + debug.nospace().noquote(); debug << "QGrpcCallOptions(deadline: " << callOpts.deadline() << ", metadata: " << callOpts.metadata() << ')'; return debug; diff --git a/src/grpc/qgrpccalloptions.h b/src/grpc/qgrpccalloptions.h index e9dfb619..70003129 100644 --- a/src/grpc/qgrpccalloptions.h +++ b/src/grpc/qgrpccalloptions.h @@ -7,15 +7,19 @@ #include #include +#include +#include #include -#include #include QT_BEGIN_NAMESPACE class QDebug; +class QVariant; + class QGrpcCallOptionsPrivate; +QT_DECLARE_QESDP_SPECIALIZATION_DTOR_WITH_EXPORT(QGrpcCallOptionsPrivate, Q_GRPC_EXPORT) class QGrpcCallOptions final { @@ -27,26 +31,32 @@ public: Q_GRPC_EXPORT QGrpcCallOptions &operator=(const QGrpcCallOptions &other); QGrpcCallOptions(QGrpcCallOptions &&other) noexcept = default; - QT_MOVE_ASSIGNMENT_OPERATOR_IMPL_VIA_MOVE_AND_SWAP(QGrpcCallOptions) + QT_MOVE_ASSIGNMENT_OPERATOR_IMPL_VIA_PURE_SWAP(QGrpcCallOptions) - void swap(QGrpcCallOptions &other) noexcept { dPtr.swap(other.dPtr); } + Q_GRPC_EXPORT Q_IMPLICIT operator QVariant() const; + + void swap(QGrpcCallOptions &other) noexcept { d_ptr.swap(other.d_ptr); } Q_GRPC_EXPORT QGrpcCallOptions &setDeadline(QGrpcDuration deadline); Q_GRPC_EXPORT QGrpcCallOptions &setMetadata(const QGrpcMetadata &metadata); - Q_GRPC_EXPORT QGrpcCallOptions &setMetadata(QGrpcMetadata &&metadata) noexcept; + Q_GRPC_EXPORT QGrpcCallOptions &setMetadata(QGrpcMetadata &&metadata); [[nodiscard]] Q_GRPC_EXPORT std::optional deadline() const noexcept; [[nodiscard]] Q_GRPC_EXPORT const QGrpcMetadata &metadata() const & noexcept; - [[nodiscard]] Q_GRPC_EXPORT QGrpcMetadata metadata() && noexcept; + [[nodiscard]] Q_GRPC_EXPORT QGrpcMetadata metadata() &&; private: - std::unique_ptr dPtr; + QExplicitlySharedDataPointer d_ptr; #ifndef QT_NO_DEBUG_STREAM friend Q_GRPC_EXPORT QDebug operator<<(QDebug debug, const QGrpcCallOptions &callOpts); #endif + + Q_DECLARE_PRIVATE(QGrpcCallOptions) }; +Q_DECLARE_SHARED(QGrpcCallOptions) + QT_END_NAMESPACE #endif // QGRPCALLOPTIONS_H diff --git a/tests/auto/grpc/CMakeLists.txt b/tests/auto/grpc/CMakeLists.txt index 5a46c063..c9158cc4 100644 --- a/tests/auto/grpc/CMakeLists.txt +++ b/tests/auto/grpc/CMakeLists.txt @@ -10,4 +10,5 @@ if(TARGET WrapgRPC::WrapLibgRPC) add_subdirectory(qgrpchttp2channel) add_subdirectory(qgrpcserializationformat) add_subdirectory(qgrpcstatus) + add_subdirectory(qgrpccalloptions) endif() diff --git a/tests/auto/grpc/qgrpccalloptions/CMakeLists.txt b/tests/auto/grpc/qgrpccalloptions/CMakeLists.txt new file mode 100644 index 00000000..222d84aa --- /dev/null +++ b/tests/auto/grpc/qgrpccalloptions/CMakeLists.txt @@ -0,0 +1,17 @@ +# Copyright (C) 2024 The Qt Company Ltd. +# SPDX-License-Identifier: BSD-3-Clause + +if(NOT QT_BUILD_STANDALONE_TESTS AND NOT QT_BUILDING_QT) + cmake_minimum_required(VERSION 3.16) + project(tst_qgrpccalloptions LANGUAGES CXX) + find_package(Qt6BuildInternals REQUIRED COMPONENTS STANDALONE_TEST) +endif() + +qt_internal_add_test(tst_qgrpccalloptions + SOURCES + tst_qgrpccalloptions.cpp + LIBRARIES + Qt::Core + Qt::Test + Qt::Grpc +) diff --git a/tests/auto/grpc/qgrpccalloptions/tst_qgrpccalloptions.cpp b/tests/auto/grpc/qgrpccalloptions/tst_qgrpccalloptions.cpp new file mode 100644 index 00000000..e7753398 --- /dev/null +++ b/tests/auto/grpc/qgrpccalloptions/tst_qgrpccalloptions.cpp @@ -0,0 +1,129 @@ +// Copyright (C) 2024 The Qt Company Ltd. +// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only + +#include + +#include + +#include + +using namespace std::chrono_literals; + +class QGrpcCallOptionsTest : public QObject +{ + Q_OBJECT + +private Q_SLOTS: + void hasSpecialMemberFunctions() const; + void hasImplicitQVariant() const; + void hasMemberSwap() const; + void propertyMetadata() const; + void propertyDeadline() const; + void streamsToDebug() const; +}; + +void QGrpcCallOptionsTest::hasSpecialMemberFunctions() const +{ + QGrpcCallOptions o1; + QVERIFY(!o1.deadline()); + QVERIFY(o1.metadata().empty()); + + o1.setDeadline(100ms); + + QGrpcCallOptions o2(o1); + QCOMPARE_EQ(o1.deadline(), o2.deadline()); + + QGrpcCallOptions o3 = o1; + QCOMPARE_EQ(o1.deadline(), o3.deadline()); + + QGrpcCallOptions o4(std::move(o1)); + QCOMPARE_EQ(o4.deadline(), o2.deadline()); + + o1 = std::move(o4); + QCOMPARE_EQ(o1.deadline(), o2.deadline()); +} + +void QGrpcCallOptionsTest::hasImplicitQVariant() const +{ + QGrpcCallOptions o1; + o1.setDeadline(250ms); + o1.setMetadata({ + { "keyA", "valA" }, + { "keyB", "valB" }, + }); + + QVariant v = o1; + QCOMPARE_EQ(v.metaType(), QMetaType::fromType()); + const auto o2 = v.value(); + QCOMPARE_EQ(o1.metadata(), o2.metadata()); + QCOMPARE_EQ(o1.deadline(), o2.deadline()); +} + +void QGrpcCallOptionsTest::hasMemberSwap() const +{ + constexpr QGrpcDuration Dur = 50ms; + + QGrpcCallOptions o1; + o1.setDeadline(Dur); + QGrpcCallOptions o2; + + QCOMPARE_EQ(o1.deadline(), Dur); + QVERIFY(!o2.deadline()); + o2.swap(o1); + QCOMPARE_EQ(o2.deadline(), Dur); + QVERIFY(!o1.deadline()); + swap(o2, o1); + QCOMPARE_EQ(o1.deadline(), Dur); + QVERIFY(!o2.deadline()); +} + +void QGrpcCallOptionsTest::propertyMetadata() const +{ + QGrpcMetadata md = { + { "keyA", "valA" }, + { "keyB", "valB" }, + }; + + QGrpcCallOptions o1; + auto o1Detach = o1; + o1.setMetadata(md); + QCOMPARE_EQ(o1.metadata(), md); + QCOMPARE_NE(o1.metadata(), o1Detach.metadata()); + + QGrpcCallOptions o2; + auto o2Detach = o2; + o2.setMetadata(std::move(md)); + QCOMPARE_EQ(o2.metadata(), o1.metadata()); + QCOMPARE_NE(o2.metadata(), o2Detach.metadata()); + + QCOMPARE_EQ(std::move(o1).metadata(), o2.metadata()); +} + +void QGrpcCallOptionsTest::propertyDeadline() const +{ + constexpr QGrpcDuration Dur = 50ms; + + QGrpcCallOptions o1; + auto o1Detach = o1; + o1.setDeadline(Dur); + QCOMPARE_EQ(o1.deadline(), Dur); + QCOMPARE_NE(o1.deadline(), o1Detach.deadline()); +} + +void QGrpcCallOptionsTest::streamsToDebug() const +{ + QGrpcCallOptions o; + QString storage; + QDebug dbg(&storage); + dbg.noquote().nospace(); + + dbg << o; + QVERIFY(!storage.isEmpty()); + + std::unique_ptr ustr(QTest::toString(o)); + QCOMPARE_EQ(storage, QString::fromUtf8(ustr.get())); +} + +QTEST_MAIN(QGrpcCallOptionsTest) + +#include "tst_qgrpccalloptions.moc"