Updated findfile() to search localpath first (#1292)

As the issue above mentions, this tutorial for Garden, is not able to parse the SDF file if it was named model.sdf. It works if the file name is changed. This was happening due to the search order used in the sdf::findFile() function. The used search order was causing the findFile() to find the model.sdf from the install path instead of the one present locally. Since the found model.sdf is actually the SDF spec file for the <model> and it is not a valid SDFormat file, it caused the script to throw a not valid error.
This PR changes the search order used in the findFile() function by searching for the local path (current directory) first and solves the issue.
---------

Signed-off-by: Jasmeet Singh <[email protected]>
Co-authored-by: Addisu Z. Taddese <[email protected]>

Migration.md

@@ -24,6 +24,7 @@ but with improved human-readability..
 
 1. ParserConfig defaults to WARN instead of LOG when parsing unrecognized
    elements.
+2. Updated search order for `sdf::findFile()` making local path (current directory) the first to be searched. 
 
 ### Deprecations
 

include/sdf/SDFImpl.hh

@@ -59,13 +59,13 @@ namespace sdf
   /// The search order in the function is as follows:
   /// 1. Using the global URI path map, search in paths associated with the URI
   ///    scheme of the input.
-  /// 2. Seach in the path defined by the macro `SDF_SHARE_PATH`.
-  /// 3. Search in the the libsdformat install path. The path is formed by
-  ///    has the pattern `SDF_SHARE_PATH/sdformat<major version>/<version>/`
-  /// 4. Directly check if the input path exists in the filesystem.
-  /// 5. Seach in the path defined by the environment variable `SDF_PATH`.
-  /// 6. If enabled via _searchLocalPath, prepend the input with the current
+  /// 2. If enabled via _searchLocalPath, prepend the input with the current
   ///    working directory and check if the result path exists.
+  /// 3. Seach in the path defined by the macro `SDF_SHARE_PATH`.
+  /// 4. Search in the the libsdformat install path. The path is formed by
+  ///    has the pattern `SDF_SHARE_PATH/sdformat<major version>/<version>/`
+  /// 5. Directly check if the input path exists in the filesystem.
+  /// 6. Seach in the path defined by the environment variable `SDF_PATH`.
   /// 7. If enabled via _useCallback and the global callback function is set,
   ///    invoke the function and return its result.
   ///

src/SDF.cc

@@ -97,6 +97,17 @@ std::string findFile(const std::string &_filename, bool _searchLocalPath,
     filename = filename.substr(idx + sep.length());
   }
 
+  // First check the local path, if the flag is set.
+  if (_searchLocalPath)
+  {
+    std::string path = sdf::filesystem::append(sdf::filesystem::current_path(),
+                                               filename);
+    if (sdf::filesystem::exists(path))
+    {
+      return path;
+    }
+  }
+
   // Next check the install path.
   std::string path = sdf::filesystem::append(SDF_SHARE_PATH, filename);
   if (sdf::filesystem::exists(path))
@@ -113,7 +124,7 @@ std::string findFile(const std::string &_filename, bool _searchLocalPath,
     return path;
   }
 
-  // Next check to see if the given file exists.
+  // Finally check to see if the given file exists.
   path = filename;
   if (sdf::filesystem::exists(path))
   {
@@ -135,16 +146,6 @@ std::string findFile(const std::string &_filename, bool _searchLocalPath,
     }
   }
 
-  // Finally check the local path, if the flag is set.
-  if (_searchLocalPath)
-  {
-    path = sdf::filesystem::append(sdf::filesystem::current_path(), filename);
-    if (sdf::filesystem::exists(path))
-    {
-      return path;
-    }
-  }
-
   // If we still haven't found the file, use the registered callback if the
   // flag has been set
   if (_useCallback)

src/SDF_TEST.cc

@@ -788,4 +788,32 @@ TEST(SDF, WriteURIPath)
   ASSERT_EQ(std::remove(tempFile.c_str()), 0);
   ASSERT_EQ(rmdir(tempDir.c_str()), 0);
 }
+
+/////////////////////////////////////////////////
+TEST(SDF, FindFileModelSDFCurrDir)
+{
+  std::string currDir;
+
+  // Get current directory path from $PWD env variable
+  currDir = sdf::filesystem::current_path();
+
+  // A file named model.sdf in current directory
+  auto tempFile = currDir + "/model.sdf";
+
+  sdf::SDF tempSDF;
+  tempSDF.Write(tempFile);
+
+  // Check file was created
+  auto fp = fopen(tempFile.c_str(), "r");
+  ASSERT_NE(nullptr, fp);
+
+  // Get path of the file returned from findFile()
+  std::string foundFile = sdf::findFile("model.sdf", true, false);
+
+  // Check that the returned file is model.sdf from curr directory
+  ASSERT_EQ(tempFile, foundFile);
+
+  // Cleanup
+  ASSERT_EQ(std::remove(tempFile.c_str()), 0);
+}
 #endif  // _WIN32