From c1af56903538e11a5c736d00da6e4ea16e4fdc05 Mon Sep 17 00:00:00 2001
From: Julien Osman <julien.osman@csgroup.eu>
Date: Tue, 7 Jul 2020 19:13:54 +0200
Subject: [PATCH] ENH: Change interface for GetMetadataValue: char* to
 std::string

---
 .../include/otbMetadataSupplierInterface.h    | 57 ++++++++++---------
 .../Metadata/include/otbXMLMetadataSupplier.h |  2 +-
 .../src/otbMetadataSupplierInterface.cxx      |  7 ++-
 .../Metadata/src/otbXMLMetadataSupplier.cxx   | 12 +++-
 Modules/IO/IOGDAL/include/otbGDALImageIO.h    |  2 +-
 Modules/IO/IOGDAL/src/otbGDALImageIO.cxx      | 36 +++++++-----
 6 files changed, 68 insertions(+), 48 deletions(-)

diff --git a/Modules/Core/Metadata/include/otbMetadataSupplierInterface.h b/Modules/Core/Metadata/include/otbMetadataSupplierInterface.h
index f5a289c01c..18d2f55f6d 100644
--- a/Modules/Core/Metadata/include/otbMetadataSupplierInterface.h
+++ b/Modules/Core/Metadata/include/otbMetadataSupplierInterface.h
@@ -74,68 +74,71 @@ public:
 
 
   /** Get the metadata value corresponding to a given path (meaning of this path
-   * depends on the specific implementation. Returns NULL when path is not found
+   * depends on the specific implementation. Returns empty string when path is not found,
+   * and hasValue is set to False.
    * If band >= 0, the metadata value is looked in the specified band*/
-  virtual const char * GetMetadataValue(const char * path, int band=-1) const = 0;
+  virtual const std::string GetMetadataValue(const std::string path, bool& hasValue, int band=-1) const = 0;
 
-  bool HasValue(const char * path, int band=-1);
+  bool HasValue(std::string path, int band=-1);
 
   // utility functions
-  template <typename T> T GetAs(const char *path, int band=-1) const
+  template <typename T> T GetAs(std::string path, int band=-1) const
+  {
+    bool hasValue;
+    std::string ret = GetMetadataValue(path, hasValue, band);
+    if (!hasValue)
     {
-    const char * ret = GetMetadataValue(path, band);
-    if (ret == nullptr)
-      {
       otbGenericExceptionMacro(MissingMetadataException,<<"Missing metadata '"<<path<<"'")
-      }
+    }
     try
-      {
+    {
       return boost::lexical_cast<T>(ret);
-      }
+    }
     catch (boost::bad_lexical_cast&)
-      {
+    {
       otbGenericExceptionMacro(MissingMetadataException,<<"Bad metadata value for '"<<path<<"', got: "<<ret)
-      }
     }
+  }
 
   /** Parse a metadata value to a std::vector,
    *  If size>=0, then the final std::vector size is checked and an exception
    *  is raised if it doesn't match the given size.*/
-  template < typename T> std::vector<T> GetAsVector(const char *path, const char sep=' ', int size=-1, int band=-1) const
+  template < typename T> std::vector<T> GetAsVector(std::string path, const char sep=' ', int size=-1, int band=-1) const
+  {
+    bool hasValue;
+    std::string ret = GetMetadataValue(path, hasValue, band);
+    if (!hasValue)
     {
-    const char * ret = GetMetadataValue(path, band);
-    if (ret == nullptr)
-      {
       otbGenericExceptionMacro(MissingMetadataException,<<"Missing metadata '"<<path<<"'")
-      }
+    }
     string_view value(ret);
     string_view filt_value = rstrip(lstrip(value,"[ "), "] ");
     std::vector<T> output;
     typedef part_range<splitter_on_delim> range_type;
     const range_type parts = split_on(filt_value, sep);
     for (auto const& part : parts)
-      {
+    {
       // TODO: check if we can use lexical_cast on a string_view
       std::string strPart = to<std::string>(part, "casting string_view to std::string");
       if (strPart.empty())
-        {
+      {
         continue;
-        }
+      }
       try
-        {
+      {
         output.push_back(boost::lexical_cast<T>(strPart));
-        }
+      }
       catch (boost::bad_lexical_cast&)
-        {
+      {
         otbGenericExceptionMacro(MissingMetadataException,<<"Bad metadata vector element in '"<<path<<"', got :"<<part)
-        }
       }
+    }
     if ((size >= 0) && (output.size() != (size_t)size))
-      {
+    {
       otbGenericExceptionMacro(MissingMetadataException,<<"Bad number of elements in vector '"<<path<<"', expected "<<size<< ", got "<<output.size())
-      }
-    return output;
     }
+    return output;
+  }
 
 };
 
diff --git a/Modules/Core/Metadata/include/otbXMLMetadataSupplier.h b/Modules/Core/Metadata/include/otbXMLMetadataSupplier.h
index 3783a124f8..225e14a4c9 100644
--- a/Modules/Core/Metadata/include/otbXMLMetadataSupplier.h
+++ b/Modules/Core/Metadata/include/otbXMLMetadataSupplier.h
@@ -47,7 +47,7 @@ public:
   /** Get the metadata value corresponding to a given path
    * Returns NULL when path is not found
    * If band >= 0, the metadata value is looked in the specified band*/
-  const char * GetMetadataValue(const char * path, int band=1) const override;
+  const std::string GetMetadataValue(const std::string path, bool& hasValue, int band=1) const override;
 
   std::string GetResourceFile(std::string="") const override;
 
diff --git a/Modules/Core/Metadata/src/otbMetadataSupplierInterface.cxx b/Modules/Core/Metadata/src/otbMetadataSupplierInterface.cxx
index c276bb6096..4cbe8a5128 100644
--- a/Modules/Core/Metadata/src/otbMetadataSupplierInterface.cxx
+++ b/Modules/Core/Metadata/src/otbMetadataSupplierInterface.cxx
@@ -23,10 +23,11 @@
 namespace otb
 {
 
-bool MetadataSupplierInterface::HasValue(const char * path, int band)
+bool MetadataSupplierInterface::HasValue(const std::string path, int band)
 {
-  const char * ret = GetMetadataValue(path, band);
-  return ret;
+  bool hasValue;
+  const std::string ret = GetMetadataValue(path, hasValue, band);
+  return hasValue;
 }
 
 
diff --git a/Modules/Core/Metadata/src/otbXMLMetadataSupplier.cxx b/Modules/Core/Metadata/src/otbXMLMetadataSupplier.cxx
index d4406642c0..ec1ca62c79 100644
--- a/Modules/Core/Metadata/src/otbXMLMetadataSupplier.cxx
+++ b/Modules/Core/Metadata/src/otbXMLMetadataSupplier.cxx
@@ -41,9 +41,17 @@ XMLMetadataSupplier::XMLMetadataSupplier(const std::string & fileName)
   CPLDestroyXMLNode(psNode);
 }
 
-const char * XMLMetadataSupplier::GetMetadataValue(const char * path, int band) const
+const std::string XMLMetadataSupplier::GetMetadataValue(const std::string path, bool& hasValue, int band) const
 {
-  return CSLFetchNameValue(m_MetadataDic, path);
+  const char * ret = CSLFetchNameValue(m_MetadataDic, path);
+  if (ret)
+    hasValue = true;
+  else
+  {
+    hasValue = false;
+    ret = "";
+  }
+  return std::string(ret);
 }
 
 std::string XMLMetadataSupplier::GetResourceFile(std::string) const
diff --git a/Modules/IO/IOGDAL/include/otbGDALImageIO.h b/Modules/IO/IOGDAL/include/otbGDALImageIO.h
index 657bea76b2..22a493a05b 100644
--- a/Modules/IO/IOGDAL/include/otbGDALImageIO.h
+++ b/Modules/IO/IOGDAL/include/otbGDALImageIO.h
@@ -210,7 +210,7 @@ public:
   std::vector<std::string> GetResourceFiles() const override;
 
   /** Get metadata item in GDALDataset, domain can specified as "domain/key" */
-  const char * GetMetadataValue(const char * path, int band = -1) const override;
+  const std::string GetMetadataValue(const std::string path, bool& hasValue, int band = -1) const override;
 
   /** Set metadata item in GDALDataset, domain can specified as prefix of the
    *  path, like "domain/key"*/
diff --git a/Modules/IO/IOGDAL/src/otbGDALImageIO.cxx b/Modules/IO/IOGDAL/src/otbGDALImageIO.cxx
index 0bbeae85eb..f50a024c04 100644
--- a/Modules/IO/IOGDAL/src/otbGDALImageIO.cxx
+++ b/Modules/IO/IOGDAL/src/otbGDALImageIO.cxx
@@ -1831,24 +1831,31 @@ std::vector<std::string> GDALImageIO::GetResourceFiles() const
   return result;
 }
 
-const char * GDALImageIO::GetMetadataValue(const char * path, int band) const
+const std::string GDALImageIO::GetMetadataValue(const std::string path, bool& hasValue, int band) const
 {
   // detect namespace if any
-  const char *slash = strchr(path,'/');
   std::string domain("");
-  const char *domain_c = nullptr;
   std::string key(path);
-  if (slash)
-    {
-    domain = std::string(path, (slash-path));
-    domain_c = domain.c_str();
-    key = std::string(slash+1);
-    }
+  std::size_t found = path.find_first_of("/");
+  if (found != std::string::npos)
+  {
+    domain = path.substr(0, found);
+    key = path.substr(found + 1);
+  }
+
+  const char* ret;
   if (band >= 0)
-    {
-    return m_Dataset->GetDataSet()->GetRasterBand(band+1)->GetMetadataItem(key.c_str(), domain_c);
-    }
-  return m_Dataset->GetDataSet()->GetMetadataItem(key.c_str(), domain_c);
+    ret = m_Dataset->GetDataSet()->GetRasterBand(band+1)->GetMetadataItem(key.c_str(), domain.c_str());
+  else
+    ret = m_Dataset->GetDataSet()->GetMetadataItem(key.c_str(), domain.c_str());
+  if (ret)
+    hasValue = true;
+  else
+  {
+    hasValue = false;
+    ret = "";
+  }
+  return std::string(ret);
 }
 
 void GDALImageIO::SetMetadataValue(const char * path, const char * value, int band)
@@ -1905,7 +1912,8 @@ void GDALImageIO::ImportMetadata()
   // Keys Starting with: MDGeomNames[MDGeom::SensorGeometry] + '.' should
   // be decoded by the (future) SensorModelFactory.
   // Use ImageMetadataBase::FromKeywordlist to ingest the metadata
-  if (std::string(GetMetadataValue("METADATATYPE")) != "OTB")
+  bool hasValue;
+  if (std::string(GetMetadataValue("METADATATYPE", hasValue)) != "OTB")
     return;
   ImageMetadataBase::Keywordlist kwl;
   GDALMetadataToKeywordlist(m_Dataset->GetDataSet()->GetMetadata(), kwl);
-- 
GitLab