Fix requirements

Query and fragment may be empty but present in a URI.
Handle this in the same way that port is handled: include
a flag for each of query and fragment, to allow an
empty but present query/fragment.

3 files changed, 139 insertions, 30 deletions

diff --git a/include/Uri/Uri.hpp b/include/Uri/Uri.hpp
index 7074990..f531d09 100644
--- a/include/Uri/Uri.hpp
+++ b/include/Uri/Uri.hpp
@@ -164,6 +164,16 @@ namespace Uri {
         bool ContainsRelativePath() const;
 
         /**
+         * This method returns an indication of whether or not the
+         * URI includes a query.
+         *
+         * @return
+         *     An indication of whether or not the
+         *     URI includes a query is returned.
+         */
+        bool HasQuery() const;
+
+        /**
          * This method returns the "query" element of the URI,
          * if it has one.
          *
@@ -176,6 +186,16 @@ namespace Uri {
         std::string GetQuery() const;
 
         /**
+         * This method returns an indication of whether or not the
+         * URI includes a fragment.
+         *
+         * @return
+         *     An indication of whether or not the
+         *     URI includes a fragment is returned.
+         */
+        bool HasFragment() const;
+
+        /**
          * This method returns the "fragment" element of the URI,
          * if it has one.
          *
@@ -267,6 +287,11 @@ namespace Uri {
         void SetPath(const std::vector< std::string >& path);
 
         /**
+         * This method removes the query element from the URI.
+         */
+        void ClearQuery();
+
+        /**
          * This method sets the query element of the URI.
          *
          * @param[in] query
@@ -275,6 +300,11 @@ namespace Uri {
         void SetQuery(const std::string& query);
 
         /**
+         * This method removes the fragment element from the URI.
+         */
+        void ClearFragment();
+
+        /**
          * This method sets the fragment element of the URI.
          *
          * @param[in] fragment
diff --git a/src/Uri.cpp b/src/Uri.cpp
index eee40ad..4e3cba0 100644
--- a/src/Uri.cpp
+++ b/src/Uri.cpp
@@ -542,12 +542,24 @@ namespace Uri {
         std::vector< std::string > path;
 
         /**
+         * This flag indicates whether or not the
+         * URI includes a query.
+         */
+        bool hasQuery = false;
+
+        /**
          * This is the "query" element of the URI,
          * if it has one.
          */
         std::string query;
 
         /**
+         * This flag indicates whether or not the
+         * URI includes a fragment.
+         */
+        bool hasFragment = false;
+
+        /**
          * This is the "fragment" element of the URI,
          * if it has one.
          */
@@ -910,10 +922,11 @@ namespace Uri {
          *     is returned.
          */
         bool ParseQuery(const std::string& queryWithDelimiter) {
-            if (queryWithDelimiter.empty()) {
-                query.clear();
-            } else {
+            hasQuery = !queryWithDelimiter.empty();
+            if (hasQuery) {
                 query = queryWithDelimiter.substr(1);
+            } else {
+                query.clear();
             }
             return DecodeQueryOrFragment(query);
         }
@@ -943,9 +956,11 @@ namespace Uri {
         ) {
             const auto fragmentDelimiter = queryAndOrFragment.find('#');
             if (fragmentDelimiter == std::string::npos) {
+                hasFragment = false;
                 fragment.clear();
                 rest = queryAndOrFragment;
             } else {
+                hasFragment = true;
                 fragment = queryAndOrFragment.substr(fragmentDelimiter + 1);
                 rest = queryAndOrFragment.substr(0, fragmentDelimiter);
             }
@@ -1199,10 +1214,18 @@ namespace Uri {
         return !impl_->IsPathAbsolute();
     }
 
+    bool Uri::HasQuery() const {
+        return impl_->hasQuery;
+    }
+
     std::string Uri::GetQuery() const {
         return impl_->query;
     }
 
+    bool Uri::HasFragment() const {
+        return impl_->hasFragment;
+    }
+
     std::string Uri::GetFragment() const {
         return impl_->fragment;
     }
@@ -1288,12 +1311,22 @@ namespace Uri {
         impl_->path = path;
     }
 
+    void Uri::ClearQuery() {
+        impl_->hasQuery = false;
+    }
+
     void Uri::SetQuery(const std::string& query) {
         impl_->query = query;
+        impl_->hasQuery = true;
+    }
+
+    void Uri::ClearFragment() {
+        impl_->hasFragment = false;
     }
 
     void Uri::SetFragment(const std::string& fragment) {
         impl_->fragment = fragment;
+        impl_->hasFragment = true;
     }
 
     std::string Uri::GenerateString() const {
@@ -1332,10 +1365,10 @@ namespace Uri {
             }
             ++i;
         }
-        if (!impl_->query.empty()) {
+        if (impl_->hasQuery) {
             buffer << '?' << impl_->query;
         }
-        if (!impl_->fragment.empty()) {
+        if (impl_->hasFragment) {
             buffer << '#' << impl_->fragment;
         }
         return buffer.str();
diff --git a/test/src/UriTests.cpp b/test/src/UriTests.cpp
index 9dbff55..75fbba3 100644
--- a/test/src/UriTests.cpp
+++ b/test/src/UriTests.cpp
@@ -764,34 +764,38 @@ TEST(UriTests, GenerateString) {
         bool hasPort;
         uint16_t port;
         std::vector< std::string > path;
+        bool hasQuery;
         std::string query;
+        bool hasFragment;
         std::string fragment;
         std::string expectedUriString;
     };
     const std::vector< TestVector > testVectors{
-        {"http", "bob", "www.example.com", true,  8080, {"", "abc", "def"}, "foobar", "ch2", "http://bob@www.example.com:8080/abc/def?foobar#ch2"},
-        {"http", "bob", "www.example.com", true,  0,    {},                 "foobar", "ch2", "http://bob@www.example.com:0?foobar#ch2"},
-        {"",     "",    "example.com",     false, 0,    {},                 "bar",    "",    "//example.com?bar"},
-        {"",     "",    "example.com",     false, 0,    {},                 "",       "",    "//example.com"},
-        {"",     "",    "example.com",     false, 0,    {""},               "",       "",    "//example.com/"},
-        {"",     "",    "example.com",     false, 0,    {"", "xyz"},        "",       "",    "//example.com/xyz"},
-        {"",     "",    "example.com",     false, 0,    {"", "xyz", ""},    "",       "",    "//example.com/xyz/"},
-        {"",     "",    "",                false, 0,    {""},               "",       "",    "/"},
-        {"",     "",    "",                false, 0,    {"", "xyz"},        "",       "",    "/xyz"},
-        {"",     "",    "",                false, 0,    {"", "xyz", ""},    "",       "",    "/xyz/"},
-        {"",     "",    "",                false, 0,    {},                 "",       "",    ""},
-        {"",     "",    "",                false, 0,    {"xyz"},            "",       "",    "xyz"},
-        {"",     "",    "",                false, 0,    {"xyz", ""},        "",       "",    "xyz/"},
-        {"",     "",    "",                false, 0,    {},                 "bar",    "",    "?bar"},
-        {"http", "",    "",                false, 0,    {},                 "bar",    "",    "http:?bar"},
-        {"http", "",    "",                false, 0,    {},                 "",       "",    "http:"},
-        {"http", "",    "::1",             false, 0,    {},                 "",       "",    "http://[::1]"},
-        {"http", "",    "::1.2.3.4",       false, 0,    {},                 "",       "",    "http://[::1.2.3.4]"},
-        {"http", "",    "1.2.3.4",         false, 0,    {},                 "",       "",    "http://1.2.3.4"},
-        {"",     "",    "",                false, 0,    {},                 "",       "",    ""},
-        {"http", "bob", "",                false, 0,    {},                 "foobar", "",    "http://bob@?foobar"},
-        {"",     "bob", "",                false, 0,    {},                 "foobar", "",    "//bob@?foobar"},
-        {"",     "bob", "",                false, 0,    {},                 "",       "",    "//bob@"},
+        {"http", "bob", "www.example.com", true,  8080, {"", "abc", "def"}, true,  "foobar", true,  "ch2", "http://bob@www.example.com:8080/abc/def?foobar#ch2"},
+        {"http", "bob", "www.example.com", true,  0,    {},                 true,  "foobar", true,  "ch2", "http://bob@www.example.com:0?foobar#ch2"},
+        {"http", "bob", "www.example.com", true,  0,    {},                 true,  "foobar", true,  "",    "http://bob@www.example.com:0?foobar#"},
+        {"",     "",    "example.com",     false, 0,    {},                 true,  "bar",    false, "",    "//example.com?bar"},
+        {"",     "",    "example.com",     false, 0,    {},                 true,  ""   ,    false, "",    "//example.com?"},
+        {"",     "",    "example.com",     false, 0,    {},                 false, "",       false, "",    "//example.com"},
+        {"",     "",    "example.com",     false, 0,    {""},               false, "",       false, "",    "//example.com/"},
+        {"",     "",    "example.com",     false, 0,    {"", "xyz"},        false, "",       false, "",    "//example.com/xyz"},
+        {"",     "",    "example.com",     false, 0,    {"", "xyz", ""},    false, "",       false, "",    "//example.com/xyz/"},
+        {"",     "",    "",                false, 0,    {""},               false, "",       false, "",    "/"},
+        {"",     "",    "",                false, 0,    {"", "xyz"},        false, "",       false, "",    "/xyz"},
+        {"",     "",    "",                false, 0,    {"", "xyz", ""},    false, "",       false, "",    "/xyz/"},
+        {"",     "",    "",                false, 0,    {},                 false, "",       false, "",    ""},
+        {"",     "",    "",                false, 0,    {"xyz"},            false, "",       false, "",    "xyz"},
+        {"",     "",    "",                false, 0,    {"xyz", ""},        false, "",       false, "",    "xyz/"},
+        {"",     "",    "",                false, 0,    {},                 true,  "bar",    false, "",    "?bar"},
+        {"http", "",    "",                false, 0,    {},                 true, "bar",    false, "",    "http:?bar"},
+        {"http", "",    "",                false, 0,    {},                 false, "",       false, "",    "http:"},
+        {"http", "",    "::1",             false, 0,    {},                 false, "",       false, "",    "http://[::1]"},
+        {"http", "",    "::1.2.3.4",       false, 0,    {},                 false, "",       false, "",    "http://[::1.2.3.4]"},
+        {"http", "",    "1.2.3.4",         false, 0,    {},                 false, "",       false, "",    "http://1.2.3.4"},
+        {"",     "",    "",                false, 0,    {},                 false, "",       false, "",    ""},
+        {"http", "bob", "",                false, 0,    {},                 true,  "foobar", false, "",    "http://bob@?foobar"},
+        {"",     "bob", "",                false, 0,    {},                 true,  "foobar", false, "",    "//bob@?foobar"},
+        {"",     "bob", "",                false, 0,    {},                 false, "",       false, "",    "//bob@"},
     };
     size_t index = 0;
     for (const auto& testVector : testVectors) {
@@ -805,10 +809,52 @@ TEST(UriTests, GenerateString) {
             uri.ClearPort();
         }
         uri.SetPath(testVector.path);
-        uri.SetQuery(testVector.query);
-        uri.SetFragment(testVector.fragment);
+        if (testVector.hasQuery) {
+            uri.SetQuery(testVector.query);
+        } else {
+            uri.ClearQuery();
+        }
+        if (testVector.hasFragment) {
+            uri.SetFragment(testVector.fragment);
+        } else {
+            uri.ClearFragment();
+        }
         const auto actualUriString = uri.GenerateString();
         ASSERT_EQ(testVector.expectedUriString, actualUriString) << index;
         ++index;
     }
 }
+
+TEST(UriTests, FragmentEmptyButPresent) {
+    Uri::Uri uri;
+    ASSERT_TRUE(uri.ParseFromString("http://example.com#"));
+    ASSERT_TRUE(uri.HasFragment());
+    ASSERT_EQ("", uri.GetFragment());
+    ASSERT_EQ("http://example.com/#", uri.GenerateString());
+    uri.ClearFragment();
+    ASSERT_EQ("http://example.com/", uri.GenerateString());
+    ASSERT_FALSE(uri.HasFragment());
+    ASSERT_TRUE(uri.ParseFromString("http://example.com"));
+    ASSERT_FALSE(uri.HasFragment());
+    uri.SetFragment("");
+    ASSERT_TRUE(uri.HasFragment());
+    ASSERT_EQ("", uri.GetFragment());
+    ASSERT_EQ("http://example.com/#", uri.GenerateString());
+}
+
+TEST(UriTests, QueryEmptyButPresent) {
+    Uri::Uri uri;
+    ASSERT_TRUE(uri.ParseFromString("http://example.com?"));
+    ASSERT_TRUE(uri.HasQuery());
+    ASSERT_EQ("", uri.GetQuery());
+    ASSERT_EQ("http://example.com/?", uri.GenerateString());
+    uri.ClearQuery();
+    ASSERT_EQ("http://example.com/", uri.GenerateString());
+    ASSERT_FALSE(uri.HasQuery());
+    ASSERT_TRUE(uri.ParseFromString("http://example.com"));
+    ASSERT_FALSE(uri.HasQuery());
+    uri.SetQuery("");
+    ASSERT_TRUE(uri.HasQuery());
+    ASSERT_EQ("", uri.GetQuery());
+    ASSERT_EQ("http://example.com/?", uri.GenerateString());
+}