Skip to content

Commit

Permalink
Support /.files-by-id/<id> prefixes for shared with me in LocationLine.
Browse files Browse the repository at this point in the history 
LocationLine assumes DRIVE_OTHER entries will be of the form
/other/path/to/file. With DriveFS, they are instead of the form
/.files-by-id/<id>/path/to/file. Support correct breadcrumbs for
directories within shared with me by treating /.files-by-id/<id> as the
prefix instead of /other when the path starts with that pattern.

Bug: 854481
Tbr: slangley@chromium.org
Change-Id: I91b55c6fbae3458b4f0f4599fc969b3c1b1fa2e8
Reviewed-on: https://chromium-review.googlesource.com/1245343
Commit-Queue: Sam McNally <sammc@chromium.org>
Reviewed-by: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594948}
  • Loading branch information
@sammc
sammc authored and Commit Bot committed Sep 28, 2018
1 parent 0e18cb6 commit 367235e
Show file tree
Hide file tree
Showing 7 changed files with 166 additions and 48 deletions.
62 changes: 49 additions & 13 deletions chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ struct AddEntriesMessage {
enum EntryType { FILE, DIRECTORY, TEAM_DRIVE };

// Represents whether an entry appears in 'Share with Me' or not.
enum SharedOption { NONE, SHARED };
enum SharedOption { NONE, SHARED, SHARED_WITH_ME, NESTED_SHARED_WITH_ME };

// The actual AddEntriesMessage contents.

Expand Down Expand Up @@ -278,6 +278,10 @@ struct AddEntriesMessage {
SharedOption* option) {
if (value == "shared")
*option = SHARED;
else if (value == "sharedWithMe")
*option = SHARED_WITH_ME;
else if (value == "nestedSharedWithMe")
*option = NESTED_SHARED_WITH_ME;
else if (value == "none")
*option = NONE;
else
Expand Down Expand Up @@ -685,12 +689,16 @@ class DriveTestVolume : public TestVolume {
case AddEntriesMessage::FILE:
CreateFile(entry.source_file_name, parent_entry->resource_id(),
target_name, entry.mime_type,
entry.shared_option == AddEntriesMessage::SHARED,
entry.shared_option == AddEntriesMessage::SHARED ||
entry.shared_option == AddEntriesMessage::SHARED_WITH_ME,
entry.last_modified_time, file_capabilities);
break;
case AddEntriesMessage::DIRECTORY:
CreateDirectory(parent_entry->resource_id(), target_name,
entry.last_modified_time, file_capabilities);
CreateDirectory(
parent_entry->resource_id(), target_name, entry.last_modified_time,
entry.shared_option == AddEntriesMessage::SHARED ||
entry.shared_option == AddEntriesMessage::SHARED_WITH_ME,
file_capabilities);
break;
case AddEntriesMessage::TEAM_DRIVE:
CreateTeamDrive(entry.team_drive_name, team_drive_capabilities);
Expand All @@ -716,6 +724,7 @@ class DriveTestVolume : public TestVolume {
const std::string& parent_id,
const std::string& target_name,
const base::Time& modification_time,
bool shared_with_me,
const google_apis::FileResourceCapabilities& capabilities) {
google_apis::DriveApiErrorCode error = google_apis::DRIVE_OTHER_ERROR;

Expand All @@ -740,6 +749,11 @@ class DriveTestVolume : public TestVolume {
base::RunLoop().RunUntilIdle();
ASSERT_TRUE(error == google_apis::HTTP_SUCCESS);
ASSERT_TRUE(entry);

if (shared_with_me) {
ASSERT_EQ(google_apis::HTTP_SUCCESS,
fake_drive_service_->SetFileAsSharedWithMe(entry->file_id()));
}
}

// Creates a test file with the given spec.
Expand Down Expand Up @@ -837,13 +851,15 @@ class DriveFsTestVolume : public DriveTestVolume {
const base::FilePath target_path = GetTargetPathForTestEntry(entry);

entries_.insert(std::make_pair(target_path, entry));
fake_drivefs_->SetMetadata(
GetRelativeDrivePathForTestEntry(entry), entry.mime_type,
base::FilePath(entry.target_path).BaseName().value(), entry.pinned,
entry.shared_option == AddEntriesMessage::SharedOption::SHARED ||
entry.shared_option ==
AddEntriesMessage::SharedOption::SHARED_WITH_ME);

switch (entry.type) {
case AddEntriesMessage::FILE: {
fake_drivefs_->SetMetadata(
GetRelativeDrivePathForTestEntry(entry), entry.mime_type,
base::FilePath(entry.target_path).BaseName().value(), entry.pinned,
entry.shared_option == AddEntriesMessage::SharedOption::SHARED);

if (entry.source_file_name.empty()) {
ASSERT_EQ(0, base::WriteFile(target_path, "", 0));
break;
Expand Down Expand Up @@ -908,7 +924,8 @@ class DriveFsTestVolume : public DriveTestVolume {
// Update the modified time of parent directories because they may be
// also affected by the update of child items.
if (path.DirName() != GetTeamDriveGrandRoot() &&
path.DirName() != GetMyDrivePath()) {
path.DirName() != GetMyDrivePath() &&
path.DirName() != GetSharedWithMePath()) {
const auto it = entries_.find(path.DirName());
if (it == entries_.end())
return false;
Expand All @@ -921,14 +938,24 @@ class DriveFsTestVolume : public DriveTestVolume {
base::FilePath GetTargetPathForTestEntry(
const AddEntriesMessage::TestEntryInfo& entry) {
const base::FilePath target_path =
entry.team_drive_name.empty()
? GetMyDrivePath().Append(entry.target_path)
: GetTeamDrivePath(entry.team_drive_name).Append(entry.target_path);
GetTargetBasePathForTestEntry(entry).Append(entry.target_path);
if (entry.name_text != entry.target_path)
return target_path.DirName().Append(entry.name_text);
return target_path;
}

base::FilePath GetTargetBasePathForTestEntry(
const AddEntriesMessage::TestEntryInfo& entry) {
if (entry.shared_option == AddEntriesMessage::SHARED_WITH_ME ||
entry.shared_option == AddEntriesMessage::NESTED_SHARED_WITH_ME) {
return GetSharedWithMePath();
}
if (!entry.team_drive_name.empty()) {
return GetTeamDrivePath(entry.team_drive_name);
}
return GetMyDrivePath();
}

base::FilePath GetRelativeDrivePathForTestEntry(
const AddEntriesMessage::TestEntryInfo& entry) {
const base::FilePath target_path = GetTargetPathForTestEntry(entry);
Expand All @@ -945,6 +972,10 @@ class DriveFsTestVolume : public DriveTestVolume {
return mount_path().Append("team_drives");
}

base::FilePath GetSharedWithMePath() {
return mount_path().Append(".files-by-id/123");
}

base::FilePath GetTeamDrivePath(const std::string& team_drive_name) {
return GetTeamDriveGrandRoot().Append(team_drive_name);
}
Expand Down Expand Up @@ -1208,6 +1239,11 @@ void FileManagerBrowserTestBase::OnCommand(const std::string& name,
return;
}

if (name == "getDriveFsEnabled") {
*output = IsDriveFsTest() ? "true" : "false";
return;
}

if (name == "getRootPaths") {
// Obtain the root paths.
const auto downloads_root = util::GetDownloadsMountPointName(profile());
Expand Down
45 changes: 22 additions & 23 deletions chromeos/components/drivefs/fake_drivefs.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,9 @@ class FakeDriveFs::SearchQuery : public mojom::SearchQuery {
static std::vector<drivefs::mojom::QueryItemPtr> SearchFiles(
const base::FilePath& mount_path) {
std::vector<drivefs::mojom::QueryItemPtr> results;
base::FileEnumerator walker(mount_path, true, base::FileEnumerator::FILES);
base::FileEnumerator walker(
mount_path, true,
base::FileEnumerator::FILES | base::FileEnumerator::DIRECTORIES);
for (auto file = walker.Next(); !file.empty(); file = walker.Next()) {
auto item = drivefs::mojom::QueryItem::New();
item->path = base::FilePath("/");
Expand Down Expand Up @@ -157,29 +159,26 @@ class FakeDriveFs::SearchQuery : public mojom::SearchQuery {

void OnComplete() {
if (--pending_callbacks_ == 0) {
if (!query_.empty() || available_offline_ || shared_with_me_) {
// Filter out non-matching results.
base::EraseIf(results_, [=](const auto& item_ptr) {
const base::FilePath path = item_ptr->path;
const drivefs::mojom::FileMetadata* metadata =
item_ptr->metadata.get();
if (!query_.empty()) {
return base::ToLowerASCII(path.BaseName().value()).find(query_) ==
std::string::npos;
}
if (available_offline_) {
if (metadata && metadata->available_offline)
return false;
if (metadata &&
metadata->type == mojom::FileMetadata::Type::kHosted)
return false;
}
if (shared_with_me_ && metadata) {
return !metadata->shared;
}
// Filter out non-matching results.
base::EraseIf(results_, [=](const auto& item_ptr) {
if (!item_ptr->metadata) {
return true;
});
}
}
const base::FilePath path = item_ptr->path;
const drivefs::mojom::FileMetadata* metadata = item_ptr->metadata.get();
if (!query_.empty()) {
return base::ToLowerASCII(path.BaseName().value()).find(query_) ==
std::string::npos;
}
if (available_offline_) {
return !metadata->available_offline &&
metadata->type != mojom::FileMetadata::Type::kHosted;
}
if (shared_with_me_) {
return !metadata->shared;
}
return false;
});

std::move(callback_).Run(drive::FileError::FILE_ERROR_OK,
{std::move(results_)});
Expand Down
13 changes: 13 additions & 0 deletions components/drive/service/fake_drive_service.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1563,6 +1563,19 @@ google_apis::DriveApiErrorCode FakeDriveService::GetFileVisibility(
return HTTP_SUCCESS;
}

google_apis::DriveApiErrorCode FakeDriveService::SetFileAsSharedWithMe(
const std::string& resource_id) {
DCHECK(thread_checker_.CalledOnValidThread());

EntryInfo* entry = FindEntryByResourceId(resource_id);
if (!entry)
return HTTP_NOT_FOUND;

entry->change_resource.mutable_file()->set_shared_with_me_date(
base::Time::Now());
return HTTP_SUCCESS;
}

void FakeDriveService::AddChangeObserver(ChangeObserver* change_observer) {
change_observers_.AddObserver(change_observer);
}
Expand Down
3 changes: 3 additions & 0 deletions components/drive/service/fake_drive_service.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,9 @@ class FakeDriveService : public DriveServiceInterface {
const std::string& resource_id,
google_apis::drive::FileVisibility* visibility);

google_apis::DriveApiErrorCode SetFileAsSharedWithMe(
const std::string& resource_id);

void AddChangeObserver(ChangeObserver* observer);
void RemoveChangeObserver(ChangeObserver* observer);

Expand Down
9 changes: 7 additions & 2 deletions ui/file_manager/file_manager/foreground/js/ui/location_line.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,13 @@ LocationLine.prototype.getComponents_ = function(entry) {
}
if (locationInfo.rootType === VolumeManagerCommon.RootType.DRIVE_OTHER) {
// When target path is a shared directory, volume should be shared with me.
displayRootUrl = this.replaceRootName_(displayRootUrl, '/other');
displayRootFullPath = '/other';
const match = entry.fullPath.match(/\/\.files-by-id\/\d+\//);
if (match) {
displayRootFullPath = match[0];
} else {
displayRootFullPath = '/other';
}
displayRootUrl = this.replaceRootName_(displayRootUrl, displayRootFullPath);
var sharedWithMeFakeEntry = locationInfo.volumeInfo.fakeEntries[
VolumeManagerCommon.RootType.DRIVE_SHARED_WITH_ME];
components.push(new LocationLine.PathComponent(
Expand Down
56 changes: 47 additions & 9 deletions ui/file_manager/integration_tests/file_manager/drive_specific.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,10 +119,20 @@ testcase.driveOpenSidebarOffline = function() {
* "shared-with-me" should be shown.
*/
testcase.driveOpenSidebarSharedWithMe = function() {
var appId;
let appId;
let isDriveFsEnabled;
StepsRunner.run([
function() {
setupAndWaitUntilReady(null, RootPath.DRIVE, this.next);
sendTestMessage({name: 'getDriveFsEnabled'}).then(this.next);
},
function(result) {
isDriveFsEnabled = result === 'true';

setupAndWaitUntilReady(
null, RootPath.DRIVE, this.next, [], BASIC_DRIVE_ENTRY_SET.concat([
ENTRIES.sharedDirectory,
ENTRIES.sharedDirectoryFile,
]));
},
// Click the icon of the Shared With Me volume.
function(results) {
Expand All @@ -132,17 +142,45 @@ testcase.driveOpenSidebarSharedWithMe = function() {
appId,
['drive_shared_with_me'], this.next);
},
// Wait until the file list is updated.
// Wait until the breadcrumb path is updated.
function(result) {
chrome.test.assertFalse(!result);
remoteCall.waitForFileListChange(appId, BASIC_DRIVE_ENTRY_SET.length).
then(this.next);
remoteCall.waitUntilCurrentDirectoryIsChanged(appId, '/Shared with me')
.then(this.next);
},
// Verify the file list.
function(actualFilesAfter) {
chrome.test.assertEq(
TestEntryInfo.getExpectedRows(SHARED_WITH_ME_ENTRY_SET).sort(),
actualFilesAfter);
function() {
remoteCall
.waitForFiles(
appId,
TestEntryInfo.getExpectedRows(
SHARED_WITH_ME_ENTRY_SET.concat([ENTRIES.sharedDirectory])))
.then(this.next);
},
// Navigate to the directory within Shared with me.
function() {
remoteCall.callRemoteTestUtil('openFile', appId, ['Shared Directory'])
.then(this.next);
},
// Wait until the breadcrumb path is updated.
function(result) {
chrome.test.assertFalse(!result);
remoteCall
.waitUntilCurrentDirectoryIsChanged(
appId,
isDriveFsEnabled ? '/Shared with me/Shared Directory' :
'/My Drive/Shared Directory')
.then(this.next);
},
// Verify the file list.
function() {
remoteCall
.waitForFiles(
appId,
TestEntryInfo.getExpectedRows([ENTRIES.sharedDirectoryFile]))
.then(this.next);
},
function() {
checkIfNoErrorsOccured(this.next);
}
]);
Expand Down
26 changes: 25 additions & 1 deletion ui/file_manager/integration_tests/test_util.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,9 @@ var EntryType = Object.freeze({
*/
var SharedOption = Object.freeze({
NONE: 'none',
SHARED: 'shared'
SHARED: 'shared',
SHARED_WITH_ME: 'sharedWithMe',
NESTED_SHARED_WITH_ME: 'nestedSharedWithMe',
});

/**
Expand Down Expand Up @@ -818,4 +820,26 @@ var ENTRIES = {
sizeText: '51 bytes',
typeText: 'Plain text'
}),

sharedDirectory: new TestEntryInfo({
type: EntryType.DIRECTORY,
targetPath: 'Shared Directory',
sharedOption: SharedOption.SHARED_WITH_ME,
lastModifiedTime: 'Jan 1, 2000, 1:00 AM',
nameText: 'Shared Directory',
sizeText: '--',
typeText: 'Folder'
}),

sharedDirectoryFile: new TestEntryInfo({
type: EntryType.FILE,
sourceFileName: 'text.txt',
targetPath: 'Shared Directory/file.txt',
mimeType: 'text/plain',
sharedOption: SharedOption.NESTED_SHARED_WITH_ME,
lastModifiedTime: 'Jan 1, 2000, 1:00 AM',
nameText: 'file.txt',
sizeText: '51 bytes',
typeText: 'Plain text'
}),
};

0 comments on commit 367235e

Please sign in to comment.