Skip to content

Commit

Permalink
[FilesBulkPinning] Update SetMetadata to use struct not params
Browse files Browse the repository at this point in the history
In preparation for adding yet another value to pass through to the
`FakeMetadata`, the params being passed through are getting needlessly
long and difficult. Given most of them are being defaulted everywhere
except the file manager tests, prefer a struct approach to have call
sites only define what they need and rely on defaults to avoid changing
every site when a new param is added.

Bug: b:278002300
Change-Id: Iaf4816fb8b72d625ea87fbd8309d1a9bb63eab58
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4551640
Commit-Queue: Ben Reich <benreich@chromium.org>
Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1147055}
  • Loading branch information
ben-reich authored and Chromium LUCI CQ committed May 22, 2023
1 parent df29f42 commit 35d6348
Show file tree
Hide file tree
Showing 7 changed files with 115 additions and 89 deletions.
Expand Up @@ -88,13 +88,11 @@ void DriveIntegrationServiceBrowserTestBase::AddDriveFileWithRelativePath(
&relative_to_drive_fs_mount);

// Update the drive file metadata.
GetFakeDriveFsForProfile(profile)->SetMetadata(
relative_to_drive_fs_mount, "text/plain",
relative_to_drive_fs_mount.BaseName().value(),
/*pinned=*/false, /*available_offline=*/false, /*shared=*/false,
/*capabilities=*/{},
/*folder_feature=*/{}, drive_file_id, /*alternate_url=*/"",
/*shortcut=*/false);
drivefs::FakeMetadata metadata;
metadata.path = relative_to_drive_fs_mount;
metadata.mime_type = "text/plain";
metadata.doc_id = drive_file_id;
GetFakeDriveFsForProfile(profile)->SetMetadata(std::move(metadata));

// Update the relative/absolute paths to the generated file.
if (new_file_relative_path)
Expand Down
38 changes: 25 additions & 13 deletions chrome/browser/ash/file_manager/file_manager_browsertest_base.cc
Expand Up @@ -1367,19 +1367,31 @@ class DriveFsTestVolume : public TestVolume {
<< "Failed to create a computer: " << target_path.value();
break;
}
fake_drivefs_helper_->fake_drivefs().SetMetadata(
relative_path, entry.mime_type, original_name.value(), entry.pinned,
entry.available_offline,
entry.shared_option == AddEntriesMessage::SharedOption::SHARED ||
entry.shared_option ==
AddEntriesMessage::SharedOption::SHARED_WITH_ME,
{entry.capabilities.can_share, entry.capabilities.can_copy,
entry.capabilities.can_delete, entry.capabilities.can_rename,
entry.capabilities.can_add_children},
{entry.folder_feature.is_machine_root,
entry.folder_feature.is_arbitrary_sync_folder,
entry.folder_feature.is_external_media},
"", entry.alternate_url, (entry.entry_type == AddEntriesMessage::LINK));
drivefs::FakeMetadata metadata;
metadata.path = relative_path;
metadata.mime_type = entry.mime_type;
metadata.original_name = original_name.value();
metadata.pinned = entry.pinned;
metadata.available_offline = entry.available_offline;
metadata.shared =
(entry.shared_option == AddEntriesMessage::SharedOption::SHARED ||
entry.shared_option ==
AddEntriesMessage::SharedOption::SHARED_WITH_ME);
metadata.capabilities.can_share = entry.capabilities.can_share;
metadata.capabilities.can_copy = entry.capabilities.can_copy;
metadata.capabilities.can_delete = entry.capabilities.can_delete;
metadata.capabilities.can_rename = entry.capabilities.can_rename,
metadata.capabilities.can_add_children =
entry.capabilities.can_add_children;
metadata.folder_feature.is_machine_root =
entry.folder_feature.is_machine_root;
metadata.folder_feature.is_arbitrary_sync_folder =
entry.folder_feature.is_arbitrary_sync_folder;
metadata.folder_feature.is_external_media =
entry.folder_feature.is_external_media;
metadata.alternate_url = entry.alternate_url;
metadata.shortcut = (entry.entry_type == AddEntriesMessage::LINK);
fake_drivefs_helper_->fake_drivefs().SetMetadata(std::move(metadata));

ASSERT_TRUE(UpdateModifiedTime(entry));
}
Expand Down
Expand Up @@ -274,11 +274,13 @@ class PendingScreencastMangerBrowserTest : public InProcessBrowserTest {
void ExpectEmptyRequestBodyForProjectorFileContent(
const std::string& file_content) {
CreateFileInDriveFsFolder(kDefaultMetadataFilePath, file_content);
GetFakeDriveFs()->SetMetadata(
base::FilePath(kDefaultMetadataFilePath), "text/plain",
kTestMetadataFile, false, false, false, {}, {}, "abc123",
/*alternate_url=*/
"https://drive.google.com/open?id=fileId", /*shortcut=*/false);
drivefs::FakeMetadata metadata;
metadata.path = base::FilePath(kDefaultMetadataFilePath);
metadata.mime_type = "text/plain";
metadata.original_name = kTestMetadataFile;
metadata.doc_id = "abc123";
metadata.alternate_url = "https://drive.google.com/open?id=fileId";
GetFakeDriveFs()->SetMetadata(std::move(metadata));

// Sets get file id callback:
base::RunLoop run_loop;
Expand Down Expand Up @@ -774,11 +776,13 @@ IN_PROC_BROWSER_TEST_F(PendingScreencastMangerBrowserTest,
"\"hypothesisParts\":[],\"startOffset\":2000,\"text\":\"another sentence."
"\"}],\"tableOfContent\":[]}";
CreateFileInDriveFsFolder(kDefaultMetadataFilePath, kProjectorFileContent);
GetFakeDriveFs()->SetMetadata(
base::FilePath(kDefaultMetadataFilePath), "text/plain", kTestMetadataFile,
false, false, false, {}, {}, "abc123",
/*alternate_url=*/"https://drive.google.com/open?id=fileId",
/*shortcut=*/false);
drivefs::FakeMetadata metadata;
metadata.path = base::FilePath(kDefaultMetadataFilePath);
metadata.mime_type = "text/plain";
metadata.original_name = kTestMetadataFile;
metadata.doc_id = "abc123";
metadata.alternate_url = "https://drive.google.com/open?id=fileId";
GetFakeDriveFs()->SetMetadata(std::move(metadata));

// Sets get file id callback:
base::RunLoop run_loop;
Expand Down Expand Up @@ -818,10 +822,12 @@ IN_PROC_BROWSER_TEST_F(PendingScreencastMangerBrowserTest,
CreateFileInDriveFsFolder(kDefaultMetadataFilePath, kTestMetadataFileBytes);
// Sets empty alternate url in metadata, which could happen when metadata is
// not fully populated.
GetFakeDriveFs()->SetMetadata(
base::FilePath(kDefaultMetadataFilePath), "text/plain", kTestMetadataFile,
false, false, false, {}, {}, "abc123",
/*alternate_url=*/std::string(), /*shortcut=*/false);
drivefs::FakeMetadata metadata;
metadata.path = base::FilePath(kDefaultMetadataFilePath);
metadata.mime_type = "text/plain";
metadata.original_name = kTestMetadataFile;
metadata.doc_id = "abc123";
GetFakeDriveFs()->SetMetadata(std::move(metadata));

TestGetFileIdFailed();
}
Expand All @@ -830,10 +836,13 @@ IN_PROC_BROWSER_TEST_F(PendingScreencastMangerBrowserTest,
UpdateIndexableTextFailByInCorrectAlternateUrl) {
CreateFileInDriveFsFolder(kDefaultMetadataFilePath, kTestMetadataFileBytes);
// Sets incorrect alternate url in metadata.
GetFakeDriveFs()->SetMetadata(
base::FilePath(kDefaultMetadataFilePath), "text/plain", kTestMetadataFile,
false, false, false, {}, {}, "abc123",
/*alternate_url=*/"alternate_url", /*shortcut=*/false);
drivefs::FakeMetadata metadata;
metadata.path = base::FilePath(kDefaultMetadataFilePath);
metadata.mime_type = "text/plain";
metadata.original_name = kTestMetadataFile;
metadata.doc_id = "abc123";
metadata.alternate_url = "alternate_url";
GetFakeDriveFs()->SetMetadata(std::move(metadata));

TestGetFileIdFailed();
}
Expand Down
Expand Up @@ -133,8 +133,13 @@ class ScreencastManagerTestWithDriveFs : public ScreencastManagerTest {
}

const base::FilePath& relative_path = GetTestFile(title, /*relative=*/true);
fake->SetMetadata(relative_path, content_type, title, false, false,
shared_with_me, {}, {}, file_id, "", /*shortcut=*/false);
drivefs::FakeMetadata metadata;
metadata.path = relative_path;
metadata.mime_type = content_type;
metadata.original_name = title;
metadata.shared = shared_with_me;
metadata.doc_id = file_id;
fake->SetMetadata(std::move(metadata));
}

// Copies a file from //media/test/data with `original_name` to default test
Expand Down
Expand Up @@ -212,15 +212,17 @@ class DriveUploadHandlerTest
// signals to the DriveFs delegate.
void SimulateDriveUploadEvents() {
// Set file metadata for `drivefs::mojom::DriveFs::GetMetadata`.
fake_drivefs().SetMetadata(
observed_relative_drive_path(),
drivefs::FakeMetadata metadata;
metadata.path = observed_relative_drive_path();
metadata.mime_type =
"application/"
"vnd.openxmlformats-officedocument.wordprocessingml.document",
test_file_name_, false, false, false, {}, {}, "abc123",
/*alternate_url=*/
"vnd.openxmlformats-officedocument.wordprocessingml.document";
metadata.original_name = test_file_name_;
metadata.doc_id = "abc123";
metadata.alternate_url =
"https://docs.google.com/document/d/"
"smalldocxid?rtpof=true&usp=drive_fs",
/*shortcut=*/false);
"smalldocxid?rtpof=true&usp=drive_fs";
fake_drivefs().SetMetadata(std::move(metadata));

// Simulate server sync events.
drivefs::mojom::SyncingStatusPtr status =
Expand Down
52 changes: 21 additions & 31 deletions chromeos/ash/components/drivefs/fake_drivefs.cc
Expand Up @@ -34,6 +34,13 @@
#include "url/gurl.h"

namespace drivefs {

FakeMetadata::FakeMetadata() = default;
FakeMetadata::~FakeMetadata() = default;

FakeMetadata::FakeMetadata(FakeMetadata&& other) = default;
FakeMetadata& FakeMetadata::operator=(FakeMetadata&& other) = default;

namespace {

std::vector<std::pair<base::RepeatingCallback<std::string()>,
Expand Down Expand Up @@ -339,37 +346,20 @@ FakeDriveFs::CreateMojoListener() {
bootstrap_receiver_.BindNewPipeAndPassRemote());
}

void FakeDriveFs::SetMetadata(const base::FilePath& path,
const std::string& mime_type,
const std::string& original_name,
bool pinned,
bool available_offline,
bool shared,
const mojom::Capabilities& capabilities,
const mojom::FolderFeature& folder_feature,
const std::string& doc_id,
const std::string& alternate_url,
bool shortcut) {
auto& stored_metadata = metadata_[path];
stored_metadata.mime_type = mime_type;
stored_metadata.original_name = original_name;
stored_metadata.hosted = (original_name != path.BaseName().value());
stored_metadata.capabilities = capabilities;
stored_metadata.folder_feature = folder_feature;
stored_metadata.doc_id = doc_id;
if (pinned) {
stored_metadata.pinned = true;
}
if (available_offline) {
stored_metadata.available_offline = true;
}
if (shared) {
stored_metadata.shared = true;
}
if (shortcut) {
stored_metadata.shortcut = true;
}
stored_metadata.alternate_url = alternate_url;
void FakeDriveFs::SetMetadata(const FakeMetadata& metadata) {
auto& stored_metadata = metadata_[metadata.path];
stored_metadata.mime_type = metadata.mime_type;
stored_metadata.original_name = metadata.original_name;
stored_metadata.hosted =
(metadata.original_name != metadata.path.BaseName().value());
stored_metadata.capabilities = metadata.capabilities;
stored_metadata.folder_feature = metadata.folder_feature;
stored_metadata.doc_id = metadata.doc_id;
stored_metadata.pinned = metadata.pinned;
stored_metadata.available_offline = metadata.available_offline;
stored_metadata.shared = metadata.shared;
stored_metadata.shortcut = metadata.shortcut;
stored_metadata.alternate_url = metadata.alternate_url;
}

void FakeDriveFs::DisplayConfirmDialog(
Expand Down
32 changes: 21 additions & 11 deletions chromeos/ash/components/drivefs/fake_drivefs.h
Expand Up @@ -23,6 +23,26 @@

namespace drivefs {

struct FakeMetadata {
FakeMetadata();
~FakeMetadata();

FakeMetadata(FakeMetadata&& other);
FakeMetadata& operator=(FakeMetadata&& other);

base::FilePath path;
std::string mime_type;
std::string original_name;
bool pinned = false;
bool available_offline = false;
bool shared = false;
mojom::Capabilities capabilities = {};
mojom::FolderFeature folder_feature = {};
std::string doc_id;
std::string alternate_url;
bool shortcut = false;
};

class FakeDriveFsBootstrapListener : public DriveFsBootstrapListener {
public:
explicit FakeDriveFsBootstrapListener(
Expand Down Expand Up @@ -56,17 +76,7 @@ class FakeDriveFs : public drivefs::mojom::DriveFs,

std::unique_ptr<drivefs::DriveFsBootstrapListener> CreateMojoListener();

void SetMetadata(const base::FilePath& path,
const std::string& mime_type,
const std::string& original_name,
bool pinned,
bool available_offline,
bool shared,
const mojom::Capabilities& capabilities,
const mojom::FolderFeature& folder_feature,
const std::string& doc_id,
const std::string& alternate_url,
bool shortcut);
void SetMetadata(const FakeMetadata& metadata);

void DisplayConfirmDialog(
drivefs::mojom::DialogReasonPtr reason,
Expand Down

0 comments on commit 35d6348

Please sign in to comment.