Bug 1455649 - DocumentL10n, part 3 - Update L10nRegistry to operate on FTLResources.
Switch to cache FTLResources per FileSource, and change the logic of
context creation to return a context if at least one source for the
permutation works.
MozReview-Commit-ID: B9fxbkaU3oX
--- a/intl/l10n/L10nRegistry.jsm
+++ b/intl/l10n/L10nRegistry.jsm
@@ -74,34 +74,50 @@ XPCOMUtils.defineLazyGlobalGetters(this,
*
* If during the life-cycle of the app a new source is added, the generator can be called again
* and will produce a new set of permutations placing the language pack provided resources
* at the top.
*/
const L10nRegistry = {
sources: new Map(),
- ctxCache: new Map(),
bootstrap: null,
/**
* Based on the list of requested languages and resource Ids,
* this function returns an lazy iterator over message context permutations.
*
* @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) {
- yield * generateContextsForLocale(locale, sourcesOrder, resourceIds);
+ for await (let set of generateResourceSetsForLocale(locale, sourcesOrder, resourceIds)) {
+ let dataSet = await Promise.all(set);
+
+ if (dataSet.every(d => d === false)) {
+ continue;
+ }
+ const pseudoNameFromPref = Services.prefs.getStringPref("intl.l10n.pseudo", "");
+ const ctx = new MessageContext(locale, {
+ ...MSG_CONTEXT_OPTIONS,
+ transform: PSEUDO_STRATEGIES[pseudoNameFromPref],
+ });
+ for (const data of dataSet) {
+ if (data !== false) {
+ ctx.addDocument(data);
+ }
+ }
+ yield ctx;
+ }
}
},
/**
* Adds a new resource source to the L10nRegistry.
*
* @param {FileSource} source
*/
@@ -121,17 +137,16 @@ const L10nRegistry = {
*
* @param {FileSource} source
*/
updateSource(source) {
if (!this.sources.has(source.name)) {
throw new Error(`Source with name "${source.name}" is not registered.`);
}
this.sources.set(source.name, source);
- this.ctxCache.clear();
Services.locale.setAvailableLocales(this.getAvailableLocales());
},
/**
* Removes a source from the L10nRegistry.
*
* @param {String} sourceId
*/
@@ -154,73 +169,47 @@ const L10nRegistry = {
locales.add(locale);
}
}
return Array.from(locales);
},
};
/**
- * A helper function for generating unique context ID used for caching
- * MessageContexts.
- *
- * @param {String} locale
- * @param {Array} sourcesOrder
- * @param {Array} resourceIds
- * @returns {String}
- */
-function generateContextID(locale, sourcesOrder, resourceIds) {
- const sources = sourcesOrder.join(",");
- const ids = resourceIds.join(",");
- return `${locale}|${sources}|${ids}`;
-}
-
-/**
* This function generates an iterator over MessageContexts for a single locale
* for a given list of resourceIds for all possible combinations of sources.
*
* This function is called recursively to generate all possible permutations
* and uses the last, optional parameter, to pass the already resolved
* sources order.
*
* @param {String} locale
* @param {Array} sourcesOrder
* @param {Array} resourceIds
* @param {Array} [resolvedOrder]
* @returns {AsyncIterator<MessageContext>}
*/
-async function* generateContextsForLocale(locale, sourcesOrder, resourceIds, resolvedOrder = []) {
+function* generateResourceSetsForLocale(locale, sourcesOrder, resourceIds, resolvedOrder = []) {
const resolvedLength = resolvedOrder.length;
const resourcesLength = resourceIds.length;
// Inside that loop we have a list of resources and the sources for them, like this:
// ['test.ftl', 'menu.ftl', 'foo.ftl']
// ['app', 'platform', 'app']
for (const sourceName of sourcesOrder) {
const order = resolvedOrder.concat(sourceName);
- // We bail only if the hasFile returns a strict false here,
- // because for FileSource it may also return undefined, which means
- // that we simply don't know if the source contains the file and we'll
- // have to perform the I/O to learn.
- if (L10nRegistry.sources.get(sourceName).hasFile(locale, resourceIds[resolvedOrder.length]) === false) {
- continue;
- }
-
// If the number of resolved sources equals the number of resources,
// create the right context and return it if it loads.
if (resolvedLength + 1 === resourcesLength) {
- const ctx = await generateContext(locale, order, resourceIds);
- if (ctx !== null) {
- yield ctx;
- }
+ yield generateResourceSet(locale, order, resourceIds);
} else if (resolvedLength < resourcesLength) {
// otherwise recursively load another generator that walks over the
// partially resolved list of sources.
- yield * generateContextsForLocale(locale, sourcesOrder, resourceIds, order);
+ yield * generateResourceSetsForLocale(locale, sourcesOrder, resourceIds, order);
}
}
}
const MSG_CONTEXT_OPTIONS = {
// Temporarily disable bidi isolation due to Microsoft not supporting FSI/PDI.
// See bug 1439018 for details.
useIsolating: Services.prefs.getBoolPref("intl.l10n.enable-bidi-marks", false),
@@ -341,45 +330,20 @@ const PSEUDO_STRATEGIES = {
* This allows the caller to be an async generator without using
* try/catch clauses.
*
* @param {String} locale
* @param {Array} sourcesOrder
* @param {Array} resourceIds
* @returns {Promise<MessageContext>}
*/
-function generateContext(locale, sourcesOrder, resourceIds) {
- const ctxId = generateContextID(locale, sourcesOrder, resourceIds);
- if (L10nRegistry.ctxCache.has(ctxId)) {
- return L10nRegistry.ctxCache.get(ctxId);
- }
-
- const fetchPromises = resourceIds.map((resourceId, i) => {
+function generateResourceSet(locale, sourcesOrder, resourceIds) {
+ return resourceIds.map((resourceId, i) => {
return L10nRegistry.sources.get(sourcesOrder[i]).fetchFile(locale, resourceId);
});
-
- const ctxPromise = Promise.all(fetchPromises).then(
- dataSets => {
- const pseudoNameFromPref = Services.prefs.getStringPref("intl.l10n.pseudo", "");
- const ctx = new MessageContext(locale, {
- ...MSG_CONTEXT_OPTIONS,
- transform: PSEUDO_STRATEGIES[pseudoNameFromPref],
- });
- for (const data of dataSets) {
- if (data === null) {
- return null;
- }
- ctx.addMessages(data);
- }
- return ctx;
- },
- () => null
- );
- L10nRegistry.ctxCache.set(ctxId, ctxPromise);
- return ctxPromise;
}
/**
* This is a basic Source for L10nRegistry.
* 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
@@ -440,38 +404,35 @@ class FileSource {
if (this.cache[fullPath].then) {
return undefined;
}
return true;
}
fetchFile(locale, path) {
if (!this.locales.includes(locale)) {
- return Promise.reject(`The source has no resources for locale "${locale}"`);
+ return false;
}
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}"`);
- }
- if (this.cache[fullPath].then) {
- return this.cache[fullPath];
+ return false;
}
} else if (this.indexed) {
- return Promise.reject(`The source has no resources for path "${fullPath}"`);
- }
+ return false;
+ }
return this.cache[fullPath] = L10nRegistry.load(fullPath).then(
data => {
- return this.cache[fullPath] = data;
+ return this.cache[fullPath] = MessageContext.parseDocument(data);
},
err => {
this.cache[fullPath] = false;
- return Promise.reject(err);
+ return false;
}
);
}
}
/**
* This is an extension of the FileSource which should be used
* for sources that can provide the list of files available in the source.
@@ -505,17 +466,17 @@ class IndexedFileSource extends FileSour
*
* @param {string} url
*
* @returns {Promise<string>}
*/
L10nRegistry.load = function(url) {
return fetch(url).then(response => {
if (!response.ok) {
- return Promise.reject(response.statusText);
+ return false;
}
return response.text();
});
};
this.L10nRegistry = L10nRegistry;
this.FileSource = FileSource;
this.IndexedFileSource = IndexedFileSource;
--- a/intl/l10n/Localization.jsm
+++ b/intl/l10n/Localization.jsm
@@ -266,17 +266,16 @@ class Localization {
observe(subject, topic, data) {
switch (topic) {
case "intl:app-locales-changed":
this.onChange();
break;
case "nsPref:changed":
switch (data) {
case "intl.l10n.pseudo":
- L10nRegistry.ctxCache.clear();
this.onChange();
}
break;
default:
break;
}
}
--- a/intl/l10n/test/test_l10nregistry.js
+++ b/intl/l10n/test/test_l10nregistry.js
@@ -35,34 +35,32 @@ add_task(async function test_empty_resou
const ctxs = L10nRegistry.generateContexts(["en-US"], []);
const done = (await ctxs.next()).done;
equal(done, true);
// cleanup
L10nRegistry.sources.clear();
- L10nRegistry.ctxCache.clear();
});
/**
* Test that passing empty sources list works.
*/
add_task(async function test_empty_sources() {
fs = {};
const ctxs = L10nRegistry.generateContexts(["en-US"], []);
const done = (await ctxs.next()).done;
equal(done, true);
// cleanup
L10nRegistry.sources.clear();
- L10nRegistry.ctxCache.clear();
});
/**
* This test tests generation of a proper context for a single
* source scenario
*/
add_task(async function test_methods_calling() {
fs = {
@@ -75,17 +73,16 @@ add_task(async function test_methods_cal
const ctxs = L10nRegistry.generateContexts(["en-US"], ["/browser/menu.ftl"]);
const ctx = (await ctxs.next()).value;
equal(ctx.hasMessage("key"), true);
// cleanup
L10nRegistry.sources.clear();
- L10nRegistry.ctxCache.clear();
});
/**
* This test verifies that the public methods return expected values
* for the single source scenario
*/
add_task(async function test_has_one_source() {
let oneSource = new FileSource("app", ["en-US"], "./app/data/locales/{locale}/");
@@ -113,17 +110,16 @@ add_task(async function test_has_one_sou
// returns no contexts for missing locale
ctxs = L10nRegistry.generateContexts(["pl"], ["test.ftl"]);
equal((await ctxs.next()).done, true);
// cleanup
L10nRegistry.sources.clear();
- L10nRegistry.ctxCache.clear();
});
/**
* This test verifies that public methods return expected values
* for the dual source scenario.
*/
add_task(async function test_has_two_sources() {
let oneSource = new FileSource("platform", ["en-US"], "./platform/data/locales/{locale}/");
@@ -170,17 +166,16 @@ add_task(async function test_has_two_sou
equal(ctx1.hasMessage("key"), true);
let msg1 = ctx1.getMessage("key");
equal(ctx1.format(msg1), "platform value");
equal((await ctxs.next()).done, true);
// cleanup
L10nRegistry.sources.clear();
- L10nRegistry.ctxCache.clear();
});
/**
* This test verifies that behavior specific to the IndexedFileSource
* works correctly.
*
* In particular it tests that IndexedFileSource correctly returns
* missing files as `false` instead of `undefined`.
@@ -198,17 +193,16 @@ add_task(async function test_indexed() {
equal(L10nRegistry.sources.has("langpack-pl"), true);
equal(oneSource.getPath("pl", "test.ftl"), "/data/locales/pl/test.ftl");
equal(oneSource.hasFile("pl", "test.ftl"), true);
equal(oneSource.hasFile("pl", "missing.ftl"), false);
// cleanup
L10nRegistry.sources.clear();
- L10nRegistry.ctxCache.clear();
});
/**
* This test checks if the correct order of contexts is used for
* scenarios where a new file source is added on top of the default one.
*/
add_task(async function test_override() {
let fileSource = new FileSource("app", ["pl"], "/app/data/locales/{locale}/");
@@ -239,17 +233,16 @@ add_task(async function test_override()
equal(ctx1.hasMessage("key"), true);
let msg1 = ctx1.getMessage("key");
equal(ctx1.format(msg1), "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}/", [
@@ -277,17 +270,16 @@ add_task(async function test_updating()
equal(L10nRegistry.sources.size, 1);
ctxs = L10nRegistry.generateContexts(["pl"], ["test.ftl"]);
ctx0 = (await ctxs.next()).value;
msg0 = ctx0.getMessage("key");
equal(ctx0.format(msg0), "new value");
// cleanup
L10nRegistry.sources.clear();
- L10nRegistry.ctxCache.clear();
});
/**
* This test verifies that generated contexts return correct values
* after sources are being removed.
*/
add_task(async function test_removing() {
let fileSource = new FileSource("app", ["pl"], "/app/data/locales/{locale}/");
@@ -343,17 +335,16 @@ add_task(async function test_removing()
equal(L10nRegistry.sources.size, 0);
ctxs = L10nRegistry.generateContexts(["pl"], ["test.ftl"]);
equal((await ctxs.next()).done, true);
// cleanup
L10nRegistry.sources.clear();
- L10nRegistry.ctxCache.clear();
});
/**
* This test verifies that the logic works correctly when there's a missing
* file in the FileSource scenario.
*/
add_task(async function test_missing_file() {
let oneSource = new FileSource("app", ["en-US"], "./app/data/locales/{locale}/");
@@ -373,25 +364,26 @@ add_task(async function test_missing_fil
equal(L10nRegistry.sources.size, 2);
equal(L10nRegistry.sources.has("app"), true);
equal(L10nRegistry.sources.has("platform"), true);
// returns a single context
let ctxs = L10nRegistry.generateContexts(["en-US"], ["test.ftl", "test2.ftl"]);
- (await ctxs.next());
- (await ctxs.next());
+ (await ctxs.next()); // [platform, platform] - both present
+ (await ctxs.next()); // [platform, app] - second missing
+ (await ctxs.next()); // [app, platform] - both present
+ (await ctxs.next()); // [app, app] - second missing
equal((await ctxs.next()).done, true);
// cleanup
L10nRegistry.sources.clear();
- L10nRegistry.ctxCache.clear();
});
/**
* This test verifies that each file is that all files requested
* by a single context are fetched at the same time, even
* if one I/O is slow.
*/
add_task(async function test_parallel_io() {
@@ -442,11 +434,10 @@ add_task(async function test_parallel_io
equal((await ctxs.next()).done, true);
// When requested again, the cache should make the load operation not
// increase the fetchedIndex count
L10nRegistry.generateContexts(["en-US"], ["test.ftl", "test2.ftl", "slow-file.ftl"]);
// cleanup
L10nRegistry.sources.clear();
- L10nRegistry.ctxCache.clear();
L10nRegistry.load = originalLoad;
});
--- a/intl/l10n/test/test_localization.js
+++ b/intl/l10n/test/test_localization.js
@@ -38,17 +38,16 @@ add_task(async function test_methods_cal
], generateMessages);
let values = await l10n.formatValues([{id: "key"}, {id: "key2"}]);
equal(values[0], "[de] Value2");
equal(values[1], "[en] Value3");
L10nRegistry.sources.clear();
- L10nRegistry.ctxCache.clear();
L10nRegistry.load = originalLoad;
Services.locale.setRequestedLocales(originalRequested);
});
add_task(async function test_builtins() {
const { L10nRegistry, FileSource } =
ChromeUtils.import("resource://gre/modules/L10nRegistry.jsm", {});
@@ -85,17 +84,16 @@ key = { PLATFORM() ->
], generateMessages);
let values = await l10n.formatValues([{id: "key"}]);
ok(values[0].includes(
`${ known_platforms[AppConstants.platform].toUpperCase() } Value`));
L10nRegistry.sources.clear();
- L10nRegistry.ctxCache.clear();
L10nRegistry.load = originalLoad;
});
add_task(async function test_add_remove_resourceIds() {
const { L10nRegistry, FileSource } =
ChromeUtils.import("resource://gre/modules/L10nRegistry.jsm", {});
const fs = {
@@ -133,12 +131,11 @@ add_task(async function test_add_remove_
l10n.removeResourceIds(["/browser/menu.ftl"]);
values = await l10n.formatValues([{id: "key1"}, {id: "key2"}]);
equal(values[0], undefined);
equal(values[1], "Value2");
L10nRegistry.sources.clear();
- L10nRegistry.ctxCache.clear();
L10nRegistry.load = originalLoad;
Services.locale.setRequestedLocales(originalRequested);
});
--- a/intl/l10n/test/test_pseudo.js
+++ b/intl/l10n/test/test_pseudo.js
@@ -89,17 +89,16 @@ add_task(async function test_accented_wo
let message = (await l10n.formatMessages([{id: "key"}]))[0];
ok(message.value.includes("This is a single message"));
ok(message.attributes[0].value.includes("This is a tooltip"));
equal(message.attributes[1].value, "f");
}
L10nRegistry.sources.clear();
- L10nRegistry.ctxCache.clear();
L10nRegistry.load = originalValues.load;
Services.locale.setRequestedLocales(originalValues.requested);
});
/**
* This test verifies that setting a bogus pseudo locale
* strategy doesn't break anything.
*/
@@ -121,12 +120,11 @@ add_task(async function test_unavailable
ok(message.value.includes("This is a single message"));
ok(message.attributes[0].value.includes("This is a tooltip"));
equal(message.attributes[1].value, "f");
}
Services.prefs.setStringPref("intl.l10n.pseudo", "");
L10nRegistry.sources.clear();
- L10nRegistry.ctxCache.clear();
L10nRegistry.load = originalValues.load;
Services.locale.setRequestedLocales(originalValues.requested);
});