Bug 1420173 - Filter and sort sources by version in L10nRegistry. r?stas
MozReview-Commit-ID: GE5WmaCXQu3
--- a/intl/l10n/L10nRegistry.jsm
+++ b/intl/l10n/L10nRegistry.jsm
@@ -13,24 +13,24 @@ Components.utils.importGlobalProperties(
*
* The generator creates all possible permutations of locales and sources to allow for
* complete fallbacking.
*
* Example:
*
* FileSource1:
* name: 'app'
- * locales: ['en-US', 'de']
+ * locales: {'en-US': 201701010000, 'de': 20170101123500}
* resources: [
* '/browser/menu.ftl',
* '/platform/toolkit.ftl',
* ]
* FileSource2:
* name: 'platform'
- * locales: ['en-US', 'de']
+ * locales: {'en-US': 201701010000, 'de': 20170101123500}
* resources: [
* '/platform/toolkit.ftl',
* ]
*
* If the user will request:
* L10nRegistry.generateContexts(['de', 'en-US'], [
* '/browser/menu.ftl',
* '/platform/toolkit.ftl'
@@ -87,18 +87,29 @@ const L10nRegistry = {
* @param {Array} requestedLangs
* @param {Array} resourceIds
* @returns {AsyncIterator<MessageContext>}
*/
async * generateContexts(requestedLangs, resourceIds) {
if (this.bootstrap !== null) {
await this.bootstrap;
}
- const sourcesOrder = Array.from(this.sources.keys()).reverse();
for (const locale of requestedLangs) {
+ // We're performing three operations here:
+ //
+ // 1) Filtering the list of sources to only get ones that have the locale
+ // 2) Reversing in order to get the newer source first in case equal versions
+ // 3) Sort by locale version
+ const sourcesOrder = [...this.sources.keys()].filter(
+ name => this.sources.get(name).locales.has(locale)
+ ).reverse().sort((a, b) => {
+ const ver1 = this.sources.get(a).locales.get(locale);
+ const ver2 = this.sources.get(b).locales.get(locale);
+ return ver1 < ver2;
+ });
yield * generateContextsForLocale(locale, sourcesOrder, resourceIds);
}
},
/**
* Adds a new resource source to the L10nRegistry.
*
* @param {FileSource} source
@@ -130,30 +141,31 @@ const L10nRegistry = {
/**
* Removes a source from the L10nRegistry.
*
* @param {String} sourceId
*/
removeSource(sourceName) {
this.sources.delete(sourceName);
+ this.ctxCache.clear();
Services.obs.notifyObservers(null, 'l10n:available-locales-changed', null);
},
/**
* Returns a list of locales for which at least one source
* has resources.
*
* @returns {Array<String>}
*/
getAvailableLocales() {
const locales = new Set();
for (const source of this.sources.values()) {
- for (const locale of source.locales) {
+ for (const locale of source.locales.keys()) {
locales.add(locale);
}
}
return Array.from(locales);
},
};
/**
@@ -264,25 +276,32 @@ function generateContext(locale, sources
* It registers its own locales and a pre-path, and when asked for a file
* it attempts to download and cache it.
*
* The Source caches the downloaded files so any consecutive loads will
* come from the cache.
**/
class FileSource {
/**
- * @param {string} name
- * @param {Array<string>} locales
- * @param {string} prePath
+ * @param {string} name
+ * @param {Object<string, number>|Array<string>} locales
+ * @param {string} prePath
*
* @returns {IndexedFileSource}
*/
constructor(name, locales, prePath) {
this.name = name;
- this.locales = locales;
+
+ if (!Array.isArray(locales) &&
+ Object.values(locales).some(ver => typeof ver !== "number")) {
+ throw new Error("All versions must be numbers");
+ }
+ this.locales = Array.isArray(locales) ?
+ new Map(locales.map(loc => [loc, 0])) :
+ new Map(Object.entries(locales));
this.prePath = prePath;
this.indexed = false;
// The cache object stores information about the resources available
// in the Source.
//
// It can take one of three states:
// * true - the resource is available but not fetched yet
@@ -298,17 +317,17 @@ class FileSource {
this.cache = {};
}
getPath(locale, path) {
return (this.prePath + path).replace(/\{locale\}/g, locale);
}
hasFile(locale, path) {
- if (!this.locales.includes(locale)) {
+ if (!this.locales.has(locale)) {
return false;
}
const fullPath = this.getPath(locale, path);
if (!this.cache.hasOwnProperty(fullPath)) {
return this.indexed ? false : undefined;
}
if (this.cache[fullPath] === false) {
@@ -316,17 +335,17 @@ class FileSource {
}
if (this.cache[fullPath].then) {
return undefined;
}
return true;
}
fetchFile(locale, path) {
- if (!this.locales.includes(locale)) {
+ if (!this.locales.has(locale)) {
return Promise.reject(`The source has no resources for locale "${locale}"`);
}
const fullPath = this.getPath(locale, path);
if (this.cache.hasOwnProperty(fullPath)) {
if (this.cache[fullPath] === false) {
return Promise.reject(`The source has no resources for path "${fullPath}"`);
@@ -355,20 +374,20 @@ class FileSource {
* This is an extension of the FileSource which should be used
* for sources that can provide the list of files available in the source.
*
* This allows for a faster lookup in cases where the source does not
* contain most of the files that the app will request for (e.g. an addon).
**/
class IndexedFileSource extends FileSource {
/**
- * @param {string} name
- * @param {Array<string>} locales
- * @param {string} prePath
- * @param {Array<string>} paths
+ * @param {string} name
+ * @param {Object<string, number>|Array<string>} locales
+ * @param {string} prePath
+ * @param {Array<string>} paths
*
* @returns {IndexedFileSource}
*/
constructor(name, locales, prePath, paths) {
super(name, locales, prePath);
this.indexed = true;
for (const path of paths) {
this.cache[path] = true;
--- a/intl/l10n/test/test_l10nregistry.js
+++ b/intl/l10n/test/test_l10nregistry.js
@@ -206,16 +206,58 @@ add_task(async function test_override()
equal((await ctxs.next()).done, true);
// cleanup
L10nRegistry.sources.clear();
L10nRegistry.ctxCache.clear();
});
/**
+ * This test checks if the correct order of contexts is used in case
+ * two sources provide the same locale but different versions.
+ */
+add_task(async function test_versions() {
+ let newerSource = new IndexedFileSource('langpack-pl', {'pl': 2}, '/langpack2/data/locales/{locale}/', [
+ '/langpack2/data/locales/pl/test.ftl'
+ ]);
+ L10nRegistry.registerSource(newerSource);
+
+ let olderSource = new IndexedFileSource('langpack-pl-old', {'pl': 1}, '/langpack1/data/locales/{locale}/', [
+ '/langpack1/data/locales/pl/test.ftl'
+ ]);
+ L10nRegistry.registerSource(olderSource);
+
+ fs = {
+ '/langpack1/data/locales/pl/test.ftl': 'key = old value',
+ '/langpack2/data/locales/pl/test.ftl': 'key = new value'
+ };
+
+ equal(L10nRegistry.sources.size, 2);
+
+ let ctxs = L10nRegistry.generateContexts(['pl'], ['test.ftl']);
+ let ctx0 = (await ctxs.next()).value;
+ equal(ctx0.locales[0], 'pl');
+ equal(ctx0.hasMessage('key'), true);
+ let msg0 = ctx0.getMessage('key');
+ equal(ctx0.format(msg0), 'new value');
+
+ let ctx1 = (await ctxs.next()).value;
+ equal(ctx1.locales[0], 'pl');
+ equal(ctx1.hasMessage('key'), true);
+ let msg1 = ctx1.getMessage('key');
+ equal(ctx1.format(msg1), 'old value');
+
+ equal((await ctxs.next()).done, true);
+
+ // cleanup
+ L10nRegistry.sources.clear();
+ L10nRegistry.ctxCache.clear();
+});
+
+/**
* This test verifies that new contexts are returned
* after source update.
*/
add_task(async function test_updating() {
let oneSource = new IndexedFileSource('langpack-pl', ['pl'], '/data/locales/{locale}/', [
'/data/locales/pl/test.ftl',
]);
L10nRegistry.registerSource(oneSource);
--- a/toolkit/components/extensions/Extension.jsm
+++ b/toolkit/components/extensions/Extension.jsm
@@ -1722,17 +1722,20 @@ this.Langpack = class extends ExtensionD
this.chromeRegistryHandle =
aomStartup.registerChrome(manifestURI, this.startupData.chromeEntries);
}
const data = await this.parseManifest();
this.langpackId = data.langpackId;
this.l10nRegistrySources = data.l10nRegistrySources;
- const languages = Object.keys(data.manifest.languages);
+ const languages = {};
+ for (const loc in data.manifest.languages) {
+ languages[loc] = parseInt(data.manifest.languages[loc].version);
+ }
resourceProtocol.setSubstitution(this.langpackId, this.rootURI);
for (const [sourceName, basePath] of Object.entries(this.l10nRegistrySources)) {
L10nRegistry.registerSource(new FileSource(
`${sourceName}-${this.langpackId}`,
languages,
`resource://${this.langpackId}/${basePath}localization/{locale}/`
));