Skip to content

Commit

Permalink
[MirrorSync] Add trie to keep track of SyncStatus
Browse files Browse the repository at this point in the history
SyncStatus events coming from DriveFs will be queried by Files app
whenever metadata is requested for files. To efficiently query them,
a trie will be used to cache those events at the chromium backend.

Bug: b/249406358
Test: out/Default/chromeos_unittests --gtest_filter=SyncStatusTrackerTest.*

Change-Id: I01676369e0aa99366289d4136396dd531f582523
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3903905
Reviewed-by: Ben Reich <benreich@chromium.org>
Reviewed-by: Austin Tankiang <austinct@chromium.org>
Commit-Queue: Marcello Salomao <msalomao@google.com>
Cr-Commit-Position: refs/heads/main@{#1052831}
  • Loading branch information
Marcello Salomao authored and Chromium LUCI CQ committed Sep 29, 2022
1 parent f636868 commit 4f38b25
Show file tree
Hide file tree
Showing 4 changed files with 296 additions and 0 deletions.
3 changes: 3 additions & 0 deletions chromeos/ash/components/drivefs/BUILD.gn
Expand Up @@ -23,6 +23,8 @@ component("drivefs") {
"drivefs_session.cc",
"drivefs_session.h",
"drivefs_util.h",
"sync_status_tracker.cc",
"sync_status_tracker.h",
]
if (!use_real_dbus_clients) {
sources += [
Expand Down Expand Up @@ -77,6 +79,7 @@ source_set("unit_tests") {
"drivefs_http_client_unittest.cc",
"drivefs_search_unittest.cc",
"drivefs_session_unittest.cc",
"sync_status_tracker_unittest.cc",
]

deps = [
Expand Down
107 changes: 107 additions & 0 deletions chromeos/ash/components/drivefs/sync_status_tracker.cc
@@ -0,0 +1,107 @@
// Copyright 2022 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "chromeos/ash/components/drivefs/sync_status_tracker.h"
#include <algorithm>
#include <deque>
#include <utility>
#include <vector>

namespace drivefs {

SyncStatusTracker::SyncStatusTracker()
: root_(std::make_unique<TrieNode>(SyncStatus::kNotFound)) {}
SyncStatusTracker::~SyncStatusTracker() = default;

// TODO(msalomao): add count of kError and kInProgress descendant nodes to each
// node and update them whenever the trie changes to avoid a recursive lookup
// on query.
void SyncStatusTracker::AddSyncStatusForPath(const base::FilePath& path,
SyncStatus status) {
if (path.empty() || !path.IsAbsolute()) {
return;
}
std::vector<base::FilePath::StringType> path_parts = path.GetComponents();
TrieNode* current_node = root_.get();
for (const auto& path_part : path_parts) {
std::unique_ptr<TrieNode>& matchingNode = current_node->children[path_part];
if (matchingNode == nullptr) {
matchingNode = std::make_unique<TrieNode>(SyncStatus::kNotFound);
}
current_node = matchingNode.get();
}
current_node->status = status;
}

SyncStatus SyncStatusTracker::GetSyncStatusForPath(const base::FilePath& path) {
if (path.empty() || !path.IsAbsolute()) {
return SyncStatus::kNotFound;
}
std::vector<base::FilePath::StringType> path_parts = path.GetComponents();
TrieNode* current_node = root_.get();
for (const auto& path_part : path_parts) {
auto it = current_node->children.find(path_part);
if (it == current_node->children.end()) {
return SyncStatus::kNotFound;
}
current_node = it->second.get();
}
if (current_node->status != SyncStatus::kNotFound) {
return current_node->status;
}
SyncStatus status = SyncStatus::kNotFound;
std::deque<TrieNode*> queue = {current_node};
while (!queue.empty()) {
TrieNode* node = queue.front();
queue.pop_front();
if (node->status == SyncStatus::kError) {
return SyncStatus::kError;
}
if (node->status > status) {
status = node->status;
}
for (const auto& child : node->children) {
queue.emplace_back(child.second.get());
}
}
return status;
}

void SyncStatusTracker::RemovePath(const base::FilePath& path) {
if (path.empty() || !path.IsAbsolute()) {
return;
}
std::vector<base::FilePath::StringType> path_parts = path.GetComponents();
TrieNode* current_node = root_.get();
std::vector<std::pair<TrieNode*, TrieNode::PathToChildMap::iterator>>
ancestors;
for (const auto& path_part : path_parts) {
auto it = current_node->children.find(path_part);
if (it == current_node->children.end()) {
return;
}
ancestors.emplace_back(current_node, it);
current_node = it->second.get();
}
if (current_node->children.size() > 0) {
return;
}
while (!ancestors.empty()) {
auto& ancestor = ancestors.back();
auto* node = ancestor.first;
auto& it = ancestor.second;
auto& child = it->second;
if (child->children.empty()) {
node->children.erase(it);
} else {
return;
}
ancestors.pop_back();
}
}

SyncStatusTracker::TrieNode::TrieNode(SyncStatus status) : status(status) {}
SyncStatusTracker::TrieNode::~TrieNode() = default;

} // namespace drivefs
55 changes: 55 additions & 0 deletions chromeos/ash/components/drivefs/sync_status_tracker.h
@@ -0,0 +1,55 @@
// Copyright 2022 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef CHROMEOS_ASH_COMPONENTS_DRIVEFS_SYNC_STATUS_TRACKER_H_
#define CHROMEOS_ASH_COMPONENTS_DRIVEFS_SYNC_STATUS_TRACKER_H_

#include <memory>

#include "base/component_export.h"
#include "base/containers/flat_map.h"
#include "base/files/file_path.h"

namespace drivefs {

// Note: The order here matters when resolving the status of directories.
// The precedence increases from top to bottom. E.g., a directory containing
// one file with SyncStatus=kInProgress and one file with SyncStatus=kError will
// be reported with SyncStatus=kError.
enum SyncStatus {
kNotFound,
kInProgress,
kError,
};

// Cache for sync status coming from DriveFs.
// Allows quick insertion, removal, and look up by file path.
class COMPONENT_EXPORT(CHROMEOS_ASH_COMPONENTS_DRIVEFS) SyncStatusTracker {
public:
SyncStatusTracker();
~SyncStatusTracker();

SyncStatusTracker(const SyncStatusTracker&) = delete;
SyncStatusTracker& operator=(const SyncStatusTracker&) = delete;

void AddSyncStatusForPath(const base::FilePath& path, SyncStatus status);
SyncStatus GetSyncStatusForPath(const base::FilePath& path);
void RemovePath(const base::FilePath& path);

private:
struct TrieNode {
explicit TrieNode(SyncStatus status);
~TrieNode();
SyncStatus status;
typedef base::flat_map<base::FilePath::StringType,
std::unique_ptr<TrieNode>>
PathToChildMap;
PathToChildMap children;
};
std::unique_ptr<TrieNode> root_ = nullptr;
};

} // namespace drivefs

#endif // CHROMEOS_ASH_COMPONENTS_DRIVEFS_SYNC_STATUS_TRACKER_H_
131 changes: 131 additions & 0 deletions chromeos/ash/components/drivefs/sync_status_tracker_unittest.cc
@@ -0,0 +1,131 @@
// Copyright 2022 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "chromeos/ash/components/drivefs/sync_status_tracker.h"

#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"

namespace drivefs {
namespace {

class SyncStatusTrackerTest : public testing::Test {
public:
SyncStatusTrackerTest() = default;

SyncStatusTrackerTest(const SyncStatusTrackerTest&) = delete;
SyncStatusTrackerTest& operator=(const SyncStatusTrackerTest&) = delete;

SyncStatus GetSyncStatus(SyncStatusTracker& tracker,
const std::string& path) {
return tracker.GetSyncStatusForPath(base::FilePath(path));
}

void AddSyncStatus(SyncStatusTracker& tracker,
const std::string& path,
SyncStatus status) {
return tracker.AddSyncStatusForPath(base::FilePath(path), status);
}
};

TEST_F(SyncStatusTrackerTest, PathReturnsValueForLeafAndAncestors) {
SyncStatusTracker tracker;

AddSyncStatus(tracker, "/a/b/c", SyncStatus::kInProgress);
EXPECT_EQ(GetSyncStatus(tracker, "/a/b/c"), SyncStatus::kInProgress);
EXPECT_EQ(GetSyncStatus(tracker, "/a/b"), SyncStatus::kInProgress);
EXPECT_EQ(GetSyncStatus(tracker, "/a"), SyncStatus::kInProgress);
EXPECT_EQ(GetSyncStatus(tracker, "/"), SyncStatus::kInProgress);
}

TEST_F(SyncStatusTrackerTest, ErrorTakesPrecedenceInAncestors) {
SyncStatusTracker tracker;

AddSyncStatus(tracker, "/a/b/c", SyncStatus::kInProgress);
AddSyncStatus(tracker, "/a/b/d", SyncStatus::kError);
EXPECT_EQ(GetSyncStatus(tracker, "/a/b/c"), SyncStatus::kInProgress);
EXPECT_EQ(GetSyncStatus(tracker, "/a/b"), SyncStatus::kError);
EXPECT_EQ(GetSyncStatus(tracker, "/a"), SyncStatus::kError);
EXPECT_EQ(GetSyncStatus(tracker, "/"), SyncStatus::kError);
}

TEST_F(SyncStatusTrackerTest, PathsNotInTrackerReturnNotFound) {
SyncStatusTracker tracker;

AddSyncStatus(tracker, "/a/b/c", SyncStatus::kInProgress);
EXPECT_EQ(GetSyncStatus(tracker, "/a/b/c"), SyncStatus::kInProgress);
EXPECT_EQ(GetSyncStatus(tracker, "/a/b/d"), SyncStatus::kNotFound);
}

TEST_F(SyncStatusTrackerTest, RemovingAPathRemovesSingleUseAncestors) {
SyncStatusTracker tracker;

AddSyncStatus(tracker, "/a/b/c/f", SyncStatus::kInProgress);
AddSyncStatus(tracker, "/a/b/d", SyncStatus::kInProgress);
AddSyncStatus(tracker, "/a/b/e", SyncStatus::kInProgress);

tracker.RemovePath(base::FilePath("/a/b/c/f"));
EXPECT_EQ(GetSyncStatus(tracker, "/a/b/c/f"), SyncStatus::kNotFound);
EXPECT_EQ(GetSyncStatus(tracker, "/a/b/c"), SyncStatus::kNotFound);
EXPECT_EQ(GetSyncStatus(tracker, "/a/b"), SyncStatus::kInProgress);
}

TEST_F(SyncStatusTrackerTest, OnlyLeafPathsCanBeRemoved) {
SyncStatusTracker tracker;

AddSyncStatus(tracker, "/a/b/c/d", SyncStatus::kInProgress);

tracker.RemovePath(base::FilePath("/a/b/c"));
tracker.RemovePath(base::FilePath("/a/b"));
tracker.RemovePath(base::FilePath("/a"));

EXPECT_EQ(GetSyncStatus(tracker, "/a/b/c/d"), SyncStatus::kInProgress);
}

TEST_F(SyncStatusTrackerTest, Utf8PathsAreSupported) {
SyncStatusTracker tracker;

AddSyncStatus(tracker, "/a/b/日本", SyncStatus::kInProgress);
EXPECT_EQ(GetSyncStatus(tracker, "/a/b/日本"), SyncStatus::kInProgress);
}

TEST_F(SyncStatusTrackerTest, DeletingNonexistingPathIsNoOp) {
SyncStatusTracker tracker;

AddSyncStatus(tracker, "/a/b/c/d", SyncStatus::kInProgress);

tracker.RemovePath(base::FilePath("/a/b/c/d/e"));

EXPECT_EQ(GetSyncStatus(tracker, "/a/b/c/d"), SyncStatus::kInProgress);
}

TEST_F(SyncStatusTrackerTest, AddingExistingPathReplacesStatus) {
SyncStatusTracker tracker;

AddSyncStatus(tracker, "/a/b/c/d", SyncStatus::kInProgress);
AddSyncStatus(tracker, "/a/b/c/d", SyncStatus::kError);

EXPECT_EQ(GetSyncStatus(tracker, "/a/b/c/d"), SyncStatus::kError);
}

TEST_F(SyncStatusTrackerTest, MalformedPathsAreSupported) {
SyncStatusTracker tracker;

AddSyncStatus(tracker, "////", SyncStatus::kInProgress);

EXPECT_EQ(GetSyncStatus(tracker, "////"), SyncStatus::kInProgress);
}

TEST_F(SyncStatusTrackerTest, RelativePathsAreNotSupported) {
SyncStatusTracker tracker;

AddSyncStatus(tracker, "./..", SyncStatus::kInProgress);
AddSyncStatus(tracker, "../", SyncStatus::kInProgress);

EXPECT_EQ(GetSyncStatus(tracker, "./.."), SyncStatus::kNotFound);
EXPECT_EQ(GetSyncStatus(tracker, "../"), SyncStatus::kNotFound);
}

} // namespace
} // namespace drivefs

0 comments on commit 4f38b25

Please sign in to comment.