Remove HTTP headers equals() / comparison

There's several ways to compare HTTP headers, and arguably
the need for it is not very high. For the time being users can use
different accessors and compare in ways that make sense for
their use cases.

Consequently since HTTP headers are no longer trivially comparable,
it makes also comparing the request factories more 'moot' because
headers are a central piece of request information. So removed
comparison from request factory as well.

These comparisons can be restored later if a clear understanding
on it's need, and on how it should be best done, emerges.

Resulted from API-review

Change-Id: Idb5ab3710268b52a8e59656db8cc7de82f0ae511
Reviewed-by: Ivan Solovev <ivan.solovev@qt.io>
Reviewed-by: Marc Mutz <marc.mutz@qt.io>
(cherry picked from commit 89dab8578c10ed4aee4e2f4b51e7c962d618c1eb)

src/network/access/qhttpheaders.cpp

@@ -90,9 +90,6 @@ class QHttpHeadersPrivate : public QSharedData
 public:
     QHttpHeadersPrivate() = default;
 
-    bool equals(const QHttpHeadersPrivate &other,
-                QHttpHeaders::CompareOptions options) const noexcept;
-
     QList<Header> headers;
 
     Q_ALWAYS_INLINE void verify([[maybe_unused]] qsizetype pos = 0,
@@ -107,18 +104,6 @@ public:
 
 QT_DEFINE_QESDP_SPECIALIZATION_DTOR(QHttpHeadersPrivate)
 
-bool QHttpHeadersPrivate::equals(const QHttpHeadersPrivate &other,
-                                 QHttpHeaders::CompareOptions options) const noexcept
-{
-    if (headers.size() != other.headers.size())
-        return false;
-
-    if (options & QHttpHeaders::CompareOption::OrderSensitive)
-        return headers == other.headers;
-    else
-        return std::is_permutation(headers.begin(), headers.end(), other.headers.begin());
-}
-
 // This list is from IANA HTTP Field Name Registry
 // https://www.iana.org/assignments/http-fields
 // It contains entries that are either "permanent"
@@ -493,24 +478,6 @@ static constexpr auto headerNames = qOffsetStringArray(
     \value ProtocolQuery
 */
 
-/*!
-    \enum QHttpHeaders::CompareOption
-
-    This enum type contains the options for comparing two
-    QHttpHeaders instances.
-
-    \value OrderInsensitive
-    Specifies that the order of headers is not significant in the comparison.
-    With this option, two QHttpHeaders instances will be considered equal
-    if they contain the same headers regardless of their order. This is
-    true with most HTTP headers and use cases.
-
-    \value OrderSensitive
-    Specifies that the order of headers is significant in the comparison.
-    With this option, two QHttpHeaders instances will be considered equal
-    only if they contain the same headers in the same exact order.
-*/
-
 /*!
     Creates a new QHttpHeaders object.
 */
@@ -1074,19 +1041,6 @@ void QHttpHeaders::reserve(qsizetype size)
     d->headers.reserve(size);
 }
 
-/*!
-    Compares this instance with \a other and returns \c true if they
-    are considered equal in accordance with the provided \a options.
-
-    The header names are always compared as case-insensitive, and values
-    as case-sensitive. For example \e Accept and \e ACCEPT header names
-    are considered equal, while values \e something and \e SOMETHING are not.
-*/
-bool QHttpHeaders::equals(const QHttpHeaders &other, CompareOptions options) const noexcept
-{
-    return d == other.d || d->equals(*other.d, options);
-}
-
 /*!
     Returns the header entries as a list of (name, value) pairs.
     Header names are case-insensitive, and the returned names are lower-cased.

src/network/access/qhttpheaders.h

@@ -201,12 +201,6 @@ public:
     };
     Q_ENUM(WellKnownHeader)
 
-    enum class CompareOption {
-        OrderInsensitive = 0x01,
-        OrderSensitive = 0x02,
-    };
-    Q_DECLARE_FLAGS(CompareOptions, CompareOption)
-
     Q_NETWORK_EXPORT QHttpHeaders();
     Q_NETWORK_EXPORT ~QHttpHeaders();
 
@@ -248,9 +242,6 @@ public:
     Q_NETWORK_EXPORT void reserve(qsizetype size);
     bool isEmpty() const noexcept { return size() == 0; }
 
-    Q_NETWORK_EXPORT bool equals(const QHttpHeaders &other,
-                           CompareOptions options = CompareOption::OrderInsensitive) const noexcept;
-
     Q_NETWORK_EXPORT static QHttpHeaders
     fromListOfPairs(const QList<std::pair<QByteArray, QByteArray>> &headers);
     Q_NETWORK_EXPORT static QHttpHeaders
@@ -269,8 +260,6 @@ private:
     QExplicitlySharedDataPointer<QHttpHeadersPrivate> d;
 };
 
-Q_DECLARE_OPERATORS_FOR_FLAGS(QHttpHeaders::CompareOptions)
-
 Q_DECLARE_SHARED(QHttpHeaders)
 
 QT_END_NAMESPACE

src/network/access/qnetworkrequestfactory.cpp

@@ -121,35 +121,6 @@ QNetworkRequestFactory &QNetworkRequestFactory::operator=(const QNetworkRequestF
     very fast and never fails.
  */
 
-/*!
-    \fn bool QNetworkRequestFactory::operator==(const QNetworkRequestFactory &lhs,
-                                              const QNetworkRequestFactory &rhs)
-
-    Returns \c true if \a lhs is considered equal with \a rhs, meaning
-    that all data in the factories match, otherwise returns \c false.
-
-    \note The headers comparison is order-insensitive.
-
-    \sa QNetworkRequestFactory::operator!=()
- */
-
-/*!
-    \fn bool QNetworkRequestFactory::operator!=(const QNetworkRequestFactory &lhs,
-                                              const QNetworkRequestFactory &rhs)
-
-    Returns \c true if \a lhs is not considered equal with \a rhs.
-
-    \sa QNetworkRequestFactory::operator==()
- */
-
-/*!
-    \internal
- */
-bool comparesEqual(const QNetworkRequestFactory &lhs, const QNetworkRequestFactory &rhs) noexcept
-{
-    return lhs.d == rhs.d || lhs.d->equals(*rhs.d);
-}
-
 /*!
     Returns the base URL used for the individual requests.
 
@@ -594,22 +565,6 @@ QUrl QNetworkRequestFactoryPrivate::requestUrl(const QString *path,
     return resultUrl;
 }
 
-bool QNetworkRequestFactoryPrivate::equals(
-        const QNetworkRequestFactoryPrivate &other) const noexcept
-{
-    return
-            transferTimeout == other.transferTimeout &&
-#if QT_CONFIG(ssl)
-            sslConfig == other.sslConfig &&
-#endif
-            baseUrl == other.baseUrl &&
-            bearerToken == other.bearerToken &&
-            userName == other.userName &&
-            password == other.password &&
-            headers.equals(other.headers) &&
-            queryParameters == other.queryParameters;
-}
-
 #ifndef QT_NO_DEBUG_STREAM
 /*!
     \fn QDebug QNetworkRequestFactory::operator<<(QDebug debug,

src/network/access/qnetworkrequestfactory.h

@@ -75,10 +75,6 @@ public:
     Q_NETWORK_EXPORT void clearQueryParameters();
 
 private:
-    friend Q_NETWORK_EXPORT bool comparesEqual(const QNetworkRequestFactory &lhs,
-                                               const QNetworkRequestFactory &rhs) noexcept;
-    Q_DECLARE_EQUALITY_COMPARABLE(QNetworkRequestFactory)
-
 #ifndef QT_NO_DEBUG_STREAM
     friend Q_NETWORK_EXPORT QDebug operator<<(QDebug debug, const QNetworkRequestFactory &reply);
 #endif

src/network/access/qnetworkrequestfactory_p.h

@@ -34,7 +34,6 @@ public:
     ~QNetworkRequestFactoryPrivate();
     QNetworkRequest newRequest(const QUrl &url) const;
     QUrl requestUrl(const QString *path = nullptr, const QUrlQuery *query = nullptr) const;
-    bool equals(const QNetworkRequestFactoryPrivate &other) const noexcept;
 
 #if QT_CONFIG(ssl)
     QSslConfiguration sslConfig;

tests/auto/network/access/qhttpheaders/tst_qhttpheaders.cpp

@@ -14,7 +14,6 @@ class tst_QHttpHeaders : public QObject
     Q_OBJECT
 
 private slots:
-    void comparison();
     void constructors();
     void accessors();
     void headerNameField();
@@ -36,71 +35,6 @@ private:
     static constexpr QAnyStringView V3{"VALUE3"};
 };
 
-void tst_QHttpHeaders::comparison()
-{
-    // Basic comparisons
-    QHttpHeaders h1;
-    QHttpHeaders h2;
-    QVERIFY(h1.equals(h2)); // empties
-    h1.append(n1, v1);
-    QVERIFY(h1.equals(h1)); // self
-    h2.append(n1, v1);
-    QVERIFY(h1.equals(h2));
-    h1.append(n2, v2);
-    QVERIFY(!h1.equals(h2));
-    h1.removeAll(n2);
-    QVERIFY(h1.equals(h2));
-
-    // 'name' case-insensitivity and 'value' case-sensitivity
-    h1.removeAll(n1);
-    QVERIFY(h1.isEmpty());
-    h1.append(N1, v1);
-    QVERIFY(h1.equals(h2));
-    h1.removeAll(n1);
-    QVERIFY(h1.isEmpty());
-    h1.append(n1, V1);
-    QVERIFY(!h1.equals(h2));
-
-    // Order-insensitivity
-    h1.clear();
-    h2.clear();
-    QVERIFY(h1.isEmpty() && h2.isEmpty());
-    // Same headers but in different order
-    h1.append(n1, v1);
-    h1.append(n2, v2);
-    h2.append(n2, v2);
-    h2.append(n1, v1);
-    QVERIFY(h1.equals(h2));
-    // Add header with different name casing
-    h1.insert(0, n1, v1);
-    h2.append(N1, v1);
-    QVERIFY(h1.equals(h2));
-    // Add header with different value casing
-    h1.insert(0, n1, v1);
-    h2.append(n1, V1);
-    QVERIFY(!h1.equals(h2));
-
-    // Order-sensitivity
-    h1.clear();
-    h2.clear();
-    QVERIFY(h1.equals(h2, QHttpHeaders::CompareOption::OrderSensitive));
-    // Same headers but in different order
-    h1.append(n1, v1);
-    h1.append(n2, v2);
-    h2.append(n2, v2);
-    h2.append(n1, v1);
-    QVERIFY(!h1.equals(h2, QHttpHeaders::CompareOption::OrderSensitive));
-
-    // Different number of headers
-    h1.clear();
-    h2.clear();
-    h1.append(n1, v1);
-    h2.append(n1, v1);
-    h2.append(n2, v2);
-    QVERIFY(!h1.equals(h2));
-    QVERIFY(!h1.equals(h2, QHttpHeaders::CompareOption::OrderSensitive));
-}
-
 void tst_QHttpHeaders::constructors()
 {
     // Default ctor
@@ -109,38 +43,44 @@ void tst_QHttpHeaders::constructors()
 
     // Copy ctor
     QHttpHeaders h2(h1);
-    QVERIFY(h2.equals(h1));
+    QCOMPARE(h2.toListOfPairs(), h1.toListOfPairs());
 
     // Copy assignment
     QHttpHeaders h3;
     h3 = h1;
-    QVERIFY(h3.equals(h1));
+    QCOMPARE(h3.toListOfPairs(), h1.toListOfPairs());
 
     // Move assignment
     QHttpHeaders h4;
     h4 = std::move(h2);
-    QVERIFY(h4.equals(h1));
+    QCOMPARE(h4.toListOfPairs(), h1.toListOfPairs());
 
     // Move ctor
     QHttpHeaders h5(std::move(h4));
-    QVERIFY(h5.equals(h1));
+    QCOMPARE(h5.toListOfPairs(), h1.toListOfPairs());
 
     // Constructors that are counterparts to 'toXXX()' conversion getters
     const QByteArray nb1{"name1"};
     const QByteArray nb2{"name2"};
     const QByteArray nv1{"value1"};
     const QByteArray nv2{"value2"};
-    // Initialize three QHttpHeaders with same content and verify they match
+    // Initialize three QHttpHeaders with similar content, and verify that they have
+    // similar header entries
+#define CONTAINS_HEADER(NAME, VALUE) \
+    QVERIFY(hlist.contains(NAME) && hmap.contains(NAME) && hhash.contains(NAME)); \
+    QCOMPARE(hlist.combinedValue(NAME), VALUE); \
+    QCOMPARE(hmap.combinedValue(NAME), VALUE);  \
+    QCOMPARE(hhash.combinedValue(NAME), VALUE); \
+
     QList<std::pair<QByteArray, QByteArray>> list{{nb1, nv1}, {nb2, nv2}, {nb2, nv2}};
     QMultiMap<QByteArray, QByteArray> map{{nb1, nv1}, {nb2, nv2}, {nb2, nv2}};
     QMultiHash<QByteArray, QByteArray> hash{{nb1, nv1}, {nb2, nv2}, {nb2, nv2}};
     QHttpHeaders hlist = QHttpHeaders::fromListOfPairs(list);
     QHttpHeaders hmap = QHttpHeaders::fromMultiMap(map);
     QHttpHeaders hhash = QHttpHeaders::fromMultiHash(hash);
-    QVERIFY(hlist.contains(nb1) && hmap.contains(nb1) && hhash.contains(nb1));
+    CONTAINS_HEADER(nb1, v1);
-    QVERIFY(!hlist.contains(n3) && !hmap.contains(n3) && !hhash.contains(n3));
+    CONTAINS_HEADER(nb2, nv2 + "," + nv2)
-    QVERIFY(hlist.equals(hmap));
+#undef CONTAINS_HEADER
-    QVERIFY(hmap.equals(hhash));
 }
 
 void tst_QHttpHeaders::accessors()

tests/auto/network/access/qnetworkrequestfactory/tst_qnetworkrequestfactory.cpp

@@ -157,7 +157,6 @@ void tst_QNetworkRequestFactory::sslConfiguration()
     // Two initially equal factories
     QNetworkRequestFactory factory1{url1};
     QNetworkRequestFactory factory2{url1};
-    QCOMPARE(factory1, factory2);
 
     // Make two differing SSL configurations (for this test it's irrelevant how they differ)
     QSslConfiguration config1;
@@ -171,9 +170,6 @@ void tst_QNetworkRequestFactory::sslConfiguration()
     factory2.setSslConfiguration(config2);
     QCOMPARE(factory2.sslConfiguration(), config2);
 
-    // Verify that the factories differ (different SSL config)
-    QCOMPARE_NE(factory1, factory2);
-
     // Verify requests are set with appropriate SSL configs
     QNetworkRequest request1 = factory1.createRequest();
     QCOMPARE(request1.sslConfiguration(), config1);
@@ -310,13 +306,9 @@ void tst_QNetworkRequestFactory::operators()
     QCOMPARE(factory1.baseUrl(), url2); // changed
     QCOMPARE(factory2.baseUrl(), url1); // remains
 
-    // Comparison
-    QVERIFY(factory2 == factory4); // factory4 was copied + moved, and originates from factory2
-    QVERIFY(factory1 != factory2); // factory1 url was changed
-
     // Move ctor
     QNetworkRequestFactory factory5{std::move(factory4)};
-    QVERIFY(factory5 == factory2); // the moved factory4 originates from factory2
+    QCOMPARE(factory5.baseUrl(), factory2.baseUrl()); // the moved factory4 originates from factory2
     QCOMPARE(factory5.baseUrl(), url1);
 }