Commit f57f5f8e authored by Luciano Pacheco's avatar Luciano Pacheco Committed by Commit Bot

Change directory tree to use Metadata update event

Metadata model provides an "update" event which is triggered anytime a
metadata item has been updated from the backend. Any UI part that
to display latest metadata should listen to it and update the UI
accordingly.

Change the directory tree to listen to Metadata "update" event to
update the "shared" icon. A listener is added for every expanded
folder to listen to updates for its sub-folders. When a folder is
collapsed its  listener is removed.

Change |onExpand_| function to fetch all metadata values from
|LIST_CONTAINER_METADATA_PREFETCH_PROPERTY_NAMES|, instead of
|DIRECTORY_TREE_METADATA_PREFETCH_PROPERTY_NAMES|. Directory tree needs
the metadata "can*" to setup permission accordingly in the context
menu. This is only fetched for Drive roots, since only Drive implements
these metadata.

Change |updateSharedStatusIcon| function to use the cached metadata
value. This fixes the issue where it would trigger metadata requests to
backend too early, because this |updateSharedStatusIcon| is called from
SubDirectoryItem constructor.

Remove Metadata cache invalidation calls (notifyEntriesChanged and
notifyEntriesRemoved) because directory tree doesn't update metadata
directly.

Fix MockVolumeManager to work with other volumes other than Drive, by
extracting the VolumeManager implementation to detect RootType to a
common function |getRootTypeFromVolumeType|.

Remove constant |DIRECTORY_TREE_METADATA_PREFETCH_PROPERTY_NAMES| since
its use has been removed in directory_tree.js.

Add MetadataModel |removeEventListener| to be able to remove listener
of MetadataModel "update" event.

manual tests for Drive "shared" icon.
New integration tests for "shared" icon will be added in follow up CL.

Bug: 880187, 880189
Test: Unittest for |insideMyDrive| and |insideDrive| functions and
Change-Id: Ibe8320e7b0981d8e9556ebb8a922d7db2f16860c
Reviewed-on: https://chromium-review.googlesource.com/1203596
Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
Reviewed-by: default avatarNoel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589846}
parent b37a343c
...@@ -246,14 +246,19 @@ MockVolumeManagerWrapper.prototype.getVolumeInfo = function(entry) { ...@@ -246,14 +246,19 @@ MockVolumeManagerWrapper.prototype.getVolumeInfo = function(entry) {
* @return {EntryLocation} Location information. * @return {EntryLocation} Location information.
*/ */
MockVolumeManagerWrapper.prototype.getLocationInfo = function(entry) { MockVolumeManagerWrapper.prototype.getLocationInfo = function(entry) {
var volumeInfo = /** @type {!VolumeInfo} */ (this.volumeInfoList.item(0)); var volumeInfo = /** @type {!VolumeInfo} */ (this.volumeInfoList.array_.find(
volumeList => volumeList.fileSystem == entry.filesystem));
if (util.isFakeEntry(entry)) { if (util.isFakeEntry(entry)) {
var fakeEntry = /** @type {!FakeEntry} */ (entry); var fakeEntry = /** @type {!FakeEntry} */ (entry);
return new EntryLocationImpl(volumeInfo, fakeEntry.rootType, true, true); return new EntryLocationImpl(volumeInfo, fakeEntry.rootType, true, true);
} }
let rootType;
let isReadOnly = false;
let isRootEntry;
if (entry.filesystem.name === VolumeManagerCommon.VolumeType.DRIVE) { if (entry.filesystem.name === VolumeManagerCommon.VolumeType.DRIVE) {
var rootType = VolumeManagerCommon.RootType.DRIVE; rootType = VolumeManagerCommon.RootType.DRIVE;
var isRootEntry = entry.fullPath === '/root'; isRootEntry = entry.fullPath === '/root';
if (entry.fullPath.startsWith('/team_drives')) { if (entry.fullPath.startsWith('/team_drives')) {
if (entry.fullPath === '/team_drives') { if (entry.fullPath === '/team_drives') {
rootType = VolumeManagerCommon.RootType.TEAM_DRIVES_GRAND_ROOT; rootType = VolumeManagerCommon.RootType.TEAM_DRIVES_GRAND_ROOT;
...@@ -265,7 +270,11 @@ MockVolumeManagerWrapper.prototype.getLocationInfo = function(entry) { ...@@ -265,7 +270,11 @@ MockVolumeManagerWrapper.prototype.getLocationInfo = function(entry) {
} }
return new EntryLocationImpl(volumeInfo, rootType, isRootEntry, true); return new EntryLocationImpl(volumeInfo, rootType, isRootEntry, true);
} }
throw new Error('Not implemented exception.');
rootType =
VolumeManagerCommon.getRootTypeFromVolumeType(volumeInfo.volumeType);
isRootEntry = util.isSameEntry(entry, volumeInfo.fileSystem.root);
return new EntryLocationImpl(volumeInfo, rootType, isRootEntry, isReadOnly);
}; };
/** /**
* @param {VolumeManagerCommon.VolumeType} volumeType Volume type. * @param {VolumeManagerCommon.VolumeType} volumeType Volume type.
......
...@@ -338,35 +338,8 @@ VolumeManagerImpl.prototype.getLocationInfo = function(entry) { ...@@ -338,35 +338,8 @@ VolumeManagerImpl.prototype.getLocationInfo = function(entry) {
return null; return null;
} }
} else { } else {
switch (volumeInfo.volumeType) { rootType =
case VolumeManagerCommon.VolumeType.DOWNLOADS: VolumeManagerCommon.getRootTypeFromVolumeType(volumeInfo.volumeType);
rootType = VolumeManagerCommon.RootType.DOWNLOADS;
break;
case VolumeManagerCommon.VolumeType.REMOVABLE:
rootType = VolumeManagerCommon.RootType.REMOVABLE;
break;
case VolumeManagerCommon.VolumeType.ARCHIVE:
rootType = VolumeManagerCommon.RootType.ARCHIVE;
break;
case VolumeManagerCommon.VolumeType.MTP:
rootType = VolumeManagerCommon.RootType.MTP;
break;
case VolumeManagerCommon.VolumeType.PROVIDED:
rootType = VolumeManagerCommon.RootType.PROVIDED;
break;
case VolumeManagerCommon.VolumeType.MEDIA_VIEW:
rootType = VolumeManagerCommon.RootType.MEDIA_VIEW;
break;
case VolumeManagerCommon.VolumeType.CROSTINI:
rootType = VolumeManagerCommon.RootType.CROSTINI;
break;
case VolumeManagerCommon.VolumeType.ANDROID_FILES:
rootType = VolumeManagerCommon.RootType.ANDROID_FILES;
break;
default:
// Programming error, throw an exception.
throw new Error('Invalid volume type: ' + volumeInfo.volumeType);
}
isRootEntry = util.isSameEntry(entry, volumeInfo.fileSystem.root); isRootEntry = util.isSameEntry(entry, volumeInfo.fileSystem.root);
// Although "Play files" root directory is writable in file system level, // Although "Play files" root directory is writable in file system level,
// we prohibit write operations on it in the UI level to avoid confusion. // we prohibit write operations on it in the UI level to avoid confusion.
......
...@@ -293,6 +293,36 @@ VolumeManagerCommon.getVolumeTypeFromRootType = function(rootType) { ...@@ -293,6 +293,36 @@ VolumeManagerCommon.getVolumeTypeFromRootType = function(rootType) {
assertNotReached('Unknown root type: ' + rootType); assertNotReached('Unknown root type: ' + rootType);
}; };
/**
* @param {VolumeManagerCommon.VolumeType} volumeType .
* @return {VolumeManagerCommon.RootType}
*/
VolumeManagerCommon.getRootTypeFromVolumeType = function(volumeType) {
switch (volumeType) {
case VolumeManagerCommon.VolumeType.ANDROID_FILES:
return VolumeManagerCommon.RootType.ANDROID_FILES;
case VolumeManagerCommon.VolumeType.ARCHIVE:
return VolumeManagerCommon.RootType.ARCHIVE;
case VolumeManagerCommon.VolumeType.CROSTINI:
return VolumeManagerCommon.RootType.CROSTINI;
case VolumeManagerCommon.VolumeType.DOWNLOADS:
return VolumeManagerCommon.RootType.DOWNLOADS;
case VolumeManagerCommon.VolumeType.DRIVE:
return VolumeManagerCommon.RootType.DRIVE;
case VolumeManagerCommon.VolumeType.MEDIA_VIEW:
return VolumeManagerCommon.RootType.MEDIA_VIEW;
case VolumeManagerCommon.VolumeType.MTP:
return VolumeManagerCommon.RootType.MTP;
case VolumeManagerCommon.VolumeType.MY_FILES:
return VolumeManagerCommon.RootType.MY_FILES;
case VolumeManagerCommon.VolumeType.PROVIDED:
return VolumeManagerCommon.RootType.PROVIDED;
case VolumeManagerCommon.VolumeType.REMOVABLE:
return VolumeManagerCommon.RootType.REMOVABLE;
}
assertNotReached('Unknown volume type: ' + volumeType);
};
/** /**
* @typedef {{ * @typedef {{
* type: VolumeManagerCommon.DriveConnectionType, * type: VolumeManagerCommon.DriveConnectionType,
......
...@@ -52,18 +52,6 @@ constants.LIST_CONTAINER_METADATA_PREFETCH_PROPERTY_NAMES = [ ...@@ -52,18 +52,6 @@ constants.LIST_CONTAINER_METADATA_PREFETCH_PROPERTY_NAMES = [
'canDelete', 'canRename', 'canAddChildren', 'canShare' 'canDelete', 'canRename', 'canAddChildren', 'canShare'
]; ];
/**
* Metadata property names used by DirectoryTree. Theis metadata is expected to
* be cached for items that are visible in the directory tree (left-hand
* folder navigation).
* TODO(sashab): Store capabilities as a set of flags to save memory. See
* https://crbug.com/849997
*
* @const {!Array<string>}
*/
constants.DIRECTORY_TREE_METADATA_PREFETCH_PROPERTY_NAMES =
['canCopy', 'canDelete', 'canRename', 'canAddChildren', 'canShare'];
/** /**
* Path for files_quick_view.html file. Allow override for testing. * Path for files_quick_view.html file. Allow override for testing.
* @type {string} * @type {string}
......
...@@ -162,6 +162,15 @@ MetadataModel.prototype.addEventListener = function(type, callback) { ...@@ -162,6 +162,15 @@ MetadataModel.prototype.addEventListener = function(type, callback) {
this.cache_.addEventListener(type, callback); this.cache_.addEventListener(type, callback);
}; };
/**
* Removes event listener from internal cache object.
* @param {string} type Name of the event to removed.
* @param {function(Event):undefined} callback Event listener.
*/
MetadataModel.prototype.removeEventListener = function(type, callback) {
this.cache_.removeEventListener(type, callback);
};
/** /**
* @param {!Array<!Entry>} entries * @param {!Array<!Entry>} entries
* @param {!Array<string>} names * @param {!Array<string>} names
......
...@@ -172,6 +172,13 @@ function DirectoryItem(label, tree) { ...@@ -172,6 +172,13 @@ function DirectoryItem(label, tree) {
item.label = label; item.label = label;
// @type {!Array<Entry>} Filled after updateSubDirectories read entries.
item.entries_ = [];
// @type {function()=} onMetadataUpdated_ bound to |this| used to listen
// metadata update events.
item.onMetadataUpdateBound_ = undefined;
return item; return item;
} }
...@@ -194,6 +201,74 @@ DirectoryItem.prototype = { ...@@ -194,6 +201,74 @@ DirectoryItem.prototype = {
*/ */
get labelElement() { get labelElement() {
return this.firstElementChild.querySelector('.label'); return this.firstElementChild.querySelector('.label');
},
/**
* Returns true if this item is inside any part of My Drive.
* @type {!boolean}
*/
get insideMyDrive() {
if (!this.entry)
return false;
const locationInfo =
this.parentTree_.volumeManager.getLocationInfo(this.entry);
return locationInfo &&
locationInfo.rootType === VolumeManagerCommon.RootType.DRIVE;
},
/**
* Returns true if this item is inside any part of Drive, including Team
* Drive.
* @type {!boolean}
*/
get insideDrive() {
if (!this.entry)
return false;
const locationInfo =
this.parentTree_.volumeManager.getLocationInfo(this.entry);
return locationInfo &&
(locationInfo.rootType === VolumeManagerCommon.RootType.DRIVE ||
locationInfo.rootType ===
VolumeManagerCommon.RootType.TEAM_DRIVES_GRAND_ROOT ||
locationInfo.rootType === VolumeManagerCommon.RootType.TEAM_DRIVE ||
locationInfo.rootType === VolumeManagerCommon.RootType.DRIVE_OFFLINE ||
locationInfo.rootType ===
VolumeManagerCommon.RootType.DRIVE_SHARED_WITH_ME ||
locationInfo.rootType ===
VolumeManagerCommon.RootType.DRIVE_FAKE_ROOT);
},
/**
* If the this directory supports 'shared' feature, as in displays shared
* icon. It's only supported inside 'My Drive', even Team Drive doesn't
* support it.
* @type {!boolean}
*/
get supportsSharedFeature() {
return this.insideMyDrive;
},
};
/**
* Handles the Metadata update event. It updates the shared icon of this item
* sub-folders.
* @param {Event} event Metadata update event.
*/
DirectoryItem.prototype.onMetadataUpdated_ = function(event) {
if (!(event.names.has('shared') && this.supportsSharedFeature))
return;
let index = 0;
while (this.entries_[index]) {
const childEntry = this.entries_[index];
const childElement = this.items[index];
if (event.entriesMap.has(childEntry.toURL()))
childElement.updateSharedStatusIcon();
index++;
} }
}; };
...@@ -316,7 +391,6 @@ DirectoryItem.prototype.remove = function(child) { ...@@ -316,7 +391,6 @@ DirectoryItem.prototype.remove = function(child) {
this.hasChildren = false; this.hasChildren = false;
}; };
/** /**
* Removes the has-children attribute which allows returning * Removes the has-children attribute which allows returning
* to the ambiguous may-have-children state. * to the ambiguous may-have-children state.
...@@ -327,20 +401,25 @@ DirectoryItem.prototype.clearHasChildren = function() { ...@@ -327,20 +401,25 @@ DirectoryItem.prototype.clearHasChildren = function() {
rowItem.removeAttribute('has-children'); rowItem.removeAttribute('has-children');
}; };
/** /**
* Invoked when the item is being expanded. * Invoked when the item is being expanded.
* @param {!Event} e Event. * @param {!Event} e Event.
* @private * @private
*/ */
DirectoryItem.prototype.onExpand_ = function(e) { DirectoryItem.prototype.onExpand_ = function(e) {
if (this.supportsSharedFeature && !this.onMetadataUpdateBound_) {
this.onMetadataUpdateBound_ = this.onMetadataUpdated_.bind(this);
this.parentTree_.metadataModel_.addEventListener(
'update', this.onMetadataUpdateBound_);
}
this.updateSubDirectories( this.updateSubDirectories(
true /* recursive */, true /* recursive */,
function() { function() {
// Retrieve metadata information for the child (expanded) items. if (!this.insideDrive)
return;
this.parentTree_.metadataModel_.get( this.parentTree_.metadataModel_.get(
this.entries_, this.entries_,
constants.DIRECTORY_TREE_METADATA_PREFETCH_PROPERTY_NAMES); constants.LIST_CONTAINER_METADATA_PREFETCH_PROPERTY_NAMES);
}.bind(this), }.bind(this),
function() { function() {
this.expanded = false; this.expanded = false;
...@@ -372,14 +451,11 @@ let getVisibleChildEntries = function(parentItem) { ...@@ -372,14 +451,11 @@ let getVisibleChildEntries = function(parentItem) {
* @private * @private
*/ */
DirectoryItem.prototype.onCollapse_ = function(e) { DirectoryItem.prototype.onCollapse_ = function(e) {
// Remove metadata information for the child (now hidden) items by recursively if (this.onMetadataUpdateBound_) {
// searching for all visible items under this item. this.parentTree_.metadataModel_.removeEventListener(
// TODO(sashab): Add a method to the cache to clear all children of a given 'update', this.onMetadataUpdateBound_);
// parent instead. this.onMetadataUpdateBound_ = undefined;
const visibleEntries = getVisibleChildEntries(this); }
this.parentTree_.metadataModel_.notifyEntriesRemoved(
visibleEntries,
constants.DIRECTORY_TREE_METADATA_PREFETCH_PROPERTY_NAMES);
if (this.delayExpansion) { if (this.delayExpansion) {
// For file systems where it is performance intensive // For file systems where it is performance intensive
...@@ -612,14 +688,9 @@ SubDirectoryItem.prototype = { ...@@ -612,14 +688,9 @@ SubDirectoryItem.prototype = {
*/ */
SubDirectoryItem.prototype.updateSharedStatusIcon = function() { SubDirectoryItem.prototype.updateSharedStatusIcon = function() {
var icon = this.querySelector('.icon'); var icon = this.querySelector('.icon');
this.parentTree_.metadataModel.notifyEntriesChanged([this.dirEntry_]); const metadata =
this.parentTree_.metadataModel this.parentTree_.metadataModel.getCache([this.dirEntry_], ['shared']);
.get( icon.classList.toggle('shared', !!(metadata[0] && metadata[0].shared));
[this.dirEntry_],
constants.LIST_CONTAINER_METADATA_PREFETCH_PROPERTY_NAMES)
.then(function(metadata) {
icon.classList.toggle('shared', !!(metadata[0] && metadata[0].shared));
});
}; };
/** /**
......
...@@ -43,6 +43,23 @@ function setUp() { ...@@ -43,6 +43,23 @@ function setUp() {
}; };
} }
/**
* Creates a plain object that can be used as mock for MetadataModel.
*/
function mockMetadataModel() {
const mock = {
notifyEntriesChanged: () => {},
get: function(entries, labels) {
// Mock a non-shared directory.
return Promise.resolve([{shared: false}]);
},
getCache: function(entries, labels) {
return [{shared: false}];
},
};
return mock;
}
/** /**
* Returns item labels of a directory tree as a list. * Returns item labels of a directory tree as a list.
* *
...@@ -136,13 +153,8 @@ function testCreateDirectoryTreeWithTeamDrive(callback) { ...@@ -136,13 +153,8 @@ function testCreateDirectoryTreeWithTeamDrive(callback) {
// Create elements. // Create elements.
var parentElement = document.createElement('div'); var parentElement = document.createElement('div');
var directoryTree = document.createElement('div'); var directoryTree = document.createElement('div');
directoryTree.metadataModel = { directoryTree.metadataModel = mockMetadataModel();
notifyEntriesChanged: function() {},
get: function(entries, labels) {
// Mock a non-shared directory
return Promise.resolve([{shared: false}]);
}
};
parentElement.appendChild(directoryTree); parentElement.appendChild(directoryTree);
// Create mocks. // Create mocks.
...@@ -203,13 +215,8 @@ function testCreateDirectoryTreeWithEmptyTeamDrive(callback) { ...@@ -203,13 +215,8 @@ function testCreateDirectoryTreeWithEmptyTeamDrive(callback) {
// Create elements. // Create elements.
var parentElement = document.createElement('div'); var parentElement = document.createElement('div');
var directoryTree = document.createElement('div'); var directoryTree = document.createElement('div');
directoryTree.metadataModel = { directoryTree.metadataModel = mockMetadataModel();
notifyEntriesChanged: function() {},
get: function(entries, labels) {
// Mock a non-shared directory
return Promise.resolve([{shared: false}]);
}
};
parentElement.appendChild(directoryTree); parentElement.appendChild(directoryTree);
// Create mocks. // Create mocks.
...@@ -366,13 +373,8 @@ function testAddFirstTeamDrive(callback) { ...@@ -366,13 +373,8 @@ function testAddFirstTeamDrive(callback) {
// Create elements. // Create elements.
var parentElement = document.createElement('div'); var parentElement = document.createElement('div');
var directoryTree = document.createElement('div'); var directoryTree = document.createElement('div');
directoryTree.metadataModel = { directoryTree.metadataModel = mockMetadataModel();
notifyEntriesChanged: () => {},
get: function(entries, labels) {
// Mock a non-shared directory
return Promise.resolve([{shared: false}]);
}
};
parentElement.appendChild(directoryTree); parentElement.appendChild(directoryTree);
// Create mocks. // Create mocks.
...@@ -441,13 +443,8 @@ function testRemoveLastTeamDrive(callback) { ...@@ -441,13 +443,8 @@ function testRemoveLastTeamDrive(callback) {
// Create elements. // Create elements.
var parentElement = document.createElement('div'); var parentElement = document.createElement('div');
var directoryTree = document.createElement('div'); var directoryTree = document.createElement('div');
directoryTree.metadataModel = { directoryTree.metadataModel = mockMetadataModel();
notifyEntriesChanged: () => {},
get: function(entries, labels) {
// Mock a non-shared directory
return Promise.resolve([{shared: false}]);
}
};
parentElement.appendChild(directoryTree); parentElement.appendChild(directoryTree);
// Create mocks. // Create mocks.
...@@ -511,3 +508,68 @@ function testRemoveLastTeamDrive(callback) { ...@@ -511,3 +508,68 @@ function testRemoveLastTeamDrive(callback) {
}), }),
callback); callback);
} }
/**
* Test DirectoryItem.insideMyDrive property, which should return true when
* inside My Drive and any of its sub-directories; Should return false for
* everything else, including within Team Drive.
*
* @param {!function(boolean)} callback A callback function which is called with
* test result.
*/
function testInsideMyDriveAndInsideDrive(callback) {
const parentElement = document.createElement('div');
const directoryTree = document.createElement('div');
directoryTree.metadataModel = mockMetadataModel();
parentElement.appendChild(directoryTree);
// Create mocks.
const directoryModel = new MockDirectoryModel();
const volumeManager = new MockVolumeManagerWrapper();
const fileOperationManager = {addEventListener: function(name, callback) {}};
// Setup My Drive and Downloads and one folder inside each of them.
const driveFileSystem = volumeManager.volumeInfoList.item(0).fileSystem;
const downloadsFileSystem = volumeManager.volumeInfoList.item(1).fileSystem;
window.webkitResolveLocalFileSystemURLEntries['filesystem:drive/root'] =
new MockDirectoryEntry(driveFileSystem, '/root');
window
.webkitResolveLocalFileSystemURLEntries['filesystem:drive/root/folder1'] =
new MockDirectoryEntry(driveFileSystem, '/root/folder1');
window
.webkitResolveLocalFileSystemURLEntries['filesystem:downloads/folder1'] =
new MockDirectoryEntry(downloadsFileSystem, '/folder1');
DirectoryTree.decorate(
directoryTree, directoryModel, volumeManager, null, fileOperationManager,
true);
directoryTree.dataModel = new MockNavigationListModel(volumeManager);
directoryTree.redraw(true);
const driveItem = directoryTree.items[0];
const downloadsItem = directoryTree.items[1];
reportPromise(
waitUntil(() => {
// Under the drive item, there exist 3 entries. In Downloads should
// exist 1 entry folder1.
return driveItem.items.length === 3 && downloadsItem.items.length === 1;
}).then(() => {
// insideMyDrive
assertTrue(driveItem.insideMyDrive, 'Drive root');
assertTrue(driveItem.items[0].insideMyDrive, 'My Drive root');
assertFalse(driveItem.items[1].insideMyDrive, 'Team Drives root');
assertFalse(driveItem.items[2].insideMyDrive, 'Offline root');
assertFalse(downloadsItem.insideMyDrive, 'Downloads root');
assertFalse(downloadsItem.items[0].insideMyDrive, 'Downloads/folder1');
// insideDrive
assertTrue(driveItem.insideDrive, 'Drive root');
assertTrue(driveItem.items[0].insideDrive, 'My Drive root');
assertTrue(driveItem.items[1].insideDrive, 'Team Drives root');
assertTrue(driveItem.items[2].insideDrive, 'Offline root');
assertFalse(downloadsItem.insideDrive, 'Downloads root');
assertFalse(downloadsItem.items[0].insideDrive, 'Downloads/folder1');
}),
callback);
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment