Skip to content

Commit

Permalink
Fixed windows download (#178)
Browse files Browse the repository at this point in the history 
* Fixed donwload on Windows

Signed-off-by: ahcorde <ahcorde@gmail.com>

* Fixed interface_TEST

Signed-off-by: ahcorde <ahcorde@gmail.com>

* Improved windows support

Signed-off-by: ahcorde <ahcorde@gmail.com>

* Make linters happy

Signed-off-by: ahcorde <ahcorde@gmail.com>

* Make linters happy

Signed-off-by: ahcorde <ahcorde@gmail.com>

* Improved

Signed-off-by: ahcorde <ahcorde@gmail.com>

* Fixed test on Windows

Signed-off-by: Alejandro Hernández <ahcorde@gmail.com>

* Fixed test

Signed-off-by: ahcorde <ahcorde@gmail.com>

* Fix some nits

Signed-off-by: ahcorde <ahcorde@gmail.com>

* Improved Windows support

Signed-off-by: ahcorde <ahcorde@gmail.com>

* Fixed test on Linux

Signed-off-by: ahcorde <ahcorde@gmail.com>

* make linters happy

Signed-off-by: ahcorde <ahcorde@gmail.com>

* Fixed windows tests

Signed-off-by: ahcorde <ahcorde@gmail.com>

* Fixed tests

Signed-off-by: ahcorde <ahcorde@gmail.com>

* Added feddback

Signed-off-by: ahcorde <ahcorde@gmail.com>

* make linters happy

Signed-off-by: ahcorde <ahcorde@gmail.com>
  • Loading branch information
@ahcorde
ahcorde authored May 14, 2021
1 parent 4c9b3c7 commit 8674ed8
Show file tree
Hide file tree
Showing 16 changed files with 222 additions and 175 deletions.
28 changes: 23 additions & 5 deletions src/ClientConfig_TEST.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,20 @@ std::string homePath()
return homePath;
}

/////////////////////////////////////////////////
/// \brief Get cache directory.
/// \return Cache directory
/// \ToDo: Move this function to ignition::common::Filesystem
std::string cachePath()
{
std::string cachePath;
#ifndef _WIN32
return std::string("/tmp/ignition/fuel");
#else
return std::string("C:\\Windows\\Temp");
#endif
}

/////////////////////////////////////////////////
/// \brief Initially only the default server in config
TEST(ClientConfig, InitiallyDefaultServers)
Expand Down Expand Up @@ -115,8 +129,9 @@ TEST(ClientConfig, CustomConfiguration)
<< "" << std::endl
<< "# Where are the assets stored in disk." << std::endl
<< "cache:" << std::endl
<< " path: /tmp/ignition/fuel" << std::endl
<< " path: " + cachePath() << std::endl
<< std::endl;
ofs.close();

EXPECT_TRUE(config.LoadConfig(testPath));

Expand All @@ -128,8 +143,7 @@ TEST(ClientConfig, CustomConfiguration)
EXPECT_EQ("https://myserver",
config.Servers().back().Url().Str());

EXPECT_EQ("/tmp/ignition/fuel", config.CacheLocation());

EXPECT_EQ(cachePath(), config.CacheLocation());
// Remove the configuration file.
removeFileTemp(testPath);
}
Expand All @@ -156,8 +170,9 @@ TEST(ClientConfig, RepeatedServerConfiguration)
<< "" << std::endl
<< "# Where are the assets stored in disk." << std::endl
<< "cache:" << std::endl
<< " path: /tmp/ignition/fuel" << std::endl
<< " path: " + cachePath() << std::endl
<< std::endl;
ofs.close();

EXPECT_TRUE(config.LoadConfig(testPath));

Expand All @@ -182,6 +197,7 @@ TEST(ClientConfig, NoServerUrlConfiguration)
<< " -" << std::endl
<< " banana: coconut" << std::endl
<< std::endl;
ofs.close();

EXPECT_FALSE(config.LoadConfig(testPath));

Expand All @@ -206,6 +222,7 @@ TEST(ClientConfig, EmptyServerUrlConfiguration)
<< " -" << std::endl
<< " url: " << std::endl
<< std::endl;
ofs.close();

EXPECT_FALSE(config.LoadConfig(testPath));

Expand All @@ -227,6 +244,7 @@ TEST(ClientConfig, NoCachePathConfiguration)
ofs << "---" << std::endl
<< "cache:" << std::endl
<< std::endl;
ofs.close();

EXPECT_FALSE(config.LoadConfig(testPath));

Expand All @@ -249,6 +267,7 @@ TEST(ClientConfig, EmptyCachePathConfiguration)
<< "cache:" << std::endl
<< " path:" << std::endl
<< std::endl;
ofs.close();

EXPECT_FALSE(config.LoadConfig(testPath));

Expand Down Expand Up @@ -417,4 +436,3 @@ int main(int argc, char **argv)
::testing::InitGoogleTest(&argc, argv);
return RUN_ALL_TESTS();
}

1 change: 0 additions & 1 deletion src/CollectionIdentifier.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,4 +156,3 @@ std::string CollectionIdentifier::AsPrettyString(
<< this->Server().AsPrettyString(_prefix + " ");
return out.str();
}

19 changes: 3 additions & 16 deletions src/CollectionIdentifier_TEST.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ TEST(CollectionIdentifier, UniqueName)
CollectionIdentifier id;
id.SetName("hello");
id.SetOwner("alice");
#ifndef _WIN32
id.SetServer(srv1);
EXPECT_EQ("https://localhost:8001/alice/collections/hello", id.UniqueName());

Expand All @@ -66,19 +65,6 @@ TEST(CollectionIdentifier, UniqueName)

id.SetServer(srv3);
EXPECT_EQ("https://localhost:8003/alice/collections/hello", id.UniqueName());
#else
id.SetServer(srv1);
EXPECT_EQ("https://localhost:8001\\alice\\collections\\hello",
id.UniqueName());

id.SetServer(srv2);
EXPECT_EQ("https://localhost:8002\\alice\\collections\\hello",
id.UniqueName());

id.SetServer(srv3);
EXPECT_EQ("https://localhost:8003\\alice\\collections\\hello",
id.UniqueName());
#endif
}

/////////////////////////////////////////////////
Expand Down Expand Up @@ -132,11 +118,12 @@ TEST(CollectionIdentifier, AsString)
common::Console::SetVerbosity(4);
{
CollectionIdentifier id;

#ifndef _WIN32
std::string str =
"Name: \n"\
"Owner: \n"\
"Unique name: https://fuel.ignitionrobotics.org//collections/\n"
"Unique name: https://fuel.ignitionrobotics.org/collections/\n"
"Server:\n"
" URL: https://fuel.ignitionrobotics.org\n"
" Version: 1.0\n"
Expand All @@ -145,7 +132,7 @@ TEST(CollectionIdentifier, AsString)
std::string str =
"Name: \n"\
"Owner: \n"\
"Unique name: https://fuel.ignitionrobotics.org\\collections\n"
"Unique name: https://fuel.ignitionrobotics.org/collections\n"
"Server:\n"
" URL: https://fuel.ignitionrobotics.org\n"
" Version: 1.0\n"
Expand Down
7 changes: 7 additions & 0 deletions src/FuelClient.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1018,6 +1018,7 @@ bool FuelClient::ParseWorldFileUrl(const common::URI &_fileUrl,

return true;
}

//////////////////////////////////////////////////
bool FuelClient::ParseCollectionUrl(const common::URI &_url,
CollectionIdentifier &_id)
Expand Down Expand Up @@ -1230,6 +1231,12 @@ Result FuelClient::CachedModelFile(const common::URI &_fileUrl,
// Check if file exists
filePath = common::joinPaths(modelPath, filePath);

std::vector<std::string> tokens = ignition::common::split(filePath, "/");
std::string sTemp;
for (auto s : tokens)
sTemp = ignition::common::joinPaths(sTemp, s);
filePath = sTemp;

if (common::exists(filePath))
{
_path = filePath;
Expand Down
12 changes: 6 additions & 6 deletions src/FuelClient_TEST.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ TEST_F(FuelClientTest, ParseModelFileURL)
/////////////////////////////////////////////////
// Protocol "https" not supported or disabled in libcurl for Windows
// https://github.com/ignitionrobotics/ign-fuel-tools/issues/105
TEST_F(FuelClientTest, IGN_UTILS_TEST_DISABLED_ON_WIN32(DownloadModel))
TEST_F(FuelClientTest, DownloadModel)
{
// Configure to use binary path as cache
ASSERT_EQ(0, ChangeDirectory(PROJECT_BINARY_PATH));
Expand Down Expand Up @@ -593,7 +593,7 @@ TEST_F(FuelClientTest, IGN_UTILS_TEST_DISABLED_ON_WIN32(DownloadModel))
/////////////////////////////////////////////////
// Windows doesn't support colons in filenames
// https://github.com/ignitionrobotics/ign-fuel-tools/issues/106
TEST_F(FuelClientTest, IGN_UTILS_TEST_DISABLED_ON_WIN32(CachedModel))
TEST_F(FuelClientTest, CachedModel)
{
// Configure to use binary path as cache and populate it
ASSERT_EQ(0, ChangeDirectory(PROJECT_BINARY_PATH));
Expand Down Expand Up @@ -983,7 +983,7 @@ TEST_F(FuelClientTest, ParseWorldFileUrl)
//////////////////////////////////////////////////
// Protocol "https" not supported or disabled in libcurl for Windows
// https://github.com/ignitionrobotics/ign-fuel-tools/issues/105
TEST_F(FuelClientTest, IGN_UTILS_TEST_DISABLED_ON_WIN32(DownloadWorld))
TEST_F(FuelClientTest, DownloadWorld)
{
// Configure to use binary path as cache
ASSERT_EQ(0, ChangeDirectory(PROJECT_BINARY_PATH));
Expand Down Expand Up @@ -1061,7 +1061,7 @@ TEST_F(FuelClientTest, IGN_UTILS_TEST_DISABLED_ON_WIN32(DownloadWorld))
/////////////////////////////////////////////////
// Windows doesn't support colons in filenames
// https://github.com/ignitionrobotics/ign-fuel-tools/issues/106
TEST_F(FuelClientTest, IGN_UTILS_TEST_DISABLED_ON_WIN32(CachedWorld))
TEST_F(FuelClientTest, CachedWorld)
{
// Configure to use binary path as cache and populate it
ASSERT_EQ(0, ChangeDirectory(PROJECT_BINARY_PATH));
Expand Down Expand Up @@ -1251,7 +1251,7 @@ TEST_F(FuelClientTest, WorldDetails)
/////////////////////////////////////////////////
// Protocol "https" not supported or disabled in libcurl for Windows
// https://github.com/ignitionrobotics/ign-fuel-tools/issues/105
TEST_F(FuelClientTest, IGN_UTILS_TEST_DISABLED_ON_WIN32(Models))
TEST_F(FuelClientTest, Models)
{
ClientConfig config;
config.SetCacheLocation(common::joinPaths(common::cwd(), "test_cache"));
Expand Down Expand Up @@ -1285,7 +1285,7 @@ TEST_F(FuelClientTest, IGN_UTILS_TEST_DISABLED_ON_WIN32(Models))
/////////////////////////////////////////////////
// Protocol "https" not supported or disabled in libcurl for Windows
// https://github.com/ignitionrobotics/ign-fuel-tools/issues/105
TEST_F(FuelClientTest, IGN_UTILS_TEST_DISABLED_ON_WIN32(Worlds))
TEST_F(FuelClientTest, Worlds)
{
ClientConfig config;
config.SetCacheLocation(common::joinPaths(common::cwd(), "test_cache"));
Expand Down
5 changes: 3 additions & 2 deletions src/Interface.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ namespace ignition
auto modelUri = _uri.substr(0,
_uri.find("files", model.UniqueName().size())-1);
_client.DownloadModel(common::URI(modelUri), result);
result += "/" + fileUrl;
result = common::joinPaths(result, fileUrl);
}
// Download the world, if it is a world URI
else if (_client.ParseWorldUrl(uri, world) &&
Expand All @@ -68,7 +68,8 @@ namespace ignition
auto worldUri = _uri.substr(0,
_uri.find("files", world.UniqueName().size())-1);
_client.DownloadWorld(common::URI(worldUri), result);
result += "/" + fileUrl;
result = common::joinPaths(result, fileUrl);

}

return result;
Expand Down
Loading

0 comments on commit 8674ed8

Please sign in to comment.