Commit 27ead8ef authored by hirono's avatar hirono Committed by Commit bot

Files.app: Handles corner cases in NewMetadataProvider.

The CL adds:
 * Assertion for property names passed to NewMetadataProvider.
 * Existance check for results obtained from getImpl.

BUG=410766
TEST=FileManagerJsTest.NewMetadataProvider

Review URL: https://codereview.chromium.org/899603002

Cr-Commit-Position: refs/heads/master@{#314297}
parent ca604c2b
...@@ -5,17 +5,29 @@ ...@@ -5,17 +5,29 @@
/** /**
* TODO(hirono): Remove 'New' from the name after removing old MetadataProvider. * TODO(hirono): Remove 'New' from the name after removing old MetadataProvider.
* @param {!MetadataProviderCache} cache * @param {!MetadataProviderCache} cache
* @param {!Array<string>} validPropertyNames
* @constructor * @constructor
* @struct * @struct
* @template T * @template T
*/ */
function NewMetadataProvider(cache) { function NewMetadataProvider(cache, validPropertyNames) {
/** /**
* @private {!MetadataProviderCache} * @private {!MetadataProviderCache}
* @const * @const
*/ */
this.cache_ = cache; this.cache_ = cache;
/**
* Set of valid property names. Key is the name of property and value is
* always true.
* @private {!Object<string, boolean>}
* @const
*/
this.validPropertyNames_ = {};
for (var i = 0; i < validPropertyNames.length; i++) {
this.validPropertyNames_[validPropertyNames[i]] = true;
}
/** /**
* @private {!Array<!MetadataProviderCallbackRequest<T>>} * @private {!Array<!MetadataProviderCallbackRequest<T>>}
* @const * @const
...@@ -25,8 +37,6 @@ function NewMetadataProvider(cache) { ...@@ -25,8 +37,6 @@ function NewMetadataProvider(cache) {
/** /**
* Obtains the metadata for the request. * Obtains the metadata for the request.
* Note: this must return all the properties requested by the argument.
* Otherwise Promise returned by NewMetadataProvider#get may not be fulfilled.
* @param {!Array<!MetadataRequest>} requests * @param {!Array<!MetadataRequest>} requests
* @return {!Promise<!Array<!T>>} * @return {!Promise<!Array<!T>>}
* @protected * @protected
...@@ -40,6 +50,11 @@ NewMetadataProvider.prototype.getImpl; ...@@ -40,6 +50,11 @@ NewMetadataProvider.prototype.getImpl;
* @return {!Promise<!Array<!T>>} * @return {!Promise<!Array<!T>>}
*/ */
NewMetadataProvider.prototype.get = function(entries, names) { NewMetadataProvider.prototype.get = function(entries, names) {
// Check if the property name is correct or not.
for (var i = 0; i < names.length; i++) {
assert(this.validPropertyNames_[names[i]]);
}
// Check if the results are cached or not. // Check if the results are cached or not.
if (this.cache_.hasFreshCache(entries, names)) if (this.cache_.hasFreshCache(entries, names))
return Promise.resolve(this.getCache(entries, names)); return Promise.resolve(this.getCache(entries, names));
...@@ -60,11 +75,19 @@ NewMetadataProvider.prototype.get = function(entries, names) { ...@@ -60,11 +75,19 @@ NewMetadataProvider.prototype.get = function(entries, names) {
// If the requests are not empty, call the requests. // If the requests are not empty, call the requests.
if (requests.length) { if (requests.length) {
var requestedEntries = [];
for (var i = 0; i < requests.length; i++) {
requestedEntries.push(requests[i].entry);
}
this.getImpl(requests).then(function(list) { this.getImpl(requests).then(function(list) {
// Obtain requested entries and ensure all the requested properties are
// contained in the result.
var requestedEntries = [];
for (var i = 0; i < requests.length; i++) {
requestedEntries.push(requests[i].entry);
for (var j = 0; j < requests[i].names.length; j++) {
var name = requests[i].names[j];
if (!(name in list[i]))
list[i][name] = undefined;
}
}
// Store cache. // Store cache.
if (this.cache_.storeProperties(requestId, requestedEntries, list)) { if (this.cache_.storeProperties(requestId, requestedEntries, list)) {
// TODO(hirono): Dispatch metadata change event here. // TODO(hirono): Dispatch metadata change event here.
......
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
<script src="metadata_cache_set.js"></script> <script src="metadata_cache_set.js"></script>
<!-- Others --> <!-- Others -->
<script src="../../../../../../ui/webui/resources/js/assert.js"></script>
<script src="../../../common/js/lru_cache.js"></script> <script src="../../../common/js/lru_cache.js"></script>
<script src="../../../common/js/unittest_util.js"></script> <script src="../../../common/js/unittest_util.js"></script>
<script src="metadata_cache_item.js"></script> <script src="metadata_cache_item.js"></script>
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
// found in the LICENSE file. // found in the LICENSE file.
function TestMetadataProvider(cache) { function TestMetadataProvider(cache) {
NewMetadataProvider.call(this, cache); NewMetadataProvider.call(this, cache, ['property', 'propertyA', 'propertyB']);
this.requestCount = 0; this.requestCount = 0;
} }
...@@ -22,6 +22,18 @@ TestMetadataProvider.prototype.getImpl = function(requests) { ...@@ -22,6 +22,18 @@ TestMetadataProvider.prototype.getImpl = function(requests) {
})); }));
}; };
function TestEmptyMetadataProvider(cache) {
NewMetadataProvider.call(this, cache, ['property']);
}
TestEmptyMetadataProvider.prototype.__proto__ = NewMetadataProvider.prototype;
TestEmptyMetadataProvider.prototype.getImpl = function(requests) {
return Promise.resolve(requests.map(function() {
return {};
}));
};
var entryA = { var entryA = {
toURL: function() { return "filesystem://A"; } toURL: function() { return "filesystem://A"; }
}; };
...@@ -113,3 +125,20 @@ function testNewMetadataProviderGetCache(callback) { ...@@ -113,3 +125,20 @@ function testNewMetadataProviderGetCache(callback) {
assertEquals('filesystem://A:property', cache[0].property); assertEquals('filesystem://A:property', cache[0].property);
}), callback); }), callback);
} }
function testNewMetadataProviderUnknownProperty() {
var cache = new MetadataProviderCache();
var provider = new TestMetadataProvider(cache);
assertThrows(function() {
provider.get([entryA], ['unknown']);
});
}
function testNewMetadataProviderEmptyResult(callback) {
var cache = new MetadataProviderCache();
var provider = new TestEmptyMetadataProvider(cache);
// getImpl returns empty result.
reportPromise(provider.get([entryA], ['property']).then(function(results) {
assertEquals(undefined, results[0].property);
}), 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