From 2b158e07968dfecb281ab2d31405bae68e9592d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Ciccola?= Date: Wed, 12 Jun 2024 11:26:03 -0300 Subject: [PATCH 01/18] add e2e tests for config-import. Additionally: * added `translation` getter in `mapeoProject` * `deleteAll(translation)` in import config --- src/mapeo-project.js | 5 +++ test-e2e/config-import.js | 80 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+) create mode 100644 test-e2e/config-import.js diff --git a/src/mapeo-project.js b/src/mapeo-project.js index 99f9697ba..53b89892f 100644 --- a/src/mapeo-project.js +++ b/src/mapeo-project.js @@ -492,6 +492,10 @@ export class MapeoProject extends TypedEmitter { return this.#dataTypes.field } + get translation() { + return this.#dataTypes.translation + } + get $member() { return this.#memberApi } @@ -720,6 +724,7 @@ export class MapeoProject extends TypedEmitter { // check for already present fields and presets and delete them if exist await deleteAll(this.preset) await deleteAll(this.field) + await deleteAll(this.translation) const config = await readConfig(configPath) /** @type {Map} */ diff --git a/test-e2e/config-import.js b/test-e2e/config-import.js new file mode 100644 index 000000000..cbab6cba1 --- /dev/null +++ b/test-e2e/config-import.js @@ -0,0 +1,80 @@ +import test from 'node:test' +import assert from 'node:assert/strict' +import { createManager } from './utils.js' +import { defaultConfigPath } from '../tests/helpers/default-config.js' + +test('config import - load default config when passed a path to `createProject`', async (t) => { + const manager = createManager('device0', t) + const project = await manager.getProject( + await manager.createProject({ configPath: defaultConfigPath }) + ) + const presets = await project.preset.getMany() + const fields = await project.field.getMany() + const translations = await project.translation.getMany() + assert.equal(presets.length, 28, 'correct number of loaded presets') + assert.equal(fields.length, 11, 'correct number of loaded fields') + assert.equal( + translations.length, + 870, + 'correct number of loaded translations' + ) +}) + +test('config import - load and re-load config manually', async (t) => { + const manager = createManager('device0', t) + const project = await manager.getProject(await manager.createProject()) + + const warnings = await project.importConfig({ configPath: defaultConfigPath }) + let presets = await project.preset.getMany() + let fields = await project.field.getMany() + let translations = await project.translation.getMany() + + assert.equal( + warnings.length, + 0, + 'no warnings when manually loading default config' + ) + assert.equal(presets.length, 28, 'correct number of manually loaded presets') + assert.equal(fields.length, 11, 'correct number of manually loaded fields') + assert.equal( + translations.length, + 870, + 'correct number of manually loaded translations' + ) + + // re load the config + await project.importConfig({ configPath: defaultConfigPath }) + presets = await project.preset.getMany() + fields = await project.field.getMany() + translations = await project.translation.getMany() + assert.equal( + presets.length, + 28, + 're-loading the same config leads to the same number of presets (since they are deleted)' + ) + assert.equal( + fields.length, + 11, + 're-loading the same config leads to the same number of fields (since they are deleted)' + ) + assert.equal( + translations.length, + 870, + 'correct number of manually loaded translations' + ) +}) + +test('load config', async (t) => { + const manager = createManager('device0', t) + const project = await manager.getProject(await manager.createProject()) + await Promise.all([ + project.importConfig({ configPath: defaultConfigPath }), + project.importConfig({ configPath: defaultConfigPath }), + ]) + const presets = await project.preset.getMany() + const fields = await project.field.getMany() + const translations = await project.translation.getMany() + console.log('presets', presets.length) + console.log('fields', fields.length) + console.log('translations', translations.length) +}) From 318f7169742f120e0de00c53c9369b50d8e6856b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Ciccola?= Date: Wed, 12 Jun 2024 11:33:23 -0300 Subject: [PATCH 02/18] rename test --- test-e2e/config-import.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test-e2e/config-import.js b/test-e2e/config-import.js index cbab6cba1..27a70e88f 100644 --- a/test-e2e/config-import.js +++ b/test-e2e/config-import.js @@ -60,11 +60,11 @@ test('config import - load and re-load config manually', async (t) => { assert.equal( translations.length, 870, - 'correct number of manually loaded translations' + 're-loading the same config leads to the same number of translations (since they are deleted)' ) }) -test('load config', async (t) => { +test('load config in parallel', async (t) => { const manager = createManager('device0', t) const project = await manager.getProject(await manager.createProject()) await Promise.all([ From de40cb9be69f447fb97aac11f37209a5910fd089 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Ciccola?= Date: Wed, 12 Jun 2024 12:02:04 -0300 Subject: [PATCH 03/18] `deleteTranslations` only delete translations that refer to deleted fields or presets --- src/mapeo-project.js | 32 +++++++++++++++++++++++++++++++- test-e2e/config-import.js | 2 +- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/src/mapeo-project.js b/src/mapeo-project.js index 53b89892f..9e30dc4e0 100644 --- a/src/mapeo-project.js +++ b/src/mapeo-project.js @@ -724,7 +724,8 @@ export class MapeoProject extends TypedEmitter { // check for already present fields and presets and delete them if exist await deleteAll(this.preset) await deleteAll(this.field) - await deleteAll(this.translation) + // delete only translations that refer to deleted fields and presets + await deleteTranslations(this.translation, this.preset, this.field) const config = await readConfig(configPath) /** @type {Map} */ @@ -845,6 +846,35 @@ async function deleteAll(dataType) { return Promise.all(deletions) } +// TODO: maybe a better signature than a bunch of any? +/** +@param {DataType} translation +@param {DataType} preset +@param {DataType} field +*/ +async function deleteTranslations(translation, preset, field) { + const deletions = [] + for (const { + docId, + docIdRef, + schemaNameRef, + } of await translation.getMany()) { + try { + if (schemaNameRef === 'presets') { + const presetToDelete = await preset.getByDocId(docIdRef) + if (presetToDelete.deleted) deletions.push(translation.delete(docId)) + } else if (schemaNameRef === 'fields') { + const fieldToDelete = await field.getByDocId(docIdRef) + if (fieldToDelete.deleted) deletions.push(translation.delete(docId)) + } + } catch (e) { + console.log('referred preset or field is not found, deleting translation') + deletions.push(translation.delete(docId)) + } + } + await Promise.all(deletions) +} + /** * Return a map of namespace -> core keypair * diff --git a/test-e2e/config-import.js b/test-e2e/config-import.js index 27a70e88f..3d94c801c 100644 --- a/test-e2e/config-import.js +++ b/test-e2e/config-import.js @@ -64,7 +64,7 @@ test('config import - load and re-load config manually', async (t) => { ) }) -test('load config in parallel', async (t) => { +test('manually load config in parallel', async (t) => { const manager = createManager('device0', t) const project = await manager.getProject(await manager.createProject()) await Promise.all([ From 84b7dd0bdd0683da152908b89cd223582307adaf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Ciccola?= Date: Wed, 12 Jun 2024 13:27:44 -0300 Subject: [PATCH 04/18] better signature for delete functions, simplify `deleteTranslations` --- src/mapeo-project.js | 60 ++++++++++++++++++++++++++++++++------------ 1 file changed, 44 insertions(+), 16 deletions(-) diff --git a/src/mapeo-project.js b/src/mapeo-project.js index 9e30dc4e0..3d7dbed12 100644 --- a/src/mapeo-project.js +++ b/src/mapeo-project.js @@ -725,7 +725,11 @@ export class MapeoProject extends TypedEmitter { await deleteAll(this.preset) await deleteAll(this.field) // delete only translations that refer to deleted fields and presets - await deleteTranslations(this.translation, this.preset, this.field) + await deleteTranslations({ + translation: this.translation, + presets: this.preset, + fields: this.field, + }) const config = await readConfig(configPath) /** @type {Map} */ @@ -836,8 +840,34 @@ function extractEditableProjectSettings(projectDoc) { return result } -// TODO: maybe a better signature than a bunch of any? -/** @param {DataType} dataType */ +/** @typedef + * {DataType< + * import('./datastore/index.js').DataStore<'config'>, + * typeof import('./schema/project.js').translationTable, + * 'translation', + * import('@mapeo/schema').Translation, + * import('@mapeo/schema').TranslationValue>} TranslationDataType + */ + +/** @typedef + * {DataType< + * import('./datastore/index.js').DataStore<'config'>, + * typeof import('./schema/project.js').presetTable, + * 'preset', + * import('@mapeo/schema').Preset, + * import('@mapeo/schema').PresetValue>} PresetDataType + */ + +/** @typedef + * {DataType< + * import('./datastore/index.js').DataStore<'config'>, + * typeof import('./schema/project.js').fieldTable, + * 'field', + * import('@mapeo/schema').Field, + * import('@mapeo/schema').FieldValue>} FieldDataType + */ + +/** @param {FieldDataType | PresetDataType} dataType */ async function deleteAll(dataType) { const deletions = [] for (const { docId } of await dataType.getMany()) { @@ -846,30 +876,28 @@ async function deleteAll(dataType) { return Promise.all(deletions) } -// TODO: maybe a better signature than a bunch of any? /** -@param {DataType} translation -@param {DataType} preset -@param {DataType} field +@param {Object} dataTypes +@param {TranslationDataType} dataTypes.translation +@param {PresetDataType} dataTypes.presets +@param {FieldDataType} dataTypes.fields */ -async function deleteTranslations(translation, preset, field) { +async function deleteTranslations(dataTypes) { const deletions = [] for (const { docId, docIdRef, schemaNameRef, - } of await translation.getMany()) { + } of await dataTypes.translation.getMany()) { try { - if (schemaNameRef === 'presets') { - const presetToDelete = await preset.getByDocId(docIdRef) - if (presetToDelete.deleted) deletions.push(translation.delete(docId)) - } else if (schemaNameRef === 'fields') { - const fieldToDelete = await field.getByDocId(docIdRef) - if (fieldToDelete.deleted) deletions.push(translation.delete(docId)) + if (schemaNameRef === 'presets' || schemaNameRef === 'fields') { + const toDelete = await dataTypes[schemaNameRef].getByDocId(docIdRef) + if (toDelete.deleted) + deletions.push(dataTypes.translation.delete(docId)) } } catch (e) { console.log('referred preset or field is not found, deleting translation') - deletions.push(translation.delete(docId)) + deletions.push(dataTypes.translation.delete(docId)) } } await Promise.all(deletions) From fcda6ae0cbc97fade64b841e0b24a4d51df2e435 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Ciccola?= Date: Wed, 12 Jun 2024 13:40:48 -0300 Subject: [PATCH 05/18] simplify types --- src/mapeo-project.js | 35 ++++------------------------------- 1 file changed, 4 insertions(+), 31 deletions(-) diff --git a/src/mapeo-project.js b/src/mapeo-project.js index 3d7dbed12..a9130f05e 100644 --- a/src/mapeo-project.js +++ b/src/mapeo-project.js @@ -840,34 +840,7 @@ function extractEditableProjectSettings(projectDoc) { return result } -/** @typedef - * {DataType< - * import('./datastore/index.js').DataStore<'config'>, - * typeof import('./schema/project.js').translationTable, - * 'translation', - * import('@mapeo/schema').Translation, - * import('@mapeo/schema').TranslationValue>} TranslationDataType - */ - -/** @typedef - * {DataType< - * import('./datastore/index.js').DataStore<'config'>, - * typeof import('./schema/project.js').presetTable, - * 'preset', - * import('@mapeo/schema').Preset, - * import('@mapeo/schema').PresetValue>} PresetDataType - */ - -/** @typedef - * {DataType< - * import('./datastore/index.js').DataStore<'config'>, - * typeof import('./schema/project.js').fieldTable, - * 'field', - * import('@mapeo/schema').Field, - * import('@mapeo/schema').FieldValue>} FieldDataType - */ - -/** @param {FieldDataType | PresetDataType} dataType */ +/** @param {MapeoProject['field'] | MapeoProject['preset']} dataType */ async function deleteAll(dataType) { const deletions = [] for (const { docId } of await dataType.getMany()) { @@ -878,9 +851,9 @@ async function deleteAll(dataType) { /** @param {Object} dataTypes -@param {TranslationDataType} dataTypes.translation -@param {PresetDataType} dataTypes.presets -@param {FieldDataType} dataTypes.fields +@param {MapeoProject['translation']} dataTypes.translation +@param {MapeoProject['preset']} dataTypes.presets +@param {MapeoProject['field']} dataTypes.fields */ async function deleteTranslations(dataTypes) { const deletions = [] From 868271c7dead2577ea6d3c10912faa117187b47c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Ciccola?= Date: Wed, 12 Jun 2024 13:45:16 -0300 Subject: [PATCH 06/18] better error text --- src/mapeo-project.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/mapeo-project.js b/src/mapeo-project.js index a9130f05e..c4969e356 100644 --- a/src/mapeo-project.js +++ b/src/mapeo-project.js @@ -869,7 +869,9 @@ async function deleteTranslations(dataTypes) { deletions.push(dataTypes.translation.delete(docId)) } } catch (e) { - console.log('referred preset or field is not found, deleting translation') + console.log( + `referred ${schemaNameRef} is not found, deleting translation` + ) deletions.push(dataTypes.translation.delete(docId)) } } From 002ca99c5b44a0a71a00a2f563351a110f1c85d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Ciccola?= Date: Thu, 13 Jun 2024 13:37:15 -0300 Subject: [PATCH 07/18] addressing review * make `deleteTranslations` a closure, pass the logger * `deleteTranslations` avoid for loop, use Promise.all(map) * make failing test case instead of logging --- src/mapeo-project.js | 53 ++++++++++++++++++++------------------- test-e2e/config-import.js | 19 +++++++++++--- 2 files changed, 43 insertions(+), 29 deletions(-) diff --git a/src/mapeo-project.js b/src/mapeo-project.js index c4969e356..7a80ccfd5 100644 --- a/src/mapeo-project.js +++ b/src/mapeo-project.js @@ -725,7 +725,7 @@ export class MapeoProject extends TypedEmitter { await deleteAll(this.preset) await deleteAll(this.field) // delete only translations that refer to deleted fields and presets - await deleteTranslations({ + await deleteTranslations(this.#l)({ translation: this.translation, presets: this.preset, fields: this.field, @@ -850,32 +850,33 @@ async function deleteAll(dataType) { } /** -@param {Object} dataTypes -@param {MapeoProject['translation']} dataTypes.translation -@param {MapeoProject['preset']} dataTypes.presets -@param {MapeoProject['field']} dataTypes.fields -*/ -async function deleteTranslations(dataTypes) { - const deletions = [] - for (const { - docId, - docIdRef, - schemaNameRef, - } of await dataTypes.translation.getMany()) { - try { - if (schemaNameRef === 'presets' || schemaNameRef === 'fields') { - const toDelete = await dataTypes[schemaNameRef].getByDocId(docIdRef) - if (toDelete.deleted) - deletions.push(dataTypes.translation.delete(docId)) - } - } catch (e) { - console.log( - `referred ${schemaNameRef} is not found, deleting translation` - ) - deletions.push(dataTypes.translation.delete(docId)) - } + * @param {Logger} logger + */ +function deleteTranslations(logger) { + /** + * @param {Object} dataTypes + * @param {MapeoProject['translation']} dataTypes.translation + * @param {MapeoProject['preset']} dataTypes.presets + * @param {MapeoProject['field']} dataTypes.fields + */ + return async function (dataTypes) { + const translations = await dataTypes.translation.getMany() + await Promise.all( + translations.map(async ({ docId, docIdRef, schemaNameRef }) => { + try { + if (schemaNameRef === 'presets' || schemaNameRef === 'fields') { + const toDelete = await dataTypes[schemaNameRef].getByDocId(docIdRef) + if (toDelete.deleted) dataTypes.translation.delete(docId) + } + } catch (e) { + logger.log( + `referred ${schemaNameRef} is not found, deleting translation` + ) + dataTypes.translation.delete(docId) + } + }) + ) } - await Promise.all(deletions) } /** diff --git a/test-e2e/config-import.js b/test-e2e/config-import.js index 3d94c801c..7aac2f670 100644 --- a/test-e2e/config-import.js +++ b/test-e2e/config-import.js @@ -74,7 +74,20 @@ test('manually load config in parallel', async (t) => { const presets = await project.preset.getMany() const fields = await project.field.getMany() const translations = await project.translation.getMany() - console.log('presets', presets.length) - console.log('fields', fields.length) - console.log('translations', translations.length) + + assert.equal( + presets.length, + 28, + 're-loading the same config leads to the same number of presets (since they are deleted)' + ) + assert.equal( + fields.length, + 11, + 're-loading the same config leads to the same number of fields (since they are deleted)' + ) + assert.equal( + translations.length, + 870, + 're-loading the same config leads to the same number of translations (since they are deleted)' + ) }) From 4d40fc674b70975399e5e37318d36932e1f50d85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Ciccola?= Date: Tue, 18 Jun 2024 11:04:52 -0300 Subject: [PATCH 08/18] revert `deleteTranslations` being a closure --- src/mapeo-project.js | 49 +++++++++++++++++++++----------------------- 1 file changed, 23 insertions(+), 26 deletions(-) diff --git a/src/mapeo-project.js b/src/mapeo-project.js index 7a80ccfd5..c5fb6bc0f 100644 --- a/src/mapeo-project.js +++ b/src/mapeo-project.js @@ -725,7 +725,8 @@ export class MapeoProject extends TypedEmitter { await deleteAll(this.preset) await deleteAll(this.field) // delete only translations that refer to deleted fields and presets - await deleteTranslations(this.#l)({ + await deleteTranslations({ + logger: this.#l, translation: this.translation, presets: this.preset, fields: this.field, @@ -850,33 +851,29 @@ async function deleteAll(dataType) { } /** - * @param {Logger} logger + * @param {Object} opts + * @param {Logger} opts.logger + * @param {MapeoProject['translation']} opts.translation + * @param {MapeoProject['preset']} opts.presets + * @param {MapeoProject['field']} opts.fields */ -function deleteTranslations(logger) { - /** - * @param {Object} dataTypes - * @param {MapeoProject['translation']} dataTypes.translation - * @param {MapeoProject['preset']} dataTypes.presets - * @param {MapeoProject['field']} dataTypes.fields - */ - return async function (dataTypes) { - const translations = await dataTypes.translation.getMany() - await Promise.all( - translations.map(async ({ docId, docIdRef, schemaNameRef }) => { - try { - if (schemaNameRef === 'presets' || schemaNameRef === 'fields') { - const toDelete = await dataTypes[schemaNameRef].getByDocId(docIdRef) - if (toDelete.deleted) dataTypes.translation.delete(docId) - } - } catch (e) { - logger.log( - `referred ${schemaNameRef} is not found, deleting translation` - ) - dataTypes.translation.delete(docId) +async function deleteTranslations(opts) { + const translations = await opts.translation.getMany() + await Promise.all( + translations.map(async ({ docId, docIdRef, schemaNameRef }) => { + try { + if (schemaNameRef === 'presets' || schemaNameRef === 'fields') { + const toDelete = await opts[schemaNameRef].getByDocId(docIdRef) + if (toDelete.deleted) opts.translation.delete(docId) } - }) - ) - } + } catch (e) { + opts.logger.log( + `referred ${schemaNameRef} is not found, deleting translation` + ) + opts.translation.delete(docId) + } + }) + ) } /** From c6d8e39ddd19d6c86a65d3be37f97431b8914755 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Ciccola?= Date: Tue, 18 Jun 2024 11:25:19 -0300 Subject: [PATCH 09/18] add boolean to avoid loading a config in parallel --- src/mapeo-project.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/mapeo-project.js b/src/mapeo-project.js index c5fb6bc0f..35762877a 100644 --- a/src/mapeo-project.js +++ b/src/mapeo-project.js @@ -87,6 +87,8 @@ export class MapeoProject extends TypedEmitter { /** @type {TranslationApi} */ #translationApi #l + /** @type {Boolean} this avoids loading multiple configs in parallel */ + #loadingConfig static EMPTY_PROJECT_SETTINGS = EMPTY_PROJECT_SETTINGS @@ -125,6 +127,7 @@ export class MapeoProject extends TypedEmitter { this.#l = Logger.create('project', logger) this.#deviceId = getDeviceId(keyManager) this.#projectId = projectKeyToId(projectKey) + this.#loadingConfig = false ///////// 1. Setup database this.#sqlite = new Database(dbPath) @@ -721,6 +724,9 @@ export class MapeoProject extends TypedEmitter { * @returns {Promise} */ async importConfig({ configPath }) { + if (this.#loadingConfig) return /** @type Error[] */ [] + this.#loadingConfig = true + // check for already present fields and presets and delete them if exist await deleteAll(this.preset) await deleteAll(this.field) @@ -826,7 +832,7 @@ export class MapeoProject extends TypedEmitter { relation: [], }, }) - + this.#loadingConfig = false return config.warnings } } From d3356f66e86f4707c7857b59950106b18946414e Mon Sep 17 00:00:00 2001 From: tomasciccola <117094913+tomasciccola@users.noreply.github.com> Date: Wed, 19 Jun 2024 13:07:42 -0300 Subject: [PATCH 10/18] refactor `deleteTranslations` Co-authored-by: Evan Hahn --- src/mapeo-project.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/mapeo-project.js b/src/mapeo-project.js index 35762877a..d9c3c5fe9 100644 --- a/src/mapeo-project.js +++ b/src/mapeo-project.js @@ -867,15 +867,19 @@ async function deleteTranslations(opts) { const translations = await opts.translation.getMany() await Promise.all( translations.map(async ({ docId, docIdRef, schemaNameRef }) => { + if (schemaNameRef !== 'presets' && schemaNameRef !== 'fields') return + + let shouldDelete = true try { - if (schemaNameRef === 'presets' || schemaNameRef === 'fields') { - const toDelete = await opts[schemaNameRef].getByDocId(docIdRef) - if (toDelete.deleted) opts.translation.delete(docId) - } + const toDelete = await opts[schemaNameRef].getByDocId(docIdRef) + shouldDelete = toDelete.deleted } catch (e) { opts.logger.log( `referred ${schemaNameRef} is not found, deleting translation` ) + } + + if (shouldDelete) { opts.translation.delete(docId) } }) From 66521906ff91e37589ba99a2778967f444d6ad63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Ciccola?= Date: Wed, 19 Jun 2024 13:07:53 -0300 Subject: [PATCH 11/18] wrap importConfig in a try/catch --- src/mapeo-project.js | 199 ++++++++++++++++++++++--------------------- 1 file changed, 102 insertions(+), 97 deletions(-) diff --git a/src/mapeo-project.js b/src/mapeo-project.js index 35762877a..c6e94d98f 100644 --- a/src/mapeo-project.js +++ b/src/mapeo-project.js @@ -727,113 +727,118 @@ export class MapeoProject extends TypedEmitter { if (this.#loadingConfig) return /** @type Error[] */ [] this.#loadingConfig = true - // check for already present fields and presets and delete them if exist - await deleteAll(this.preset) - await deleteAll(this.field) - // delete only translations that refer to deleted fields and presets - await deleteTranslations({ - logger: this.#l, - translation: this.translation, - presets: this.preset, - fields: this.field, - }) + try { + // check for already present fields and presets and delete them if exist + await deleteAll(this.preset) + await deleteAll(this.field) + // delete only translations that refer to deleted fields and presets + await deleteTranslations({ + logger: this.#l, + translation: this.translation, + presets: this.preset, + fields: this.field, + }) - const config = await readConfig(configPath) - /** @type {Map} */ - const iconNameToId = new Map() - /** @type {Map} */ - const fieldNameToId = new Map() - /** @type {Map} */ - const presetNameToId = new Map() - - // Do this in serial not parallel to avoid memory issues (avoid keeping all icon buffers in memory) - for await (const icon of config.icons()) { - const iconId = await this.#iconApi.create(icon) - iconNameToId.set(icon.name, iconId) - } + const config = await readConfig(configPath) + /** @type {Map} */ + const iconNameToId = new Map() + /** @type {Map} */ + const fieldNameToId = new Map() + /** @type {Map} */ + const presetNameToId = new Map() + + // Do this in serial not parallel to avoid memory issues (avoid keeping all icon buffers in memory) + for await (const icon of config.icons()) { + const iconId = await this.#iconApi.create(icon) + iconNameToId.set(icon.name, iconId) + } - // Ok to create fields and presets in parallel - const fieldPromises = [] - for (const { name, value } of config.fields()) { - fieldPromises.push( - this.#dataTypes.field.create(value).then(({ docId }) => { - fieldNameToId.set(name, docId) + // Ok to create fields and presets in parallel + const fieldPromises = [] + for (const { name, value } of config.fields()) { + fieldPromises.push( + this.#dataTypes.field.create(value).then(({ docId }) => { + fieldNameToId.set(name, docId) + }) + ) + } + await Promise.all(fieldPromises) + + const presetsWithRefs = [] + for (const { fieldNames, iconName, value, name } of config.presets()) { + const fieldIds = fieldNames.map((fieldName) => { + const id = fieldNameToId.get(fieldName) + if (!id) { + throw new Error( + `field ${fieldName} not found (referenced by preset ${value.name})})` + ) + } + return id }) - ) - } - await Promise.all(fieldPromises) + presetsWithRefs.push({ + preset: { + ...value, + iconId: iconName && iconNameToId.get(iconName), + fieldIds, + }, + name, + }) + } + + const presetPromises = [] + for (const { preset, name } of presetsWithRefs) { + presetPromises.push( + this.preset.create(preset).then(({ docId }) => { + presetNameToId.set(name, docId) + }) + ) + } + + await Promise.all(presetPromises) - const presetsWithRefs = [] - for (const { fieldNames, iconName, value, name } of config.presets()) { - const fieldIds = fieldNames.map((fieldName) => { - const id = fieldNameToId.get(fieldName) - if (!id) { + const translationPromises = [] + for (const { name, value } of config.translations()) { + let docIdRef + if (value.schemaNameRef === 'fields') { + docIdRef = fieldNameToId.get(name) + } else if (value.schemaNameRef === 'presets') { + docIdRef = presetNameToId.get(name) + } else { + throw new Error(`invalid schemaNameRef ${value.schemaNameRef}`) + } + if (docIdRef) { + translationPromises.push( + this.$translation.put({ + ...value, + docIdRef, + }) + ) + } else { throw new Error( - `field ${fieldName} not found (referenced by preset ${value.name})})` + `docIdRef for preset or field with name ${name} not found` ) } - return id - }) - presetsWithRefs.push({ - preset: { - ...value, - iconId: iconName && iconNameToId.get(iconName), - fieldIds, + } + await Promise.all(translationPromises) + + // close the zip handles after we know we won't be needing them anymore + await config.close() + + await this.$setProjectSettings({ + defaultPresets: { + point: [...presetNameToId.values()], + line: [], + area: [], + vertex: [], + relation: [], }, - name, }) + return config.warnings + } catch (e) { + this.#l.log('error loading config', e) + this.#loadingConfig = false + return /** @type Error[] */ [] } - - const presetPromises = [] - for (const { preset, name } of presetsWithRefs) { - presetPromises.push( - this.preset.create(preset).then(({ docId }) => { - presetNameToId.set(name, docId) - }) - ) - } - - await Promise.all(presetPromises) - - const translationPromises = [] - for (const { name, value } of config.translations()) { - let docIdRef - if (value.schemaNameRef === 'fields') { - docIdRef = fieldNameToId.get(name) - } else if (value.schemaNameRef === 'presets') { - docIdRef = presetNameToId.get(name) - } else { - throw new Error(`invalid schemaNameRef ${value.schemaNameRef}`) - } - if (docIdRef) { - translationPromises.push( - this.$translation.put({ - ...value, - docIdRef, - }) - ) - } else { - throw new Error( - `docIdRef for preset or field with name ${name} not found` - ) - } - } - await Promise.all(translationPromises) - - // close the zip handles after we know we won't be needing them anymore - await config.close() - - await this.$setProjectSettings({ - defaultPresets: { - point: [...presetNameToId.values()], - line: [], - area: [], - vertex: [], - relation: [], - }, - }) - this.#loadingConfig = false - return config.warnings } } From 9bc8e725b9e51e97c446846927517bbbbecd2b8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Ciccola?= Date: Wed, 19 Jun 2024 13:14:36 -0300 Subject: [PATCH 12/18] re-invert logic in `deleteTranslations` to please typescript --- src/mapeo-project.js | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/src/mapeo-project.js b/src/mapeo-project.js index 7665b9bfc..a462ccd8a 100644 --- a/src/mapeo-project.js +++ b/src/mapeo-project.js @@ -872,20 +872,17 @@ async function deleteTranslations(opts) { const translations = await opts.translation.getMany() await Promise.all( translations.map(async ({ docId, docIdRef, schemaNameRef }) => { - if (schemaNameRef !== 'presets' && schemaNameRef !== 'fields') return - - let shouldDelete = true - try { - const toDelete = await opts[schemaNameRef].getByDocId(docIdRef) - shouldDelete = toDelete.deleted - } catch (e) { - opts.logger.log( - `referred ${schemaNameRef} is not found, deleting translation` - ) - } - - if (shouldDelete) { - opts.translation.delete(docId) + if (schemaNameRef === 'presets' || schemaNameRef === 'fields') { + let shouldDelete = false + try { + const toDelete = await opts[schemaNameRef].getByDocId(docIdRef) + shouldDelete = toDelete.deleted + } catch (e) { + opts.logger.log(`referred ${docIdRef} is not found`) + } + if (shouldDelete) { + await opts.translation.delete(docId) + } } }) ) From 8a1e2085843b64aeb3e2955d3fe72497b255f714 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Ciccola?= Date: Wed, 19 Jun 2024 14:20:34 -0300 Subject: [PATCH 13/18] fix logic error, change assertion on parallel test --- src/mapeo-project.js | 7 +++++-- test-e2e/config-import.js | 31 ++++++++----------------------- 2 files changed, 13 insertions(+), 25 deletions(-) diff --git a/src/mapeo-project.js b/src/mapeo-project.js index a462ccd8a..e8ad482eb 100644 --- a/src/mapeo-project.js +++ b/src/mapeo-project.js @@ -494,7 +494,6 @@ export class MapeoProject extends TypedEmitter { get field() { return this.#dataTypes.field } - get translation() { return this.#dataTypes.translation } @@ -724,7 +723,10 @@ export class MapeoProject extends TypedEmitter { * @returns {Promise} */ async importConfig({ configPath }) { - if (this.#loadingConfig) return /** @type Error[] */ [] + assert( + !this.#loadingConfig, + 'Cannot run multiple config imports at the same time' + ) this.#loadingConfig = true try { @@ -833,6 +835,7 @@ export class MapeoProject extends TypedEmitter { relation: [], }, }) + this.#loadingConfig = false return config.warnings } catch (e) { this.#l.log('error loading config', e) diff --git a/test-e2e/config-import.js b/test-e2e/config-import.js index 7aac2f670..e12ed6f53 100644 --- a/test-e2e/config-import.js +++ b/test-e2e/config-import.js @@ -64,30 +64,15 @@ test('config import - load and re-load config manually', async (t) => { ) }) -test('manually load config in parallel', async (t) => { +test('failing on loading multiple configs in parallel', async (t) => { const manager = createManager('device0', t) const project = await manager.getProject(await manager.createProject()) - await Promise.all([ - project.importConfig({ configPath: defaultConfigPath }), - project.importConfig({ configPath: defaultConfigPath }), - ]) - const presets = await project.preset.getMany() - const fields = await project.field.getMany() - const translations = await project.translation.getMany() - - assert.equal( - presets.length, - 28, - 're-loading the same config leads to the same number of presets (since they are deleted)' - ) - assert.equal( - fields.length, - 11, - 're-loading the same config leads to the same number of fields (since they are deleted)' - ) - assert.equal( - translations.length, - 870, - 're-loading the same config leads to the same number of translations (since they are deleted)' + assert.throws( + async () => + await Promise.all([ + project.importConfig({ configPath: defaultConfigPath }), + project.importConfig({ configPath: defaultConfigPath }), + ]), + 'Cannot run multiple config imports at the same time' ) }) From 3f934399adee29e8d105915947548dcdc5d0e97a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Ciccola?= Date: Wed, 19 Jun 2024 14:22:29 -0300 Subject: [PATCH 14/18] expose translationApi.dataType --- src/mapeo-project.js | 5 +---- src/translation-api.js | 3 +++ test-e2e/config-import.js | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/mapeo-project.js b/src/mapeo-project.js index e8ad482eb..ade14fd50 100644 --- a/src/mapeo-project.js +++ b/src/mapeo-project.js @@ -494,9 +494,6 @@ export class MapeoProject extends TypedEmitter { get field() { return this.#dataTypes.field } - get translation() { - return this.#dataTypes.translation - } get $member() { return this.#memberApi @@ -736,7 +733,7 @@ export class MapeoProject extends TypedEmitter { // delete only translations that refer to deleted fields and presets await deleteTranslations({ logger: this.#l, - translation: this.translation, + translation: this.$translation.dataType, presets: this.preset, fields: this.field, }) diff --git a/src/translation-api.js b/src/translation-api.js index c8b24c0ba..d25f1e752 100644 --- a/src/translation-api.js +++ b/src/translation-api.js @@ -126,4 +126,7 @@ export default class TranslationApi { get [ktranslatedLanguageCodeToSchemaNames]() { return this.#translatedLanguageCodeToSchemaNames } + get dataType() { + return this.#dataType + } } diff --git a/test-e2e/config-import.js b/test-e2e/config-import.js index e12ed6f53..ea4068b03 100644 --- a/test-e2e/config-import.js +++ b/test-e2e/config-import.js @@ -10,7 +10,7 @@ test('config import - load default config when passed a path to `createProject`' ) const presets = await project.preset.getMany() const fields = await project.field.getMany() - const translations = await project.translation.getMany() + const translations = await project.$translation.dataType.getMany() assert.equal(presets.length, 28, 'correct number of loaded presets') assert.equal(fields.length, 11, 'correct number of loaded fields') assert.equal( @@ -27,7 +27,7 @@ test('config import - load and re-load config manually', async (t) => { const warnings = await project.importConfig({ configPath: defaultConfigPath }) let presets = await project.preset.getMany() let fields = await project.field.getMany() - let translations = await project.translation.getMany() + let translations = await project.$translation.dataType.getMany() assert.equal( warnings.length, @@ -46,7 +46,7 @@ test('config import - load and re-load config manually', async (t) => { await project.importConfig({ configPath: defaultConfigPath }) presets = await project.preset.getMany() fields = await project.field.getMany() - translations = await project.translation.getMany() + translations = await project.$translation.dataType.getMany() assert.equal( presets.length, 28, From 74cbf6e50639f792830b0e4b93f8d9e4d14270a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Ciccola?= Date: Wed, 19 Jun 2024 14:39:17 -0300 Subject: [PATCH 15/18] fix type error --- src/mapeo-project.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mapeo-project.js b/src/mapeo-project.js index ade14fd50..f93dea201 100644 --- a/src/mapeo-project.js +++ b/src/mapeo-project.js @@ -864,7 +864,7 @@ async function deleteAll(dataType) { /** * @param {Object} opts * @param {Logger} opts.logger - * @param {MapeoProject['translation']} opts.translation + * @param {MapeoProject['$translation']['dataType']} opts.translation * @param {MapeoProject['preset']} opts.presets * @param {MapeoProject['field']} opts.fields */ From a01651cd75a757f60470b47c249738edb76a1b1f Mon Sep 17 00:00:00 2001 From: tomasciccola <117094913+tomasciccola@users.noreply.github.com> Date: Mon, 24 Jun 2024 13:24:08 -0300 Subject: [PATCH 16/18] assert.throws -> assert.rejects Co-authored-by: Evan Hahn --- test-e2e/config-import.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test-e2e/config-import.js b/test-e2e/config-import.js index ea4068b03..fff02d197 100644 --- a/test-e2e/config-import.js +++ b/test-e2e/config-import.js @@ -67,7 +67,7 @@ test('config import - load and re-load config manually', async (t) => { test('failing on loading multiple configs in parallel', async (t) => { const manager = createManager('device0', t) const project = await manager.getProject(await manager.createProject()) - assert.throws( + await assert.rejects( async () => await Promise.all([ project.importConfig({ configPath: defaultConfigPath }), From bcbf119057f036965a15dfa1bb2f701e0bdd06a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Ciccola?= Date: Mon, 24 Jun 2024 13:29:42 -0300 Subject: [PATCH 17/18] change test message --- test-e2e/config-import.js | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/test-e2e/config-import.js b/test-e2e/config-import.js index fff02d197..b98a75d56 100644 --- a/test-e2e/config-import.js +++ b/test-e2e/config-import.js @@ -67,12 +67,10 @@ test('config import - load and re-load config manually', async (t) => { test('failing on loading multiple configs in parallel', async (t) => { const manager = createManager('device0', t) const project = await manager.getProject(await manager.createProject()) - await assert.rejects( - async () => - await Promise.all([ - project.importConfig({ configPath: defaultConfigPath }), - project.importConfig({ configPath: defaultConfigPath }), - ]), - 'Cannot run multiple config imports at the same time' - ) + await assert.rejects(async () => { + await Promise.all([ + project.importConfig({ configPath: defaultConfigPath }), + project.importConfig({ configPath: defaultConfigPath }), + ]) + }, 'loading configs in parallell should throw') }) From 6f58b33cec3201a31db7e3bdbb94ec7b263dcb65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Ciccola?= Date: Tue, 25 Jun 2024 10:04:29 -0300 Subject: [PATCH 18/18] fix last test --- test-e2e/config-import.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test-e2e/config-import.js b/test-e2e/config-import.js index b98a75d56..984d0a7b6 100644 --- a/test-e2e/config-import.js +++ b/test-e2e/config-import.js @@ -3,7 +3,7 @@ import assert from 'node:assert/strict' import { createManager } from './utils.js' import { defaultConfigPath } from '../tests/helpers/default-config.js' -test('config import - load default config when passed a path to `createProject`', async (t) => { +test(' config import - load default config when passed a path to `createProject`', async (t) => { const manager = createManager('device0', t) const project = await manager.getProject( await manager.createProject({ configPath: defaultConfigPath }) @@ -67,10 +67,10 @@ test('config import - load and re-load config manually', async (t) => { test('failing on loading multiple configs in parallel', async (t) => { const manager = createManager('device0', t) const project = await manager.getProject(await manager.createProject()) - await assert.rejects(async () => { - await Promise.all([ - project.importConfig({ configPath: defaultConfigPath }), - project.importConfig({ configPath: defaultConfigPath }), - ]) - }, 'loading configs in parallell should throw') + const results = await Promise.allSettled([ + project.importConfig({ configPath: defaultConfigPath }), + project.importConfig({ configPath: defaultConfigPath }), + ]) + assert.equal(results[0]?.status, 'fulfilled', 'first import should work') + assert.equal(results[1]?.status, 'rejected', 'second import should fail') })