Skip to content

Commit

Permalink
--[BUGFIX] -Ellipses in paths (#2218)
Browse files Browse the repository at this point in the history
* --filter paths of ellipses and their parent, referencing directories
If an OS link spans the ellipsis, certain functions, like glob, get lost when attempting to navigate the ellipsis.
* --add test
* --fix for glob call
--glob doesn't know how to back-step across an os link, so it gets lost if it encounters one spanned by an ellipsis.
* --rename to normalizePath for consistency with other similar functions.
* --tailor the functionality only to specific use case
In an effort to limit the impact of this change, it has been narrowed to only affect paths prefixed with an ellipse in the scene dataset config. Since the scene dataset config is expected to exist at the root of the dataset, and since the scene dataset may be accessed via a link, having an ellipses from the dataset root back-pedals across a link. Some functions, like glob(), don't handle this properly and get lost. This commit addresses only these cases.
  • Loading branch information
jturner65 authored Sep 27, 2023
1 parent 9826bd1 commit 7ed84da
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 12 deletions.
25 changes: 25 additions & 0 deletions src/esp/io/Io.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,33 @@ std::string changeExtension(const std::string& filename,

} // changeExtension

std::string normalizePath(const std::string& srcPath) {
std::size_t ellipsisLoc = srcPath.find("/../");
if ((ellipsisLoc == std::string::npos) || (ellipsisLoc == 0)) {
// not found
return srcPath;
}
// ellipsisLoc is location of first instance of "/../"
// Get rid of ellipsis inside a specified path, and the previous directory
// it references, so that if the ellisis spans a link, consumers don't get
// lost.

// Get end of path/filename after location of ellipsis
auto suffixString = srcPath.substr(ellipsisLoc + 4);
// Get beginning of path up to directory before ellipsis
std::size_t prevLoc = srcPath.find_last_of('/', ellipsisLoc - 1);
// Get paths leading up to ellipsis-cancelled path
auto prefixString = srcPath.substr(0, prevLoc);
// recurses to get subsequent ellipses.
auto filteredPath = Cr::Utility::formatString("{}/{}", prefixString,
normalizePath(suffixString));
return filteredPath;
} // normalizePath

std::vector<std::string> globDirs(const std::string& pattern) {
// Check for ellipsis, if so process here.
glob_t glob_result;

glob(pattern.c_str(), GLOB_MARK, nullptr, &glob_result);
std::vector<std::string> ret(glob_result.gl_pathc);
for (int i = 0; i < glob_result.gl_pathc; ++i) {
Expand Down
11 changes: 11 additions & 0 deletions src/esp/io/Io.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,17 @@ namespace io {
*/
std::string changeExtension(const std::string& file, const std::string& ext);

/**
* @brief This function will recursively remove any ellipsis in paths or path
* patterns, as well as the parent directory the ellisis is bypassing. This is
* so that if the ellipsis spans an OS link any function consuming the path does
* not get lost. If the ellipsis is at the beginning of @p srcPath then it is
* ignored.
* @param srcPath The path to filter
* @return The filtered path.
*/
std::string normalizePath(const std::string& srcPath);

/**
* @brief This function will perform [glob-based pattern matching]
* (https://en.wikipedia.org/wiki/Glob_(programming)) to find and return all the
Expand Down
36 changes: 25 additions & 11 deletions src/esp/metadata/managers/AttributesManagerBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -362,28 +362,42 @@ void AttributesManager<T, Access>::buildAttrSrcPathsFromJSONAndLoad(
const std::string& configDir,
const std::string& extType,
const io::JsonGenericValue& filePaths) {
std::size_t cfgLastDirLoc = configDir.find_last_of('/');
for (rapidjson::SizeType i = 0; i < filePaths.Size(); ++i) {
if (!filePaths[i].IsString()) {
ESP_WARNING() << "<" << this->objectType_
<< "> : Invalid path value in file path array element @ idx"
<< i << ". Skipping.";
ESP_WARNING(Mn::Debug::Flag::NoSpace)
<< "<" << this->objectType_
<< "> : Invalid path value in file path array element @ idx" << i
<< ". Skipping.";
continue;
}
std::string absolutePath =
Cr::Utility::Path::join(configDir, filePaths[i].GetString());
std::vector<std::string> globPaths = io::globDirs(absolutePath);
const char* fileString = filePaths[i].GetString();
// Only normalize paths for paths from config starting with ellipses.
// This is so that glob doesn't fail when the filepath from the config is
// trying to back-pedal across an OS link.
bool normalizePaths = (fileString[0] == '.') && (fileString[1] == '.') &&
(fileString[2] == '/');

// TODO Eventually we should normalize all metadata paths in the system
std::string dsFilePath =
normalizePaths
? Cr::Utility::Path::join(configDir.substr(0, cfgLastDirLoc),
std::string(fileString).substr(3))
: Cr::Utility::Path::join(configDir, fileString);

std::vector<std::string> globPaths = io::globDirs(dsFilePath);
if (globPaths.size() > 0) {
for (const auto& globPath : globPaths) {
// load all object templates available as configs in absolutePath
ESP_VERY_VERBOSE() << "<" << this->objectType_
<< "> : Glob path result for" << absolutePath << ":"
<< globPath;
// load all object templates available as configs in dsFilePath
ESP_VERY_VERBOSE(Mn::Debug::Flag::NoSpace)
<< "<" << this->objectType_ << "> : Glob path result for"
<< dsFilePath << ":" << globPath;
this->loadAllTemplatesFromPathAndExt(globPath, extType, true);
}
} else {
ESP_WARNING(Mn::Debug::Flag::NoSpace)
<< "<" << this->objectType_ << "> : No Glob path result found for `"
<< absolutePath << "` so unable to load templates from that path.";
<< dsFilePath << "` so unable to load templates from that path.";
}
}
ESP_DEBUG(Mn::Debug::Flag::NoSpace)
Expand Down
33 changes: 32 additions & 1 deletion src/tests/IOTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ const std::string dataDir = Corrade::Utility::Path::join(SCENE_DATASETS, "../");
struct IOTest : Cr::TestSuite::Tester {
explicit IOTest();
void fileReplaceExtTest();
void testEllipsisFilter();
void parseURDF();
void testJson();
void testJsonBuiltinTypes();
Expand All @@ -50,7 +51,8 @@ struct IOTest : Cr::TestSuite::Tester {
};

IOTest::IOTest() {
addTests({&IOTest::fileReplaceExtTest, &IOTest::parseURDF, &IOTest::testJson,
addTests({&IOTest::fileReplaceExtTest, &IOTest::testEllipsisFilter,
&IOTest::parseURDF, &IOTest::testJson,
&IOTest::testJsonBuiltinTypes, &IOTest::testJsonStlTypes,
&IOTest::testJsonMagnumTypes, &IOTest::testJsonEspTypes,
&IOTest::testJsonUserType});
Expand Down Expand Up @@ -98,6 +100,35 @@ void IOTest::fileReplaceExtTest() {
CORRADE_COMPARE(result, ".jpg.png");
}

void IOTest::testEllipsisFilter() {
std::string testPath1 = "/test/path/to/test/no-op.txt";
std::string res = esp::io::normalizePath(testPath1);
CORRADE_COMPARE(res, "/test/path/to/test/no-op.txt");

testPath1 = "/test/path/DELETE/../to/test/removal.txt";
res = esp::io::normalizePath(testPath1);
CORRADE_COMPARE(res, "/test/path/to/test/removal.txt");
testPath1 =
"/test/path/DELETE/../to/DELETE/../test/DELETE/../multiple/removal.txt";
res = esp::io::normalizePath(testPath1);
CORRADE_COMPARE(res, "/test/path/to/test/multiple/removal.txt");

testPath1 = "test/path/DELETE/../to/test/removal.txt";
res = esp::io::normalizePath(testPath1);
CORRADE_COMPARE(res, "test/path/to/test/removal.txt");

testPath1 =
"test/path/DELETE/../to/DELETE/../test/DELETE/../multiple/removal.txt";
res = esp::io::normalizePath(testPath1);
CORRADE_COMPARE(res, "test/path/to/test/multiple/removal.txt");

// ignore intitial ellipsis
testPath1 = "/../test/path/DELETE/../to/test/initial/ignore.txt";
res = esp::io::normalizePath(testPath1);
CORRADE_COMPARE(res, "/../test/path/DELETE/../to/test/initial/ignore.txt");

} // IOTest::testEllipsisFilter

void IOTest::parseURDF() {
const std::string iiwaURDF = Cr::Utility::Path::join(
TEST_ASSETS, "urdf/kuka_iiwa/model_free_base.urdf");
Expand Down

0 comments on commit 7ed84da

Please sign in to comment.