From 7f756eea816650b56580ecfe9e2dcaf9eda253ae Mon Sep 17 00:00:00 2001 From: Nate Koenig Date: Wed, 29 Mar 2023 11:11:18 -0700 Subject: [PATCH 01/11] Support link referral download (#333) * Support downloading from a referal link Signed-off-by: Nate Koenig * spelling Signed-off-by: Nate Koenig * Address comments Signed-off-by: Nate Koenig --------- Signed-off-by: Nate Koenig --- src/FuelClient.cc | 84 ++++++++++++++++++++++++++++++++++++++++++++--- src/RestClient.cc | 29 +++++++++------- 2 files changed, 97 insertions(+), 16 deletions(-) diff --git a/src/FuelClient.cc b/src/FuelClient.cc index fe8150a6..cd0e52d9 100644 --- a/src/FuelClient.cc +++ b/src/FuelClient.cc @@ -34,6 +34,7 @@ #include #include +#include #include #include @@ -192,6 +193,11 @@ class gz::fuel_tools::FuelClientPrivate /// DEPRECATED/DEPRECATION: remove this function in Gazebo H. public: void CheckForDeprecatedUri(const common::URI &_uri); + /// \brief Get zip data from a REST response. This is used by world and + /// model download. + public: void ZipFromResponse(const RestResponse &_resp, + std::string &_zip); + /// \brief Client configuration public: ClientConfig config; @@ -623,7 +629,7 @@ Result FuelClient::DownloadModel(const ModelIdentifier &_id, Rest rest; RestResponse resp; resp = rest.Request(HttpMethod::GET, _id.Server().Url().Str(), - _id.Server().Version(), route.Str(), {}, + _id.Server().Version(), route.Str(), {"link=true"}, headersIncludingServerConfig, ""); if (resp.statusCode != 200) { @@ -657,9 +663,12 @@ Result FuelClient::DownloadModel(const ModelIdentifier &_id, } newId.SetVersion(version); + std::string zipData; + this->dataPtr->ZipFromResponse(resp, zipData); + // Save // Note that the save function doesn't return the path - if (!this->dataPtr->cache->SaveModel(newId, resp.data, true)) + if (zipData.empty() || !this->dataPtr->cache->SaveModel(newId, zipData, true)) return Result(ResultType::FETCH_ERROR); return this->ModelDependencies(_id, _dependencies); @@ -792,7 +801,7 @@ Result FuelClient::DownloadWorld(WorldIdentifier &_id) Rest rest; RestResponse resp; resp = rest.Request(HttpMethod::GET, _id.Server().Url().Str(), - _id.Server().Version(), route.Str(), {}, + _id.Server().Version(), route.Str(), {"link=true"}, headersIncludingServerConfig, ""); if (resp.statusCode != 200) { @@ -826,9 +835,12 @@ Result FuelClient::DownloadWorld(WorldIdentifier &_id) } _id.SetVersion(version); + std::string zipData; + this->dataPtr->ZipFromResponse(resp, zipData); + // Save - if (!this->dataPtr->cache->SaveWorld(_id, resp.data, true)) - return Result(ResultType::FETCH_ERROR); + if (zipData.empty() || !this->dataPtr->cache->SaveWorld(_id, zipData, true)) + return Result(ResultType::FETCH_ERROR); return Result(ResultType::FETCH); } @@ -1804,3 +1816,65 @@ void FuelClientPrivate::CheckForDeprecatedUri(const common::URI &_uri) } } +////////////////////////////////////////////////// +void FuelClientPrivate::ZipFromResponse(const RestResponse &_resp, + std::string &_zip) +{ + // Check the content-type which could be empty (ideally not): + // * text/plain indicates the data is a download link. + // * application/zip indicates the data is a zip file. + // * binary/octet-stream indicates the data is a zip file. + auto contentTypeIter = _resp.headers.find("Content-Type"); + if (contentTypeIter != _resp.headers.end()) + { + // If text/plain then data might be a link to a zip file. + if (contentTypeIter->second.find("text/plain") != std::string::npos) + { + std::string linkUri = _resp.data; + + // Check for valid URI + if (common::URI::Valid(linkUri)) + { + igndbg << "Downloading from a referral link [" << linkUri << "]\n"; + // Get the zip data. + RestResponse linkResp = rest.Request(HttpMethod::GET, + // URL + linkUri, + // Version + "", + // Path + "", + // Query strings + {}, + // Headers + {}, + // Data + ""); + + return this->ZipFromResponse(linkResp, _zip); + } + else + { + ignerr << "Invalid referral link URI [" << linkUri << "]. " + << "Unable to download.\n"; + } + } + else if (contentTypeIter->second.find("application/zip") != + std::string::npos || + contentTypeIter->second.find("binary/octet-stream") != + std::string::npos) + { + _zip = std::move(_resp.data); + } + else + { + ignerr << "Invalid content-type of [" << contentTypeIter->second << "]. " + << "Unable to download.\n"; + } + } + else + { + // If content-type is missing, then assume the data is the zip file. + _zip = std::move(_resp.data); + } +} diff --git a/src/RestClient.cc b/src/RestClient.cc index 85df36aa..62827ac9 100644 --- a/src/RestClient.cc +++ b/src/RestClient.cc @@ -212,19 +212,25 @@ RestResponse Rest::Request(HttpMethod _method, if (_url.empty()) return res; - std::string url = RestJoinUrl(_url, _version); + std::string url = _url; + if (!_version.empty()) + url = RestJoinUrl(_url, _version); CURL *curl = curl_easy_init(); + char *encodedPath = nullptr; - // First, unescape the _path since it might have %XX encodings. If this - // step is not performed, then curl_easy_escape will encode the %XX - // encodings resulting in an incorrect URL. - int decodedSize; - char *decodedPath = curl_easy_unescape(curl, - _path.c_str(), _path.size(), &decodedSize); - - char *encodedPath = curl_easy_escape(curl, decodedPath, decodedSize); - url = RestJoinUrl(url, encodedPath); + if (!_path.empty()) + { + // First, unescape the _path since it might have %XX encodings. If this + // step is not performed, then curl_easy_escape will encode the %XX + // encodings resulting in an incorrect URL. + int decodedSize; + char *decodedPath = curl_easy_unescape(curl, + _path.c_str(), _path.size(), &decodedSize); + + encodedPath = curl_easy_escape(curl, decodedPath, decodedSize); + url = RestJoinUrl(url, encodedPath); + } // Process query strings. if (!_queryStrings.empty()) @@ -358,7 +364,8 @@ RestResponse Rest::Request(HttpMethod _method, curl_formfree(formpost); // free encoded path char* - curl_free(encodedPath); + if (encodedPath) + curl_free(encodedPath); // free the headers curl_slist_free_all(headers); From d8ce77b4b3c5155228c024d1c1748b0b976ecade Mon Sep 17 00:00:00 2001 From: Nate Koenig Date: Wed, 29 Mar 2023 12:54:22 -0700 Subject: [PATCH 02/11] 4.8.3 release (#342) * 4.8.3 release Signed-off-by: Nate Koenig * 4.8.3 Signed-off-by: Nate Koenig --------- Signed-off-by: Nate Koenig --- CMakeLists.txt | 2 +- Changelog.md | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 877946db..49fb69fc 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -3,7 +3,7 @@ cmake_minimum_required(VERSION 3.5.1 FATAL_ERROR) #============================================================================ # Initialize the project #============================================================================ -project(ignition-fuel_tools4 VERSION 4.8.2) +project(ignition-fuel_tools4 VERSION 4.8.3) #============================================================================ # Find ignition-cmake diff --git a/Changelog.md b/Changelog.md index 288ca98e..0566ebe1 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,5 +1,10 @@ ## Gazebo Fuel Tools 4.x +### Gazebo Fuel Tools 4.8.3 (2023-03-29) + +1. Support link referral download + * [Pull request #333](https://github.com/gazebosim/gz-fuel-tools/pull/333) + ### Gazebo Fuel Tools 4.8.2 (2023-03-21) 1. Allow Curl redirect on get. From fe66f33b3235a70855a43e79437653612cea1897 Mon Sep 17 00:00:00 2001 From: "Addisu Z. Taddese" Date: Tue, 25 Apr 2023 19:08:44 -0500 Subject: [PATCH 03/11] Reflect pagination of RESTful requests in iterator API (#350) The ModelIter iterator was fetching all available pages before making the first model available. This PR makes it so that each page is fetched from Fuel when the iterator is advanced. This also adds a new member function FuelClient::Models(onst ModelIdentifier &_id, bool _checkCache) that allows bypassing the cache when getting a list of models owned by a user from the server. --------- Signed-off-by: Addisu Z. Taddese --- include/gz/fuel_tools/FuelClient.hh | 37 ++++++++--- include/gz/fuel_tools/ModelIterPrivate.hh | 20 +++++- src/FuelClient.cc | 27 ++++++-- src/FuelClient_TEST.cc | 46 +++++++++++++ src/ModelIter.cc | 78 ++++++++++------------- 5 files changed, 148 insertions(+), 60 deletions(-) diff --git a/include/gz/fuel_tools/FuelClient.hh b/include/gz/fuel_tools/FuelClient.hh index 7a7dec15..a9d92100 100644 --- a/include/gz/fuel_tools/FuelClient.hh +++ b/include/gz/fuel_tools/FuelClient.hh @@ -132,25 +132,46 @@ namespace ignition /// \brief Returns models matching a given identifying criteria /// \param[in] _id a partially filled out identifier used to fetch models /// \remarks Fulfills Get-One requirement - /// \remarks It's not yet clear if model names are unique, so this API + /// \remarks Model names are unique to the owner, so this API /// allows the possibility of getting multiple models with the - /// same name. + /// same name if the owner is not specified. /// \return An iterator of models with names matching the criteria public: ModelIter Models(const ModelIdentifier &_id); /// \brief Returns models matching a given identifying criteria /// \param[in] _id a partially filled out identifier used to fetch models /// \remarks Fulfills Get-One requirement - /// \remarks It's not yet clear if model names are unique, so this API + /// \remarks Model names are unique to the owner, so this API /// allows the possibility of getting multiple models with the - /// same name. + /// same name if the owner is not specified. /// \return An iterator of models with names matching the criteria public: ModelIter Models(const ModelIdentifier &_id) const; - /// \brief Returns an iterator for the models found in a collection. - /// \param[in] _id a partially filled out identifier used to fetch a - /// collection. - /// \return An iterator of models in the collection. + /// \brief Returns models matching a given identifying criteria + /// \param[in] _id a partially filled out identifier used to fetch models + /// \param[in] _checkCache Whether to check the cache. + /// \remarks Fulfills Get-One requirement + /// \remarks Model names are unique to the owner, so this API + /// allows the possibility of getting multiple models with the + /// same name if the owner is not specified. + /// \return An iterator of models with names matching the criteria + public: ModelIter Models(const ModelIdentifier &_id, bool _checkCache); + + /// \brief Returns models matching a given identifying criteria + /// \param[in] _id a partially filled out identifier used to fetch models + /// \param[in] _checkCache Whether to check the cache. + /// \remarks Fulfills Get-One requirement + /// \remarks Model names are unique to the owner, so this API + /// allows the possibility of getting multiple models with the + /// same name if the owner is not specified. + /// \return An iterator of models with names matching the criteria + public: ModelIter Models(const ModelIdentifier &_id, + bool _checkCache) const; + + /// \brief Returns an iterator for the models found in a collection. + /// \param[in] _id a partially filled out identifier used to fetch a + /// collection. + /// \return An iterator of models in the collection. public: ModelIter Models(const CollectionIdentifier &_id) const; /// \brief Returns worlds matching a given identifying criteria diff --git a/include/gz/fuel_tools/ModelIterPrivate.hh b/include/gz/fuel_tools/ModelIterPrivate.hh index 52200c7a..36693854 100644 --- a/include/gz/fuel_tools/ModelIterPrivate.hh +++ b/include/gz/fuel_tools/ModelIterPrivate.hh @@ -130,7 +130,7 @@ namespace ignition }; /// \brief class for iterating through model ids from a rest API - class IGNITION_FUEL_TOOLS_VISIBLE IterRestIds: public ModelIterPrivate + class IGNITION_FUEL_TOOLS_HIDDEN IterRestIds: public ModelIterPrivate { /// \brief constructor public: IterRestIds(const Rest &_rest, @@ -153,11 +153,29 @@ namespace ignition /// \brief RESTful client public: Rest rest; + /// \brief The API (path) of the RESTful requests + public: const std::string api; + + /// \brief Make a RESTful request for the given page + /// \param[in] _page Page number to request + /// \return Response from the request + protected: RestResponse MakeRestRequest(std::size_t _page); + + /// \brief Parse model identifiers from a RESTful response + /// \param[in] _resp RESTful response + /// \return A vector of identifiers extracted from the response. + protected: std::vector ParseIdsFromResponse( + const RestResponse &_resp); + /// \brief Model identifiers in the current page protected: std::vector ids; /// \brief Where the current iterator is in the list of ids protected: std::vector::iterator idIter; + + /// \brief Keep track of page number for pagination of response data from + /// server. + protected: std::size_t currentPage{0}; }; } } diff --git a/src/FuelClient.cc b/src/FuelClient.cc index cd0e52d9..58f4b4fe 100644 --- a/src/FuelClient.cc +++ b/src/FuelClient.cc @@ -378,16 +378,31 @@ WorldIter FuelClient::Worlds(const ServerConfig &_server) const ////////////////////////////////////////////////// ModelIter FuelClient::Models(const ModelIdentifier &_id) { - return const_cast(this)->Models(_id); + return this->Models(_id, true); } ////////////////////////////////////////////////// ModelIter FuelClient::Models(const ModelIdentifier &_id) const { - // Check local cache first - ModelIter localIter = this->dataPtr->cache->MatchingModels(_id); - if (localIter) - return localIter; + return this->Models(_id, true); +} + +////////////////////////////////////////////////// +ModelIter FuelClient::Models(const ModelIdentifier &_id, bool _checkCache) +{ + return const_cast(this)->Models(_id, _checkCache); +} + +////////////////////////////////////////////////// +ModelIter FuelClient::Models(const ModelIdentifier &_id, bool _checkCache) const +{ + if (_checkCache) + { + // Check local cache first + ModelIter localIter = this->dataPtr->cache->MatchingModels(_id); + if (localIter) + return localIter; + } // TODO(nkoenig) try to fetch model directly from a server // Note: ign-fuel-server doesn't like URLs ending in / @@ -398,7 +413,7 @@ ModelIter FuelClient::Models(const ModelIdentifier &_id) const path = path / _id.Owner() / "models"; if (path.Str().empty()) - return localIter; + return ModelIterFactory::Create(); ignmsg << _id.UniqueName() << " not found in cache, attempting download\n"; diff --git a/src/FuelClient_TEST.cc b/src/FuelClient_TEST.cc index 6dde9259..f4259dbd 100644 --- a/src/FuelClient_TEST.cc +++ b/src/FuelClient_TEST.cc @@ -1384,6 +1384,52 @@ TEST_F(FuelClientTest, Models) } } +////////////////////////////////////////////////// +TEST_F(FuelClientTest, ModelsCheckCached) +{ + ClientConfig config; + std::string cacheDir = common::joinPaths(PROJECT_BINARY_PATH, "test_cache"); + common::removeAll(cacheDir ); + ASSERT_TRUE(common::createDirectories(cacheDir)); + config.SetCacheLocation(cacheDir); + FuelClient client(config); + ServerConfig serverConfig; + ModelIdentifier modelId; + modelId.SetOwner("openroboticstest"); + std::vector modelInfos; + { + for (ModelIter iter = client.Models(modelId, true); iter; ++iter) + { + modelInfos.push_back(*iter); + } + } + ASSERT_FALSE(modelInfos.empty()); + // Download one of the models to test the behavior of Models() with + // different values for _checkCache + EXPECT_TRUE(client.DownloadModel(modelInfos.front().Identification())); + { + std::size_t counter = 0; + for (ModelIter iter = client.Models(modelId, true); iter; ++iter, ++counter) + { + } + std::cout << "counter: " << counter << std::endl; + // Expect only one result with checkCache=true because we've downloaded only + // one model + EXPECT_EQ(counter, 1u); + EXPECT_GT(modelInfos.size(), counter); + } + + { + std::size_t counter = 0; + for (ModelIter iter = client.Models(modelId, false); iter; + ++iter, ++counter) + { + } + std::cout << "counter: " << counter << std::endl; + EXPECT_EQ(counter, modelInfos.size()); + } +} + ///////////////////////////////////////////////// // Protocol "https" not supported or disabled in libcurl for Windows // https://github.com/gazebosim/gz-fuel-tools/issues/105 diff --git a/src/ModelIter.cc b/src/ModelIter.cc index e65832f2..66294b6a 100644 --- a/src/ModelIter.cc +++ b/src/ModelIter.cc @@ -147,65 +147,55 @@ IterRestIds::~IterRestIds() { } +std::vector IterRestIds::ParseIdsFromResponse( + const RestResponse &resp) +{ + if (resp.data == "null\n" || resp.statusCode != 200) + return {}; + + // Parse the response. + return JSONParser::ParseModels(resp.data, this->config); +} + ////////////////////////////////////////////////// IterRestIds::IterRestIds(const Rest &_rest, const ServerConfig &_config, const std::string &_api) - : config(_config), rest(_rest) + : config(_config), rest(_rest), api(_api) +{ + this->idIter = this->ids.begin(); + this->Next(); +} + +////////////////////////////////////////////////// +RestResponse IterRestIds::MakeRestRequest(std::size_t _page) { HttpMethod method = HttpMethod::GET; - this->config = _config; - int page = 1; std::vector headers = {"Accept: application/json"}; - RestResponse resp; std::vector modelIds; - this->ids.clear(); - - do - { - // Prepare the request with the next page. - std::string queryStrPage = "page=" + std::to_string(page); - std::string path = _api; - ++page; - - // Fire the request. - resp = this->rest.Request(method, this->config.Url().Str(), + // Prepare the request with the requested page. + std::string queryStrPage = "page=" + std::to_string(_page); + std::string path = this->api; + // Fire the request. + return this->rest.Request(method, this->config.Url().Str(), this->config.Version(), std::regex_replace(path, std::regex(R"(\\)"), "/"), {queryStrPage}, headers, ""); - - // TODO(nkoenig): resp.statusCode should return != 200 when the page - // requested does - // not exist. When this happens we should stop without calling ParseModels() - if (resp.data == "null\n" || resp.statusCode != 200) - break; - - // Parse the response. - modelIds = JSONParser::ParseModels(resp.data, this->config); - - // Add the vector of models to the list. - this->ids.insert(std::end(this->ids), std::begin(modelIds), - std::end(modelIds)); - } while (!modelIds.empty()); - - if (this->ids.empty()) - return; - - this->idIter = this->ids.begin(); - - // make first model - std::shared_ptr ptr(new ModelPrivate); - ptr->id = *(this->idIter); - ptr->id.SetServer(this->config); - this->model = Model(ptr); - - igndbg << "Got response [" << resp.data << "]\n"; } ////////////////////////////////////////////////// void IterRestIds::Next() { // advance pointer - ++(this->idIter); + if (this->idIter != this->ids.end()) + ++(this->idIter); + + if (this->idIter == this->ids.end()) + { + ++this->currentPage; + RestResponse resp = this->MakeRestRequest(this->currentPage); + this->ids = this->ParseIdsFromResponse(resp); + this->idIter = this->ids.begin(); + } // Update personal model class if (this->idIter != this->ids.end()) @@ -215,7 +205,6 @@ void IterRestIds::Next() ptr->id.SetServer(this->config); this->model = Model(ptr); } - // TODO(nkoenig) request next page if api is paginated } ////////////////////////////////////////////////// @@ -256,7 +245,6 @@ ModelIter::operator bool() const ////////////////////////////////////////////////// ModelIter &ModelIter::operator++() { - // TODO(nkoenig) Request more data if there are more pages if (!this->dataPtr->HasReachedEnd()) { this->dataPtr->Next(); From e51c672a9d6fb13c83025a283a7747de0d6c43b7 Mon Sep 17 00:00:00 2001 From: "Addisu Z. Taddese" Date: Tue, 2 May 2023 09:58:07 -0500 Subject: [PATCH 04/11] Add bash completion (#329) Signed-off-by: Addisu Z. Taddese --- CMakeLists.txt | 1 + src/cmd/CMakeLists.txt | 62 +++++++++-- src/cmd/cmdfuel.rb.in | 14 ++- src/cmd/fuel.bash_completion.sh | 191 ++++++++++++++++++++++++++++++++ 4 files changed, 256 insertions(+), 12 deletions(-) create mode 100755 src/cmd/fuel.bash_completion.sh diff --git a/CMakeLists.txt b/CMakeLists.txt index 49fb69fc..545ab1e3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -68,6 +68,7 @@ ign_find_package(ignition-tools QUIET) if (ignition-tools_FOUND) set (HAVE_IGN_TOOLS TRUE) endif() +set (IGN_TOOLS_VER 1) #============================================================================ # Configure the build diff --git a/src/cmd/CMakeLists.txt b/src/cmd/CMakeLists.txt index 083a7588..788bd694 100644 --- a/src/cmd/CMakeLists.txt +++ b/src/cmd/CMakeLists.txt @@ -1,16 +1,60 @@ -# Generate a the ruby script. +#=============================================================================== +# Generate the ruby script for internal testing. # Note that the major version of the library is included in the name. # Ex: cmdfuel0.rb -if (APPLE) - set(IGN_LIBRARY_NAME lib${PROJECT_NAME_LOWER}.dylib) -else() - set(IGN_LIBRARY_NAME lib${PROJECT_NAME_LOWER}.so) -endif() +# Unlike other gz libraries, the ruby script for the fuel_tools library is called cmdfuel.rb instead of cmdfuel_tools.rb +set(CMD_NAME cmdfuel) +set(cmd_script_generated_test "${CMAKE_BINARY_DIR}/test/lib/$/ruby/ignition/${CMD_NAME}${PROJECT_VERSION_MAJOR}.rb") +set(cmd_script_configured_test "${CMAKE_CURRENT_BINARY_DIR}/test_${CMD_NAME}${PROJECT_VERSION_MAJOR}.rb.configured") + +# Set the library_location variable to the full path of the library file within +# the build directory. +set(library_location "$") configure_file( - "cmdfuel.rb.in" - "${CMAKE_CURRENT_BINARY_DIR}/cmdfuel${PROJECT_VERSION_MAJOR}.rb" @ONLY) + "${CMD_NAME}.rb.in" + "${cmd_script_configured_test}" + @ONLY) + +file(GENERATE + OUTPUT "${cmd_script_generated_test}" + INPUT "${cmd_script_configured_test}") +#=============================================================================== +# Used for the installed version. +# Generate the ruby script that gets installed. +# Note that the major version of the library is included in the name. +# Ex: cmdfuel0.rb +set(cmd_script_generated "${CMAKE_CURRENT_BINARY_DIR}/$/${CMD_NAME}${PROJECT_VERSION_MAJOR}.rb") +set(cmd_script_configured "${CMAKE_CURRENT_BINARY_DIR}/${CMD_NAME}${PROJECT_VERSION_MAJOR}.rb.configured") + +# Set the library_location variable to the relative path to the library file +# within the install directory structure. +set(library_location "../../../${CMAKE_INSTALL_LIBDIR}/$") + +configure_file( + "${CMD_NAME}.rb.in" + "${cmd_script_configured}" + @ONLY) + +file(GENERATE + OUTPUT "${cmd_script_generated}" + INPUT "${cmd_script_configured}") + # Install the ruby command line library in an unversioned location. -install(FILES ${CMAKE_CURRENT_BINARY_DIR}/cmdfuel${PROJECT_VERSION_MAJOR}.rb DESTINATION lib/ruby/ignition) +install(FILES ${cmd_script_generated} DESTINATION lib/ruby/ignition) + + +#=============================================================================== +# Bash completion + +# Tack version onto and install the bash completion script +configure_file( + "fuel.bash_completion.sh" + "${CMAKE_CURRENT_BINARY_DIR}/fuel${PROJECT_VERSION_MAJOR}.bash_completion.sh" @ONLY) +install( + FILES + ${CMAKE_CURRENT_BINARY_DIR}/fuel${PROJECT_VERSION_MAJOR}.bash_completion.sh + DESTINATION + ${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_DATAROOTDIR}/gz/gz${IGN_TOOLS_VER}.completion.d) diff --git a/src/cmd/cmdfuel.rb.in b/src/cmd/cmdfuel.rb.in index 71288740..5ba6f3db 100755 --- a/src/cmd/cmdfuel.rb.in +++ b/src/cmd/cmdfuel.rb.in @@ -28,7 +28,7 @@ end require 'optparse' # Constants. -LIBRARY_NAME = '@IGN_LIBRARY_NAME@' +LIBRARY_NAME = '@library_location@' LIBRARY_VERSION = '@PROJECT_VERSION_FULL@' MAX_PARALLEL_JOBS = 16 @@ -311,7 +311,15 @@ class Cmd options = parse(args) # Read the plugin that handles the command. - plugin = LIBRARY_NAME + if LIBRARY_NAME[0] == '/' + # If the first character is a slash, we'll assume that we've been given an + # absolute path to the library. This is only used during test mode. + plugin = LIBRARY_NAME + else + # We're assuming that the library path is relative to the current + # location of this script. + plugin = File.expand_path(File.join(File.dirname(__FILE__), LIBRARY_NAME)) + end conf_version = LIBRARY_VERSION begin @@ -405,7 +413,7 @@ class Cmd end rescue puts "Library error: Problem running [#{options['subcommand']}]() "\ - "from @IGN_LIBRARY_NAME@." + "from #{plugin}." end # begin end # execute end # class diff --git a/src/cmd/fuel.bash_completion.sh b/src/cmd/fuel.bash_completion.sh new file mode 100755 index 00000000..d29f4a21 --- /dev/null +++ b/src/cmd/fuel.bash_completion.sh @@ -0,0 +1,191 @@ +#!/usr/bin/env bash +# +# Copyright (C) 2023 Open Source Robotics Foundation +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +# bash tab-completion + +# This is a per-library function definition, used in conjunction with the +# top-level entry point in ign-tools. + +GZ_FUEL_SUBCOMMANDS=" +delete +download +edit +list +meta +upload +" + +# TODO: In Fortress+, for each of the completion lists, remove --force-version +# and --versions. Add --help-all. Update ../gz_TEST.cc accordingly. +GZ_FUEL_COMPLETION_LIST=" + -c --config + -h --help + -v --verbose + --force-version + --versions +" + +GZ_DELETE_COMPLETION_LIST=" + --header + -c --config + -h --help + -u --url + --force-version + --versions +" + +GZ_DOWNLOAD_COMPLETION_LIST=" + --header + -c --config + -h --help + -j --jobs + -t --type + -u --url + --force-version + --versions +" + +GZ_EDIT_COMPLETION_LIST=" + --header + -b --public + -c --config + -h -help + -m --model + -p --private + -u --url + --force-version + --versions +" + +GZ_LIST_COMPLETION_LIST=" + -c --config + -h --help + -o --owner + -r --raw + -t --type + -u --url + --force-version + --versions +" + +GZ_META_COMPLETION_LIST=" + --config2pbtxt + --pbtxt2config + -c --config + -h --help + --force-version + --versions +" + +GZ_UPLOAD_COMPLETION_LIST=" + --header + -c --config + -h --help + -m --model + -p --private + -u --url + --force-version + --versions +" + +function __get_comp_from_list { + if [[ ${COMP_WORDS[COMP_CWORD]} == -* ]]; then + # Specify options (-*) word list for this subcommand + COMPREPLY=($(compgen -W "$@" \ + -- "${COMP_WORDS[COMP_CWORD]}" )) + return + else + # Just use bash default auto-complete, because we never have two + # subcommands in the same line. If that is ever needed, change here to + # detect subsequent subcommands + COMPREPLY=($(compgen -o default -- "${COMP_WORDS[COMP_CWORD]}")) + return + fi +} + +function _gz_fuel_delete +{ + __get_comp_from_list "$GZ_DELETE_COMPLETION_LIST" +} + +function _gz_fuel_download +{ + __get_comp_from_list "$GZ_DOWNLOAD_COMPLETION_LIST" +} + +function _gz_fuel_edit +{ + __get_comp_from_list "$GZ_EDIT_COMPLETION_LIST" +} + +function _gz_fuel_list +{ + __get_comp_from_list "$GZ_LIST_COMPLETION_LIST" +} + +function _gz_fuel_meta +{ + __get_comp_from_list "$GZ_META_COMPLETION_LIST" +} + +function _gz_fuel_upload +{ + __get_comp_from_list "$GZ_UPLOAD_COMPLETION_LIST" +} + +# This searches the current list of typed words for one of the subcommands +# listed in GZ_FUEL_SUBCOMMANDS. This should work for most cases, but may fail +# if a word that looks like a subocmmand is used as an argument to a flag. Eg. +# `gz fuel --config download list`. Here `download` is an argument to +# `--config`, but this function will think that it's the subcommand. Since this +# seems like a rare scenario, we accept this failure mode. +function __get_subcommand +{ + local known_subcmd + local subcmd + for ((i=2; $i<=$COMP_CWORD; i++)); do + for subcmd in $GZ_FUEL_SUBCOMMANDS; do + if [[ "${COMP_WORDS[i]}" == "$subcmd" ]]; then + known_subcmd="$subcmd" + fi + done + done + echo "$known_subcmd" +} + +function _gz_fuel +{ + if [[ $COMP_CWORD > 2 ]]; then + local known_subcmd=$(__get_subcommand) + if [[ ! -z $known_subcmd ]]; then + local subcmd_func="_gz_fuel_$known_subcmd" + if [[ "$(type -t $subcmd_func)" == 'function' ]]; then + $subcmd_func + return + fi + fi + fi + + # If a subcommand is not found, assume we're still completing the subcommands + # or flags for `fuel`. + if [[ ${COMP_WORDS[COMP_CWORD]} == -* ]]; then + COMPREPLY=($(compgen -W "$GZ_FUEL_COMPLETION_LIST" \ + -- "${COMP_WORDS[COMP_CWORD]}" )) + else + COMPREPLY=($(compgen -W "${GZ_FUEL_SUBCOMMANDS}" -- ${cur})) + fi +} From fb27cacd3068cbbf23205ddea4b7b6e7c9e19349 Mon Sep 17 00:00:00 2001 From: "Addisu Z. Taddese" Date: Tue, 2 May 2023 10:43:46 -0500 Subject: [PATCH 05/11] =?UTF-8?q?=F0=9F=8E=88=204.9.0=20(#353)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Addisu Z. Taddese --- CMakeLists.txt | 2 +- Changelog.md | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 545ab1e3..b5976f84 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -3,7 +3,7 @@ cmake_minimum_required(VERSION 3.5.1 FATAL_ERROR) #============================================================================ # Initialize the project #============================================================================ -project(ignition-fuel_tools4 VERSION 4.8.3) +project(ignition-fuel_tools4 VERSION 4.9.0) #============================================================================ # Find ignition-cmake diff --git a/Changelog.md b/Changelog.md index 0566ebe1..8816bc6f 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,5 +1,13 @@ ## Gazebo Fuel Tools 4.x +### Gazebo Fuel Tools 4.9.0 (2023-05-03) + +1. Add bash completion + * [Pull request #329](https://github.com/gazebosim/gz-fuel-tools/pull/329) + +1. Reflect pagination of RESTful requests in iterator API + * [Pull request #350](https://github.com/gazebosim/gz-fuel-tools/pull/350) + ### Gazebo Fuel Tools 4.8.3 (2023-03-29) 1. Support link referral download From 0bdbb7a0f54a6db260abcc391de064662f193d3e Mon Sep 17 00:00:00 2001 From: "Addisu Z. Taddese" Date: Tue, 13 Jun 2023 22:47:01 -0500 Subject: [PATCH 06/11] 7.3.0 Release (#359) Signed-off-by: Addisu Z. Taddese --- CMakeLists.txt | 2 +- Changelog.md | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 0db75aa7..b11de784 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -3,7 +3,7 @@ cmake_minimum_required(VERSION 3.5.1 FATAL_ERROR) #============================================================================ # Initialize the project #============================================================================ -project(ignition-fuel_tools7 VERSION 7.2.2) +project(ignition-fuel_tools7 VERSION 7.3.0) #============================================================================ # Find ignition-cmake diff --git a/Changelog.md b/Changelog.md index 40aa9cab..184a7432 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,5 +1,19 @@ ## Gazebo Fuel Tools 7.x +### Gazebo Fuel Tools 7.3.0 (2023-06-13) + +1. Forward merges + * [Pull request #355](https://github.com/gazebosim/gz-fuel-tools/pull/355) + +1. Add bash completion + * [Pull request #329](https://github.com/gazebosim/gz-fuel-tools/pull/329) + +1. Reflect pagination of RESTful requests in iterator API + * [Pull request #350](https://github.com/gazebosim/gz-fuel-tools/pull/350) + +1. Support link referral download + * [Pull request #333](https://github.com/gazebosim/gz-fuel-tools/pull/333) + ### Gazebo Fuel Tools 7.2.2 (2023-03-29) 1. Support link referral download From f9fd6497750f8eac869d0b729b88209ed10f1c1a Mon Sep 17 00:00:00 2001 From: shameekganguly Date: Wed, 14 Jun 2023 05:40:17 -0700 Subject: [PATCH 07/11] Minor cleanup (#357) * Replace ign log macros with gz Signed-off-by: Shameek Ganguly * Clangtidy fixes Signed-off-by: Shameek Ganguly * More clangtidy fixes Signed-off-by: Shameek Ganguly --------- Signed-off-by: Shameek Ganguly --- src/ClientConfig.cc | 6 +++--- src/FuelClient.cc | 30 +++++++++++++++--------------- src/Interface.cc | 2 +- src/Model.cc | 3 +-- src/ModelIdentifier.cc | 4 ++-- src/Model_TEST.cc | 3 +-- src/WorldIdentifier.cc | 8 ++++---- 7 files changed, 27 insertions(+), 29 deletions(-) diff --git a/src/ClientConfig.cc b/src/ClientConfig.cc index f0d24ef2..1b11c76e 100644 --- a/src/ClientConfig.cc +++ b/src/ClientConfig.cc @@ -240,9 +240,9 @@ ClientConfig::ClientConfig(const ClientConfig &_copy) } ////////////////////////////////////////////////// -ClientConfig &ClientConfig::operator=(const ClientConfig &_rhs) +ClientConfig &ClientConfig::operator=(const ClientConfig &_copy) { - *(this->dataPtr) = *(_rhs.dataPtr); + *(this->dataPtr) = *(_copy.dataPtr); return *this; } @@ -526,7 +526,7 @@ std::string ClientConfig::AsString(const std::string &_prefix) const << _prefix << "Cache location: " << this->CacheLocation() << std::endl << _prefix << "Servers:" << std::endl; - for (auto s : this->Servers()) + for (const auto &s : this->Servers()) { out << _prefix << " ---" << std::endl; out << _prefix << s.AsString(" "); diff --git a/src/FuelClient.cc b/src/FuelClient.cc index d7acdea7..a30feaaa 100644 --- a/src/FuelClient.cc +++ b/src/FuelClient.cc @@ -530,7 +530,7 @@ void FuelClient::AddServerConfigParametersToHeaders( std::vector &_headers) const { bool privateTokenDefined = false; - for (auto header : _headers) + for (const auto &header : _headers) { if (header.find("Private-token:") != std::string::npos) { @@ -627,7 +627,7 @@ Result FuelClient::DownloadModel(const ModelIdentifier &_id, if(!res) return res; - for (ModelIdentifier dep : dependencies) + for (const ModelIdentifier &dep : dependencies) { // Download dependency if not in the local cache if (!this->dataPtr->cache->MatchingModel(dep)) @@ -787,7 +787,7 @@ Result FuelClient::ModelDependencies( std::vector &_dependencies) { std::vector newDeps; - for (auto modelId : _ids) + for (const auto &modelId : _ids) { std::vector modelDeps; auto result = this->ModelDependencies(modelId, modelDeps); @@ -946,7 +946,7 @@ std::vector FuelClient::DownloadModels( std::lock_guard lock(idsMutex); gzdbg << "Adding " << dependencies.size() << " model dependencies to queue from " << id.Name() << "\n"; - for (auto dep : dependencies) + for (const auto &dep : dependencies) { if (uniqueIds.count(dep) == 0) { @@ -1191,15 +1191,15 @@ bool FuelClient::ParseWorldUrl(const common::URI &_worldUrl, } ////////////////////////////////////////////////// -bool FuelClient::ParseModelFileUrl(const common::URI &_fileUrl, +bool FuelClient::ParseModelFileUrl(const common::URI &_modelFileUrl, ModelIdentifier &_id, std::string &_filePath) { - if (!_fileUrl.Valid()) + if (!_modelFileUrl.Valid()) return false; - this->dataPtr->CheckForDeprecatedUri(_fileUrl); + this->dataPtr->CheckForDeprecatedUri(_modelFileUrl); - auto urlStr = _fileUrl.Str(); + auto urlStr = _modelFileUrl.Str(); std::smatch match; std::string scheme; @@ -1262,13 +1262,13 @@ bool FuelClient::ParseModelFileUrl(const common::URI &_fileUrl, } ////////////////////////////////////////////////// -bool FuelClient::ParseWorldFileUrl(const common::URI &_fileUrl, +bool FuelClient::ParseWorldFileUrl(const common::URI &_worldFileUrl, WorldIdentifier &_id, std::string &_filePath) { - if (!_fileUrl.Valid()) + if (!_worldFileUrl.Valid()) return false; - auto urlStr = _fileUrl.Str(); + auto urlStr = _worldFileUrl.Str(); std::smatch match; std::string scheme; @@ -1935,7 +1935,7 @@ void FuelClientPrivate::CheckForDeprecatedUri(const common::URI &_uri) { std::string newUrl = _uri.Str(); newUrl.replace(ignFuelPos, oldServer.size(), "fuel.gazebosim.org"); - ignwarn << "The " << oldServer << " URL is deprecrated. Pleasse change " + gzwarn << "The " << oldServer << " URL is deprecrated. Pleasse change " << _uri.Str() << " to " << newUrl << std::endl; } } @@ -1959,7 +1959,7 @@ void FuelClientPrivate::ZipFromResponse(const RestResponse &_resp, // Check for valid URI if (common::URI::Valid(linkUri)) { - igndbg << "Downloading from a referral link [" << linkUri << "]\n"; + gzdbg << "Downloading from a referral link [" << linkUri << "]\n"; // Get the zip data. RestResponse linkResp = rest.Request(HttpMethod::GET, // URL @@ -1979,7 +1979,7 @@ void FuelClientPrivate::ZipFromResponse(const RestResponse &_resp, } else { - ignerr << "Invalid referral link URI [" << linkUri << "]. " + gzerr << "Invalid referral link URI [" << linkUri << "]. " << "Unable to download.\n"; } } @@ -1992,7 +1992,7 @@ void FuelClientPrivate::ZipFromResponse(const RestResponse &_resp, } else { - ignerr << "Invalid content-type of [" << contentTypeIter->second << "]. " + gzerr << "Invalid content-type of [" << contentTypeIter->second << "]. " << "Unable to download.\n"; } } diff --git a/src/Interface.cc b/src/Interface.cc index d2c5e82e..c1c9412a 100644 --- a/src/Interface.cc +++ b/src/Interface.cc @@ -140,7 +140,7 @@ namespace gz if (!basename.empty() && basename.find(".sdf") != std::string::npos) { std::string extension = basename.substr( - basename.find_last_of(".") + 1); + basename.find_last_of('.') + 1); if (extension == "sdf") return *dirIter; } diff --git a/src/Model.cc b/src/Model.cc index 5d760fdb..90f85560 100644 --- a/src/Model.cc +++ b/src/Model.cc @@ -24,9 +24,8 @@ #include "ModelPrivate.hh" -namespace ignft = gz::fuel_tools; using namespace gz; -using namespace ignft; +using namespace fuel_tools; ////////////////////////////////////////////////// Model::Model() : dataPtr(nullptr) diff --git a/src/ModelIdentifier.cc b/src/ModelIdentifier.cc index 2e96c43f..45df4b4d 100644 --- a/src/ModelIdentifier.cc +++ b/src/ModelIdentifier.cc @@ -174,10 +174,10 @@ bool ModelIdentifier::SetName(const std::string &_name) } ////////////////////////////////////////////////// -bool ModelIdentifier::SetOwner(const std::string &_owner) +bool ModelIdentifier::SetOwner(const std::string &_name) { bool success = false; - std::string owner = common::lowercase(_owner); + std::string owner = common::lowercase(_name); if (this->dataPtr->ValidName(owner)) { success = true; diff --git a/src/Model_TEST.cc b/src/Model_TEST.cc index 620341d3..a30502da 100644 --- a/src/Model_TEST.cc +++ b/src/Model_TEST.cc @@ -18,9 +18,8 @@ #include #include "gz/fuel_tools/Model.hh" -namespace ignft = gz::fuel_tools; using namespace gz; -using namespace ignft; +using namespace fuel_tools; ///////////////////////////////////////////////// /// \brief Nothing crashes diff --git a/src/WorldIdentifier.cc b/src/WorldIdentifier.cc index 38697476..9c9296a0 100644 --- a/src/WorldIdentifier.cc +++ b/src/WorldIdentifier.cc @@ -106,9 +106,9 @@ std::string WorldIdentifier::Owner() const } ////////////////////////////////////////////////// -bool WorldIdentifier::SetOwner(const std::string &_owner) +bool WorldIdentifier::SetOwner(const std::string &_name) { - this->dataPtr->owner = common::lowercase(_owner); + this->dataPtr->owner = common::lowercase(_name); return true; } @@ -177,9 +177,9 @@ std::string WorldIdentifier::LocalPath() const } ////////////////////////////////////////////////// -bool WorldIdentifier::SetLocalPath(const std::string &_localPath) +bool WorldIdentifier::SetLocalPath(const std::string &_path) { - this->dataPtr->localPath = _localPath; + this->dataPtr->localPath = _path; return true; } From 9036b5863d513a7ce9dd2a06cf73c946ad5c75e3 Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Wed, 21 Jun 2023 15:08:27 -0500 Subject: [PATCH 08/11] Support for bazel in Garden (#328) Signed-off-by: Michael Carroll Signed-off-by: Michael Carroll Co-authored-by: Nate Koenig --- BUILD.bazel | 110 +++++++++++++++++++++++++++++++++++++++ CMakeLists.txt | 2 +- src/CMakeLists.txt | 1 + src/ClientConfig_TEST.cc | 95 +++++++++++++-------------------- src/FuelClient_TEST.cc | 42 ++++----------- src/Interface_TEST.cc | 36 +++++++------ src/LocalCache_TEST.cc | 37 ++++--------- src/RestClient_TEST.cc | 1 - test/BUILD.bazel | 6 +++ 9 files changed, 196 insertions(+), 134 deletions(-) create mode 100644 BUILD.bazel create mode 100644 test/BUILD.bazel diff --git a/BUILD.bazel b/BUILD.bazel new file mode 100644 index 00000000..0bd4aa6a --- /dev/null +++ b/BUILD.bazel @@ -0,0 +1,110 @@ +load( + "@gz//bazel/skylark:build_defs.bzl", + "GZ_FEATURES", + "GZ_ROOT", + "GZ_VISIBILITY", + "gz_configure_header", + "gz_export_header", + "gz_include_header", +) +load( + "@gz//bazel/lint:lint.bzl", + "add_lint_tests", +) + +package( + default_visibility = GZ_VISIBILITY, + features = GZ_FEATURES, +) + +licenses(["notice"]) # Apache-2.0 + +exports_files(["LICENSE"]) + +gz_configure_header( + name = "fuel_tools_config_hh", + src = "include/gz/fuel_tools/config.hh.in", + cmakelists = ["CMakeLists.txt"], + defines = { + # These definitions are unused, + # this is merely to suppress generator warnings + "CMAKE_INSTALL_PREFIX": "unused", + }, + package = "fuel_tools", +) + +gz_export_header( + name = "include/gz/fuel_tools/Export.hh", + export_base = "GZ_FUEL_TOOLS", + lib_name = "gz-fuel_tools", + visibility = ["//visibility:private"], +) + +public_headers_no_gen = glob([ + "include/gz/fuel_tools/*.hh", +]) + +private_headers = glob(["src/*.hh"]) + +sources = glob( + ["src/*.cc"], + exclude = [ + "src/gz.cc", + "src/*_TEST.cc", + ], +) + +gz_include_header( + name = "fuel_tools_hh_genrule", + out = "include/gz/fuel_tools.hh", + hdrs = public_headers_no_gen + [ + "include/gz/fuel_tools/config.hh", + "include/gz/fuel_tools/Export.hh", + ], +) + +public_headers = public_headers_no_gen + [ + "include/gz/fuel_tools/config.hh", + "include/gz/fuel_tools/Export.hh", + "include/gz/fuel_tools.hh", +] + +cc_library( + name = "fuel_tools", + srcs = sources + private_headers, + hdrs = public_headers, + includes = ["include"], + deps = [ + GZ_ROOT + "common", + GZ_ROOT + "msgs", + "@curl", + "@jsoncpp", + "@yaml", + "@zip", + ], +) + +test_sources = glob( + include = ["src/*_TEST.cc"], + exclude = [ + "src/gz_TEST.cc", + "src/gz_src_TEST.cc", + ], +) + +[cc_test( + name = src.replace("/", "_").replace(".cc", "").replace("src_", ""), + srcs = [src], + env = { + "GZ_BAZEL": "1", + "GZ_BAZEL_PATH": "fuel_tools", + }, + deps = [ + ":fuel_tools", + GZ_ROOT + "common/testing", + "@gtest", + "@gtest//:gtest_main", + ], +) for src in test_sources] + +add_lint_tests() diff --git a/CMakeLists.txt b/CMakeLists.txt index 16920633..f6b06899 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -54,7 +54,7 @@ set(GZ_UTILS_VER ${gz-utils2_VERSION_MAJOR}) #-------------------------------------- # Find gz-common -gz_find_package(gz-common5 REQUIRED PRIVATE) +gz_find_package(gz-common5 REQUIRED PRIVATE COMPONENTS testing) set(GZ_COMMON_VER ${gz-common5_VERSION_MAJOR}) #-------------------------------------- diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index aad437d8..c2ec97ac 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -70,6 +70,7 @@ gz_build_tests(TYPE UNIT TEST_LIST test_targets LIB_DEPS gz-common${GZ_COMMON_VER}::gz-common${GZ_COMMON_VER} + gz-common${GZ_COMMON_VER}::testing TINYXML2::TINYXML2 ) diff --git a/src/ClientConfig_TEST.cc b/src/ClientConfig_TEST.cc index ea672e27..efff93c0 100644 --- a/src/ClientConfig_TEST.cc +++ b/src/ClientConfig_TEST.cc @@ -20,28 +20,16 @@ #include #include #include +#include +#include #include + +#include "gz/fuel_tools/config.hh" #include "gz/fuel_tools/ClientConfig.hh" -#include "test_config.hh" using namespace gz; using namespace fuel_tools; -///////////////////////////////////////////////// -/// \brief Helper to remove file according to OS, while Windows -/// has this issue: -/// https://github.com/gazebosim/gz-common/issues/51 -/// \todo(anyone) Remove this once Windows issue is solved. -/// \param[in] _path Path to file to be removed. -void removeFileTemp(const std::string &_path) -{ -#ifndef _WIN32 - EXPECT_TRUE(gz::common::removeFile(_path)); -#else - gz::common::removeFile(_path); -#endif -} - ///////////////////////////////////////////////// /// \brief Get home directory. /// \return Home directory or empty string if home wasn't found. @@ -54,26 +42,34 @@ std::string homePath() #else gz::common::env("USERPROFILE", homePath); #endif - return homePath; } ///////////////////////////////////////////////// -/// \brief Get cache directory. -/// \return Cache directory -/// \ToDo: Move this function to gz::common::Filesystem -std::string cachePath() +class ClientConfigTest: public ::testing::Test { -#ifndef _WIN32 - return std::string("/tmp/gz/fuel"); -#else - return std::string("C:\\Windows\\Temp"); -#endif -} + public: void SetUp() override + { + gz::common::Console::SetVerbosity(4); + tempDir = gz::common::testing::MakeTestTempDirectory(); + ASSERT_TRUE(tempDir->Valid()) << tempDir->Path(); + + gz::common::chdir(tempDir->Path()); + } + + public: std::string cachePath() + { + return this->tempDir->Path(); + } + + public: std::shared_ptr tempDir; +}; + +class ServerConfigTest: public ClientConfigTest {}; ///////////////////////////////////////////////// /// \brief Initially only the default server in config -TEST(ClientConfig, InitiallyDefaultServers) +TEST_F(ClientConfigTest, InitiallyDefaultServers) { ClientConfig config; EXPECT_EQ(2u, config.Servers().size()); @@ -81,7 +77,7 @@ TEST(ClientConfig, InitiallyDefaultServers) ///////////////////////////////////////////////// /// \brief Servers can be added -TEST(ClientConfig, ServersCanBeAdded) +TEST_F(ClientConfigTest, ServersCanBeAdded) { ClientConfig config; ServerConfig srv; @@ -94,7 +90,7 @@ TEST(ClientConfig, ServersCanBeAdded) ///////////////////////////////////////////////// /// \brief We can load the default configuration file. -TEST(ClientConfig, CustomDefaultConfiguration) +TEST_F(ClientConfigTest, CustomDefaultConfiguration) { ClientConfig config; ASSERT_EQ(2u, config.Servers().size()); @@ -110,7 +106,7 @@ TEST(ClientConfig, CustomDefaultConfiguration) ///////////////////////////////////////////////// /// \brief We can load custom settings in a configuration file. -TEST(ClientConfig, CustomConfiguration) +TEST_F(ClientConfigTest, CustomConfiguration) { ClientConfig config; @@ -147,13 +143,11 @@ TEST(ClientConfig, CustomConfiguration) config.Servers().back().Url().Str()); EXPECT_EQ(cachePath(), config.CacheLocation()); - // Remove the configuration file. - removeFileTemp(testPath); } ///////////////////////////////////////////////// /// \brief A server contains an already used URL. -TEST(ClientConfig, RepeatedServerConfiguration) +TEST_F(ClientConfigTest, RepeatedServerConfiguration) { ClientConfig config; @@ -178,14 +172,11 @@ TEST(ClientConfig, RepeatedServerConfiguration) ofs.close(); EXPECT_TRUE(config.LoadConfig(testPath)); - - // Remove the configuration file. - removeFileTemp(testPath); } ///////////////////////////////////////////////// /// \brief A server without URL is not valid. -TEST(ClientConfig, NoServerUrlConfiguration) +TEST_F(ClientConfigTest, NoServerUrlConfiguration) { ClientConfig config; @@ -203,14 +194,11 @@ TEST(ClientConfig, NoServerUrlConfiguration) ofs.close(); EXPECT_FALSE(config.LoadConfig(testPath)); - - // Remove the configuration file. - removeFileTemp(testPath); } ///////////////////////////////////////////////// /// \brief A server with an empty URL is not valid. -TEST(ClientConfig, EmptyServerUrlConfiguration) +TEST_F(ClientConfigTest, EmptyServerUrlConfiguration) { ClientConfig config; @@ -228,14 +216,11 @@ TEST(ClientConfig, EmptyServerUrlConfiguration) ofs.close(); EXPECT_FALSE(config.LoadConfig(testPath)); - - // Remove the configuration file. - removeFileTemp(testPath); } ///////////////////////////////////////////////// /// \brief The "cache" option requires to set "path". -TEST(ClientConfig, NoCachePathConfiguration) +TEST_F(ClientConfigTest, NoCachePathConfiguration) { ClientConfig config; @@ -250,14 +235,11 @@ TEST(ClientConfig, NoCachePathConfiguration) ofs.close(); EXPECT_FALSE(config.LoadConfig(testPath)); - - // Remove the configuration file. - removeFileTemp(testPath); } ///////////////////////////////////////////////// /// \brief The path parameter cannot be empty. -TEST(ClientConfig, EmptyCachePathConfiguration) +TEST_F(ClientConfigTest, EmptyCachePathConfiguration) { ClientConfig config; @@ -273,13 +255,10 @@ TEST(ClientConfig, EmptyCachePathConfiguration) ofs.close(); EXPECT_FALSE(config.LoadConfig(testPath)); - - // Remove the configuration file. - removeFileTemp(testPath); } ///////////////////////////////////////////////// -TEST(ClientConfig, UserAgent) +TEST_F(ClientConfigTest, UserAgent) { ClientConfig config; EXPECT_EQ("IgnitionFuelTools-" GZ_FUEL_TOOLS_VERSION_FULL, @@ -290,7 +269,7 @@ TEST(ClientConfig, UserAgent) } ///////////////////////////////////////////////// -TEST(ServerConfig, ApiKey) +TEST_F(ServerConfigTest, ApiKey) { ServerConfig config; EXPECT_TRUE(config.ApiKey().empty()); @@ -303,7 +282,7 @@ TEST(ServerConfig, ApiKey) } ///////////////////////////////////////////////// -TEST(ClientConfig, AsString) +TEST_F(ClientConfigTest, AsString) { common::Console::SetVerbosity(4); { @@ -366,7 +345,7 @@ TEST(ClientConfig, AsString) } ///////////////////////////////////////////////// -TEST(ClientConfig, AsPrettyString) +TEST_F(ClientConfigTest, AsPrettyString) { common::Console::SetVerbosity(4); @@ -394,7 +373,7 @@ TEST(ClientConfig, AsPrettyString) } ///////////////////////////////////////////////// -TEST(ServerConfig, Url) +TEST_F(ServerConfigTest, Url) { // Invalid URL string { diff --git a/src/FuelClient_TEST.cc b/src/FuelClient_TEST.cc index 38385bcd..ade73331 100644 --- a/src/FuelClient_TEST.cc +++ b/src/FuelClient_TEST.cc @@ -26,15 +26,7 @@ #include "gz/fuel_tools/Result.hh" #include "gz/fuel_tools/WorldIdentifier.hh" -#include "test_config.hh" - -#ifdef _WIN32 -#include -#define ChangeDirectory _chdir -#else -#include -#define ChangeDirectory chdir -#endif +#include using namespace gz; using namespace gz::fuel_tools; @@ -126,12 +118,21 @@ void createLocalWorld(ClientConfig &_conf) } ///////////////////////////////////////////////// -class FuelClientTest : public ::testing::Test +class FuelClientTest: public ::testing::Test { public: void SetUp() override { gz::common::Console::SetVerbosity(4); + tempDir = gz::common::testing::MakeTestTempDirectory(); + ASSERT_TRUE(tempDir->Valid()) << tempDir->Path(); + + gz::common::chdir(tempDir->Path()); + ASSERT_FALSE(common::exists("test_cache")); + ASSERT_TRUE(common::createDirectories("test_cache")); + ASSERT_TRUE(common::isDirectory("test_cache")); } + + public: std::shared_ptr tempDir; }; ///////////////////////////////////////////////// @@ -414,10 +415,6 @@ INSTANTIATE_TEST_SUITE_P( // https://github.com/gazebosim/gz-fuel-tools/issues/105 TEST_P(FuelClientDownloadTest, DownloadModel) { - // Configure to use binary path as cache - ASSERT_EQ(0, ChangeDirectory(PROJECT_BINARY_PATH)); - common::removeAll("test_cache"); - ASSERT_TRUE(common::createDirectories("test_cache")); ClientConfig config; config.SetCacheLocation(common::joinPaths(common::cwd(), "test_cache")); @@ -621,10 +618,6 @@ TEST_P(FuelClientDownloadTest, DownloadModel) // https://github.com/gazebosim/gz-fuel-tools/issues/106 TEST_F(FuelClientTest, GZ_UTILS_TEST_DISABLED_ON_WIN32(ModelDependencies)) { - // Configure to use binary path as cache - ASSERT_EQ(0, ChangeDirectory(PROJECT_BINARY_PATH)); - common::removeAll("test_cache"); - ASSERT_TRUE(common::createDirectories("test_cache")); ClientConfig config; config.SetCacheLocation(common::joinPaths(common::cwd(), "test_cache")); @@ -697,10 +690,6 @@ TEST_F(FuelClientTest, GZ_UTILS_TEST_DISABLED_ON_WIN32(ModelDependencies)) // See https://github.com/gazebosim/gz-fuel-tools/issues/231 TEST_F(FuelClientTest, GZ_UTILS_TEST_DISABLED_ON_WIN32(CachedModel)) { - // Configure to use binary path as cache and populate it - ASSERT_EQ(0, ChangeDirectory(PROJECT_BINARY_PATH)); - common::removeAll("test_cache"); - ASSERT_TRUE(common::createDirectories("test_cache")); ClientConfig config; config.SetCacheLocation(common::joinPaths(common::cwd(), "test_cache")); createLocalModel(config); @@ -1089,11 +1078,6 @@ TEST_F(FuelClientTest, ParseWorldFileUrl) // https://github.com/gazebosim/gz-fuel-tools/issues/105 TEST_F(FuelClientTest, DownloadWorld) { - // Configure to use binary path as cache - ASSERT_EQ(0, ChangeDirectory(PROJECT_BINARY_PATH)); - common::removeAll("test_cache"); - ASSERT_TRUE(common::createDirectories("test_cache")); - ServerConfig server; server.SetUrl(common::URI( "https://fuel.gazebosim.org")); @@ -1167,10 +1151,6 @@ TEST_F(FuelClientTest, DownloadWorld) // https://github.com/gazebosim/gz-fuel-tools/issues/106 TEST_F(FuelClientTest, CachedWorld) { - // Configure to use binary path as cache and populate it - ASSERT_EQ(0, ChangeDirectory(PROJECT_BINARY_PATH)); - common::removeAll("test_cache"); - ASSERT_TRUE(common::createDirectories("test_cache")); ClientConfig config; config.SetCacheLocation(common::joinPaths(common::cwd(), "test_cache")); createLocalWorld(config); diff --git a/src/Interface_TEST.cc b/src/Interface_TEST.cc index e389b4ee..3d5435c4 100644 --- a/src/Interface_TEST.cc +++ b/src/Interface_TEST.cc @@ -23,30 +23,34 @@ #include "gz/fuel_tools/FuelClient.hh" #include "gz/fuel_tools/Interface.hh" -#include "test_config.hh" - -#ifdef _WIN32 -#include -#define ChangeDirectory _chdir -#else -#include -#define ChangeDirectory chdir -#endif +#include using namespace gz; using namespace gz::fuel_tools; +///////////////////////////////////////////////// +class InterfaceTest: public ::testing::Test +{ + public: void SetUp() override + { + gz::common::Console::SetVerbosity(4); + tempDir = gz::common::testing::MakeTestTempDirectory(); + ASSERT_TRUE(tempDir->Valid()) << tempDir->Path(); + + gz::common::chdir(tempDir->Path()); + ASSERT_FALSE(common::exists("test_cache")); + ASSERT_TRUE(common::createDirectories("test_cache")); + ASSERT_TRUE(common::isDirectory("test_cache")); + } + + public: std::shared_ptr tempDir; +}; + ///////////////////////////////////////////////// // Protocol "https" not supported or disabled in libcurl for Windows // https://github.com/gazebosim/gz-fuel-tools/issues/105 -TEST(Interface, FetchResources) +TEST_F(InterfaceTest, FetchResources) { - common::Console::SetVerbosity(4); - - // Configure to use binary path as cache - ASSERT_EQ(0, ChangeDirectory(PROJECT_BINARY_PATH)); - common::removeAll("test_cache"); - ASSERT_TRUE(common::createDirectories("test_cache")); ClientConfig config; config.SetCacheLocation(common::joinPaths(common::cwd(), "test_cache")); diff --git a/src/LocalCache_TEST.cc b/src/LocalCache_TEST.cc index 15b93384..4e9844b4 100644 --- a/src/LocalCache_TEST.cc +++ b/src/LocalCache_TEST.cc @@ -22,21 +22,13 @@ #include #include #include +#include #include #include "gz/fuel_tools/ClientConfig.hh" #include "gz/fuel_tools/WorldIdentifier.hh" #include "LocalCache.hh" -#include "test_config.hh" - -#ifdef _WIN32 -#include -#define ChangeDirectory _chdir -#else -#include -#define ChangeDirectory chdir -#endif using namespace gz; using namespace fuel_tools; @@ -219,7 +211,16 @@ class LocalCacheTest : public ::testing::Test public: void SetUp() override { gz::common::Console::SetVerbosity(4); + tempDir = gz::common::testing::MakeTestTempDirectory(); + ASSERT_TRUE(tempDir->Valid()) << tempDir->Path(); + + gz::common::chdir(tempDir->Path()); + ASSERT_FALSE(common::exists("test_cache")); + ASSERT_TRUE(common::createDirectories("test_cache")); + ASSERT_TRUE(common::isDirectory("test_cache")); } + + public: std::shared_ptr tempDir; }; ///////////////////////////////////////////////// @@ -227,9 +228,6 @@ class LocalCacheTest : public ::testing::Test // See https://github.com/gazebosim/gz-fuel-tools/issues/231 TEST_F(LocalCacheTest, GZ_UTILS_TEST_DISABLED_ON_WIN32(AllModels)) { - ASSERT_EQ(0, ChangeDirectory(PROJECT_BINARY_PATH)); - EXPECT_TRUE(common::removeAll("test_cache")); - ASSERT_TRUE(common::createDirectories("test_cache")); ClientConfig conf; conf.SetCacheLocation(common::joinPaths(common::cwd(), "test_cache")); createLocal6Models(conf); @@ -256,9 +254,6 @@ TEST_F(LocalCacheTest, GZ_UTILS_TEST_DISABLED_ON_WIN32(AllModels)) // See https://github.com/gazebosim/gz-fuel-tools/issues/307 TEST_F(LocalCacheTest, GZ_UTILS_TEST_DISABLED_ON_WIN32(MatchingModels)) { - ASSERT_EQ(0, ChangeDirectory(PROJECT_BINARY_PATH)); - EXPECT_TRUE(common::removeAll("test_cache")); - ASSERT_TRUE(common::createDirectories("test_cache")); ClientConfig conf; conf.Clear(); conf.SetCacheLocation(common::joinPaths(common::cwd(), "test_cache")); @@ -303,9 +298,6 @@ TEST_F(LocalCacheTest, GZ_UTILS_TEST_DISABLED_ON_WIN32(MatchingModels)) // See https://github.com/gazebosim/gz-fuel-tools/issues/307 TEST_F(LocalCacheTest, GZ_UTILS_TEST_DISABLED_ON_WIN32(MatchingModel)) { - ASSERT_EQ(0, ChangeDirectory(PROJECT_BINARY_PATH)); - EXPECT_TRUE(common::removeAll("test_cache")); - ASSERT_TRUE(common::createDirectories("test_cache")); ClientConfig conf; conf.SetCacheLocation(common::joinPaths(common::cwd(), "test_cache")); createLocal6Models(conf); @@ -360,9 +352,6 @@ TEST_F(LocalCacheTest, GZ_UTILS_TEST_DISABLED_ON_WIN32(MatchingModel)) // See https://github.com/gazebosim/gz-fuel-tools/issues/307 TEST_F(LocalCacheTest, GZ_UTILS_TEST_DISABLED_ON_WIN32(AllWorlds)) { - ASSERT_EQ(0, ChangeDirectory(PROJECT_BINARY_PATH)); - EXPECT_TRUE(common::removeAll("test_cache")); - ASSERT_TRUE(common::createDirectories("test_cache")); ClientConfig conf; conf.SetCacheLocation(common::joinPaths(common::cwd(), "test_cache")); createLocal6Worlds(conf); @@ -393,9 +382,6 @@ TEST_F(LocalCacheTest, GZ_UTILS_TEST_DISABLED_ON_WIN32(AllWorlds)) // See https://github.com/gazebosim/gz-fuel-tools/issues/307 TEST_F(LocalCacheTest, GZ_UTILS_TEST_DISABLED_ON_WIN32(MatchingWorlds)) { - ASSERT_EQ(0, ChangeDirectory(PROJECT_BINARY_PATH)); - EXPECT_TRUE(common::removeAll("test_cache")); - ASSERT_TRUE(common::createDirectories("test_cache")); ClientConfig conf; conf.Clear(); conf.SetCacheLocation(common::joinPaths(common::cwd(), "test_cache")); @@ -428,9 +414,6 @@ TEST_F(LocalCacheTest, GZ_UTILS_TEST_DISABLED_ON_WIN32(MatchingWorlds)) // See https://github.com/gazebosim/gz-fuel-tools/issues/307 TEST_F(LocalCacheTest, GZ_UTILS_TEST_DISABLED_ON_WIN32(MatchingWorld)) { - ASSERT_EQ(0, ChangeDirectory(PROJECT_BINARY_PATH)); - EXPECT_TRUE(common::removeAll("test_cache")); - ASSERT_TRUE(common::createDirectories("test_cache")); ClientConfig conf; conf.SetCacheLocation(common::joinPaths(common::cwd(), "test_cache")); createLocal6Worlds(conf); diff --git a/src/RestClient_TEST.cc b/src/RestClient_TEST.cc index b88c93b9..748794b1 100644 --- a/src/RestClient_TEST.cc +++ b/src/RestClient_TEST.cc @@ -18,7 +18,6 @@ #include #include #include "gz/fuel_tools/RestClient.hh" -#include "test_config.hh" ///////////////////////////////////////////////// TEST(RestClient, UserAgent) diff --git a/test/BUILD.bazel b/test/BUILD.bazel new file mode 100644 index 00000000..5a263079 --- /dev/null +++ b/test/BUILD.bazel @@ -0,0 +1,6 @@ +load( + "@gz//bazel/lint:lint.bzl", + "add_lint_tests", +) + +add_lint_tests() From a0ef9727c5e92217af3cb5d610adffaa0a301674 Mon Sep 17 00:00:00 2001 From: Silvio Traversaro Date: Tue, 27 Jun 2023 18:48:56 +0200 Subject: [PATCH 09/11] Deprecated non-relocatable macro (#352) Signed-off-by: Silvio Traversaro Co-authored-by: Jose Luis Rivero Co-authored-by: Nate Koenig --- include/gz/fuel_tools/config.hh.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/gz/fuel_tools/config.hh.in b/include/gz/fuel_tools/config.hh.in index 74707ab8..96da880a 100644 --- a/include/gz/fuel_tools/config.hh.in +++ b/include/gz/fuel_tools/config.hh.in @@ -30,6 +30,6 @@ #define GZ_FUEL_TOOLS_VERSION_HEADER "Gazebo Fuel Tools, version ${PROJECT_VERSION_FULL}\nCopyright (C) 2017 Open Source Robotics Foundation.\nReleased under the Apache 2.0 License.\n\n" -#define GZ_FUEL_INITIAL_CONFIG_PATH "${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_DATAROOTDIR}/ignition/${GZ_DESIGNATION}${PROJECT_VERSION_MAJOR}/" +#define GZ_FUEL_INITIAL_CONFIG_PATH _Pragma ("GCC warning \"'GZ_FUEL_INITIAL_CONFIG_PATH' macro is deprecated and should not be used. \"") "${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_DATAROOTDIR}/ignition/${GZ_DESIGNATION}${PROJECT_VERSION_MAJOR}/" #endif From ed4cea14c3ca5f9f2330a6b3fd4766887fc45bbc Mon Sep 17 00:00:00 2001 From: Steve Peters Date: Mon, 17 Jul 2023 08:11:08 -0700 Subject: [PATCH 10/11] Zip: use non-deprecated methods (#360) Fixes compiler warnings on macOS. Signed-off-by: Steve Peters --- src/Zip.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Zip.cc b/src/Zip.cc index 81ada1f3..93718d39 100644 --- a/src/Zip.cc +++ b/src/Zip.cc @@ -37,7 +37,7 @@ bool CompressFile(zip *_archive, const std::string &_file, { if (common::isDirectory(_file)) { - if (zip_add_dir(_archive, _entry.c_str()) < 0) + if (zip_dir_add(_archive, _entry.c_str(), 0) < 0) { ignerr << "Error adding directory to zip: " << _file << std::endl; return false; @@ -69,7 +69,7 @@ bool CompressFile(zip *_archive, const std::string &_file, return false; } - if (zip_add(_archive, _entry.c_str(), source) + if (zip_file_add(_archive, _entry.c_str(), source, 0) < 0) { ignerr << "Error adding file to zip: " << _file << std::endl; From ae5dbe11f6393bfb73d4d7b6ecd371cf593caba4 Mon Sep 17 00:00:00 2001 From: Steve Peters Date: Thu, 20 Jul 2023 13:58:52 -0700 Subject: [PATCH 11/11] Remove GZ_FUEL_INITIAL_CONFIG_PATH macro (#363) Signed-off-by: Steve Peters --- Migration.md | 6 ++++++ include/gz/fuel_tools/config.hh.in | 2 -- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/Migration.md b/Migration.md index 20d5ef6e..014416f4 100644 --- a/Migration.md +++ b/Migration.md @@ -1,3 +1,9 @@ +## Gazebo Fuel Tools 8.X to 9.X + +### Removals + +* `GZ_FUEL_INITIAL_CONFIG_PATH` macro removed from `gz/fuel_tools/config.hh` + ## Gazebo Fuel Tools 7.X to 8.X ### Deprecations diff --git a/include/gz/fuel_tools/config.hh.in b/include/gz/fuel_tools/config.hh.in index 96da880a..28a909e0 100644 --- a/include/gz/fuel_tools/config.hh.in +++ b/include/gz/fuel_tools/config.hh.in @@ -30,6 +30,4 @@ #define GZ_FUEL_TOOLS_VERSION_HEADER "Gazebo Fuel Tools, version ${PROJECT_VERSION_FULL}\nCopyright (C) 2017 Open Source Robotics Foundation.\nReleased under the Apache 2.0 License.\n\n" -#define GZ_FUEL_INITIAL_CONFIG_PATH _Pragma ("GCC warning \"'GZ_FUEL_INITIAL_CONFIG_PATH' macro is deprecated and should not be used. \"") "${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_DATAROOTDIR}/ignition/${GZ_DESIGNATION}${PROJECT_VERSION_MAJOR}/" - #endif