From 8bfe4c40412152dfdb8a707d1f66b3cda41f1f13 Mon Sep 17 00:00:00 2001 From: Kevin Ring Date: Mon, 11 Mar 2024 18:50:31 +1100 Subject: [PATCH 1/7] Add Uri::getPath and setPath. --- CesiumUtility/include/CesiumUtility/Uri.h | 20 +++++ CesiumUtility/src/Uri.cpp | 103 ++++++++++++++++++++++ CesiumUtility/test/TestUri.cpp | 58 ++++++++++++ 3 files changed, 181 insertions(+) create mode 100644 CesiumUtility/test/TestUri.cpp diff --git a/CesiumUtility/include/CesiumUtility/Uri.h b/CesiumUtility/include/CesiumUtility/Uri.h index 63b95a941..1f6ae94a8 100644 --- a/CesiumUtility/include/CesiumUtility/Uri.h +++ b/CesiumUtility/include/CesiumUtility/Uri.h @@ -24,5 +24,25 @@ class Uri final { const std::function& substitutionCallback); static std::string escape(const std::string& s); + + /** + * @brief Gets the path portion of the URI. + * + * @param uri The URI from which to get the path. + * @return The path, or empty string if the URI could not be parsed. + */ + static std::string getPath(const std::string& uri); + + /** + * @brief Sets the path portion of a URI to a new value. The other portions of + * the URI are left unmodified. + * + * @param uri The URI for which to set the path. + * @param The new path portion of the URI. + * @returns The new URI after setting the path. If the original URI cannot be + * parsed, it is returned unmodified. + */ + static std::string + setPath(const std::string& uri, const std::string& newPath); }; } // namespace CesiumUtility diff --git a/CesiumUtility/src/Uri.cpp b/CesiumUtility/src/Uri.cpp index c7e3679d4..f8847fcef 100644 --- a/CesiumUtility/src/Uri.cpp +++ b/CesiumUtility/src/Uri.cpp @@ -1,7 +1,10 @@ #include "CesiumUtility/Uri.h" +#include + #include +#include #include #include @@ -175,4 +178,104 @@ std::string Uri::escape(const std::string& s) { result.resize(size_t(pTerminator - result.data())); return result; } + +std::string Uri::getPath(const std::string& uri) { + UriUriA parsedUri; + if (uriParseSingleUriA(&parsedUri, uri.c_str(), nullptr) != URI_SUCCESS) { + // Could not parse the URI, so return an empty string. + return std::string(); + } + + // The initial string in this vector can be thought of as the "nothing" before + // the first slash in the path. + std::vector parts{std::string()}; + + UriPathSegmentA* pCurrent = parsedUri.pathHead; + while (pCurrent != nullptr) { + parts.emplace_back(std::string( + pCurrent->text.first, + size_t(pCurrent->text.afterLast - pCurrent->text.first))); + pCurrent = pCurrent->next; + } + + uriFreeUriMembersA(&parsedUri); + + return joinToString(parts, "/"); +} + +std::string Uri::setPath(const std::string& uri, const std::string& newPath) { + UriUriA parsedUri; + if (uriParseSingleUriA(&parsedUri, uri.c_str(), nullptr) != URI_SUCCESS) { + // Could not parse the URI, so return an empty string. + return std::string(); + } + + // Free the existing path. Strangely, uriparser doesn't provide any simple way + // to do this. + UriPathSegmentA* pCurrent = parsedUri.pathHead; + while (pCurrent != nullptr) { + UriPathSegmentA* pNext = pCurrent->next; + free(pCurrent); + pCurrent = pNext; + } + + parsedUri.pathHead = nullptr; + parsedUri.pathTail = nullptr; + + // Set the new path. + if (!newPath.empty()) { + std::string::size_type startPos = 0; + do { + std::string::size_type nextSlashIndex = newPath.find('/', startPos); + + // Skip the initial slash if there is one. + if (nextSlashIndex == 0) { + startPos = 1; + continue; + } + + UriPathSegmentA* pSegment = + static_cast(malloc(sizeof(UriPathSegmentA))); + memset(pSegment, 0, sizeof(UriPathSegmentA)); + + if (parsedUri.pathHead == nullptr) { + parsedUri.pathHead = pSegment; + parsedUri.pathTail = pSegment; + } else { + parsedUri.pathTail->next = pSegment; + parsedUri.pathTail = parsedUri.pathTail->next; + } + + pSegment->text.first = newPath.data() + startPos; + + if (nextSlashIndex != std::string::npos) { + pSegment->text.afterLast = newPath.data() + nextSlashIndex; + startPos = nextSlashIndex + 1; + } else { + pSegment->text.afterLast = newPath.data() + newPath.size(); + startPos = nextSlashIndex; + } + } while (startPos != std::string::npos); + } + + int charsRequired; + if (uriToStringCharsRequiredA(&parsedUri, &charsRequired) != URI_SUCCESS) { + uriFreeUriMembersA(&parsedUri); + return uri; + } + + std::string result(static_cast(charsRequired), ' '); + + if (uriToStringA( + const_cast(result.c_str()), + &parsedUri, + charsRequired + 1, + nullptr) != URI_SUCCESS) { + uriFreeUriMembersA(&parsedUri); + return uri; + } + + return result; +} + } // namespace CesiumUtility diff --git a/CesiumUtility/test/TestUri.cpp b/CesiumUtility/test/TestUri.cpp new file mode 100644 index 000000000..a2ba060ed --- /dev/null +++ b/CesiumUtility/test/TestUri.cpp @@ -0,0 +1,58 @@ +#include + +#include + +using namespace CesiumUtility; + +TEST_CASE("Uri::getPath") { + CHECK(Uri::getPath("https://example.com/") == "/"); + CHECK(Uri::getPath("https://example.com/foo/bar") == "/foo/bar"); + CHECK(Uri::getPath("https://example.com/foo/bar/") == "/foo/bar/"); + CHECK(Uri::getPath("https://example.com/?some=parameter") == "/"); + CHECK( + Uri::getPath("https://example.com/foo/bar?some=parameter") == "/foo/bar"); + CHECK( + Uri::getPath("https://example.com/foo/bar/?some=parameter") == + "/foo/bar/"); + CHECK(Uri::getPath("https://example.com") == ""); + CHECK(Uri::getPath("https://example.com?some=parameter") == ""); + CHECK(Uri::getPath("not a valid uri") == ""); +} + +TEST_CASE("Uri::setPath") { + CHECK(Uri::setPath("https://example.com", "") == "https://example.com"); + CHECK( + Uri::setPath("https://example.com?some=parameter", "") == + "https://example.com?some=parameter"); + CHECK(Uri::setPath("https://example.com", "/") == "https://example.com/"); + CHECK( + Uri::setPath("https://example.com?some=parameter", "/") == + "https://example.com/?some=parameter"); + CHECK( + Uri::setPath("https://example.com/foo?some=parameter", "/bar") == + "https://example.com/bar?some=parameter"); + CHECK( + Uri::setPath("https://example.com/foo", "/bar") == + "https://example.com/bar"); + CHECK( + Uri::setPath("https://example.com/foo?some=parameter", "/bar") == + "https://example.com/bar?some=parameter"); + CHECK( + Uri::setPath("https://example.com/foo/", "/bar") == + "https://example.com/bar"); + CHECK( + Uri::setPath("https://example.com/foo/?some=parameter", "/bar") == + "https://example.com/bar?some=parameter"); + CHECK( + Uri::setPath("https://example.com/foo", "/bar/") == + "https://example.com/bar/"); + CHECK( + Uri::setPath("https://example.com/foo?some=parameter", "/bar/") == + "https://example.com/bar/?some=parameter"); + CHECK( + Uri::setPath("https://example.com/foo/bar", "/foo/bar") == + "https://example.com/foo/bar"); + CHECK( + Uri::setPath("https://example.com/foo/bar?some=parameter", "/foo/bar") == + "https://example.com/foo/bar?some=parameter"); +} From 1a6b1f812e54275af12e9c9c93a1b8f4bcee6127 Mon Sep 17 00:00:00 2001 From: Kevin Ring Date: Fri, 5 Apr 2024 20:30:00 +1100 Subject: [PATCH 2/7] Update CHANGES.md. --- CHANGES.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index 963e7ed33..77fbcc8af 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,11 @@ # Change Log +### ? - ? + +##### Additions :tada: + +- Added `Uri::getPath` and `Uri::setPath`. + ### v0.34.0 - 2024-04-01 ##### Breaking Changes :mega: From 3d7ff14c70a0cef7b2078aa901e8943da22d41fd Mon Sep 17 00:00:00 2001 From: Kevin Ring Date: Fri, 5 Apr 2024 20:59:04 +1100 Subject: [PATCH 3/7] Fix joinToString with initial empty string. --- .../include/CesiumUtility/joinToString.h | 27 +++++++------------ CesiumUtility/test/TestJoinToString.cpp | 22 +++++++++++++++ 2 files changed, 31 insertions(+), 18 deletions(-) create mode 100644 CesiumUtility/test/TestJoinToString.cpp diff --git a/CesiumUtility/include/CesiumUtility/joinToString.h b/CesiumUtility/include/CesiumUtility/joinToString.h index 848889913..d92a53fef 100644 --- a/CesiumUtility/include/CesiumUtility/joinToString.h +++ b/CesiumUtility/include/CesiumUtility/joinToString.h @@ -17,16 +17,17 @@ namespace CesiumUtility { template std::string joinToString(TIterator begin, TIterator end, const std::string& separator) { + if (begin == end) + return std::string(); + + std::string first = *begin; + return std::accumulate( - begin, + ++begin, end, - std::string(), + std::move(first), [&separator](const std::string& acc, const std::string& element) { - if (!acc.empty()) { - return acc + separator + element; - } else { - return element; - } + return acc + separator + element; }); } @@ -41,16 +42,6 @@ joinToString(TIterator begin, TIterator end, const std::string& separator) { */ template std::string joinToString(TCollection collection, const std::string& separator) { - return std::accumulate( - collection.cbegin(), - collection.cend(), - std::string(), - [&separator](const std::string& acc, const std::string& element) { - if (!acc.empty()) { - return acc + separator + element; - } else { - return element; - } - }); + return joinToString(collection.cbegin(), collection.cend(), separator); } } // namespace CesiumUtility diff --git a/CesiumUtility/test/TestJoinToString.cpp b/CesiumUtility/test/TestJoinToString.cpp new file mode 100644 index 000000000..c8ad8b476 --- /dev/null +++ b/CesiumUtility/test/TestJoinToString.cpp @@ -0,0 +1,22 @@ +#include + +#include + +using namespace CesiumUtility; + +TEST_CASE("joinToString") { + CHECK( + joinToString(std::vector{"test", "this"}, "--") == + "test--this"); + CHECK( + joinToString(std::vector{"test", "this", "thing"}, " ") == + "test this thing"); + CHECK( + joinToString(std::vector{"test", "this", "thing"}, "") == + "testthisthing"); + CHECK(joinToString(std::vector{"test"}, "--") == "test"); + CHECK(joinToString(std::vector{""}, "--") == ""); + CHECK(joinToString(std::vector{"", "aa", ""}, "--") == "--aa--"); + CHECK(joinToString(std::vector{"", ""}, "--") == "--"); + CHECK(joinToString(std::vector{}, "--") == ""); +} From 79d0e9b0f559079c44586a1f5e3d764b4f356c33 Mon Sep 17 00:00:00 2001 From: Kevin Ring Date: Fri, 5 Apr 2024 21:03:00 +1100 Subject: [PATCH 4/7] Update CHANGES.md. --- CHANGES.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index 77fbcc8af..cabf139b2 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -6,6 +6,10 @@ - Added `Uri::getPath` and `Uri::setPath`. +##### Fixes :wrench: + +- Fixed a bug in + ### v0.34.0 - 2024-04-01 ##### Breaking Changes :mega: From 7952973afe674e94dba12391549bd43283ff2f02 Mon Sep 17 00:00:00 2001 From: Kevin Ring Date: Fri, 5 Apr 2024 21:34:27 +1100 Subject: [PATCH 5/7] Update CHANGES.md again. --- CHANGES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index cabf139b2..5b3fef983 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -8,7 +8,7 @@ ##### Fixes :wrench: -- Fixed a bug in +- Fixed a bug in `joinToString` when given a collection containing empty strings. ### v0.34.0 - 2024-04-01 From f962a69cb6e61fdc42d5afe0bee8fe9c3a5de6e6 Mon Sep 17 00:00:00 2001 From: Kevin Ring Date: Fri, 5 Apr 2024 21:51:25 +1100 Subject: [PATCH 6/7] Add missing header. --- CesiumUtility/src/Uri.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/CesiumUtility/src/Uri.cpp b/CesiumUtility/src/Uri.cpp index f8847fcef..560d1700a 100644 --- a/CesiumUtility/src/Uri.cpp +++ b/CesiumUtility/src/Uri.cpp @@ -7,6 +7,7 @@ #include #include #include +#include namespace CesiumUtility { std::string Uri::resolve( From b25f169877e59c3caff662ce115cec56e6b28cca Mon Sep 17 00:00:00 2001 From: Janine Liu Date: Fri, 5 Apr 2024 14:05:05 -0400 Subject: [PATCH 7/7] Separate test lines, add quick doc tweaks --- CesiumUtility/include/CesiumUtility/Uri.h | 5 +- CesiumUtility/test/TestJoinToString.cpp | 40 +++++--- CesiumUtility/test/TestUri.cpp | 119 +++++++++++++--------- 3 files changed, 101 insertions(+), 63 deletions(-) diff --git a/CesiumUtility/include/CesiumUtility/Uri.h b/CesiumUtility/include/CesiumUtility/Uri.h index 1f6ae94a8..aa21c63d9 100644 --- a/CesiumUtility/include/CesiumUtility/Uri.h +++ b/CesiumUtility/include/CesiumUtility/Uri.h @@ -26,7 +26,8 @@ class Uri final { static std::string escape(const std::string& s); /** - * @brief Gets the path portion of the URI. + * @brief Gets the path portion of the URI. This will not include path + * parameters, if present. * * @param uri The URI from which to get the path. * @return The path, or empty string if the URI could not be parsed. @@ -35,7 +36,7 @@ class Uri final { /** * @brief Sets the path portion of a URI to a new value. The other portions of - * the URI are left unmodified. + * the URI are left unmodified, including any path parameters. * * @param uri The URI for which to set the path. * @param The new path portion of the URI. diff --git a/CesiumUtility/test/TestJoinToString.cpp b/CesiumUtility/test/TestJoinToString.cpp index c8ad8b476..7edbba008 100644 --- a/CesiumUtility/test/TestJoinToString.cpp +++ b/CesiumUtility/test/TestJoinToString.cpp @@ -5,18 +5,30 @@ using namespace CesiumUtility; TEST_CASE("joinToString") { - CHECK( - joinToString(std::vector{"test", "this"}, "--") == - "test--this"); - CHECK( - joinToString(std::vector{"test", "this", "thing"}, " ") == - "test this thing"); - CHECK( - joinToString(std::vector{"test", "this", "thing"}, "") == - "testthisthing"); - CHECK(joinToString(std::vector{"test"}, "--") == "test"); - CHECK(joinToString(std::vector{""}, "--") == ""); - CHECK(joinToString(std::vector{"", "aa", ""}, "--") == "--aa--"); - CHECK(joinToString(std::vector{"", ""}, "--") == "--"); - CHECK(joinToString(std::vector{}, "--") == ""); + SECTION("joins vector with non-empty elements") { + CHECK( + joinToString(std::vector{"test", "this"}, "--") == + "test--this"); + CHECK( + joinToString(std::vector{"test", "this", "thing"}, " ") == + "test this thing"); + CHECK( + joinToString(std::vector{"test", "this", "thing"}, "") == + "testthisthing"); + } + + SECTION("joins vector with empty elements") { + CHECK( + joinToString(std::vector{"", "aa", ""}, "--") == "--aa--"); + CHECK(joinToString(std::vector{"", ""}, "--") == "--"); + } + + SECTION("handles single-element vector") { + CHECK(joinToString(std::vector{"test"}, "--") == "test"); + CHECK(joinToString(std::vector{""}, "--") == ""); + } + + SECTION("handles empty vector") { + CHECK(joinToString(std::vector{}, "--") == ""); + } } diff --git a/CesiumUtility/test/TestUri.cpp b/CesiumUtility/test/TestUri.cpp index a2ba060ed..c3cde6844 100644 --- a/CesiumUtility/test/TestUri.cpp +++ b/CesiumUtility/test/TestUri.cpp @@ -5,54 +5,79 @@ using namespace CesiumUtility; TEST_CASE("Uri::getPath") { - CHECK(Uri::getPath("https://example.com/") == "/"); - CHECK(Uri::getPath("https://example.com/foo/bar") == "/foo/bar"); - CHECK(Uri::getPath("https://example.com/foo/bar/") == "/foo/bar/"); - CHECK(Uri::getPath("https://example.com/?some=parameter") == "/"); - CHECK( - Uri::getPath("https://example.com/foo/bar?some=parameter") == "/foo/bar"); - CHECK( - Uri::getPath("https://example.com/foo/bar/?some=parameter") == - "/foo/bar/"); - CHECK(Uri::getPath("https://example.com") == ""); - CHECK(Uri::getPath("https://example.com?some=parameter") == ""); - CHECK(Uri::getPath("not a valid uri") == ""); + SECTION("returns path") { + CHECK(Uri::getPath("https://example.com/") == "/"); + CHECK(Uri::getPath("https://example.com/foo/bar") == "/foo/bar"); + CHECK(Uri::getPath("https://example.com/foo/bar/") == "/foo/bar/"); + } + + SECTION("ignores path parameters") { + CHECK(Uri::getPath("https://example.com/?some=parameter") == "/"); + CHECK( + Uri::getPath("https://example.com/foo/bar?some=parameter") == + "/foo/bar"); + CHECK( + Uri::getPath("https://example.com/foo/bar/?some=parameter") == + "/foo/bar/"); + } + + SECTION("returns empty path for nonexistent paths") { + CHECK(Uri::getPath("https://example.com") == ""); + CHECK(Uri::getPath("https://example.com?some=parameter") == ""); + } + + SECTION("returns empty path for invalid uri") { + CHECK(Uri::getPath("not a valid uri") == ""); + } } TEST_CASE("Uri::setPath") { - CHECK(Uri::setPath("https://example.com", "") == "https://example.com"); - CHECK( - Uri::setPath("https://example.com?some=parameter", "") == - "https://example.com?some=parameter"); - CHECK(Uri::setPath("https://example.com", "/") == "https://example.com/"); - CHECK( - Uri::setPath("https://example.com?some=parameter", "/") == - "https://example.com/?some=parameter"); - CHECK( - Uri::setPath("https://example.com/foo?some=parameter", "/bar") == - "https://example.com/bar?some=parameter"); - CHECK( - Uri::setPath("https://example.com/foo", "/bar") == - "https://example.com/bar"); - CHECK( - Uri::setPath("https://example.com/foo?some=parameter", "/bar") == - "https://example.com/bar?some=parameter"); - CHECK( - Uri::setPath("https://example.com/foo/", "/bar") == - "https://example.com/bar"); - CHECK( - Uri::setPath("https://example.com/foo/?some=parameter", "/bar") == - "https://example.com/bar?some=parameter"); - CHECK( - Uri::setPath("https://example.com/foo", "/bar/") == - "https://example.com/bar/"); - CHECK( - Uri::setPath("https://example.com/foo?some=parameter", "/bar/") == - "https://example.com/bar/?some=parameter"); - CHECK( - Uri::setPath("https://example.com/foo/bar", "/foo/bar") == - "https://example.com/foo/bar"); - CHECK( - Uri::setPath("https://example.com/foo/bar?some=parameter", "/foo/bar") == - "https://example.com/foo/bar?some=parameter"); + SECTION("sets empty path") { + CHECK(Uri::setPath("https://example.com", "") == "https://example.com"); + } + + SECTION("sets new path") { + CHECK(Uri::setPath("https://example.com", "/") == "https://example.com/"); + CHECK( + Uri::setPath("https://example.com/foo", "/bar") == + "https://example.com/bar"); + CHECK( + Uri::setPath("https://example.com/foo/", "/bar") == + "https://example.com/bar"); + CHECK( + Uri::setPath("https://example.com/foo", "/bar/") == + "https://example.com/bar/"); + } + + SECTION("preserves path parameters") { + CHECK( + Uri::setPath("https://example.com?some=parameter", "") == + "https://example.com?some=parameter"); + CHECK( + Uri::setPath("https://example.com?some=parameter", "/") == + "https://example.com/?some=parameter"); + CHECK( + Uri::setPath("https://example.com/foo?some=parameter", "/bar") == + "https://example.com/bar?some=parameter"); + CHECK( + Uri::setPath("https://example.com/foo/?some=parameter", "/bar") == + "https://example.com/bar?some=parameter"); + CHECK( + Uri::setPath("https://example.com/foo?some=parameter", "/bar/") == + "https://example.com/bar/?some=parameter"); + } + + SECTION("sets same path") { + CHECK( + Uri::setPath("https://example.com/foo/bar", "/foo/bar") == + "https://example.com/foo/bar"); + CHECK( + Uri::setPath( + "https://example.com/foo/bar?some=parameter", + "/foo/bar") == "https://example.com/foo/bar?some=parameter"); + } + + SECTION("returns empty path for invalid uri") { + CHECK(Uri::setPath("not a valid uri", "/foo/") == ""); + } }