Skip to content

Commit acc543b

Browse files
smilesa-maurice
authored andcommitted
Fixed Path::IsParent()
Path::StartsWith() was incorrectly performing a common prefix check, instead the method should have been determining whether the other path is a child of the current path. i.e Path("foo/bar").IsParent("foo/bar/baz") yields true where Path("foo/bar/baz").IsParent("foo/bar") yields false --- Cloned from cl/259864083 added missing test cases and fixed naming. PiperOrigin-RevId: 259953511
1 parent 72f025a commit acc543b

File tree

3 files changed

+24
-22
lines changed

3 files changed

+24
-22
lines changed

app/src/path.cc

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616

1717
#include "app/src/path.h"
18+
1819
#include <algorithm>
1920
#include <cstring>
2021
#include <iterator>
@@ -130,16 +131,16 @@ const char* Path::GetBaseName() const {
130131
return path_.c_str() + index + 1;
131132
}
132133

133-
bool Path::StartsWith(const Path& prefix) const {
134-
if (prefix.empty()) return true;
135-
if (prefix.path_.size() > path_.size()) return false;
134+
bool Path::IsParent(const Path& other) const {
135+
if (empty()) return true;
136+
if (path_.size() > other.path_.size()) return false;
137+
auto other_iter = other.path_.begin();
136138
auto this_iter = path_.begin();
137-
auto prefix_iter = prefix.path_.begin();
138-
for (; this_iter != path_.end() && prefix_iter != prefix.path_.end();
139-
++this_iter, ++prefix_iter) {
140-
if (*this_iter != *prefix_iter) break;
139+
for (; other_iter != other.path_.end() && this_iter != path_.end();
140+
++other_iter, ++this_iter) {
141+
if (*other_iter != *this_iter) break;
141142
}
142-
return this_iter == path_.end() || *this_iter == '/';
143+
return other_iter == other.path_.end() || *other_iter == '/';
143144
}
144145

145146
std::vector<std::string> Path::GetDirectories() const {

app/src/path.h

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
#include <string>
2121
#include <vector>
22+
2223
#include "app/src/include/firebase/internal/common.h"
2324
#include "app/src/optional.h"
2425

@@ -73,11 +74,11 @@ class Path {
7374
// In the path: path/to/object/in/database, it would return "database".
7475
const char* GetBaseName() const;
7576

76-
// Returns true if this path starts with the prefix path. The prefix path is
77-
// compared on a per-directory basis, not per-character. That is, for the path
78-
// "foo/bar/baz", the path "foo/bar" would be return true, but "foo/ba" would
79-
// return false.
80-
bool StartsWith(const Path& prefix) const;
77+
// Returns true if this path is the parent of the other path. The other path
78+
// is compared on a per-directory basis, not per-character. That is, for the
79+
// path "foo/bar/baz", the path "foo/bar" would be return true, but "foo/ba"
80+
// would return false.
81+
bool IsParent(const Path& other) const;
8182

8283
// Returns a vector containing each directory in the path in order.
8384
// The path "foo/bar/baz" would return a vector containing "foo", "bar", and

database/src/desktop/core/write_tree.cc

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ bool WriteTree::RemoveWrite(WriteId write_id) {
9797
// The removed write was completely shadowed by a subsequent write.
9898
removed_write_was_visible = false;
9999
break;
100-
} else if (write_to_remove.path.StartsWith(current_write.path)) {
100+
} else if (write_to_remove.path.IsParent(current_write.path)) {
101101
// Either we're covering some writes or they're covering part of us
102102
// (depending on which came first).
103103
removed_write_overlaps_with_other_writes = true;
@@ -198,8 +198,8 @@ Optional<Variant> WriteTree::CalcCompleteEventCache(
198198
filter_data->write_ids_to_exclude->end(),
199199
write.write_id);
200200
if (iter == filter_data->write_ids_to_exclude->end()) {
201-
return write.path.StartsWith(*filter_data->tree_path) ||
202-
filter_data->tree_path->StartsWith(write.path);
201+
return write.path.IsParent(*filter_data->tree_path) ||
202+
filter_data->tree_path->IsParent(write.path);
203203
}
204204
}
205205
return false;
@@ -358,12 +358,12 @@ Optional<Variant> WriteTree::ShadowingWrite(const Path& path) const {
358358
bool WriteTree::RecordContainsPath(const UserWriteRecord& write_record,
359359
const Path& path) {
360360
if (write_record.is_overwrite) {
361-
return write_record.path.StartsWith(path);
361+
return write_record.path.IsParent(path);
362362
} else {
363363
bool result = false;
364364
write_record.merge.write_tree().CallOnEach(
365365
Path(), [&](const Path& current_path, Variant) {
366-
if (write_record.path.GetChild(current_path).StartsWith(path)) {
366+
if (write_record.path.GetChild(current_path).IsParent(path)) {
367367
result = true;
368368
}
369369
});
@@ -394,12 +394,12 @@ CompoundWrite WriteTree::LayerTree(const std::vector<UserWriteRecord>& writes,
394394
if (filter(write, filter_userdata)) {
395395
Path write_path = write.path;
396396
if (write.is_overwrite) {
397-
if (tree_root.StartsWith(write_path)) {
397+
if (tree_root.IsParent(write_path)) {
398398
Optional<Path> relative_path =
399399
Path::GetRelative(tree_root, write_path);
400400
compound_write =
401401
compound_write.AddWrite(*relative_path, write.overwrite);
402-
} else if (write_path.StartsWith(tree_root)) {
402+
} else if (write_path.IsParent(tree_root)) {
403403
compound_write = compound_write.AddWrite(
404404
Path(),
405405
VariantGetChild(&write.overwrite,
@@ -409,12 +409,12 @@ CompoundWrite WriteTree::LayerTree(const std::vector<UserWriteRecord>& writes,
409409
// write
410410
}
411411
} else {
412-
if (tree_root.StartsWith(write_path)) {
412+
if (tree_root.IsParent(write_path)) {
413413
Optional<Path> relative_path =
414414
Path::GetRelative(tree_root, write_path);
415415
compound_write =
416416
compound_write.AddWrites(*relative_path, write.merge);
417-
} else if (write_path.StartsWith(tree_root)) {
417+
} else if (write_path.IsParent(tree_root)) {
418418
Optional<Path> relative_path =
419419
Path::GetRelative(write_path, tree_root);
420420
if (relative_path->empty()) {

0 commit comments

Comments
 (0)