From 1e4926fd80cc9a34c6dc1caad353cca342ceca60 Mon Sep 17 00:00:00 2001 From: Artyom Savchenko Date: Thu, 15 Jan 2026 21:29:48 +0700 Subject: [PATCH] Fix controlled doc sequence conflicts (#10406) * Fix document sequence conflicts Signed-off-by: Artem Savchenko * Clean up Signed-off-by: Artem Savchenko --------- Signed-off-by: Artem Savchenko --- .../core/packages/core/src/operations.ts | 11 ++ .../server/packages/middleware/src/applyTx.ts | 14 +- plugins/controlled-documents/src/docutils.ts | 140 ++++++++++++++++-- 3 files changed, 155 insertions(+), 10 deletions(-) diff --git a/foundations/core/packages/core/src/operations.ts b/foundations/core/packages/core/src/operations.ts index 8a182e5514..8e64f87928 100644 --- a/foundations/core/packages/core/src/operations.ts +++ b/foundations/core/packages/core/src/operations.ts @@ -531,6 +531,17 @@ export class ApplyOperations extends TxOperations { if (typeof window === 'object' && window !== null && this.measureName != null) { console.log(`measure ${this.measureName}`, dnow - st, 'server time', result.serverTime) } + if (!result.success) { + console.warn('ops.commit() failed', { + scope: this.scope, + measureName: this.measureName, + matchCount: this.matches.length, + notMatchCount: this.notMatches.length, + txCount: this.txes.length, + matches: this.matches.map((m) => ({ _class: m._class, query: m.query })), + notMatches: this.notMatches.map((m) => ({ _class: m._class, query: m.query })) + }) + } this.txes = [] return { result: result.success, diff --git a/foundations/server/packages/middleware/src/applyTx.ts b/foundations/server/packages/middleware/src/applyTx.ts index e1275ef253..a6e9407799 100644 --- a/foundations/server/packages/middleware/src/applyTx.ts +++ b/foundations/server/packages/middleware/src/applyTx.ts @@ -63,6 +63,14 @@ export class ApplyTxMiddleware extends BaseMiddleware implements Middleware { } applyResult.serverTime = Date.now() - st } else { + ctx.warn('TxApplyIf failed', { + scope: applyIf.scope, + reason: passed.reason, + measureName: applyIf.measureName, + matchCount: applyIf.match?.length ?? 0, + notMatchCount: applyIf.notMatch?.length ?? 0, + txCount: applyIf.txes.length + }) result.push({ success: false }) @@ -92,6 +100,7 @@ export class ApplyTxMiddleware extends BaseMiddleware implements Middleware { ): Promise<{ onEnd: () => void passed: boolean + reason?: string }> { if (applyIf.scope == null) { return { passed: true, onEnd: () => {} } @@ -115,11 +124,13 @@ export class ApplyTxMiddleware extends BaseMiddleware implements Middleware { }) ) let passed = true + let reason: string | undefined if (applyIf.match != null) { for (const { _class, query } of applyIf.match) { const res = await this.provideFindAll(ctx, _class, query, { limit: 1 }) if (res.length === 0) { passed = false + reason = `match query failed: class=${_class}, query=${JSON.stringify(query)}` break } } @@ -129,10 +140,11 @@ export class ApplyTxMiddleware extends BaseMiddleware implements Middleware { const res = await this.provideFindAll(ctx, _class, query, { limit: 1 }) if (res.length > 0) { passed = false + reason = `notMatch query failed: class=${_class}, query=${JSON.stringify(query)} (found ${res.length} matching document(s))` break } } } - return { passed, onEnd } + return { passed, onEnd, reason } } } diff --git a/plugins/controlled-documents/src/docutils.ts b/plugins/controlled-documents/src/docutils.ts index 1e980e094c..ecdca27339 100644 --- a/plugins/controlled-documents/src/docutils.ts +++ b/plugins/controlled-documents/src/docutils.ts @@ -30,7 +30,7 @@ import { import { makeRank } from '@hcengineering/rank' import documents from './plugin' -import { getFirstRank, TEMPLATE_PREFIX } from './utils' +import { getDocumentId, getFirstRank, TEMPLATE_PREFIX } from './utils' async function getParentPath (client: TxOperations, parent: Ref): Promise>> { const parentDocObj = await client.findOne(documents.class.ProjectDocument, { @@ -68,8 +68,10 @@ export async function createControlledDocFromTemplate ( return { seqNumber: -1, success: false } } - const { seqNumber, prefix, content, category } = await useDocumentTemplate(client, templateId) - const { success, documentMetaId } = await createControlledDocMetadata( + // Try fast path first (assumes template sequence is in sync) + let { seqNumber, prefix, content, category } = await useDocumentTemplate(client, templateId, false) + let actualCode = getDocumentId({ prefix, seqNumber }) + let { success, documentMetaId } = await createControlledDocMetadata( client, templateId, documentId, @@ -78,10 +80,35 @@ export async function createControlledDocFromTemplate ( parent, prefix, seqNumber, - spec.code, + actualCode, spec.title ) + // If creation failed due to seqNumber conflict, retry with full uniqueness check + if (!success) { + // Retry with expensive check to find actual max seqNumber + const retryResult = await useDocumentTemplate(client, templateId, true) + seqNumber = retryResult.seqNumber + prefix = retryResult.prefix + content = retryResult.content + category = retryResult.category + actualCode = getDocumentId({ prefix, seqNumber }) + const retryMetadata = await createControlledDocMetadata( + client, + templateId, + documentId, + space, + project, + parent, + prefix, + seqNumber, + actualCode, + spec.title + ) + success = retryMetadata.success + documentMetaId = retryMetadata.documentMetaId + } + if (!success) { return { seqNumber: -1, success: false } } @@ -107,9 +134,33 @@ export async function createControlledDocFromTemplate ( return { seqNumber, success: true } } +/** + * Calculate the next available seqNumber by checking existing documents with the template. + */ +async function calculateNextSeqNumberWithCheck ( + client: TxOperations, + templateId: Ref, + currentTemplateSequence: number +): Promise { + const existingDocs = await client.findAll( + documents.class.Document, + { + template: templateId + }, + { + projection: { seqNumber: 1 } + } + ) + + const maxExistingSeqNumber = existingDocs.length > 0 ? Math.max(...existingDocs.map((doc) => doc.seqNumber ?? 0)) : -1 + + return Math.max(currentTemplateSequence, maxExistingSeqNumber) + 1 +} + export async function useDocumentTemplate ( client: TxOperations, - templateId: Ref + templateId: Ref, + checkExisting: boolean = false ): Promise<{ seqNumber: number, prefix: string, content: Ref | null, category: Ref }> { const template = await client.findOne(documents.mixin.DocumentTemplate, { _id: templateId @@ -119,15 +170,27 @@ export async function useDocumentTemplate ( return { seqNumber: -1, prefix: '', content: null, category: '' as Ref } } + let nextSeqNumber: number + + if (checkExisting) { + nextSeqNumber = await calculateNextSeqNumberWithCheck(client, templateId, template.sequence) + } else { + nextSeqNumber = template.sequence + 1 + } + + // Update template sequence to nextSeqNumber in a single atomic operation await client.updateMixin(templateId, documents.class.Document, template.space, documents.mixin.DocumentTemplate, { - $inc: { sequence: 1 } + sequence: nextSeqNumber }) - // FIXME: not concurrency safe - const seqNumber = template.sequence + 1 const prefix = template.docPrefix - return { seqNumber, prefix, content: template.content, category: template.category as Ref } + return { + seqNumber: nextSeqNumber, + prefix, + content: template.content, + category: template.category as Ref + } } export async function createControlledDocMetadata ( @@ -203,6 +266,22 @@ export async function createControlledDocMetadata ( const success = await ops.commit() + if (!success.result) { + console.warn('createControlledDocMetadata: ops.commit() failed', { + templateId, + documentId, + space, + project, + parent, + prefix, + seqNumber, + specCode, + specTitle, + documentMetaId, + projectDocumentId + }) + } + return { success: success.result, seqNumber, documentMetaId, projectDocumentId } } @@ -261,6 +340,20 @@ export async function createDocumentTemplate ( }) const commit = await ops.commit() + if (!commit.result) { + console.warn('createDocumentTemplate: ops.commit() failed', { + _class, + space, + _mixin, + project, + parent, + templateId, + prefix, + category, + author + }) + } + return { seqNumber, success: commit.result } } @@ -355,6 +448,24 @@ export async function createDocumentTemplateMetadata ( const success = await ops.commit() + if (!success.result) { + console.warn('createDocumentTemplateMetadata: ops.commit() failed', { + _class, + space, + _mixin, + project, + parent, + templateId, + prefix, + specCode, + specTitle, + seqNumber, + code, + documentMetaId, + projectDocumentId + }) + } + return { success: success.result, seqNumber, code, documentMetaId, projectDocumentId } } @@ -407,5 +518,16 @@ export async function createNewFolder ( const success = await ops.commit() + if (!success.result) { + console.warn('createNewFolder: ops.commit() failed', { + space, + project, + parent, + title, + documentMetaId, + projectDocumentId + }) + } + return { success: success.result, documentMetaId, projectDocumentId } }