Restore support for model:// URIs

Migration.md

@@ -12,8 +12,8 @@ release will remove the deprecated code.
 1. Corrected `BAYER_RGGR8` to `BAYER_BGGR8` in `PixelFormatName` and
    `PixelFormatType` located in `graphics/include/ignition/common/Image.hh`.
 
-1. URI parsing has updated to follow the specification more closely. Changes
-   include:
+1. URI parsing has updated to follow the specification more closely when
+   `URI::Authority` is set. Changes include:
     * An empty URI Path is valid.
     * Double forward slashes, `//`, are valid in a URI Path.
     * A URI Query does not require a `key=value` format. For example

include/ignition/common/SystemPaths.hh

@@ -71,9 +71,13 @@ namespace ignition
       /// \param[in] _uri the uniform resource identifier
       /// \return Returns full path name to file with platform-specific
       /// directory separators, or an empty string if URI couldn't be found.
+      /// \sa FindFileURI(const ignition::common::URI &_uri)
       public: std::string FindFileURI(const std::string &_uri) const;
 
-      /// \brief Find a file or path using a URI
+      /// \brief Find a file or path using a URI.
+      /// If URI is not an absolute path, the URI's path will be matched against
+      /// all added paths and environment variables (including URI authority as
+      /// needed).
       /// \param[in] _uri the uniform resource identifier
       /// \return Returns full path name to file with platform-specific
       /// directory separators, or an empty string if URI couldn't be found.
@@ -87,10 +91,12 @@ namespace ignition
       /// \param[in] _filename Name of the file to find.
       /// \param[in] _searchLocalPath True to search in the current working
       /// directory.
+      /// \param[in] _verbose False to omit console messages.
       /// \return Returns full path name to file with platform-specific
       /// directory separators, or empty string on error.
       public: std::string FindFile(const std::string &_filename,
-                                   const bool _searchLocalPath = true) const;
+                                   const bool _searchLocalPath = true,
+                                   const bool _verbose = true) const;
 
       /// \brief Find a shared library by name in the plugin paths
       ///

include/ignition/common/URI.hh

@@ -52,6 +52,13 @@ namespace ignition
     /// authority. For example, `file://abs/path` will result in a host
     /// value of `abs` and the URI path will be `/path`. You can specify
     /// a relative path using `file:abs/path`.
+    ///
+    /// URIs that are set not to have an authority
+    /// (i.e. `Authority() == false`) preserve the legacy behaviour, which
+    /// is:
+    /// * `file:/abs/path` has the path `/abs/path`
+    /// * `file://abs/path` has the path `abs/path`
+    /// * `file:///abs/path` has the path `/abs/path`
     class IGNITION_COMMON_VISIBLE URIAuthority
     {
       /// \brief Constructor
@@ -148,7 +155,15 @@ namespace ignition
       IGN_COMMON_WARN_RESUME__DLL_INTERFACE_MISSING
     };
 
-    /// \brief The path component of a URI
+    /// \brief A URI path contains a sequence of segments separated by `/`.
+    /// The path may be empty in a valid URI.
+    /// When an authority is present, the path must start with a `/`.
+    ///
+    /// In the following URI:
+    ///
+    /// scheme://authority.com/seg1/seg2?query
+    ///
+    /// The path is `/seg1/seg2`
     class IGNITION_COMMON_VISIBLE URIPath
     {
       /// \brief Constructor
@@ -203,6 +218,16 @@ namespace ignition
       /// and a Windows drive specifier (e.g. 'C:') is pushed to it.
       public: void PushBack(const std::string &_part);
 
+      /// \brief Remove the part that's in the front of this path and return it.
+      /// \return Popped part.
+      /// Returns empty string if path doesn't have parts to be popped.
+      public: std::string PopFront();
+
+      /// \brief Remove the part that's in the back of this path and return it.
+      /// \return Popped part.
+      /// Returns empty string if path doesn't have parts to be popped.
+      public: std::string PopBack();
+
       /// \brief Compound assignment operator.
       /// \param[in] _part A new path to append.
       /// \return A new Path that consists of "this / _part"
@@ -383,9 +408,11 @@ namespace ignition
       /// \brief Default constructor
       public: URI();
 
-      /// \brief Construct a URI object from a string.
-      /// \param[in] _str A string.
-      public: explicit URI(const std::string &_str);
+      /// \brief Default constructor
+      /// \param[in] _hasAuthority False if the URI doesn't have an authority.
+      /// Defaults to false.
+      public: explicit URI(const std::string &_str,
+          bool _hasAuthority = false);
 
       /// \brief Copy constructor
       /// \param[in] _uri Another URI.
@@ -412,13 +439,13 @@ namespace ignition
       /// \param[in] _scheme New scheme.
       public: void SetScheme(const std::string &_scheme);
 
-      /// \brief Get a mutable version of the URI's authority.
+      /// \brief Set the URI's authority.
       /// \return The authority
-      public: URIAuthority &Authority();
+      public: void SetAuthority(const URIAuthority &_authority);
 
-      /// \brief Get a const reference of the URI's authority.
+      /// \brief Get a copy of the URI's authority.
       /// \return The authority
-      public: const URIAuthority &Authority() const;
+      public: std::optional<URIAuthority> Authority() const;
 
       /// \brief Get a mutable version of the path component
       /// \return A reference to the path

src/SystemPaths.cc

@@ -313,15 +313,30 @@ std::string SystemPaths::FindFileURI(const std::string &_uri) const
 std::string SystemPaths::FindFileURI(const ignition::common::URI &_uri) const
 {
   std::string prefix = _uri.Scheme();
-  std::string suffix = _uri.Path().Str() + _uri.Query().Str();
-  std::string filename;
-
-  if (prefix == "file")
+  std::string suffix;
+  if (_uri.Authority())
   {
-    // First try to find the file on the current system
-    filename = this->FindFile(ignition::common::copyFromUnixPath(suffix));
+    // Strip //
+    suffix = (*_uri.Authority()).Str().substr(2) + _uri.Path().Str();
   }
-  else if (this->dataPtr->findFileURICB)
+  else
+  {
+    // Strip /
+    if (_uri.Path().IsAbsolute() && prefix != "file")
+      suffix += _uri.Path().Str().substr(1);
+    else
+      suffix += _uri.Path().Str();
+  }
+  suffix += _uri.Query().Str();
+
+  std::string filename;
+
+  // First try to find the file on the current system
+  filename = this->FindFile(ignition::common::copyFromUnixPath(suffix),
+      true, false);
+
+  // Try URI callback
+  if (filename.empty() && this->dataPtr->findFileURICB)
   {
     filename = this->dataPtr->findFileURICB(_uri.Str());
   }
@@ -371,7 +386,8 @@ std::string SystemPaths::FindFileURI(const ignition::common::URI &_uri) const
 
 //////////////////////////////////////////////////
 std::string SystemPaths::FindFile(const std::string &_filename,
-                                  const bool _searchLocalPath) const
+                                  const bool _searchLocalPath,
+                                  const bool _verbose) const
 {
   std::string path;
   std::string filename = _filename;
@@ -441,14 +457,20 @@ std::string SystemPaths::FindFile(const std::string &_filename,
 
   if (path.empty())
   {
-    ignerr << "Could not resolve file [" << _filename << "]" << std::endl;
+    if (_verbose)
+    {
+      ignerr << "Could not resolve file [" << _filename << "]" << std::endl;
+    }
     return std::string();
   }
 
   if (!exists(path))
   {
-    ignerr << "File [" << _filename << "] resolved to path [" << path <<
-              "] but the path does not exist" << std::endl;
+    if (_verbose)
+    {
+      ignerr << "File [" << _filename << "] resolved to path [" << path <<
+                "] but the path does not exist" << std::endl;
+    }
     return std::string();
   }
 

src/SystemPaths_TEST.cc

@@ -126,8 +126,8 @@ TEST_F(SystemPathsFixture, FileSystemPaths)
   filePaths.push_back("test_dir1");
   common::setenv(kFilePath, SystemPathsJoin(filePaths));
   paths.SetFilePathEnv(kFilePath);
-  EXPECT_EQ(file1, paths.FindFile("test_f1")) << paths.FindFile("test_f1");
-  EXPECT_EQ(file1, paths.FindFile("model:test_f1"));
+  EXPECT_EQ(file1, paths.FindFile("test_f1"));
+  EXPECT_EQ(file1, paths.FindFile("model://test_f1"));
 }
 
 /////////////////////////////////////////////////
@@ -234,7 +234,8 @@ TEST_F(SystemPathsFixture, FindFileURI)
   auto osrfCb = [dir2](const ignition::common::URI &_uri)
   {
     return _uri.Scheme() == "osrf" ?
-           ignition::common::joinPaths(dir2, _uri.Path().Str()) : "";
+           ignition::common::joinPaths(dir2, ignition::common::copyFromUnixPath(
+           _uri.Path().Str())) : "";
   };
   auto robot2Cb = [dir2](const ignition::common::URI &_uri)
   {
@@ -243,7 +244,7 @@ TEST_F(SystemPathsFixture, FindFileURI)
   };
 
   EXPECT_EQ("", sp.FindFileURI("robot://test_f1"));
-  EXPECT_EQ("", sp.FindFileURI("osrf:test_f2"));
+  EXPECT_EQ("", sp.FindFileURI("osrf://test_f2"));
 
   // We still want to test the deprecated function until it is removed.
 #ifndef _WIN32
@@ -256,31 +257,31 @@ TEST_F(SystemPathsFixture, FindFileURI)
 #endif
 
   EXPECT_EQ(file1, sp.FindFileURI("robot://test_f1"));
-  EXPECT_EQ("", sp.FindFileURI("osrf:test_f2"));
+  EXPECT_EQ("", sp.FindFileURI("osrf://test_f2"));
 
   sp.AddFindFileURICallback(osrfCb);
   EXPECT_EQ(file1, sp.FindFileURI("robot://test_f1"));
-  EXPECT_EQ(file2, sp.FindFileURI("osrf:test_f2"));
+  EXPECT_EQ(file2, sp.FindFileURI("osrf://test_f2"));
 
   // Test that th CB from SetFindFileURICallback is called first even when a
   // second handler for the same protocol is available
   sp.AddFindFileURICallback(robot2Cb);
   EXPECT_EQ(file1, sp.FindFileURI("robot://test_f1"));
-  EXPECT_EQ(file2, sp.FindFileURI("osrf:test_f2"));
+  EXPECT_EQ(file2, sp.FindFileURI("osrf://test_f2"));
 
   // URI + env var
   common::setenv(kFilePath, dir1);
   sp.SetFilePathEnv(kFilePath);
   EXPECT_EQ(kFilePath, sp.FilePathEnv());
-  EXPECT_EQ(file1, sp.FindFileURI("anything:test_f1"));
-  EXPECT_NE(file2, sp.FindFileURI("anything:test_f2"));
+  EXPECT_EQ(file1, sp.FindFileURI("anything://test_f1"));
+  EXPECT_NE(file2, sp.FindFileURI("anything://test_f2"));
 
   std::string newEnv{"IGN_NEW_FILE_PATH"};
   common::setenv(newEnv, dir2);
   sp.SetFilePathEnv(newEnv);
   EXPECT_EQ(newEnv, sp.FilePathEnv());
-  EXPECT_NE(file1, sp.FindFileURI("anything:test_f1"));
-  EXPECT_EQ(file2, sp.FindFileURI("anything:test_f2"));
+  EXPECT_NE(file1, sp.FindFileURI("anything://test_f1"));
+  EXPECT_EQ(file2, sp.FindFileURI("anything://test_f2"));
 }
 
 //////////////////////////////////////////////////
@@ -317,6 +318,7 @@ TEST_F(SystemPathsFixture, FindFile)
   EXPECT_EQ("", sp.FindFile(this->filesystemRoot + "no_such_file"));
   EXPECT_EQ("", sp.FindFile("no_such_file"));
   EXPECT_EQ(file1, sp.FindFile(common::joinPaths("test_dir1", "test_f1")));
+
   EXPECT_EQ(file1, sp.FindFile("file:test_dir1/test_f1"));
 
   // Existing absolute paths