From 77d7be43fa77baf7f0911a7fbab69ee1b5c9c12e Mon Sep 17 00:00:00 2001 From: jstet Date: Fri, 25 Sep 2026 16:48:00 +0200 Subject: [PATCH] refactor(xlsform2lstsv): per-call conversion state; synchronous internals XLSFormToTSVConverter kept all per-conversion state on the instance and reset it with seven clear() calls at the start of convert(). Because convert() awaited inside every row, two concurrent calls on one instance corrupted each other. Reproduced: the second call's reset wiped the first call's choice lists ("list 'colors' has no rows on the choices sheet"). - The collaborators and state now belong to an internal Conversion object that convert() creates for each call. XLSFormToTSVConverter holds only its resolved options (resolveConfig), so an instance can be reused, including concurrently. The collaborators' clear() methods are gone. - The internals are synchronous: the XPath transpiler gains sync cores (xpathToLimeSurveySync, convertRelevanceSync, convertConstraintSync). The public xpathToLimeSurvey/convertRelevance/convertConstraint stay Promise-returning, and so does convert(), which rejects on error. Public API unchanged. Every TSV snapshot is identical. tests/ts/integration/converterConcurrency.test.ts runs four surveys concurrently on one instance and compares them with separate instances; it fails on main and passes here. Closes #62 Co-Authored-By: Claude Opus 5.5 (1M context) --- src/pipelines/xlsform2lstsv/choiceManager.ts | 7 - src/pipelines/xlsform2lstsv/counters.ts | 7 - src/pipelines/xlsform2lstsv/groupEmitter.ts | 40 +++-- src/pipelines/xlsform2lstsv/groupProcessor.ts | 5 - src/pipelines/xlsform2lstsv/index.ts | 143 +++++++++++------- src/pipelines/xlsform2lstsv/matrixHandler.ts | 37 ++--- src/pipelines/xlsform2lstsv/rowEmitter.ts | 4 - .../xlsform2lstsv/surveySettingsEmitter.ts | 5 - .../xlsform2lstsv/transpilerHelper.ts | 18 +-- .../xlsform2lstsv/xpathTranspiler.ts | 43 +++++- .../integration/converterConcurrency.test.ts | 56 +++++++ 11 files changed, 217 insertions(+), 148 deletions(-) create mode 100644 tests/ts/integration/converterConcurrency.test.ts diff --git a/src/pipelines/xlsform2lstsv/choiceManager.ts b/src/pipelines/xlsform2lstsv/choiceManager.ts index fb32808..8f1022a 100644 --- a/src/pipelines/xlsform2lstsv/choiceManager.ts +++ b/src/pipelines/xlsform2lstsv/choiceManager.ts @@ -15,13 +15,6 @@ export class ChoiceManager { constructor(private fieldSanitizer: FieldSanitizer) {} - clear(): void { - this.choicesMap.clear(); - this.answerCodeMap.clear(); - this.questionToListMap.clear(); - this.questionBaseTypeMap.clear(); - } - getChoicesMap(): Map { return this.choicesMap; } diff --git a/src/pipelines/xlsform2lstsv/counters.ts b/src/pipelines/xlsform2lstsv/counters.ts index 3f23c65..e7bd1c3 100644 --- a/src/pipelines/xlsform2lstsv/counters.ts +++ b/src/pipelines/xlsform2lstsv/counters.ts @@ -10,13 +10,6 @@ export class Counters { answerSeq = 0; subquestionSeq = 0; - clear(): void { - this.groupSeq = 0; - this.questionSeq = 0; - this.answerSeq = 0; - this.subquestionSeq = 0; - } - setAnswerSeq(value: number): void { this.answerSeq = value; } diff --git a/src/pipelines/xlsform2lstsv/groupEmitter.ts b/src/pipelines/xlsform2lstsv/groupEmitter.ts index f062017..1935f69 100644 --- a/src/pipelines/xlsform2lstsv/groupEmitter.ts +++ b/src/pipelines/xlsform2lstsv/groupEmitter.ts @@ -17,7 +17,7 @@ export type GroupCounters = Counters; /** Helpers passed to handleBeginGroup for name resolution + relevance. */ export interface GroupHelpers { sanitizeName: (name: string) => string; - convertRelevance: (relevant?: string) => Promise; + convertRelevance: (relevant?: string) => string; } /** @@ -37,12 +37,6 @@ export class GroupEmitter { private counters: GroupCounters, ) {} - clear(): void { - this.currentGroup = null; - this.groupStack = []; - this.pendingGroupNotes = []; - } - getCurrentGroup(): string | null { return this.currentGroup; } @@ -89,11 +83,11 @@ export class GroupEmitter { ); } - async addGroup( + addGroup( row: SurveyRow, sanitizeName: (name: string) => string, - convertRelevance: (relevant?: string) => Promise, - ): Promise { + convertRelevance: (relevant?: string) => string, + ): void { const groupName = row.name && row.name.trim() !== '' ? sanitizeName(row.name.trim()) @@ -105,7 +99,7 @@ export class GroupEmitter { // type/scale is used as a stable group sequence key for LimeSurvey's TSV importer // to correctly match group translations across languages. const groupSeqKey = String(this.counters.getGroupSeq()); - const relevance = await convertRelevance(row.relevant); + const relevance = convertRelevance(row.relevant); this.rowEmitter.emitForEachLanguage( (lang) => ({ @@ -149,10 +143,10 @@ export class GroupEmitter { /** * Emit pending parent-only group labels as note questions (type X). */ - async emitPendingGroupNotes( + emitPendingGroupNotes( sanitizeName: (name: string) => string, - convertRelevance: (relevant?: string) => Promise, - ): Promise { + convertRelevance: (relevant?: string) => string, + ): void { for (const noteRow of this.pendingGroupNotes) { const noteName = noteRow.name && noteRow.name.trim() !== '' @@ -160,7 +154,7 @@ export class GroupEmitter { : `GN${this.counters.getQuestionSeq()}`; this.counters.bumpQuestionSeq(); - const relevance = await convertRelevance(noteRow.relevant); + const relevance = convertRelevance(noteRow.relevant); this.rowEmitter.emitForEachLanguage((lang) => ({ class: 'Q', @@ -185,13 +179,13 @@ export class GroupEmitter { * note, do not emit. * - otherwise: push, emit G row, flush pending notes. */ - async handleBeginGroup( + handleBeginGroup( row: SurveyRow, isMessageOnly: boolean, isParentOnly: boolean, helpers: GroupHelpers, - onTableList: (sanitizedName: string) => Promise, - ): Promise { + onTableList: (sanitizedName: string) => void, + ): void { const { sanitizeName, convertRelevance } = helpers; const originalName = (row.name || '').trim(); const sanitizedName = originalName @@ -208,9 +202,9 @@ export class GroupEmitter { emittedAsGroup: true, }); this.rowEmitter.flushGroupContent(); - await this.addGroup(row, sanitizeName, convertRelevance); - await this.emitPendingGroupNotes(sanitizeName, convertRelevance); - await onTableList(sanitizedName); + this.addGroup(row, sanitizeName, convertRelevance); + this.emitPendingGroupNotes(sanitizeName, convertRelevance); + onTableList(sanitizedName); return; } @@ -239,7 +233,7 @@ export class GroupEmitter { emittedAsGroup: true, }); this.rowEmitter.flushGroupContent(); - await this.addGroup(row, sanitizeName, convertRelevance); - await this.emitPendingGroupNotes(sanitizeName, convertRelevance); + this.addGroup(row, sanitizeName, convertRelevance); + this.emitPendingGroupNotes(sanitizeName, convertRelevance); } } diff --git a/src/pipelines/xlsform2lstsv/groupProcessor.ts b/src/pipelines/xlsform2lstsv/groupProcessor.ts index 5f9d71b..a150a39 100644 --- a/src/pipelines/xlsform2lstsv/groupProcessor.ts +++ b/src/pipelines/xlsform2lstsv/groupProcessor.ts @@ -58,11 +58,6 @@ export class GroupProcessor { constructor(private configManager: ConfigManager) {} - clear(): void { - this.messageOnlyGroups.clear(); - this.parentOnlyGroups.clear(); - } - getMessageOnlyGroups(): Set { return this.messageOnlyGroups; } diff --git a/src/pipelines/xlsform2lstsv/index.ts b/src/pipelines/xlsform2lstsv/index.ts index 175f820..1016fff 100644 --- a/src/pipelines/xlsform2lstsv/index.ts +++ b/src/pipelines/xlsform2lstsv/index.ts @@ -1,4 +1,5 @@ import { ConfigManager } from '../../config/ConfigManager.js'; +import { resolveConfig } from '../../config/resolveConfig.js'; import type { LstsvConfig } from '../../config/types.js'; import { SurveyRow, ChoiceRow, SettingsRow } from '../../xlsform/types.js'; import { FieldSanitizer } from '../../xlsform/sanitize.js'; @@ -38,7 +39,14 @@ import { // - xfTypeInfo: Parsed XLSForm type information (TypeInfo interface) // - lsType: LimeSurvey type information (LSType interface) -export class XLSFormToTSVConverter { +/** + * One XLSForm → LimeSurvey TSV conversion. Every collaborator and all + * per-conversion state (choices, field names, counters, buffered rows, + * languages) belong to this object, which {@link XLSFormToTSVConverter.convert} + * creates fresh for each call and drops afterwards. So nothing leaks from one + * conversion into the next, and concurrent calls can't interfere. + */ +class Conversion { private configManager: ConfigManager; private fieldSanitizer: FieldSanitizer; private typeMapper: TypeMapper; @@ -55,12 +63,13 @@ export class XLSFormToTSVConverter { private fieldNameHandler: FieldNameHandler; private appearanceHandler: AppearanceHandler; private counters: Counters; - private fileChoices: Record = {}; + private fileChoices: Record; private surveySettingsEmitter: SurveySettingsEmitter; private surveyDataCache: SurveyRow[] = []; - constructor(config?: Partial) { + constructor(config: Readonly) { this.configManager = new ConfigManager(config); + this.fileChoices = {}; this.fieldSanitizer = new FieldSanitizer(); this.choiceManager = new ChoiceManager(this.fieldSanitizer); @@ -130,43 +139,14 @@ export class XLSFormToTSVConverter { }; } - // ── Public API ─────────────────────────────────────────────────────── - - /** - * Get the current configuration - */ - getConfig(): Readonly { - return this.configManager.getConfig(); - } - - /** - * Update configuration at runtime - */ - updateConfig(partialConfig: Partial): void { - this.configManager.updateConfig(partialConfig); - } - - async convert( + /** Run the conversion; synchronous, throws on the first problem. */ + run( surveyData: SurveyRow[], choicesData: ChoiceRow[], settingsData: SettingsRow[], - // Choices for external-file lists (`select_*_from_file .csv`), keyed by - // the referenced filename. Registered vocabularies (registry/vocab/) are - // built in; entries here add unregistered ones or override a registered one - // (the CLI reads CSVs beside the form via resolveFileChoices). A from_file - // question is emitted as its base select with these choices inlined + a - // `cdl_vocab` attribute naming the source vocabulary. - fileChoices: Record = {}, - ): Promise { - // Reset state + fileChoices: Record, + ): string { this.fileChoices = { ...registeredFileChoices(surveyData), ...fileChoices }; - this.choiceManager.clear(); - this.tsvGenerator.clear(); - this.counters.clear(); - this.rowEmitter.clear(); - this.surveySettingsEmitter.clear(); - this.groupEmitter.clear(); - this.matrixHandler.clear(); // Pre-scan for welcome/end notes (must happen before group identification) this.surveySettingsEmitter.captureNotes(surveyData); @@ -223,7 +203,7 @@ export class XLSFormToTSVConverter { // Process survey rows for (const row of surveyData) { - await this.processRow(row); + this.processRow(row); } // Flush any pending matrix at the end @@ -246,7 +226,7 @@ export class XLSFormToTSVConverter { // ── Row processing ─────────────────────────────────────────────────── - private async processRow(row: SurveyRow): Promise { + private processRow(row: SurveyRow): void { const xfType = (row.type || '').trim(); if (!xfType) return; @@ -270,7 +250,7 @@ export class XLSFormToTSVConverter { this.validateRowType(xfType, baseType, row.name); if (xfType === 'begin_group' || xfType === 'begin group') { - await this.handleBeginGroup(row); + this.handleBeginGroup(row); return; } if (xfType === 'end_group' || xfType === 'end group') { @@ -284,7 +264,7 @@ export class XLSFormToTSVConverter { this.groupEmitter.addAutoGroupForOrphans(); } - await this.addQuestion(row); + this.addQuestion(row); } /** @@ -380,10 +360,10 @@ export class XLSFormToTSVConverter { } } - private async handleBeginGroup(row: SurveyRow): Promise { + private handleBeginGroup(row: SurveyRow): void { this.matrixHandler.flushMatrix(this.matrixHelpers()); const originalName = (row.name || '').trim(); - await this.groupEmitter.handleBeginGroup( + this.groupEmitter.handleBeginGroup( row, this.groupProcessor.getMessageOnlyGroups().has(originalName), this.groupProcessor.getParentOnlyGroups().has(originalName), @@ -416,7 +396,7 @@ export class XLSFormToTSVConverter { // ── Question emission ──────────────────────────────────────────────── - private async addQuestion(row: SurveyRow): Promise { + private addQuestion(row: SurveyRow): void { let xfTypeInfo = this.typeMapper.parseType(row.type || ''); // select_*_from_file → emit as its base select with the referenced CSV's @@ -436,7 +416,7 @@ export class XLSFormToTSVConverter { // The handler returns true if it consumed the row as part of a matrix, // false if it should be processed as a regular question (and the pending // matrix has been flushed). - const handledByMatrix = await this.matrixHandler.dispatchRow( + const handledByMatrix = this.matrixHandler.dispatchRow( row, xfTypeInfo, appearance, @@ -468,7 +448,7 @@ export class XLSFormToTSVConverter { xfTypeInfo.base, ); - const fields = await this.computeQuestionFields(row, xfTypeInfo, lsType); + const fields = this.computeQuestionFields(row, xfTypeInfo, lsType); const ctx: QuestionRowContext = { lsType, @@ -502,11 +482,11 @@ export class XLSFormToTSVConverter { * Compute the per-question fields that don't vary by language * (relevance, validation, mandatory, other, default, hidden, hide_tip). */ - private async computeQuestionFields( + private computeQuestionFields( row: SurveyRow, xfTypeInfo: { base: string }, lsType: { other?: boolean; dateFormat?: string }, - ): Promise<{ + ): { calculationExpr: string; relevance: string; emValidation: string; @@ -517,18 +497,16 @@ export class XLSFormToTSVConverter { hideTip: string; isNote: boolean; isCalculate: boolean; - }> { + } { const isNote = xfTypeInfo.base === 'note'; const isCalculate = xfTypeInfo.base === 'calculate'; const isNoteOrCalc = isNote || isCalculate; - const calculationExpr = await this.computeCalculation(row, isCalculate); - const relevance = await this.transpilerHelper.convertRelevance( - row.relevant, - ); + const calculationExpr = this.computeCalculation(row, isCalculate); + const relevance = this.transpilerHelper.convertRelevance(row.relevant); const emValidation = isNoteOrCalc ? '' - : await this.transpilerHelper.convertConstraint(row.constraint || ''); + : this.transpilerHelper.convertConstraint(row.constraint || ''); const mandatory = isNoteOrCalc ? '' : this.mandatoryValue(row); const other = this.computeOtherFlag(row, lsType, isNoteOrCalc); const defaultVal = isNoteOrCalc ? '' : row.default || ''; @@ -556,10 +534,7 @@ export class XLSFormToTSVConverter { } /** Transpile `row.calculation` to EM, only meaningful for `calculate` questions. */ - private async computeCalculation( - row: SurveyRow, - isCalculate: boolean, - ): Promise { + private computeCalculation(row: SurveyRow, isCalculate: boolean): string { if (!isCalculate || !row.calculation) return ''; return this.transpilerHelper.convertCalculation(row.calculation); } @@ -660,3 +635,57 @@ interface QuestionRowContext { /** LS question attributes from the `parameters` column (parameters.ts). */ attributes: Record; } + +/** + * XLSForm → LimeSurvey structure TSV. The instance only holds its resolved + * options; each {@link convert} call runs in its own {@link Conversion}, so one + * instance can be reused, including for concurrent calls. + */ +export class XLSFormToTSVConverter { + private config: Readonly; + + constructor(config?: Partial) { + this.config = resolveConfig(config); + } + + /** The resolved options. */ + getConfig(): Readonly { + return this.config; + } + + /** Replace the options: `partialConfig` merged over the defaults. */ + updateConfig(partialConfig: Partial): void { + this.config = resolveConfig(partialConfig); + } + + /** + * Convert parsed XLSForm sheets to LimeSurvey TSV. + * + * @param fileChoices Choices for external-file lists + * (`select_*_from_file .csv`), keyed by the referenced filename. + * Registered vocabularies (registry/vocab/) are built in; entries here add + * unregistered ones or override a registered one (the CLI reads CSVs beside + * the form via resolveFileChoices). + */ + convert( + surveyData: SurveyRow[], + choicesData: ChoiceRow[], + settingsData: SettingsRow[], + fileChoices: Record = {}, + ): Promise { + try { + return Promise.resolve( + new Conversion(this.config).run( + surveyData, + choicesData, + settingsData, + fileChoices, + ), + ); + } catch (error: unknown) { + return Promise.reject( + error instanceof Error ? error : new Error(String(error)), + ); + } + } +} diff --git a/src/pipelines/xlsform2lstsv/matrixHandler.ts b/src/pipelines/xlsform2lstsv/matrixHandler.ts index 1330951..74acae4 100644 --- a/src/pipelines/xlsform2lstsv/matrixHandler.ts +++ b/src/pipelines/xlsform2lstsv/matrixHandler.ts @@ -11,7 +11,7 @@ export type MatrixCounters = Counters; export interface MatrixHelpers { sanitizeName(name: string): string; sanitizeAnswerCode(code: string): string; - convertRelevance(relevant?: string): Promise; + convertRelevance(relevant?: string): string; } /** @@ -35,12 +35,6 @@ export class MatrixHandler { private counters: MatrixCounters, ) {} - clear(): void { - this.inMatrix = false; - this.matrixListName = null; - this.inTableListMatrix = false; - } - isInMatrix(): boolean { return this.inMatrix; } @@ -59,18 +53,18 @@ export class MatrixHandler { * - appearance=list-nolabel + in matrix + select_one → addMatrixSubquestion * - otherwise → flush any pending matrix, return false */ - async dispatchRow( + dispatchRow( row: SurveyRow, xfTypeInfo: TypeInfo, appearance: string, helpers: MatrixHelpers, - ): Promise { + ): boolean { // Inside a `table-list` group: each select_one child is a subquestion of // the enclosing array. Capture the shared list from the first child so // flushMatrix can emit its answer scale. if (this.inTableListMatrix && xfTypeInfo.base === 'select_one') { if (!this.matrixListName) this.matrixListName = xfTypeInfo.listName; - await this.addMatrixSubquestion(row, helpers); + this.addMatrixSubquestion(row, helpers); return true; } @@ -81,7 +75,7 @@ export class MatrixHandler { xfTypeInfo.listName ) { this.flushMatrix(helpers); - await this.addMatrixHeader(row, xfTypeInfo, helpers); + this.addMatrixHeader(row, xfTypeInfo, helpers); return true; } @@ -91,7 +85,7 @@ export class MatrixHandler { this.inMatrix && xfTypeInfo.base === 'select_one' ) { - await this.addMatrixSubquestion(row, helpers); + this.addMatrixSubquestion(row, helpers); return true; } @@ -114,18 +108,18 @@ export class MatrixHandler { * shared answer scale is emitted by flushMatrix (on end_group). No G row is * emitted — the group *is* the array. */ - async addTableListHeader( + addTableListHeader( row: SurveyRow, questionName: string, helpers: MatrixHelpers, - ): Promise { + ): void { this.counters.bumpGroupSeq(); this.inMatrix = true; this.inTableListMatrix = true; this.matrixListName = null; this.counters.setSubquestionSeq(0); - const relevance = await helpers.convertRelevance(row.relevant); + const relevance = helpers.convertRelevance(row.relevant); const mandatory = row.required === 'yes' || row.required === 'true' ? 'Y' : ''; @@ -143,11 +137,11 @@ export class MatrixHandler { })); } - async addMatrixHeader( + addMatrixHeader( row: SurveyRow, xfTypeInfo: TypeInfo, helpers: MatrixHelpers, - ): Promise { + ): void { const questionName = row.name && row.name.trim() !== '' ? helpers.sanitizeName(row.name.trim()) @@ -158,7 +152,7 @@ export class MatrixHandler { this.matrixListName = xfTypeInfo.listName; this.counters.setSubquestionSeq(0); - const relevance = await helpers.convertRelevance(row.relevant); + const relevance = helpers.convertRelevance(row.relevant); const mandatory = row.required === 'yes' || row.required === 'true' ? 'Y' : ''; @@ -176,17 +170,14 @@ export class MatrixHandler { })); } - async addMatrixSubquestion( - row: SurveyRow, - helpers: MatrixHelpers, - ): Promise { + addMatrixSubquestion(row: SurveyRow, helpers: MatrixHelpers): void { const sqName = row.name && row.name.trim() !== '' ? helpers.sanitizeName(row.name.trim()) : `SQ${this.counters.getSubquestionSeq()}`; this.counters.bumpSubquestionSeq(); - const relevance = await helpers.convertRelevance(row.relevant); + const relevance = helpers.convertRelevance(row.relevant); const mandatory = row.required === 'yes' || row.required === 'true' ? 'Y' : ''; diff --git a/src/pipelines/xlsform2lstsv/rowEmitter.ts b/src/pipelines/xlsform2lstsv/rowEmitter.ts index a6d02ad..31953cc 100644 --- a/src/pipelines/xlsform2lstsv/rowEmitter.ts +++ b/src/pipelines/xlsform2lstsv/rowEmitter.ts @@ -14,10 +14,6 @@ export class RowEmitter { private languageHandler: LanguageHandler, ) {} - clear(): void { - this.buffer = []; - } - /** * Build a TSVRowData with sensible defaults. Only `class` and `name` are required; * all other fields default to empty strings (relevance defaults to '1'). diff --git a/src/pipelines/xlsform2lstsv/surveySettingsEmitter.ts b/src/pipelines/xlsform2lstsv/surveySettingsEmitter.ts index 2104a52..337f7fa 100644 --- a/src/pipelines/xlsform2lstsv/surveySettingsEmitter.ts +++ b/src/pipelines/xlsform2lstsv/surveySettingsEmitter.ts @@ -18,11 +18,6 @@ export class SurveySettingsEmitter { private languageHandler: LanguageHandler, ) {} - clear(): void { - this.welcomeNote = null; - this.endNote = null; - } - /** * Pre-scan for welcome/end notes (must happen before group identification). * Promotes a row named "welcome"/"end" to its own SL row so the message diff --git a/src/pipelines/xlsform2lstsv/transpilerHelper.ts b/src/pipelines/xlsform2lstsv/transpilerHelper.ts index 68bdc04..d081b43 100644 --- a/src/pipelines/xlsform2lstsv/transpilerHelper.ts +++ b/src/pipelines/xlsform2lstsv/transpilerHelper.ts @@ -1,9 +1,9 @@ import { FieldSanitizer } from '../../xlsform/sanitize.js'; import { ChoiceManager } from './choiceManager.js'; import { - convertRelevance, - convertConstraint, - xpathToLimeSurvey, + convertRelevanceSync, + convertConstraintSync, + xpathToLimeSurveySync, TranspilerContext, } from './xpathTranspiler.js'; @@ -44,16 +44,16 @@ export class TranspilerHelper { }; } - async convertRelevance(relevant?: string): Promise { + convertRelevance(relevant?: string): string { if (!relevant) return '1'; - return await convertRelevance(relevant, this.buildTranspilerContext()); + return convertRelevanceSync(relevant, this.buildTranspilerContext()); } - async convertCalculation(calculation: string): Promise { - return await xpathToLimeSurvey(calculation, this.buildTranspilerContext()); + convertCalculation(calculation: string): string { + return xpathToLimeSurveySync(calculation, this.buildTranspilerContext()); } - async convertConstraint(constraint: string): Promise { - return await convertConstraint(constraint); + convertConstraint(constraint: string): string { + return convertConstraintSync(constraint); } } diff --git a/src/pipelines/xlsform2lstsv/xpathTranspiler.ts b/src/pipelines/xlsform2lstsv/xpathTranspiler.ts index 1ecda75..d0d73cf 100644 --- a/src/pipelines/xlsform2lstsv/xpathTranspiler.ts +++ b/src/pipelines/xlsform2lstsv/xpathTranspiler.ts @@ -273,19 +273,31 @@ export function xpathToLimeSurvey( xpathExpr: string, ctx?: TranspilerContext, ): Promise { - // Stays Promise-returning: it's public API, and callers await it. - if (!xpathExpr || xpathExpr.trim() === '') { - return Promise.resolve('1'); // Default relevance expression + // Promise-returning public API; the converter uses the sync core. + try { + return Promise.resolve(xpathToLimeSurveySync(xpathExpr, ctx)); + } catch (error: unknown) { + return Promise.reject( + error instanceof Error ? error : new Error(String(error)), + ); } +} + +/** Synchronous core of {@link xpathToLimeSurvey}; throws on failure. */ +export function xpathToLimeSurveySync( + xpathExpr: string, + ctx?: TranspilerContext, +): string { + if (!xpathExpr || xpathExpr.trim() === '') return '1'; // default relevance const processedExpr = preprocessExpression(xpathExpr); try { - return Promise.resolve(transpile(parseXPath(processedExpr), ctx)); + return transpile(parseXPath(processedExpr), ctx); } catch (error: unknown) { const wrapped = new Error( `Cannot convert XPath expression "${xpathExpr}" to LimeSurvey: ${(error as Error).message}`, ); (wrapped as Error & { cause?: unknown }).cause = error; - return Promise.reject(wrapped); + throw wrapped; } } @@ -341,7 +353,8 @@ export function convertConstraint(constraint: string): Promise { return Promise.resolve(convertConstraintSync(constraint)); } -function convertConstraintSync(constraint: string): string { +/** Synchronous core of {@link convertConstraint}. */ +export function convertConstraintSync(constraint: string): string { if (!constraint) return ''; const processedExpr = preprocessExpression(constraint); @@ -454,10 +467,24 @@ function parseRegexMatchArguments(argsString: string): string[] { * @param xpath - The XPath relevance expression * @returns LimeSurvey Expression Manager syntax */ -export async function convertRelevance( +export function convertRelevance( xpathExpr: string, ctx?: TranspilerContext, ): Promise { + try { + return Promise.resolve(convertRelevanceSync(xpathExpr, ctx)); + } catch (error: unknown) { + return Promise.reject( + error instanceof Error ? error : new Error(String(error)), + ); + } +} + +/** Synchronous core of {@link convertRelevance}. */ +export function convertRelevanceSync( + xpathExpr: string, + ctx?: TranspilerContext, +): string { if (!xpathExpr) return '1'; // XPath operators are lowercase; accept AND/OR as XLSForm authors write them @@ -465,7 +492,7 @@ export async function convertRelevance( .replace(/\bAND\b/gi, 'and') .replace(/\bOR\b/gi, 'or'); - const result = await xpathToLimeSurvey(normalizedXPath, ctx); + const result = xpathToLimeSurveySync(normalizedXPath, ctx); // Handle edge case: selected() with just {field} (without $) if (result && typeof result === 'string' && result.includes('selected(')) { diff --git a/tests/ts/integration/converterConcurrency.test.ts b/tests/ts/integration/converterConcurrency.test.ts new file mode 100644 index 0000000..5786d5d --- /dev/null +++ b/tests/ts/integration/converterConcurrency.test.ts @@ -0,0 +1,56 @@ +/** + * #62: XLSFormToTSVConverter keeps no per-conversion state on the instance, so + * one instance can run several conversions at once. Before, a second call's + * reset wiped the first call's choice lists mid-run. + */ +import { readFileSync } from 'node:fs'; +import { join, resolve } from 'node:path'; + +import { describe, expect, test } from 'vitest'; + +import { XLSFormToTSVConverter } from '../../../src/index.js'; + +const SURVEYS = resolve(__dirname, '../../fixtures/surveys'); +type Form = { survey: never[]; choices?: never[]; settings?: never[] }; +const load = (name: string) => + JSON.parse( + readFileSync(join(SURVEYS, name, 'xlsform.json'), 'utf-8'), + ) as Form; +const run = (c: XLSFormToTSVConverter, f: Form) => + c.convert(f.survey, f.choices ?? [], f.settings ?? []); + +const NAMES = [ + 'complex_survey', + 'multilingual_survey', + 'complex_xpath_survey', + 'validation_relevance_survey', +]; + +describe('XLSFormToTSVConverter concurrency', () => { + test('concurrent convert() calls on one instance match separate instances', async () => { + const forms = NAMES.map(load); + const expected = await Promise.all( + forms.map((f) => run(new XLSFormToTSVConverter(), f)), + ); + + const shared = new XLSFormToTSVConverter(); + const actual = await Promise.all(forms.map((f) => run(shared, f))); + expect(actual).toEqual(expected); + }); + + test('reusing one instance sequentially gives identical output', async () => { + const c = new XLSFormToTSVConverter(); + const f = load('complex_survey'); + expect(await run(c, f)).toBe(await run(c, f)); + }); + + test('a conversion error rejects the promise; it does not throw synchronously', async () => { + const c = new XLSFormToTSVConverter(); + const p = c.convert( + [{ type: 'select_one missing', name: 'q', label: 'Q' }] as never[], + [], + [], + ); + await expect(p).rejects.toThrow(/list 'missing' has no rows/); + }); +});