From 964db6f4bfaba795446f4cf5bdd8a545d2e2ee9a Mon Sep 17 00:00:00 2001
From: Louise Poubel <louise@openrobotics.org>
Date: Fri, 15 Jan 2021 14:18:55 -0800
Subject: [PATCH] Edifice deprecations (#159)

Signed-off-by: Louise Poubel <louise@openrobotics.org>
---
 Migration.md                              | 17 +++++++++++++
 include/ignition/fuel_tools/FuelClient.hh | 17 ++++++++-----
 src/FuelClient.cc                         | 29 +++++++++++++----------
 src/FuelClient_TEST.cc                    |  7 ++++++
 4 files changed, 52 insertions(+), 18 deletions(-)

diff --git a/Migration.md b/Migration.md
index 622d9dc6..f7363e09 100644
--- a/Migration.md
+++ b/Migration.md
@@ -1,3 +1,20 @@
+## Ignition Fuel Tools 5.X to 6.X
+
+### Deprecations
+
+* **Deprecation**: `FuelClient::DeleteModel`
+* **Replacement**: `FielClient::DeleteUrl`
+
+* **Deprecation**: `FuelClient` constructor that takes `LocalCache`
+* **Replacement**: `FielClient` constructor without `LocalCache`
+
+## Ignition Fuel Tools 4.X to 5.X
+
+### Deprecations
+
+* **Deprecation**: `LocalCache`
+* **Replacement**: None
+
 ## Ignition Fuel Tools 3.X to 4.X
 
 ### Modifications
diff --git a/include/ignition/fuel_tools/FuelClient.hh b/include/ignition/fuel_tools/FuelClient.hh
index 64c4ab0e..baff1bd8 100644
--- a/include/ignition/fuel_tools/FuelClient.hh
+++ b/include/ignition/fuel_tools/FuelClient.hh
@@ -54,6 +54,13 @@ namespace ignition
       /// \brief Default constructor.
       public: FuelClient();
 
+      /// \brief Constructor accepts server and auth configuration
+      /// \param[in] _config configuration about servers to connect to
+      /// \param[in] _rest A REST request.
+      /// \remarks the client saves a copy of the config passed into it
+      public: FuelClient(const ClientConfig &_config,
+                         const Rest &_rest = Rest());
+
       /// \brief Constructor accepts server and auth configuration
       /// \param[in] _config configuration about servers to connect to
       /// \param[in] _rest A REST request.
@@ -62,9 +69,9 @@ namespace ignition
       ///            destructed. If set to nullptr the client will instantiate
       ///            it's own cache.
       /// \remarks the client saves a copy of the config passed into it
-      public: FuelClient(const ClientConfig &_config,
-                         const Rest &_rest = Rest(),
-                         LocalCache *_cache = nullptr);
+      public: IGN_DEPRECATED(6) FuelClient(const ClientConfig &_config,
+                                           const Rest &_rest,
+                                           LocalCache *_cache);
 
       /// \brief Destructor
       public: ~FuelClient();
@@ -177,9 +184,7 @@ namespace ignition
       /// \brief Remove a model from ignition fuel
       /// \param[in] _id The model identifier.
       /// \return Result of the delete operation
-      /// Deprecate this function in ign-fuel-tools6. DeleteUrl
-      /// replaces this function.
-      public: Result DeleteModel(const ModelIdentifier &_id);
+      public: Result IGN_DEPRECATED(6) DeleteModel(const ModelIdentifier &_id);
 
       /// \brief Remove a resource, such as a model or world, from Ignition Fuel
       /// \param[in] _uri The full URI of the resource, e.g:
diff --git a/src/FuelClient.cc b/src/FuelClient.cc
index 05ad827e..6e2f0c36 100644
--- a/src/FuelClient.cc
+++ b/src/FuelClient.cc
@@ -217,34 +217,31 @@ class ignition::fuel_tools::FuelClientPrivate
 
 //////////////////////////////////////////////////
 FuelClient::FuelClient()
-  : FuelClient(ClientConfig(), Rest(), nullptr)
+  : FuelClient(ClientConfig(), Rest())
 {
 }
 
 //////////////////////////////////////////////////
-FuelClient::FuelClient(const ClientConfig &_config, const Rest &_rest,
-    LocalCache *_cache)
-  : dataPtr(new FuelClientPrivate)
+FuelClient::FuelClient(const ClientConfig &_config, const Rest &_rest)
+  : dataPtr(std::make_unique<FuelClientPrivate>())
 {
   this->dataPtr->config = _config;
   this->dataPtr->rest = _rest;
   this->dataPtr->rest.SetUserAgent(this->dataPtr->config.UserAgent());
 
-  if (nullptr == _cache)
-  {
 #ifndef _WIN32
 # pragma GCC diagnostic push
 # pragma GCC diagnostic ignored "-Wdeprecated-declarations"
+#else
+# pragma warning(push)
+# pragma warning(disable: 4996)
 #endif
-    this->dataPtr->cache.reset(new LocalCache(&(this->dataPtr->config)));
+  this->dataPtr->cache = std::make_unique<LocalCache>(&(this->dataPtr->config));
 #ifndef _WIN32
 # pragma GCC diagnostic pop
+#else
+# pragma warning(pop)
 #endif
-  }
-  else
-  {
-    this->dataPtr->cache.reset(_cache);
-  }
 
   this->dataPtr->urlModelRegex.reset(new std::regex(
     this->dataPtr->kModelUrlRegexStr));
@@ -258,6 +255,14 @@ FuelClient::FuelClient(const ClientConfig &_config, const Rest &_rest,
     this->dataPtr->kCollectionUrlRegexStr));
 }
 
+//////////////////////////////////////////////////
+FuelClient::FuelClient(const ClientConfig &_config, const Rest &_rest,
+      LocalCache *_cache) : FuelClient(_config, _rest)
+{
+  if (_cache != nullptr)
+    this->dataPtr->cache.reset(_cache);
+}
+
 //////////////////////////////////////////////////
 FuelClient::~FuelClient()
 {
diff --git a/src/FuelClient_TEST.cc b/src/FuelClient_TEST.cc
index 54e6d4f0..22bda451 100644
--- a/src/FuelClient_TEST.cc
+++ b/src/FuelClient_TEST.cc
@@ -1342,7 +1342,14 @@ TEST_F(FuelClientTest, DeleteModelFail)
   FuelClient client;
   ModelIdentifier modelId;
 
+#ifndef _WIN32
+# pragma GCC diagnostic push
+# pragma GCC diagnostic ignored "-Wdeprecated-declarations"
+#endif
   Result result = client.DeleteModel(modelId);
+#ifndef _WIN32
+# pragma GCC diagnostic pop
+#endif
   EXPECT_EQ(ResultType::DELETE_ERROR, result.Type());
 }