From 934d28d241e2dcdfb10ed0ff8d4cf0cae56a4c9f Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Thu, 12 Oct 2023 14:49:41 -0500 Subject: [PATCH] Re-enabling Windows tests (#376) Fix tests around Windows' handling (or not handling) of : and @ in filesystem paths. Signed-off-by: Michael Carroll --- include/gz/fuel_tools/FuelClient.hh | 7 +++ include/gz/fuel_tools/Helpers.hh | 15 +++++ src/ClientConfig.cc | 2 +- src/CollectionIdentifier_TEST.cc | 2 +- src/FuelClient.cc | 71 +++++++++++++++--------- src/FuelClient_TEST.cc | 85 +++++++++++++++++------------ src/Helpers.cc | 12 +++- src/Helpers_TEST.cc | 32 ++++++++--- src/LocalCache.cc | 4 +- src/LocalCache_TEST.cc | 60 ++++++++++---------- src/ModelIdentifier.cc | 8 ++- src/ModelIdentifier_TEST.cc | 20 +++---- src/WorldIdentifier.cc | 10 ++-- src/WorldIdentifier_TEST.cc | 30 +++------- 14 files changed, 209 insertions(+), 149 deletions(-) diff --git a/include/gz/fuel_tools/FuelClient.hh b/include/gz/fuel_tools/FuelClient.hh index d148de47..703d3032 100644 --- a/include/gz/fuel_tools/FuelClient.hh +++ b/include/gz/fuel_tools/FuelClient.hh @@ -315,6 +315,13 @@ namespace gz const std::vector &_ids, size_t _jobs = 2); + /// \brief Check if a model is already present in the local cache. + /// \param[in] _id The model identifier + /// \param[out] _path Local path where the model can be found. + /// \return FETCH_ERROR if not cached, FETCH_ALREADY_EXISTS if cached. + public: Result CachedModel(const ModelIdentifier &_id, + std::string &_path); + /// \brief Check if a model is already present in the local cache. /// \param[in] _modelUrl The unique URL of the model on a Fuel server. /// E.g.: https://fuel.gazebosim.org/1.0/caguero/models/Beer diff --git a/include/gz/fuel_tools/Helpers.hh b/include/gz/fuel_tools/Helpers.hh index 82989018..96df74b9 100644 --- a/include/gz/fuel_tools/Helpers.hh +++ b/include/gz/fuel_tools/Helpers.hh @@ -39,6 +39,21 @@ namespace gz { namespace fuel_tools { +/// \brief Convert the authority portion of a URI to a string +/// suitable to be used as a path on disk for all platforms. +/// +/// It encodes illegal characters on Windows and Linux filesystems with +/// their corresponding URL-encoded values. +/// +/// This assumes an authority of the form: username@host:port +/// "@" is encoded as %40 +/// ":" is encoded as %3A +/// +/// \param[in] _uriAuthority the authority section of the URI to convert. +/// \return String suitable to use in file paths +GZ_FUEL_TOOLS_VISIBLE +std::string sanitizeAuthority(const std::string &_uriAuthority); + /// \brief Convert a URI to a string suitable to use as a path on disk. /// It strips the scheme and authority's `//` prefix. /// \param[in] _uri URI to convert. diff --git a/src/ClientConfig.cc b/src/ClientConfig.cc index 0a2764f1..02ba8a13 100644 --- a/src/ClientConfig.cc +++ b/src/ClientConfig.cc @@ -83,7 +83,7 @@ class gz::fuel_tools::ServerConfigPrivate } /// \brief URL to reach server - public: common::URI url{"https://fuel.gazebosim.org"}; + public: common::URI url{"https://fuel.gazebosim.org", true}; /// \brief A key to auth with the server public: std::string key = ""; diff --git a/src/CollectionIdentifier_TEST.cc b/src/CollectionIdentifier_TEST.cc index a7025263..e18b393a 100644 --- a/src/CollectionIdentifier_TEST.cc +++ b/src/CollectionIdentifier_TEST.cc @@ -45,7 +45,7 @@ TEST(CollectionIdentifier, SetFields) ///////////////////////////////////////////////// /// \brief Unique Name // See https://github.com/gazebosim/gz-fuel-tools/issues/231 -TEST(CollectionIdentifier, GZ_UTILS_TEST_DISABLED_ON_WIN32(UniqueName)) +TEST(CollectionIdentifier, UniqueName) { gz::fuel_tools::ServerConfig srv1; srv1.SetUrl(common::URI("https://localhost:8001")); diff --git a/src/FuelClient.cc b/src/FuelClient.cc index 45902eec..28e5bf47 100644 --- a/src/FuelClient.cc +++ b/src/FuelClient.cc @@ -730,7 +730,8 @@ Result FuelClient::ModelDependencies(const ModelIdentifier &_id, // Locate any dependencies from the input model and download them. std::string path; gz::msgs::FuelMetadata meta; - if (this->CachedModel(gz::common::URI(_id.UniqueName()), path)) + + if (this->CachedModel(_id, path)) { std::string metadataPath = gz::common::joinPaths(path, "metadata.pbtxt"); @@ -1097,7 +1098,11 @@ bool FuelClient::ParseModelUrl(const common::URI &_modelUrl, } // Get remaining server information from config - _id.Server().SetUrl(common::URI(scheme + "://" + server)); + common::URI serverUri; + serverUri.SetScheme(scheme); + serverUri.SetAuthority(gz::common::URIAuthority("//" + server)); + + _id.Server().SetUrl(serverUri); _id.Server().SetVersion(apiVersion); for (const auto &s : this->dataPtr->config.Servers()) { @@ -1163,7 +1168,11 @@ bool FuelClient::ParseWorldUrl(const common::URI &_worldUrl, } // Get remaining server information from config - _id.Server().SetUrl(common::URI(scheme + "://" + server)); + common::URI serverUri; + serverUri.SetScheme(scheme); + serverUri.SetAuthority(gz::common::URIAuthority("//" + server)); + + _id.Server().SetUrl(serverUri); _id.Server().SetVersion(apiVersion); for (const auto &s : this->dataPtr->config.Servers()) { @@ -1231,7 +1240,11 @@ bool FuelClient::ParseModelFileUrl(const common::URI &_modelFileUrl, } // Get remaining server information from config - _id.Server().SetUrl(common::URI(scheme + "://" + server)); + common::URI serverUri; + serverUri.SetScheme(scheme); + serverUri.SetAuthority(gz::common::URIAuthority("//" + server)); + + _id.Server().SetUrl(serverUri); _id.Server().SetVersion(apiVersion); for (const auto &s : this->dataPtr->config.Servers()) { @@ -1300,7 +1313,11 @@ bool FuelClient::ParseWorldFileUrl(const common::URI &_worldFileUrl, } // Get remaining server information from config - _id.Server().SetUrl(common::URI(scheme + "://" + server)); + common::URI serverUri; + serverUri.SetScheme(scheme); + serverUri.SetAuthority(gz::common::URIAuthority("//" + server)); + + _id.Server().SetUrl(serverUri); _id.Server().SetVersion(apiVersion); for (const auto &s : this->dataPtr->config.Servers()) { @@ -1367,7 +1384,11 @@ bool FuelClient::ParseCollectionUrl(const common::URI &_url, } // Get remaining server information from config - _id.Server().SetUrl(common::URI(scheme + "://" + server)); + common::URI serverUri; + serverUri.SetScheme(scheme); + serverUri.SetAuthority(gz::common::URIAuthority("//" + server)); + + _id.Server().SetUrl(serverUri); _id.Server().SetVersion(apiVersion); for (const auto &s : this->dataPtr->config.Servers()) { @@ -1414,9 +1435,6 @@ Result FuelClient::DownloadModel(const common::URI &_modelUrl, if (!result) return result; - // TODO(anyone) We shouldn't need to reconstruct the path, SaveModel should - // be changed to return it - // We need to figure out the version for the tip if (id.Version() == 0 || id.VersionStr() == "tip") { @@ -1425,8 +1443,7 @@ Result FuelClient::DownloadModel(const common::URI &_modelUrl, } _path = gz::common::joinPaths(this->Config().CacheLocation(), - id.Server().Url().Path().Str(), id.Owner(), "models", id.Name(), - id.VersionStr()); + id.UniqueName(), id.VersionStr()); return result; } @@ -1453,15 +1470,23 @@ Result FuelClient::DownloadWorld(const common::URI &_worldUrl, } ////////////////////////////////////////////////// -bool FuelClient::CachedModel(const common::URI &_modelUrl) +Result FuelClient::CachedModel(const ModelIdentifier &_id, + std::string &_path) { - // Get data from URL - ModelIdentifier id; - if (!this->ParseModelUrl(_modelUrl, id)) - return Result(ResultType::FETCH_ERROR); + auto modelIter = this->dataPtr->cache->MatchingModel(_id); + if (modelIter) + { + _path = modelIter.PathToModel(); + return Result(ResultType::FETCH_ALREADY_EXISTS); + } + return Result(ResultType::FETCH_ERROR); +} - // Check local cache - return this->dataPtr->cache->MatchingModel(id) ? true : false; +////////////////////////////////////////////////// +bool FuelClient::CachedModel(const common::URI &_modelUrl) +{ + std::string path; + return this->CachedModel(_modelUrl, path).Type() != ResultType::FETCH_ERROR; } ////////////////////////////////////////////////// @@ -1475,15 +1500,7 @@ Result FuelClient::CachedModel(const common::URI &_modelUrl, return Result(ResultType::FETCH_ERROR); } - // Check local cache - auto modelIter = this->dataPtr->cache->MatchingModel(id); - if (modelIter) - { - _path = modelIter.PathToModel(); - return Result(ResultType::FETCH_ALREADY_EXISTS); - } - - return Result(ResultType::FETCH_ERROR); + return this->CachedModel(id, _path); } ////////////////////////////////////////////////// diff --git a/src/FuelClient_TEST.cc b/src/FuelClient_TEST.cc index 86350733..c7569f75 100644 --- a/src/FuelClient_TEST.cc +++ b/src/FuelClient_TEST.cc @@ -23,6 +23,7 @@ #include "gz/fuel_tools/FuelClient.hh" #include "gz/fuel_tools/ClientConfig.hh" +#include "gz/fuel_tools/Helpers.hh" #include "gz/fuel_tools/Result.hh" #include "gz/fuel_tools/WorldIdentifier.hh" @@ -40,7 +41,9 @@ void createLocalModel(ClientConfig &_conf) gzdbg << "Creating local model in [" << common::cwd() << "]" << std::endl; auto modelPath = common::joinPaths( - "test_cache", "localhost:8007", "alice", "models", "My Model"); + "test_cache", + sanitizeAuthority("localhost:8007"), + "alice", "models", "My Model"); ASSERT_TRUE(common::createDirectories( common::joinPaths(modelPath, "2", "meshes"))); @@ -82,7 +85,7 @@ void createLocalModel(ClientConfig &_conf) } gz::fuel_tools::ServerConfig srv; - srv.SetUrl(common::URI("http://localhost:8007/")); + srv.SetUrl(common::URI("http://localhost:8007/", true)); _conf.AddServer(srv); } @@ -95,7 +98,9 @@ void createLocalWorld(ClientConfig &_conf) gzdbg << "Creating local world in [" << common::cwd() << "]" << std::endl; auto worldPath = common::joinPaths( - "test_cache", "localhost:8007", "banana", "worlds", "My World"); + "test_cache", + sanitizeAuthority("localhost:8007"), + "banana", "worlds", "My World"); ASSERT_TRUE(common::createDirectories(common::joinPaths(worldPath, "2"))); ASSERT_TRUE(common::createDirectories(common::joinPaths(worldPath, "3"))); @@ -113,7 +118,7 @@ void createLocalWorld(ClientConfig &_conf) } gz::fuel_tools::ServerConfig srv; - srv.SetUrl(gz::common::URI("http://localhost:8007/")); + srv.SetUrl(gz::common::URI("http://localhost:8007/", true)); _conf.AddServer(srv); } @@ -130,6 +135,9 @@ class FuelClientTest: public ::testing::Test ASSERT_FALSE(common::exists("test_cache")); ASSERT_TRUE(common::createDirectories("test_cache")); ASSERT_TRUE(common::isDirectory("test_cache")); + ASSERT_FALSE(common::exists("test_cache/fuel.gazebosim.org")); + ASSERT_TRUE(common::createDirectories("test_cache/fuel.gazebosim.org")); + ASSERT_TRUE(common::isDirectory("test_cache/fuel.gazebosim.org")); } public: std::shared_ptr tempDir; @@ -615,7 +623,7 @@ TEST_P(FuelClientDownloadTest, DownloadModel) ///////////////////////////////////////////////// // Windows doesn't support colons in filenames // https://github.com/gazebosim/gz-fuel-tools/issues/106 -TEST_F(FuelClientTest, GZ_UTILS_TEST_DISABLED_ON_WIN32(ModelDependencies)) +TEST_F(FuelClientTest, ModelDependencies) { ClientConfig config; config.SetCacheLocation(common::joinPaths(common::cwd(), "test_cache")); @@ -687,7 +695,7 @@ TEST_F(FuelClientTest, GZ_UTILS_TEST_DISABLED_ON_WIN32(ModelDependencies)) // Windows doesn't support colons in filenames // https://github.com/gazebosim/gz-fuel-tools/issues/106 // See https://github.com/gazebosim/gz-fuel-tools/issues/231 -TEST_F(FuelClientTest, GZ_UTILS_TEST_DISABLED_ON_WIN32(CachedModel)) +TEST_F(FuelClientTest, CachedModel) { ClientConfig config; config.SetCacheLocation(common::joinPaths(common::cwd(), "test_cache")); @@ -697,66 +705,69 @@ TEST_F(FuelClientTest, GZ_UTILS_TEST_DISABLED_ON_WIN32(CachedModel)) FuelClient client(config); EXPECT_EQ(config.CacheLocation(), client.Config().CacheLocation()); + auto basePath = common::joinPaths(common::cwd(), "test_cache", + sanitizeAuthority("localhost:8007")); + // Cached model (no version) { - common::URI url{"http://localhost:8007/1.0/alice/models/My Model"}; + common::URI url{"http://localhost:8007/1.0/alice/models/My Model", true}; std::string path; auto result = client.CachedModel(url, path); EXPECT_TRUE(result); EXPECT_EQ(ResultType::FETCH_ALREADY_EXISTS, result.Type()); - EXPECT_EQ(common::joinPaths(common::cwd(), "test_cache", "localhost:8007", + EXPECT_EQ(common::joinPaths(basePath, "alice", "models", "My Model", "3"), path); } // Cached model (tip) { - common::URI url{"http://localhost:8007/1.0/alice/models/My Model/tip"}; + common::URI url{"http://localhost:8007/1.0/alice/models/My Model/tip", true}; std::string path; auto result = client.CachedModel(url, path); EXPECT_TRUE(result); EXPECT_EQ(ResultType::FETCH_ALREADY_EXISTS, result.Type()); - EXPECT_EQ(common::joinPaths(common::cwd(), "test_cache", "localhost:8007", + EXPECT_EQ(common::joinPaths(basePath, "alice", "models", "My Model", "3"), path); } // Cached model (version number) { - common::URI url{"http://localhost:8007/1.0/alice/models/My Model/2"}; + common::URI url{"http://localhost:8007/1.0/alice/models/My Model/2", true}; std::string path; auto result = client.CachedModel(url, path); EXPECT_TRUE(result); EXPECT_EQ(ResultType::FETCH_ALREADY_EXISTS, result.Type()); - EXPECT_EQ(common::joinPaths(common::cwd(), "test_cache", "localhost:8007", + EXPECT_EQ(common::joinPaths(basePath, "alice", "models", "My Model", "2"), path); } // Cached model file (tip) { common::URI url{ - "http://localhost:8007/1.0/alice/models/My Model/tip/files/model.sdf"}; + "http://localhost:8007/1.0/alice/models/My Model/tip/files/model.sdf", true}; std::string path; auto result = client.CachedModelFile(url, path); EXPECT_TRUE(result); EXPECT_EQ(ResultType::FETCH_ALREADY_EXISTS, result.Type()); - EXPECT_EQ(common::joinPaths(common::cwd(), "test_cache", "localhost:8007", + EXPECT_EQ(common::joinPaths(basePath, "alice", "models", "My Model", "3", "model.sdf"), path); } // Deeper cached model file { common::URI url{"http://localhost:8007/1.0/alice/models/My Model/2/files/" - "meshes/model.dae"}; + "meshes/model.dae", true}; std::string path; auto result = client.CachedModelFile(url, path); EXPECT_TRUE(result); EXPECT_EQ(ResultType::FETCH_ALREADY_EXISTS, result.Type()); - EXPECT_EQ(common::joinPaths(common::cwd(), "test_cache", "localhost:8007", + EXPECT_EQ(common::joinPaths(basePath, "alice", "models", "My Model", "2", "meshes", "model.dae"), path); } // Non-cached model { - common::URI url{"http://localhost:8007/1.0/alice/models/Banana"}; + common::URI url{"http://localhost:8007/1.0/alice/models/Banana", true}; std::string path; auto result = client.CachedModel(url, path); EXPECT_FALSE(result); @@ -765,7 +776,7 @@ TEST_F(FuelClientTest, GZ_UTILS_TEST_DISABLED_ON_WIN32(CachedModel)) // Non-cached model (when looking for file) { - common::URI url{"http://localhost:8007/1.0/alice/models/Banana/model.sdf"}; + common::URI url{"http://localhost:8007/1.0/alice/models/Banana/model.sdf", true}; std::string path; auto result = client.CachedModelFile(url, path); EXPECT_FALSE(result); @@ -775,7 +786,7 @@ TEST_F(FuelClientTest, GZ_UTILS_TEST_DISABLED_ON_WIN32(CachedModel)) // Non-cached model file { common::URI url{"http://localhost:8007/1.0/alice/models/My Model/tip/files/" - "meshes/banana.dae" + "meshes/banana.dae", true }; std::string path; auto result = client.CachedModelFile(url, path); @@ -785,7 +796,7 @@ TEST_F(FuelClientTest, GZ_UTILS_TEST_DISABLED_ON_WIN32(CachedModel)) // Model root URL to model file { - common::URI url{"http://localhost:8007/1.0/alice/models/My Model"}; + common::URI url{"http://localhost:8007/1.0/alice/models/My Model", true}; std::string path; auto result = client.CachedModelFile(url, path); EXPECT_FALSE(result); @@ -1158,49 +1169,52 @@ TEST_F(FuelClientTest, CachedWorld) FuelClient client(config); EXPECT_EQ(config.CacheLocation(), client.Config().CacheLocation()); + auto basePath = common::joinPaths(common::cwd(), "test_cache", + sanitizeAuthority("localhost:8007")); + // Cached world (no version) { - common::URI url{"http://localhost:8007/1.0/banana/worlds/My World"}; + common::URI url{"http://localhost:8007/1.0/banana/worlds/My World", true}; std::string path; auto result = client.CachedWorld(url, path); EXPECT_TRUE(result); EXPECT_EQ(ResultType::FETCH_ALREADY_EXISTS, result.Type()); - EXPECT_EQ(common::joinPaths(common::cwd(), "test_cache", - "localhost:8007", "banana", "worlds", "My World", "3"), path); + EXPECT_EQ(common::joinPaths(basePath, + "banana", "worlds", "My World", "3"), path); } // Cached world (tip) { - common::URI url{"http://localhost:8007/1.0/banana/worlds/My World/tip"}; + common::URI url{"http://localhost:8007/1.0/banana/worlds/My World/tip", true}; std::string path; auto result = client.CachedWorld(url, path); EXPECT_TRUE(result); EXPECT_EQ(ResultType::FETCH_ALREADY_EXISTS, result.Type()); - EXPECT_EQ(common::joinPaths(common::cwd(), "test_cache", - "localhost:8007", "banana", "worlds", "My World", "3"), path); + EXPECT_EQ(common::joinPaths(basePath, + "banana", "worlds", "My World", "3"), path); } // Cached world (version number) { - common::URI url{"http://localhost:8007/1.0/banana/worlds/My World/2"}; + common::URI url{"http://localhost:8007/1.0/banana/worlds/My World/2", true}; std::string path; auto result = client.CachedWorld(url, path); EXPECT_TRUE(result); EXPECT_EQ(ResultType::FETCH_ALREADY_EXISTS, result.Type()); - EXPECT_EQ(common::joinPaths(common::cwd(), "test_cache", - "localhost:8007", "banana", "worlds", "My World", "2"), path); + EXPECT_EQ(common::joinPaths(basePath, + "banana", "worlds", "My World", "2"), path); } // Cached world file (tip) { common::URI url{"http://localhost:8007/1.0/banana/worlds/My World/tip/" - "files/strawberry.world"}; + "files/strawberry.world", true}; std::string path; auto result = client.CachedWorldFile(url, path); EXPECT_TRUE(result); EXPECT_EQ(ResultType::FETCH_ALREADY_EXISTS, result.Type()); - EXPECT_EQ(common::joinPaths(common::cwd(), "test_cache", - "localhost:8007", "banana", "worlds", "My World", "3", + EXPECT_EQ(common::joinPaths(basePath, + "banana", "worlds", "My World", "3", "strawberry.world"), path); } @@ -1212,8 +1226,8 @@ TEST_F(FuelClientTest, CachedWorld) auto result = client.CachedWorldFile(url, path); EXPECT_TRUE(result); EXPECT_EQ(ResultType::FETCH_ALREADY_EXISTS, result.Type()); - EXPECT_EQ(common::joinPaths(common::cwd(), "test_cache", - "localhost:8007", "banana", "worlds", "My World", "2", + EXPECT_EQ(common::joinPaths(basePath, + "banana", "worlds", "My World", "2", "strawberry.world"), path); } @@ -1498,7 +1512,8 @@ TEST_F(FuelClientTest, PatchModelFail) // Bad model.config result = client.PatchModel(modelId, headers, - common::joinPaths(common::cwd(), "test_cache", "localhost:8007", + common::joinPaths(common::cwd(), "test_cache", + sanitizeAuthority("localhost:8007"), "alice", "models", "My Model", "3")); EXPECT_EQ(ResultType::UPLOAD_ERROR, result.Type()); diff --git a/src/Helpers.cc b/src/Helpers.cc index b8dfc395..e9be8d35 100644 --- a/src/Helpers.cc +++ b/src/Helpers.cc @@ -19,8 +19,15 @@ #include "gz/fuel_tools/Helpers.hh" -using namespace gz; -using namespace fuel_tools; +////////////////////////////////////////////////// +std::string gz::fuel_tools::sanitizeAuthority(const std::string &_uriAuthority) +{ + // Take an authority of the form userinfo@host:port and turn it into a valid + // subset of characters for a path. + auto output = gz::common::replaceAll(_uriAuthority, ":", "%3A"); + output = gz::common::replaceAll(output, "@", "%40"); + return output; +} ////////////////////////////////////////////////// std::string gz::fuel_tools::uriToPath(const common::URI &_uri) @@ -43,6 +50,7 @@ std::string gz::fuel_tools::uriToPath(const common::URI &_uri) authority = authority.substr(2); } + authority = sanitizeAuthority(authority); if (authority.empty()) { return path; diff --git a/src/Helpers_TEST.cc b/src/Helpers_TEST.cc index 15171c60..e28b9e9e 100644 --- a/src/Helpers_TEST.cc +++ b/src/Helpers_TEST.cc @@ -22,21 +22,31 @@ using namespace gz; using namespace fuel_tools; +///////////////////////////////////////////////// +TEST(HelpersTEST, SanitizeAuthority) +{ + EXPECT_EQ("localhost%3A8000", sanitizeAuthority("localhost:8000")); + EXPECT_EQ("localhost%3A8001", sanitizeAuthority("localhost:8001")); + EXPECT_EQ("localhost%3A%3A8002", sanitizeAuthority("localhost::8002")); + EXPECT_EQ("gazebo%40localhost", sanitizeAuthority("gazebo@localhost")); +} + ///////////////////////////////////////////////// TEST(HelpersTEST, UriToPathNoAuthority) { -// TO-DO: Update this test after gz-fuel-tools#204 is addressed #ifdef WIN32 + const std::string testStr0 = R"(localhost:8000)"; const std::string testStr1 = R"(localhost:8000\some\path)"; const std::string testStr2 = R"(localhost:8000\some\path\)"; #else + const std::string testStr0 = R"(localhost:8000)"; const std::string testStr1 = R"(localhost:8000/some/path)"; const std::string testStr2 = R"(localhost:8000/some/path/)"; #endif { common::URI uri{"http://localhost:8000"}; - EXPECT_EQ("localhost:8000", uriToPath(uri)); + EXPECT_EQ(testStr0, uriToPath(uri)); } { @@ -53,20 +63,28 @@ TEST(HelpersTEST, UriToPathNoAuthority) ///////////////////////////////////////////////// TEST(HelpersTEST, UriToPathHasAuthority) { +#ifdef WIN32 + const std::string testStr0 = R"(localhost%3A8000)"; + const std::string testStr1 = R"(localhost%3A8000\some\path)"; + const std::string testStr2 = R"(localhost%3A8000\some\path\)"; +#else + const std::string testStr0 = R"(localhost%3A8000)"; + const std::string testStr1 = R"(localhost%3A8000/some/path)"; + const std::string testStr2 = R"(localhost%3A8000/some/path/)"; +#endif + { common::URI uri{"http://localhost:8000", true}; - EXPECT_EQ("localhost:8000", uriToPath(uri)); + EXPECT_EQ(testStr0, uriToPath(uri)); } { common::URI uri{"http://localhost:8000/some/path", true}; - EXPECT_EQ(common::joinPaths("localhost:8000", "some", "path"), - uriToPath(uri)); + EXPECT_EQ(testStr1, uriToPath(uri)); } { common::URI uri{"http://localhost:8000/some/path/", true}; - EXPECT_EQ(common::separator(common::joinPaths("localhost:8000", "some", - "path")), uriToPath(uri)); + EXPECT_EQ(testStr2, uriToPath(uri)); } } diff --git a/src/LocalCache.cc b/src/LocalCache.cc index 60023534..481da466 100644 --- a/src/LocalCache.cc +++ b/src/LocalCache.cc @@ -389,7 +389,8 @@ bool LocalCache::SaveModel( std::string cacheLocation = this->dataPtr->config->CacheLocation(); std::string modelRootDir = common::joinPaths(cacheLocation, - uriToPath(_id.Server().Url()), _id.Owner(), "models", _id.Name()); + _id.UniqueName()); + std::string modelVersionedDir = common::joinPaths(modelRootDir, _id.VersionStr()); @@ -741,7 +742,6 @@ bool LocalCache::SaveWorld( } auto cacheLocation = this->dataPtr->config->CacheLocation(); - auto worldRootDir = common::joinPaths(cacheLocation, _id.UniqueName()); auto worldVersionedDir = common::joinPaths(worldRootDir, _id.VersionStr()); diff --git a/src/LocalCache_TEST.cc b/src/LocalCache_TEST.cc index 4e9844b4..52649e88 100644 --- a/src/LocalCache_TEST.cc +++ b/src/LocalCache_TEST.cc @@ -26,6 +26,7 @@ #include #include "gz/fuel_tools/ClientConfig.hh" +#include "gz/fuel_tools/Helpers.hh" #include "gz/fuel_tools/WorldIdentifier.hh" #include "LocalCache.hh" @@ -38,7 +39,8 @@ void createLocal6Models(ClientConfig &_conf) { gzdbg << "Creating 6 local models in [" << common::cwd() << "]" << std::endl; - auto serverPath = common::joinPaths("test_cache", "localhost:8001"); + auto serverPath = + common::joinPaths("test_cache", sanitizeAuthority("localhost:8001")); ASSERT_TRUE(common::createDirectories(common::joinPaths(serverPath, "alice", "models", "am1", "2"))); ASSERT_TRUE(common::createDirectories(common::joinPaths(serverPath, @@ -81,7 +83,7 @@ void createLocal6Models(ClientConfig &_conf) "trudy", "models", "tm2", "2", "model.config"))); gz::fuel_tools::ServerConfig srv; - srv.SetUrl(common::URI("http://localhost:8001/")); + srv.SetUrl(common::URI("http://localhost:8001/", true)); _conf.AddServer(srv); } @@ -90,7 +92,8 @@ void createLocal3Models(ClientConfig &_conf) { gzdbg << "Creating 3 local models in [" << common::cwd() << "]" << std::endl; - auto serverPath = common::joinPaths("test_cache", "localhost:8007"); + auto serverPath = + common::joinPaths("test_cache", sanitizeAuthority("localhost:8007")); ASSERT_TRUE(common::createDirectories(common::joinPaths(serverPath, "alice", "models", "am1", "2"))); ASSERT_TRUE(common::createDirectories(common::joinPaths(serverPath, @@ -115,7 +118,7 @@ void createLocal3Models(ClientConfig &_conf) "trudy", "models", "tm1", "3", "model.config"))); gz::fuel_tools::ServerConfig srv; - srv.SetUrl(gz::common::URI("http://localhost:8007/")); + srv.SetUrl(gz::common::URI("http://localhost:8007/", true)); _conf.AddServer(srv); } @@ -124,7 +127,8 @@ void createLocal6Worlds(ClientConfig &_conf) { gzdbg << "Creating 6 local worlds in [" << common::cwd() << "]" << std::endl; - auto serverPath = common::joinPaths("test_cache", "localhost:8001"); + auto serverPath = + common::joinPaths("test_cache", sanitizeAuthority("localhost:8001")); ASSERT_TRUE(common::createDirectories(common::joinPaths(serverPath, "alice", "worlds", "am1", "2"))); ASSERT_TRUE(common::createDirectories(common::joinPaths(serverPath, @@ -167,7 +171,7 @@ void createLocal6Worlds(ClientConfig &_conf) "trudy", "worlds", "tm2", "2", "world.world"))); gz::fuel_tools::ServerConfig srv; - srv.SetUrl(gz::common::URI("http://localhost:8001/")); + srv.SetUrl(gz::common::URI("http://localhost:8001/", true)); _conf.AddServer(srv); } @@ -176,7 +180,8 @@ void createLocal3Worlds(ClientConfig &_conf) { gzdbg << "Creating 3 local worlds in [" << common::cwd() << "]" << std::endl; - auto serverPath = common::joinPaths("test_cache", "localhost:8007"); + auto serverPath = common::joinPaths( + "test_cache", sanitizeAuthority("localhost:8007")); ASSERT_TRUE(common::createDirectories(common::joinPaths(serverPath, "alice", "worlds", "am1", "2"))); ASSERT_TRUE(common::createDirectories(common::joinPaths(serverPath, @@ -201,7 +206,7 @@ void createLocal3Worlds(ClientConfig &_conf) "trudy", "worlds", "tm1", "3", "world.world"))); gz::fuel_tools::ServerConfig srv; - srv.SetUrl(common::URI("http://localhost:8007/")); + srv.SetUrl(common::URI("http://localhost:8007/", true)); _conf.AddServer(srv); } @@ -218,6 +223,10 @@ class LocalCacheTest : public ::testing::Test ASSERT_FALSE(common::exists("test_cache")); ASSERT_TRUE(common::createDirectories("test_cache")); ASSERT_TRUE(common::isDirectory("test_cache")); + + // This suppresses some unnecessary console spam. + common::createDirectories( + common::joinPaths("test_cache", "fuel.gazebosim.org")); } public: std::shared_ptr tempDir; @@ -225,8 +234,7 @@ class LocalCacheTest : public ::testing::Test ///////////////////////////////////////////////// /// \brief Iterate through all models in cache -// See https://github.com/gazebosim/gz-fuel-tools/issues/231 -TEST_F(LocalCacheTest, GZ_UTILS_TEST_DISABLED_ON_WIN32(AllModels)) +TEST_F(LocalCacheTest, AllModels) { ClientConfig conf; conf.SetCacheLocation(common::joinPaths(common::cwd(), "test_cache")); @@ -245,14 +253,13 @@ TEST_F(LocalCacheTest, GZ_UTILS_TEST_DISABLED_ON_WIN32(AllModels)) EXPECT_EQ(9u, uniqueNames.size()); EXPECT_NE(uniqueNames.end(), uniqueNames.find( - "http://localhost:8001/alice/models/am1")); + "localhost%3A8001/alice/models/am1")); } ///////////////////////////////////////////////// /// \brief Get all models that match some fields /// \brief Iterate through all models in cache -// See https://github.com/gazebosim/gz-fuel-tools/issues/307 -TEST_F(LocalCacheTest, GZ_UTILS_TEST_DISABLED_ON_WIN32(MatchingModels)) +TEST_F(LocalCacheTest, MatchingModels) { ClientConfig conf; conf.Clear(); @@ -295,8 +302,7 @@ TEST_F(LocalCacheTest, GZ_UTILS_TEST_DISABLED_ON_WIN32(MatchingModels)) ///////////////////////////////////////////////// /// \brief Get a specific model from cache /// \brief Iterate through all models in cache -// See https://github.com/gazebosim/gz-fuel-tools/issues/307 -TEST_F(LocalCacheTest, GZ_UTILS_TEST_DISABLED_ON_WIN32(MatchingModel)) +TEST_F(LocalCacheTest, MatchingModel) { ClientConfig conf; conf.SetCacheLocation(common::joinPaths(common::cwd(), "test_cache")); @@ -305,10 +311,10 @@ TEST_F(LocalCacheTest, GZ_UTILS_TEST_DISABLED_ON_WIN32(MatchingModel)) gz::fuel_tools::LocalCache cache(&conf); gz::fuel_tools::ServerConfig srv1; - srv1.SetUrl(common::URI("http://localhost:8001/")); + srv1.SetUrl(common::URI("http://localhost:8001/", true)); gz::fuel_tools::ServerConfig srv2; - srv2.SetUrl(common::URI("http://localhost:8002/")); + srv2.SetUrl(common::URI("http://localhost:8002/", true)); ModelIdentifier am1; am1.SetServer(srv1); @@ -349,8 +355,7 @@ TEST_F(LocalCacheTest, GZ_UTILS_TEST_DISABLED_ON_WIN32(MatchingModel)) ///////////////////////////////////////////////// /// \brief Iterate through all worlds in cache /// \brief Iterate through all models in cache -// See https://github.com/gazebosim/gz-fuel-tools/issues/307 -TEST_F(LocalCacheTest, GZ_UTILS_TEST_DISABLED_ON_WIN32(AllWorlds)) +TEST_F(LocalCacheTest, AllWorlds) { ClientConfig conf; conf.SetCacheLocation(common::joinPaths(common::cwd(), "test_cache")); @@ -367,20 +372,14 @@ TEST_F(LocalCacheTest, GZ_UTILS_TEST_DISABLED_ON_WIN32(AllWorlds)) ++iter; } EXPECT_EQ(9u, uniqueNames.size()); -#ifdef _WIN32 - EXPECT_NE(uniqueNames.end(), uniqueNames.find( - gz::common::joinPaths("localhost8001", "alice", "worlds", "am1"))); -#else EXPECT_NE(uniqueNames.end(), uniqueNames.find( - gz::common::joinPaths("localhost:8001", "alice", "worlds", "am1"))); -#endif + sanitizeAuthority("localhost:8001") + "/alice/worlds/am1")); } ///////////////////////////////////////////////// /// \brief Get all worlds that match some fields /// \brief Iterate through all models in cache -// See https://github.com/gazebosim/gz-fuel-tools/issues/307 -TEST_F(LocalCacheTest, GZ_UTILS_TEST_DISABLED_ON_WIN32(MatchingWorlds)) +TEST_F(LocalCacheTest, MatchingWorlds) { ClientConfig conf; conf.Clear(); @@ -411,8 +410,7 @@ TEST_F(LocalCacheTest, GZ_UTILS_TEST_DISABLED_ON_WIN32(MatchingWorlds)) ///////////////////////////////////////////////// /// \brief Get a specific world from cache /// \brief Iterate through all models in cache -// See https://github.com/gazebosim/gz-fuel-tools/issues/307 -TEST_F(LocalCacheTest, GZ_UTILS_TEST_DISABLED_ON_WIN32(MatchingWorld)) +TEST_F(LocalCacheTest, MatchingWorld) { ClientConfig conf; conf.SetCacheLocation(common::joinPaths(common::cwd(), "test_cache")); @@ -421,10 +419,10 @@ TEST_F(LocalCacheTest, GZ_UTILS_TEST_DISABLED_ON_WIN32(MatchingWorld)) gz::fuel_tools::LocalCache cache(&conf); gz::fuel_tools::ServerConfig srv1; - srv1.SetUrl(gz::common::URI("http://localhost:8001/")); + srv1.SetUrl(gz::common::URI("http://localhost:8001/", true)); gz::fuel_tools::ServerConfig srv2; - srv2.SetUrl(gz::common::URI("http://localhost:8002/")); + srv2.SetUrl(gz::common::URI("http://localhost:8002/", true)); WorldIdentifier am1; am1.SetServer(srv1); diff --git a/src/ModelIdentifier.cc b/src/ModelIdentifier.cc index 45df4b4d..39c536e0 100644 --- a/src/ModelIdentifier.cc +++ b/src/ModelIdentifier.cc @@ -21,6 +21,7 @@ #include #include +#include #include #include "gz/fuel_tools/ClientConfig.hh" @@ -149,9 +150,10 @@ ModelIdentifier::~ModelIdentifier() ////////////////////////////////////////////////// std::string ModelIdentifier::UniqueName() const { - return common::joinPaths(this->dataPtr->server.Url().Str(), - this->dataPtr->owner, "models", - this->dataPtr->name); + return common::copyToUnixPath(common::joinPaths( + uriToPath(this->dataPtr->server.Url()), + this->dataPtr->owner, "models", + this->dataPtr->name)); } ////////////////////////////////////////////////// diff --git a/src/ModelIdentifier_TEST.cc b/src/ModelIdentifier_TEST.cc index 8e27299d..deabbcd3 100644 --- a/src/ModelIdentifier_TEST.cc +++ b/src/ModelIdentifier_TEST.cc @@ -56,32 +56,28 @@ TEST(ModelIdentifier, SetFields) ///////////////////////////////////////////////// /// \brief Unique Name -// See https://github.com/gazebosim/gz-fuel-tools/issues/231 -TEST(ModelIdentifier, GZ_UTILS_TEST_DISABLED_ON_WIN32(UniqueName)) +TEST(ModelIdentifier, UniqueName) { gz::fuel_tools::ServerConfig srv1; - srv1.SetUrl(common::URI("https://localhost:8001")); + srv1.SetUrl(common::URI("https://localhost:8001", true)); gz::fuel_tools::ServerConfig srv2; - srv2.SetUrl(common::URI("https://localhost:8002")); + srv2.SetUrl(common::URI("https://localhost:8002", true)); gz::fuel_tools::ServerConfig srv3; - srv3.SetUrl(common::URI("https://localhost:8003")); + srv3.SetUrl(common::URI("https://localhost:8003", true)); ModelIdentifier id; id.SetName("hello"); id.SetOwner("alice"); id.SetServer(srv1); - EXPECT_EQ(common::joinPaths("https://localhost:8001", "alice", "models", - "hello"), id.UniqueName()); + EXPECT_EQ("localhost%3A8001/alice/models/hello", id.UniqueName()); id.SetServer(srv2); - EXPECT_EQ(common::joinPaths("https://localhost:8002", "alice", "models", - "hello"), id.UniqueName()); + EXPECT_EQ("localhost%3A8002/alice/models/hello", id.UniqueName()); id.SetServer(srv3); - EXPECT_EQ(common::joinPaths("https://localhost:8003", "alice", "models", - "hello"), id.UniqueName()); + EXPECT_EQ("localhost%3A8003/alice/models/hello", id.UniqueName()); } ///////////////////////////////////////////////// @@ -156,7 +152,7 @@ TEST(ModelIdentifier, AsString) "Name: \n"\ "Owner: \n"\ "Version: tip\n"\ - "Unique name: https://fuel.gazebosim.org/models/\n" + "Unique name: fuel.gazebosim.org/models/\n" "Description: \n" "File size: 0\n" "Upload date: 0\n" diff --git a/src/WorldIdentifier.cc b/src/WorldIdentifier.cc index 9c9296a0..6af8c802 100644 --- a/src/WorldIdentifier.cc +++ b/src/WorldIdentifier.cc @@ -20,6 +20,7 @@ #include #include +#include #include #include "gz/fuel_tools/ClientConfig.hh" @@ -80,10 +81,10 @@ WorldIdentifier::~WorldIdentifier() ////////////////////////////////////////////////// std::string WorldIdentifier::UniqueName() const { - return common::joinPaths(uriToPath(this->dataPtr->server.Url()), - this->dataPtr->owner, - "worlds", - this->dataPtr->name); + return common::copyToUnixPath(common::joinPaths( + uriToPath(this->dataPtr->server.Url()), + this->dataPtr->owner, "worlds", + this->dataPtr->name)); } ////////////////////////////////////////////////// @@ -228,4 +229,3 @@ std::string WorldIdentifier::AsPrettyString(const std::string &_prefix) const << this->Server().AsPrettyString(_prefix + " "); return out.str(); } - diff --git a/src/WorldIdentifier_TEST.cc b/src/WorldIdentifier_TEST.cc index 947d8fd1..91c2d963 100644 --- a/src/WorldIdentifier_TEST.cc +++ b/src/WorldIdentifier_TEST.cc @@ -44,29 +44,26 @@ TEST(WorldIdentifier, SetFields) TEST(WorldIdentifier, UniqueName) { gz::fuel_tools::ServerConfig srv1; - srv1.SetUrl(gz::common::URI("https://localhost:8001/")); + srv1.SetUrl(gz::common::URI("https://localhost:8001/", true)); gz::fuel_tools::ServerConfig srv2; - srv2.SetUrl(gz::common::URI("https://localhost:8002")); + srv2.SetUrl(gz::common::URI("https://localhost:8002", true)); gz::fuel_tools::ServerConfig srv3; - - srv3.SetUrl(gz::common::URI("https://localhost:8003/")); + srv3.SetUrl(gz::common::URI("https://localhost:8003/", true)); WorldIdentifier id; id.SetName("hello"); id.SetOwner("alice"); + id.SetServer(srv1); - EXPECT_EQ(common::joinPaths("localhost:8001", "alice", "worlds", "hello"), - id.UniqueName()); + EXPECT_EQ("localhost%3A8001/alice/worlds/hello", id.UniqueName()); id.SetServer(srv2); - EXPECT_EQ(common::joinPaths("localhost:8002", "alice", "worlds", "hello"), - id.UniqueName()); + EXPECT_EQ("localhost%3A8002/alice/worlds/hello", id.UniqueName()); id.SetServer(srv3); - EXPECT_EQ(common::joinPaths("localhost:8003", "alice", "worlds", "hello"), - id.UniqueName()); + EXPECT_EQ("localhost%3A8003/alice/worlds/hello", id.UniqueName()); } ///////////////////////////////////////////////// @@ -115,7 +112,6 @@ TEST(WorldIdentifier, AsString) common::Console::SetVerbosity(4); { WorldIdentifier id; -#ifndef _WIN32 std::string str = "Name: \n"\ "Owner: \n"\ @@ -126,18 +122,6 @@ TEST(WorldIdentifier, AsString) " URL: https://fuel.gazebosim.org\n" " Version: 1.0\n" " API key: \n"; -#else - std::string str = - "Name: \n"\ - "Owner: \n"\ - "Version: tip\n"\ - "Unique name: fuel.gazebosim.org\\worlds\\\n" - "Local path: \n" - "Server:\n" - " URL: https://fuel.gazebosim.org\n" - " Version: 1.0\n" - " API key: \n"; -#endif EXPECT_EQ(str, id.AsString()); }