Improve the API of QCoapOption

 - The RFC specifies the following option formats: empty, opaque, uint
   and string. Leave only the construcors for the specified types.
 - Rename value getters to reflect the expected value type.
 - Move the internally used setter methods to the private class.

This change is based on the feedback from API review.

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

4 files changed, 34 insertions, 42 deletions

diff --git a/tests/auto/qcoapinternalreply/tst_qcoapinternalreply.cpp b/tests/auto/qcoapinternalreply/tst_qcoapinternalreply.cpp
index 7915f44..f9f46bd 100644
--- a/tests/auto/qcoapinternalreply/tst_qcoapinternalreply.cpp
+++ b/tests/auto/qcoapinternalreply/tst_qcoapinternalreply.cpp
@@ -160,7 +160,7 @@ void tst_QCoapInternalReply::parseReplyPdu()
         QCoapOption option = reply->message()->optionAt(i);
         QCOMPARE(option.name(), optionsNames.at(i));
         QCOMPARE(option.length(), optionsLengths.at(i));
-        QCOMPARE(option.value(), optionsValues.at(i));
+        QCOMPARE(option.opaqueValue(), optionsValues.at(i));
     }
     QCOMPARE(reply->message()->payload(), payload);
 }
diff --git a/tests/auto/qcoapinternalrequest/tst_qcoapinternalrequest.cpp b/tests/auto/qcoapinternalrequest/tst_qcoapinternalrequest.cpp
index 1a1a293..d3e367b 100644
--- a/tests/auto/qcoapinternalrequest/tst_qcoapinternalrequest.cpp
+++ b/tests/auto/qcoapinternalrequest/tst_qcoapinternalrequest.cpp
@@ -177,33 +177,34 @@ void tst_QCoapInternalRequest::parseUri_data()
                         << QUrl()
                         << QVector<QCoapOption>({
                             QCoapOption(QCoapOption::UriPort, 1234),
-                            QCoapOption(QCoapOption::UriPath, "test"),
-                            QCoapOption(QCoapOption::UriPath, "path1") });
+                            QCoapOption(QCoapOption::UriPath, QByteArray("test")),
+                            QCoapOption(QCoapOption::UriPath, QByteArray("path1")) });
 
     QTest::newRow("path_query")
                         << QUrl("coap://10.20.30.40/test/path1/?rd=25&nd=4")
                         << QUrl()
                         << QVector<QCoapOption>({
-                            QCoapOption(QCoapOption::UriPath, "test"),
-                            QCoapOption(QCoapOption::UriPath, "path1"),
-                            QCoapOption(QCoapOption::UriQuery, "rd=25"),
-                            QCoapOption(QCoapOption::UriQuery, "nd=4") });
+                            QCoapOption(QCoapOption::UriPath, QByteArray("test")),
+                            QCoapOption(QCoapOption::UriPath, QByteArray("path1")),
+                            QCoapOption(QCoapOption::UriQuery, QByteArray("rd=25")),
+                            QCoapOption(QCoapOption::UriQuery, QByteArray("nd=4")) });
 
     QTest::newRow("host_path_query")
                         << QUrl("coap://aa.bb.cc.com:5683/test/path1/?rd=25&nd=4")
                         << QUrl()
                         << QVector<QCoapOption>({
-                            QCoapOption(QCoapOption::UriHost, "aa.bb.cc.com"),
-                            QCoapOption(QCoapOption::UriPath, "test"),
-                            QCoapOption(QCoapOption::UriPath, "path1"),
-                            QCoapOption(QCoapOption::UriQuery, "rd=25"),
-                            QCoapOption(QCoapOption::UriQuery, "nd=4") });
+                            QCoapOption(QCoapOption::UriHost, QByteArray("aa.bb.cc.com")),
+                            QCoapOption(QCoapOption::UriPath, QByteArray("test")),
+                            QCoapOption(QCoapOption::UriPath, QByteArray("path1")),
+                            QCoapOption(QCoapOption::UriQuery, QByteArray("rd=25")),
+                            QCoapOption(QCoapOption::UriQuery, QByteArray("nd=4")) });
 
     QTest::newRow("proxy_url")
                         << QUrl("coap://aa.bb.cc.com:5683/test/path1/?rd=25&nd=4")
                         << QUrl("coap://10.20.30.40/test:5684/othertest/path")
                         << QVector<QCoapOption>({
-                            QCoapOption(QCoapOption::ProxyUri, "coap://10.20.30.40/test:5684/othertest/path") });
+                            QCoapOption(QCoapOption::ProxyUri,
+                            QByteArray("coap://10.20.30.40/test:5684/othertest/path")) });
 }
 
 void tst_QCoapInternalRequest::parseUri()
@@ -227,9 +228,9 @@ void tst_QCoapInternalRequest::urlOptions_data()
     QTest::addColumn<QVector<QCoapOption>>("options");
 
     QVector<QCoapOption> options = {
-        { QCoapOption::UriHost, "example.com" },
-        { QCoapOption::UriPath, "~sensors" },
-        { QCoapOption::UriPath, "temp.xml" }
+        { QCoapOption::UriHost, QByteArray("example.com") },
+        { QCoapOption::UriPath, QByteArray("~sensors") },
+        { QCoapOption::UriPath, QByteArray("temp.xml") }
     };
 
     QTest::newRow("url_with_default_port")
diff --git a/tests/auto/qcoapmessage/tst_qcoapmessage.cpp b/tests/auto/qcoapmessage/tst_qcoapmessage.cpp
index e13281e..56579e5 100644
--- a/tests/auto/qcoapmessage/tst_qcoapmessage.cpp
+++ b/tests/auto/qcoapmessage/tst_qcoapmessage.cpp
@@ -123,7 +123,7 @@ void tst_QCoapMessage::addOption()
 
     QVERIFY(std::all_of(message.options().cbegin(), message.options().cend(),
                         [value](const QCoapOption opt) -> bool {
-                            return opt.value() == value;
+                            return opt.opaqueValue() == value;
            }));
 }
 
@@ -131,7 +131,7 @@ void tst_QCoapMessage::addOption_string_data()
 {
     QTest::addColumn<QVector<QCoapOption>>("options");
 
-    QVector<QCoapOption> single_char_option = { { QCoapOption::LocationPath, "path1" } };
+    QVector<QCoapOption> single_string_option = { { QCoapOption::LocationPath, QString("path1") } };
     QVector<QCoapOption> single_ba_option = {
         { QCoapOption::LocationPath, QByteArray("\xAF\x01\xC2") }
     };
@@ -140,7 +140,7 @@ void tst_QCoapMessage::addOption_string_data()
         { QCoapOption::LocationPath, QString("str_path3") }
     };
 
-    QTest::newRow("single_char_option") << single_char_option;
+    QTest::newRow("single_char_option") << single_string_option;
     QTest::newRow("single_ba_option") << single_ba_option;
     QTest::newRow("multiple_string_options") << multiple_string_options;
 }
@@ -184,18 +184,18 @@ void tst_QCoapMessage::addOption_uint()
     message.addOption(option);
 
     QCOMPARE(message.options(name).size(), 1);
-    QCOMPARE(message.option(name).valueToInt(), value);
-    QCOMPARE(option.value().size(), size);
+    QCOMPARE(message.option(name).uintValue(), value);
+    QCOMPARE(option.opaqueValue().size(), size);
 }
 
 void tst_QCoapMessage::removeOption_data()
 {
     QTest::addColumn<QVector<QCoapOption>>("options");
 
-    QVector<QCoapOption> single_option = { { QCoapOption::LocationPath, "path1" } };
+    QVector<QCoapOption> single_option = { { QCoapOption::LocationPath, QByteArray("path1") } };
     QVector<QCoapOption> multiple_options = {
-        { QCoapOption::LocationPath, "path2" },
-        { QCoapOption::LocationPath, "path3" }
+        { QCoapOption::LocationPath, QByteArray("path2") },
+        { QCoapOption::LocationPath, QByteArray("path3") }
     };
 
     QTest::newRow("single_option") << single_option;
@@ -227,10 +227,10 @@ void tst_QCoapMessage::removeOptionByName_data()
     QTest::addColumn<QVector<QCoapOption>>("options");
     QTest::addColumn<QCoapOption::OptionName>("name");
 
-    QVector<QCoapOption> single_option = { { QCoapOption::LocationPath, "path1" } };
+    QVector<QCoapOption> single_option = { { QCoapOption::LocationPath, QByteArray("path1") } };
     QVector<QCoapOption> multiple_options = {
-        { QCoapOption::LocationPath, "path2" },
-        { QCoapOption::LocationPath, "path3" }
+        { QCoapOption::LocationPath, QByteArray("path2") },
+        { QCoapOption::LocationPath, QByteArray("path3") }
     };
 
     QTest::newRow("remove_single_option") << single_option << single_option.back().name();
diff --git a/tests/auto/qcoapoption/tst_qcoapoption.cpp b/tests/auto/qcoapoption/tst_qcoapoption.cpp
index 2541fad..fec22b1 100644
--- a/tests/auto/qcoapoption/tst_qcoapoption.cpp
+++ b/tests/auto/qcoapoption/tst_qcoapoption.cpp
@@ -38,8 +38,7 @@ class tst_QCoapOption : public QObject
 
 private Q_SLOTS:
     void constructWithQByteArray();
-    void constructWithQStringView();
-    void constructWithCString();
+    void constructWithQString();
     void constructWithInteger();
     void constructWithUtf8Characters();
 };
@@ -49,23 +48,15 @@ void tst_QCoapOption::constructWithQByteArray()
     QByteArray ba = "some data";
     QCoapOption option(QCoapOption::LocationPath, ba);
 
-    QCOMPARE(option.value(), ba);
+    QCOMPARE(option.opaqueValue(), ba);
 }
 
-void tst_QCoapOption::constructWithQStringView()
+void tst_QCoapOption::constructWithQString()
 {
     QString str = "some data";
     QCoapOption option(QCoapOption::LocationPath, str);
 
-    QCOMPARE(option.value(), str.toUtf8());
-}
-
-void tst_QCoapOption::constructWithCString()
-{
-    const char *str = "some data";
-    QCoapOption option(QCoapOption::LocationPath, str);
-
-    QCOMPARE(option.value(), QByteArray(str));
+    QCOMPARE(option.opaqueValue(), str.toUtf8());
 }
 
 void tst_QCoapOption::constructWithInteger()
@@ -73,7 +64,7 @@ void tst_QCoapOption::constructWithInteger()
     quint32 value = 64000;
     QCoapOption option(QCoapOption::Size1, value);
 
-    QCOMPARE(option.valueToInt(), value);
+    QCOMPARE(option.uintValue(), value);
 }
 
 void tst_QCoapOption::constructWithUtf8Characters()
@@ -81,7 +72,7 @@ void tst_QCoapOption::constructWithUtf8Characters()
     QByteArray ba = "\xc3\xa9~\xce\xbb\xe2\x82\xb2";
     QCoapOption option(QCoapOption::LocationPath, ba);
 
-    QCOMPARE(option.value(), ba);
+    QCOMPARE(option.opaqueValue(), ba);
 }
 
 QTEST_APPLESS_MAIN(tst_QCoapOption)