Forbid creation of QCoapReply and QCoapResourceDiscoveryReply objects

Replies are created internally, and pointers to created objects are
returned to the user. Users should not be able to create instances of
QCoapReply or QCoapResourceDiscoveryReply.

This change is based on the feedback from API review.

Change-Id: Ibd59c62fd19c4b312253d4c4621b195bf5816340
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>

8 files changed, 58 insertions, 103 deletions

diff --git a/src/coap/qcoapclient.cpp b/src/coap/qcoapclient.cpp
index af09686..372128a 100644
--- a/src/coap/qcoapclient.cpp
+++ b/src/coap/qcoapclient.cpp
@@ -36,6 +36,7 @@
 #include "qcoapsecurityconfiguration.h"
 #include "qcoapqudpconnection_p.h"
 #include "qcoaprequest_p.h"
+#include "qcoapreply_p.h"
 #include <QtCore/qiodevice.h>
 #include <QtCore/qurl.h>
 #include <QtCore/qloggingcategory.h>
@@ -514,7 +515,7 @@ QCoapReply *QCoapClientPrivate::sendRequest(const QCoapRequest &request)
     Q_Q(QCoapClient);
 
     // Prepare the reply
-    QCoapReply *reply = new QCoapReply(request, q);
+    QCoapReply *reply = QCoapReplyPrivate::createCoapReply(request, q);
 
     if (!send(reply)) {
         delete reply;
diff --git a/src/coap/qcoapreply.cpp b/src/coap/qcoapreply.cpp
index 7d6ba40..2880fd6 100644
--- a/src/coap/qcoapreply.cpp
+++ b/src/coap/qcoapreply.cpp
@@ -243,15 +243,6 @@ void QCoapReplyPrivate::_q_setError(QtCoap::ResponseCode code)
 */
 
 /*!
-    Constructs a new CoAP reply for the \a request and sets \a parent as
-    its parent.
-*/
-QCoapReply::QCoapReply(const QCoapRequest &request, QObject *parent) :
-    QCoapReply(*new QCoapReplyPrivate(request), parent)
-{
-}
-
-/*!
     \internal
     Constructs a new CoAP reply with \a dd as the d_ptr.
     This constructor must be used when subclassing internally
@@ -427,6 +418,16 @@ void QCoapReply::abortRequest()
     emit finished(this);
 }
 
+/*!
+    \internal
+
+    Creates a new instance of QCoapReply and returns a pointer to it.
+*/
+QCoapReply *QCoapReplyPrivate::createCoapReply(const QCoapRequest &request, QObject *parent)
+{
+    return new QCoapReply(*new QCoapReplyPrivate(request), parent);
+}
+
 QT_END_NAMESPACE
 
 #include "moc_qcoapreply.cpp"
diff --git a/src/coap/qcoapreply.h b/src/coap/qcoapreply.h
index afdd5ea..019fba5 100644
--- a/src/coap/qcoapreply.h
+++ b/src/coap/qcoapreply.h
@@ -45,8 +45,6 @@ class Q_COAP_EXPORT QCoapReply : public QIODevice
 {
     Q_OBJECT
 public:
-
-    explicit QCoapReply(const QCoapRequest &request, QObject *parent = nullptr);
     ~QCoapReply() override;
 
     QtCoap::ResponseCode responseCode() const;
@@ -68,11 +66,6 @@ Q_SIGNALS:
     void aborted(const QCoapToken &token);
 
 protected:
-    friend class QCoapProtocol;
-    friend class QCoapProtocolPrivate;
-
-    explicit QCoapReply(QCoapReplyPrivate &dd, QObject *parent = nullptr);
-
     qint64 readData(char *data, qint64 maxSize) override;
     qint64 writeData(const char *data, qint64 maxSize) override;
 
@@ -85,6 +78,10 @@ protected:
     Q_PRIVATE_SLOT(d_func(), void _q_setFinished(QtCoap::Error))
     Q_PRIVATE_SLOT(d_func(), void _q_setError(QtCoap::ResponseCode))
     Q_PRIVATE_SLOT(d_func(), void _q_setError(QtCoap::Error))
+
+private:
+    explicit QCoapReply(QCoapReplyPrivate &dd, QObject *parent = nullptr);
+    friend class QCoapResourceDiscoveryReply;
 };
 
 QT_END_NAMESPACE
diff --git a/src/coap/qcoapreply_p.h b/src/coap/qcoapreply_p.h
index 68e9062..500c1dd 100644
--- a/src/coap/qcoapreply_p.h
+++ b/src/coap/qcoapreply_p.h
@@ -62,6 +62,8 @@ public:
     void _q_setError(QtCoap::ResponseCode code);
     void _q_setError(QtCoap::Error);
 
+    static QCoapReply *createCoapReply(const QCoapRequest &request, QObject *parent = nullptr);
+
     QCoapRequest request;
     QCoapMessage message;
     QtCoap::ResponseCode responseCode = QtCoap::ResponseCode::InvalidCode;
diff --git a/src/coap/qcoapresourcediscoveryreply.cpp b/src/coap/qcoapresourcediscoveryreply.cpp
index dabdeb6..c709331 100644
--- a/src/coap/qcoapresourcediscoveryreply.cpp
+++ b/src/coap/qcoapresourcediscoveryreply.cpp
@@ -100,6 +100,8 @@ void QCoapResourceDiscoveryReplyPrivate::_q_setContent(const QHostAddress &sende
 */
 
 /*!
+    \internal
+
     Constructs a new CoAP discovery reply from the \a request and sets \a parent
     as its parent.
 */
diff --git a/src/coap/qcoapresourcediscoveryreply.h b/src/coap/qcoapresourcediscoveryreply.h
index 65f386d..bd575e0 100644
--- a/src/coap/qcoapresourcediscoveryreply.h
+++ b/src/coap/qcoapresourcediscoveryreply.h
@@ -43,14 +43,15 @@ class Q_COAP_EXPORT QCoapResourceDiscoveryReply : public QCoapReply
     Q_OBJECT
 
 public:
-    explicit QCoapResourceDiscoveryReply(const QCoapRequest &request, QObject *parent = nullptr);
-
     QVector<QCoapResource> resources() const;
 
 Q_SIGNALS:
     void discovered(QCoapResourceDiscoveryReply *reply, QVector<QCoapResource> resources);
 
 private:
+    explicit QCoapResourceDiscoveryReply(const QCoapRequest &request, QObject *parent = nullptr);
+    friend class QCoapClientPrivate;
+
     Q_DECLARE_PRIVATE(QCoapResourceDiscoveryReply)
 };
 
diff --git a/tests/auto/qcoapinternalreply/tst_qcoapinternalreply.cpp b/tests/auto/qcoapinternalreply/tst_qcoapinternalreply.cpp
index 61a3dbc..7915f44 100644
--- a/tests/auto/qcoapinternalreply/tst_qcoapinternalreply.cpp
+++ b/tests/auto/qcoapinternalreply/tst_qcoapinternalreply.cpp
@@ -45,8 +45,6 @@ private Q_SLOTS:
     void parseReplyPdu();
     void updateReply_data();
     void updateReply();
-    void requestData();
-    void abortRequest();
 };
 
 void tst_QCoapInternalReply::parseReplyPdu_data()
@@ -167,24 +165,6 @@ void tst_QCoapInternalReply::parseReplyPdu()
     QCOMPARE(reply->message()->payload(), payload);
 }
 
-class QCoapReplyForTests : public QCoapReply
-{
-public:
-    QCoapReplyForTests(const QCoapRequest &req) : QCoapReply (req) {}
-
-    void setRunning(const QCoapToken &token, QCoapMessageId messageId)
-    {
-        Q_D(QCoapReply);
-        d->_q_setRunning(token, messageId);
-    }
-    void setContentAndFinished(const QCoapInternalReply *internal)
-    {
-        Q_D(QCoapReply);
-        d->_q_setContent(internal->senderAddress(), *internal->message(), internal->responseCode());
-        d->_q_setFinished();
-    }
-};
-
 void tst_QCoapInternalReply::updateReply_data()
 {
     QTest::addColumn<QByteArray>("data");
@@ -196,39 +176,19 @@ void tst_QCoapInternalReply::updateReply()
 {
     QFETCH(QByteArray, data);
 
-    QCoapReplyForTests reply((QCoapRequest()));
+    QScopedPointer<QCoapReply> reply(QCoapReplyPrivate::createCoapReply(QCoapRequest()));
     QCoapInternalReply internalReply;
     internalReply.message()->setPayload(data);
-    QSignalSpy spyReplyFinished(&reply, &QCoapReply::finished);
+    QSignalSpy spyReplyFinished(reply.data(), &QCoapReply::finished);
 
-    reply.setContentAndFinished(&internalReply);
+    QMetaObject::invokeMethod(reply.data(), "_q_setContent",
+                              Q_ARG(QHostAddress, internalReply.senderAddress()),
+                              Q_ARG(QCoapMessage, *internalReply.message()),
+                              Q_ARG(QtCoap::ResponseCode, internalReply.responseCode()));
+    QMetaObject::invokeMethod(reply.data(), "_q_setFinished", Q_ARG(QtCoap::Error, QtCoap::Error::Ok));
 
     QTRY_COMPARE_WITH_TIMEOUT(spyReplyFinished.count(), 1, 1000);
-    QCOMPARE(reply.readAll(), data);
-}
-
-void tst_QCoapInternalReply::requestData()
-{
-    QCoapReplyForTests reply((QCoapRequest()));
-    reply.setRunning("token", 543);
-
-    QCOMPARE(reply.request().token(), QByteArray("token"));
-    QCOMPARE(reply.request().messageId(), 543);
-}
-
-void tst_QCoapInternalReply::abortRequest()
-{
-    QCoapReplyForTests reply((QCoapRequest()));
-    reply.setRunning("token", 543);
-
-    QSignalSpy spyAborted(&reply, &QCoapReply::aborted);
-    QSignalSpy spyFinished(&reply, &QCoapReply::finished);
-    reply.abortRequest();
-
-    QTRY_COMPARE_WITH_TIMEOUT(spyAborted.count(), 1, 1000);
-    QList<QVariant> arguments = spyAborted.takeFirst();
-    QTRY_COMPARE_WITH_TIMEOUT(spyFinished.count(), 1, 1000);
-    QVERIFY(arguments.at(0).toByteArray() == "token");
+    QCOMPARE(reply->readAll(), data);
 }
 
 #else
diff --git a/tests/auto/qcoapreply/tst_qcoapreply.cpp b/tests/auto/qcoapreply/tst_qcoapreply.cpp
index f22098e..f893840 100644
--- a/tests/auto/qcoapreply/tst_qcoapreply.cpp
+++ b/tests/auto/qcoapreply/tst_qcoapreply.cpp
@@ -48,18 +48,6 @@ private Q_SLOTS:
     void abortRequest();
 };
 
-class QCoapReplyForTests : public QCoapReply
-{
-public:
-    QCoapReplyForTests(const QCoapRequest &req) : QCoapReply (req) {}
-
-    void setRunning(const QCoapToken &token, QCoapMessageId messageId)
-    {
-        Q_D(QCoapReply);
-        d->_q_setRunning(token, messageId);
-    }
-};
-
 void tst_QCoapReply::updateReply_data()
 {
     QTest::addColumn<QByteArray>("payload");
@@ -93,22 +81,22 @@ void tst_QCoapReply::updateReply()
     const QByteArray token = "\xAF\x01\xC2";
     const quint16 id = 645;
 
-    QCoapReply reply(QCoapRequest{});
+    QScopedPointer<QCoapReply> reply(QCoapReplyPrivate::createCoapReply(QCoapRequest()));
     QCoapMessage message;
     message.setToken(token);
     message.setMessageId(id);
     message.setPayload(payload);
 
-    QSignalSpy spyReplyFinished(&reply, &QCoapReply::finished);
-    QSignalSpy spyReplyNotified(&reply, &QCoapReply::notified);
-    QSignalSpy spyReplyError(&reply, &QCoapReply::error);
-    QSignalSpy spyReplyAborted(&reply, &QCoapReply::aborted);
+    QSignalSpy spyReplyFinished(reply.data(), &QCoapReply::finished);
+    QSignalSpy spyReplyNotified(reply.data(), &QCoapReply::notified);
+    QSignalSpy spyReplyError(reply.data(), &QCoapReply::error);
+    QSignalSpy spyReplyAborted(reply.data(), &QCoapReply::aborted);
 
-    QMetaObject::invokeMethod(&reply, "_q_setContent",
+    QMetaObject::invokeMethod(reply.data(), "_q_setContent",
                               Q_ARG(QHostAddress, QHostAddress()),
                               Q_ARG(QCoapMessage, message),
                               Q_ARG(QtCoap::ResponseCode, responseCode));
-    QMetaObject::invokeMethod(&reply, "_q_setFinished",
+    QMetaObject::invokeMethod(reply.data(), "_q_setFinished",
                               Q_ARG(QtCoap::Error, error));
 
     QCOMPARE(spyReplyFinished.count(), 1);
@@ -116,42 +104,45 @@ void tst_QCoapReply::updateReply()
     QCOMPARE(spyReplyAborted.count(), 0);
     if (error != QtCoap::Error::Ok || QtCoap::isError(responseCode)) {
         QVERIFY(spyReplyError.count() > 0);
-        QCOMPARE(reply.isSuccessful(), false);
+        QCOMPARE(reply->isSuccessful(), false);
     } else {
         QCOMPARE(spyReplyError.count(), 0);
-        QCOMPARE(reply.isSuccessful(), true);
+        QCOMPARE(reply->isSuccessful(), true);
     }
 
-    QCOMPARE(reply.readAll(), payload);
-    QCOMPARE(reply.readAll(), QByteArray());
-    QCOMPARE(reply.responseCode(), responseCode);
-    QCOMPARE(reply.message().token(), token);
-    QCOMPARE(reply.message().messageId(), id);
+    QCOMPARE(reply->readAll(), payload);
+    QCOMPARE(reply->readAll(), QByteArray());
+    QCOMPARE(reply->responseCode(), responseCode);
+    QCOMPARE(reply->message().token(), token);
+    QCOMPARE(reply->message().messageId(), id);
 }
 
 void tst_QCoapReply::requestData()
 {
-    QCoapReplyForTests reply((QCoapRequest()));
-    reply.setRunning("token", 543);
+    QScopedPointer<QCoapReply> reply(QCoapReplyPrivate::createCoapReply(QCoapRequest()));
+    QMetaObject::invokeMethod(reply.data(), "_q_setRunning",
+                              Q_ARG(QCoapToken, "token"),
+                              Q_ARG(QCoapMessageId, 543));
 
-    QCOMPARE(reply.request().token(), QByteArray("token"));
-    QCOMPARE(reply.request().messageId(), 543);
+    QCOMPARE(reply->request().token(), QByteArray("token"));
+    QCOMPARE(reply->request().messageId(), 543);
 }
 
 void tst_QCoapReply::abortRequest()
 {
-    QCoapReplyForTests reply((QCoapRequest()));
-    reply.setRunning("token", 543);
+    QScopedPointer<QCoapReply> reply(QCoapReplyPrivate::createCoapReply(QCoapRequest()));
+    QMetaObject::invokeMethod(reply.data(), "_q_setRunning",
+                              Q_ARG(QCoapToken, "token"),
+                              Q_ARG(QCoapMessageId, 543));
 
-    QSignalSpy spyAborted(&reply, &QCoapReply::aborted);
-    QSignalSpy spyFinished(&reply, &QCoapReply::finished);
-    reply.abortRequest();
+    QSignalSpy spyAborted(reply.data(), &QCoapReply::aborted);
+    QSignalSpy spyFinished(reply.data(), &QCoapReply::finished);
+    reply->abortRequest();
 
     QTRY_COMPARE_WITH_TIMEOUT(spyAborted.count(), 1, 1000);
     QList<QVariant> arguments = spyAborted.takeFirst();
     QTRY_COMPARE_WITH_TIMEOUT(spyFinished.count(), 1, 1000);
     QVERIFY(arguments.at(0).toByteArray() == "token");
-    QCOMPARE(reply.isSuccessful(), false);
 }
 
 #else