diff --git a/include/ignition/fuel_tools/FuelClient.hh b/include/ignition/fuel_tools/FuelClient.hh index 634454c3..4a007140 100644 --- a/include/ignition/fuel_tools/FuelClient.hh +++ b/include/ignition/fuel_tools/FuelClient.hh @@ -348,6 +348,24 @@ namespace ignition const ignition::fuel_tools::ModelIdentifier &_model, const std::vector &_headers); + /// \brief Update a model using a PATCH request. + /// + /// Model fields that are patched by this function: + /// * private + /// * Model files contained in _pathToModelDir. + /// * Description, tags, license, and other attributes found in the + /// metadata.pbtxt or model.config file. + /// + /// \param[in] _model The model to patch. The contents of this model + /// will be sent in the PATCH request. + /// \param[in] _headers Headers to set on the HTTP request. + /// \param[in] _pathToModelDir a path to a directory containing a model. + /// \return Result of the patch operation. + public: Result PatchModel( + const ignition::fuel_tools::ModelIdentifier &_model, + const std::vector &_headers, + const std::string &_pathToModelDir); + /// \brief Parse Collection identifer from URL. /// \param[in] _url The unique URL of a collection. It may also be a /// unique name, which is a URL without the server version. diff --git a/src/FuelClient.cc b/src/FuelClient.cc index 2fdf0a34..4797cd6c 100644 --- a/src/FuelClient.cc +++ b/src/FuelClient.cc @@ -162,6 +162,29 @@ class ignition::fuel_tools::FuelClientPrivate public: void AllFiles(const std::string &_path, std::vector &_files) const; + /// \brief Populate a model form such that it can be used during + /// model creation and editing REST calls. + /// \param[in] _pathToModelDir Path to the model directory. + /// \param[in] _id Model identifier information. + /// \param[in] _private True if this model should be private. + /// \param[in] _form Form to fill. + /// \return True if the operation completed successfully. + public: bool FillModelForm(const std::string &_pathToModelDir, + const ModelIdentifier &_id, bool _private, + std::multimap &_form); + + /// \brief This function requests the available licenses from the + /// Fuel server and stores this information locally. + /// + /// The UploadModel function can use this information to set + /// appropriate license information based on a model's metadata.pbtxt + /// file. If license information is not available, then the + /// UploadModel function will default to the + /// "Creative Commons - Public Domain" license. + /// \param[in] _server Information about the server that provides + /// license information. + public: void PopulateLicenses(const ServerConfig &_server); + /// \brief Client configuration public: ClientConfig config; @@ -412,130 +435,9 @@ Result FuelClient::UploadModel(const std::string &_pathToModelDir, ignition::fuel_tools::Rest rest; RestResponse resp; - if (!common::exists(_pathToModelDir)) - { - ignerr << "The model path[" << _pathToModelDir << "] doesn't exist.\n"; - return Result(ResultType::UPLOAD_ERROR); - } - - ignition::msgs::FuelMetadata meta; - - // Try the `metadata.pbtxt` file first since it contains more information - // than `model.config`. - if (common::exists(common::joinPaths(_pathToModelDir, "metadata.pbtxt"))) - { - std::string filePath = common::joinPaths(_pathToModelDir, "metadata.pbtxt"); - - ignmsg << "Parsing " << filePath << std::endl; - - // Read the pbtxt file. - std::ifstream inputFile(filePath); - std::string inputStr((std::istreambuf_iterator(inputFile)), - std::istreambuf_iterator()); - - // Parse the file into the fuel metadata message - google::protobuf::TextFormat::ParseFromString(inputStr, &meta); - } - else if (common::exists(common::joinPaths(_pathToModelDir, "model.config"))) - { - std::string filePath = common::joinPaths(_pathToModelDir, "model.config"); - - ignmsg << "Parsing " << filePath << std::endl; - - std::ifstream inputFile(filePath); - std::string inputStr((std::istreambuf_iterator(inputFile)), - std::istreambuf_iterator()); - - if (!ignition::msgs::ConvertFuelMetadata(inputStr, meta)) - { - ignerr << "Unable to convert model config[" << _pathToModelDir << "].\n"; - return Result(ResultType::UPLOAD_ERROR); - } - } - else - { - ignerr << "Provided model directory[" << _pathToModelDir - << "] needs a metadata.pbtxt or a model.confg file."; + std::multimap form; + if (!this->dataPtr->FillModelForm(_pathToModelDir, _id, _private, form)) return Result(ResultType::UPLOAD_ERROR); - } - - std::multimap form = - { - {"name", meta.name()}, - {"description", meta.description()}, - {"private", _private ? "1" : "0"}, - }; - - // \todo(nkoenig) The ign-fuelserver expects an integer number for the - // license information. The fuelserver should be modified to accept - // a string. Otherwise, we have to bake into each client a mapping of - // license name to integer. - // - // If we have legal, then attempt to fill in the correct license information. - if (meta.has_legal()) - { - // Attempt to retrieve the available licenses, if we have no available - // licenses. - if (this->dataPtr->licenses.empty()) - { - this->PopulateLicenses(_id.Server()); - // Fail if a license has been requested, but we couldn't get the - // available licenses. - if (this->dataPtr->licenses.empty()) - { - return Result(ResultType::UPLOAD_ERROR); - } - } - - // Find the license by name. - std::map::const_iterator licenseIt = - this->dataPtr->licenses.find(meta.legal().license()); - if (licenseIt != this->dataPtr->licenses.end()) - { - form.emplace("license", std::to_string(licenseIt->second)); - } - // No license found, print an error and return. - else - { - std::string validLicenseNames; - auto end = this->dataPtr->licenses.end(); - std::advance(end, -1); - for (licenseIt = this->dataPtr->licenses.begin(); - licenseIt != end; ++licenseIt) - { - validLicenseNames += " " + licenseIt->first + "\n"; - } - validLicenseNames += " " + licenseIt->first; - - ignerr << "Invalid license[" << meta.legal().license() << "].\n" - << " Valid licenses include:\n" - << validLicenseNames << std::endl; - - return Result(ResultType::UPLOAD_ERROR); - } - } - // If there is no license information, then default to - // "Creative Commons - Public Domain" - else - { - form.emplace("license", "1"); - } - - // Add tags - std::string tags; - for (int i = 0; i < meta.tags_size(); ++i) - tags += meta.tags(i) + ","; - if (!tags.empty()) - form.emplace("tags", tags); - - // Recursively get all the files. - std::vector files; - this->dataPtr->AllFiles(_pathToModelDir, files); - for (const std::string &file : files) - { - form.emplace("file", std::string("@") + file + ";" - + file.substr(_pathToModelDir.size()+1)); - } // Send the request. resp = rest.Request(HttpMethod::POST_FORM, _id.Server().Url().Str(), @@ -1332,6 +1234,15 @@ Result FuelClient::CachedWorldFile(const common::URI &_fileUrl, Result FuelClient::PatchModel( const ignition::fuel_tools::ModelIdentifier &_model, const std::vector &_headers) +{ + return this->PatchModel(_model, _headers, ""); +} + +////////////////////////////////////////////////// +Result FuelClient::PatchModel( + const ignition::fuel_tools::ModelIdentifier &_model, + const std::vector &_headers, + const std::string &_pathToModelDir) { ignition::fuel_tools::Rest rest; RestResponse resp; @@ -1341,10 +1252,18 @@ Result FuelClient::PatchModel( common::URIPath path; path = path / _model.Owner() / "models" / _model.Name(); - std::multimap form = + std::multimap form; + + if (!_pathToModelDir.empty() && + !this->dataPtr->FillModelForm(_pathToModelDir, _model, + _model.Private(), form)) { - {"private", _model.Private() ? "1" : "0"} - }; + return Result(ResultType::UPLOAD_ERROR); + } + else + { + form.emplace("private", _model.Private() ? "1" : "0"); + } resp = rest.Request(HttpMethod::PATCH_FORM, serverUrl, version, path.Str(), {}, _headers, "", form); @@ -1372,22 +1291,188 @@ void FuelClientPrivate::AllFiles(const std::string &_path, } } - ////////////////////////////////////////////////// void FuelClient::PopulateLicenses(const ServerConfig &_server) { - ignition::fuel_tools::Rest rest; + this->dataPtr->PopulateLicenses(_server); +} + +////////////////////////////////////////////////// +bool FuelClientPrivate::FillModelForm(const std::string &_pathToModelDir, + const ModelIdentifier &_id, bool _private, + std::multimap &_form) +{ + if (!common::exists(_pathToModelDir)) + { + ignerr << "The model path[" << _pathToModelDir << "] doesn't exist.\n"; + return false; + } + + ignition::msgs::FuelMetadata meta; + + // Try the `metadata.pbtxt` file first since it contains more information + // than `model.config`. + if (common::exists(common::joinPaths(_pathToModelDir, "metadata.pbtxt"))) + { + std::string filePath = common::joinPaths(_pathToModelDir, "metadata.pbtxt"); + + igndbg << "Parsing " << filePath << std::endl; + + // Read the pbtxt file. + std::ifstream inputFile(filePath); + std::string inputStr((std::istreambuf_iterator(inputFile)), + std::istreambuf_iterator()); + + // Parse the file into the fuel metadata message + google::protobuf::TextFormat::ParseFromString(inputStr, &meta); + } + else if (common::exists(common::joinPaths(_pathToModelDir, "model.config"))) + { + std::string filePath = common::joinPaths(_pathToModelDir, "model.config"); + + igndbg << "Parsing " << filePath << std::endl; + + std::ifstream inputFile(filePath); + std::string inputStr((std::istreambuf_iterator(inputFile)), + std::istreambuf_iterator()); + + if (!ignition::msgs::ConvertFuelMetadata(inputStr, meta)) + { + ignerr << "Unable to convert model config[" << _pathToModelDir << "].\n"; + return false; + } + } + else + { + ignerr << "Provided model directory[" << _pathToModelDir + << "] needs a metadata.pbtxt or a model.confg file."; + return false; + } + + _form = + { + {"name", meta.name()}, + {"description", meta.description()}, + {"private", _private ? "1" : "0"}, + }; + + // \todo(nkoenig) The ign-fuelserver expects an integer number for the + // license information. The fuelserver should be modified to accept + // a string. Otherwise, we have to bake into each client a mapping of + // license name to integer. + // + // If we have legal, then attempt to fill in the correct license information. + if (meta.has_legal()) + { + // Attempt to retrieve the available licenses, if we have no available + // licenses. + if (this->licenses.empty()) + { + this->PopulateLicenses(_id.Server()); + // Fail if a license has been requested, but we couldn't get the + // available licenses. + if (this->licenses.empty()) + { + return false; + } + } + + // Find the license by name. + std::map::const_iterator licenseIt = + this->licenses.find(meta.legal().license()); + if (licenseIt != this->licenses.end()) + { + _form.emplace("license", std::to_string(licenseIt->second)); + } + // No license found, print an error and return. + else + { + std::string validLicenseNames; + auto end = this->licenses.end(); + std::advance(end, -1); + for (licenseIt = this->licenses.begin(); licenseIt != end; ++licenseIt) + { + validLicenseNames += " " + licenseIt->first + "\n"; + } + validLicenseNames += " " + licenseIt->first; + + ignerr << "Invalid license[" << meta.legal().license() << "].\n" + << " Valid licenses include:\n" + << validLicenseNames << std::endl; + + return false; + } + } + // If there is no license information, then default to + // "Creative Commons - Public Domain" + else + { + _form.emplace("license", "1"); + } + + // Add tags + std::string tags; + for (int i = 0; i < meta.tags_size(); ++i) + tags += meta.tags(i) + ","; + if (!tags.empty()) + _form.emplace("tags", tags); + + // Add categories + /* \todo: Uncomment in version 5.0 + std::string categories; + if (meta.has_categories()) + { + // Add the first category, if present. + if (!meta.categories().first().empty()) + categories = meta.categories().first(); + + // Add the second category, if present. + if (!meta.categories().second().empty()) + { + // Add a comma separator if the first category was not empty. + if (!categories.empty()) + categories += ", "; + categories += meta.categories().second(); + } + } + if (!categories.empty()) + _form.emplace("categories", categories); + */ + + // Add annotations as metadata. + for (const auto &annotation : meta.annotations()) + { + std::string formAnnotation = std::string("{\"key\":\"") + + annotation.first + "\",\"value\":\"" + annotation.second + "\"}"; + _form.emplace("metadata", formAnnotation); + } + + // Recursively get all the files. + std::vector files; + this->AllFiles(_pathToModelDir, files); + for (const std::string &file : files) + { + _form.emplace("file", std::string("@") + file + ";" + + file.substr(_pathToModelDir.size()+1)); + } + + return true; +} + +////////////////////////////////////////////////// +void FuelClientPrivate::PopulateLicenses(const ServerConfig &_server) +{ RestResponse resp; // Send the request. - resp = rest.Request(HttpMethod::GET, _server.Url().Str(), + resp = this->rest.Request(HttpMethod::GET, _server.Url().Str(), _server.Version(), "licenses", {}, {}, ""); if (resp.statusCode != 200) { ignerr << "Failed to get license information from " << _server.Url().Str() << "/" << _server.Version() << std::endl; } - else if (!JSONParser::ParseLicenses(resp.data, this->dataPtr->licenses)) + else if (!JSONParser::ParseLicenses(resp.data, this->licenses)) { ignerr << "Failed to parse license information[" << resp.data << "]\n"; } diff --git a/src/FuelClient_TEST.cc b/src/FuelClient_TEST.cc index 3d0242f8..54e6d4f0 100644 --- a/src/FuelClient_TEST.cc +++ b/src/FuelClient_TEST.cc @@ -1357,6 +1357,37 @@ TEST_F(FuelClientTest, UploadModelFail) EXPECT_EQ(ResultType::UPLOAD_ERROR, result.Type()); } +////////////////////////////////////////////////// +TEST_F(FuelClientTest, PatchModelFail) +{ + FuelClient client; + ModelIdentifier modelId; + + std::vector headers; + + // Bad directory + Result result = client.PatchModel(modelId, headers, "bad"); + EXPECT_EQ(ResultType::UPLOAD_ERROR, result.Type()); + + // Missing metadata.pbtxt and model.config + result = client.PatchModel(modelId, headers, common::cwd()); + EXPECT_EQ(ResultType::UPLOAD_ERROR, result.Type()); + + ClientConfig config; + config.SetCacheLocation(common::joinPaths(common::cwd(), "test_cache")); + createLocalModel(config); + + // Bad model.config + result = client.PatchModel(modelId, headers, + common::joinPaths(common::cwd(), "test_cache", "localhost:8007", + "alice", "models", "My Model", "3")); + EXPECT_EQ(ResultType::UPLOAD_ERROR, result.Type()); + + // Patch error + result = client.PatchModel(modelId, headers); + EXPECT_EQ(ResultType::PATCH_ERROR, result.Type()); +} + ////////////////////////////////////////////////// int main(int argc, char **argv) { diff --git a/src/cmd/cmdfuel.rb.in b/src/cmd/cmdfuel.rb.in index 737472f7..20988b74 100755 --- a/src/cmd/cmdfuel.rb.in +++ b/src/cmd/cmdfuel.rb.in @@ -98,6 +98,7 @@ SUBCOMMANDS = { " ign fuel edit [options] \n"\ " \n"\ "Available Options: \n"\ + " -m [--model] arg Path to directory containing the model. \n"\ " -u [--url] arg URL of the server that should receive \n"\ " the model. If unspecified, the server will be\n"\ " https://fuel.ignitionrobotics.org. \n"\ @@ -289,6 +290,15 @@ class Cmd puts "Missing model path." exit(-1) end + if options['url'] == '' + puts "Missing URL (e.g. --url https://fuel.ignitionrobotics.org)." + exit(-1) + end + when 'edit' + if options['url'] == '' + puts "Missing resource URL (e.g. --url https://fuel.ignitionrobotics.org/1.0/OpenRobotics/models/Ambulance)." + exit(-1) + end end options @@ -346,9 +356,9 @@ class Cmd exit(-1) end when 'edit' - Importer.extern 'int editUrl(const char *, const char *, const char *)' + Importer.extern 'int editUrl(const char *, const char *, const char *, const char *)' if not Importer.editUrl(options['url'], options['header'], - options['private']) + options['private'], options['model']) exit(-1) end when 'list' diff --git a/src/ign.cc b/src/ign.cc index 0182426c..74402dcf 100644 --- a/src/ign.cc +++ b/src/ign.cc @@ -902,7 +902,8 @@ extern "C" IGNITION_FUEL_TOOLS_VISIBLE int pbtxt2Config(const char *_path) ////////////////////////////////////////////////// extern "C" IGNITION_FUEL_TOOLS_VISIBLE int editUrl( - const char *_url, const char *_header, const char *_private) + const char *_url, const char *_header, const char *_private, + const char *_path) { ignition::fuel_tools::ClientConfig conf; conf.SetUserAgent("FuelTools " IGNITION_FUEL_TOOLS_VERSION_FULL); @@ -925,6 +926,17 @@ extern "C" IGNITION_FUEL_TOOLS_VISIBLE int editUrl( ignition::fuel_tools::ModelIdentifier model; + std::string modelPath; + if (_path && std::strlen(_path) != 0) + { + if (!ignition::common::exists(_path)) + { + ignerr << "The model path[" << _path << "] doesn't exist.\n"; + return 0; + } + modelPath = _path; + } + // Check to see if a model has been specified in the the URI. if (client.ParseModelUrl(url, model)) { @@ -944,11 +956,11 @@ extern "C" IGNITION_FUEL_TOOLS_VISIBLE int editUrl( } // Change the privacy setting, if a change is present. - if (privateBool.has_value()) + if (privateBool.has_value() || !modelPath.empty()) { details.SetPrivate(*privateBool); - if (!client.PatchModel(details, headers)) + if (!client.PatchModel(details, headers, modelPath)) { ignerr << "Failed to patch model[" << model.Name() << "].\n"; return 0; diff --git a/src/ign.hh b/src/ign.hh index 3da6d00d..a13d0866 100644 --- a/src/ign.hh +++ b/src/ign.hh @@ -118,9 +118,11 @@ extern "C" IGNITION_FUEL_TOOLS_VISIBLE int pbtxt2Config(const char *_path); /// \param[in] _header An HTTP header. /// \param[in] _private "1" to make the resource private, "0" to make it /// public +/// \param[in] _path Resource path. /// \return 1 if successful, 0 if not. extern "C" IGNITION_FUEL_TOOLS_VISIBLE int editUrl( const char *_url, const char *_header = nullptr, - const char *_private = nullptr); + const char *_private = nullptr, + const char *_path = nullptr); #endif