From da1f325b06f1c1bdd9bced0f67e87df871d0ed80 Mon Sep 17 00:00:00 2001
From: Jasmeet Singh <jasmeet0915@gmail.com>
Date: Tue, 27 Jun 2023 04:13:48 +0530
Subject: [PATCH] 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 <jasmeet0915@gmail.com>
Co-authored-by: Addisu Z. Taddese <addisu@openrobotics.org>
---
 Migration.md           |  1 +
 include/sdf/SDFImpl.hh | 12 ++++++------
 src/SDF.cc             | 23 ++++++++++++-----------
 src/SDF_TEST.cc        | 28 ++++++++++++++++++++++++++++
 4 files changed, 47 insertions(+), 17 deletions(-)

diff --git a/Migration.md b/Migration.md
index 904ec8b7f..9358a0087 100644
--- a/Migration.md
+++ b/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
 
diff --git a/include/sdf/SDFImpl.hh b/include/sdf/SDFImpl.hh
index c74412094..20e1ec19f 100644
--- a/include/sdf/SDFImpl.hh
+++ b/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.
   ///
diff --git a/src/SDF.cc b/src/SDF.cc
index 395119a01..07056d792 100644
--- a/src/SDF.cc
+++ b/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)
diff --git a/src/SDF_TEST.cc b/src/SDF_TEST.cc
index 315343e12..43d2e4732 100644
--- a/src/SDF_TEST.cc
+++ b/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