Fix QueueGR myDATA aa numbering skips for non-submitted receipts#666
Open
StefanKert wants to merge 12 commits into
Open
Fix QueueGR myDATA aa numbering skips for non-submitted receipts#666StefanKert wants to merge 12 commits into
StefanKert wants to merge 12 commits into
Conversation
Storage scaffolding for the myDATA aa counter: introduces InvoiceSeries, InvoiceNumerator, LastInvoiceMoment, LastInvoiceQueueItemId, LastInvoiceMark on ftQueueGR, mirroring the shape of ftQueueES.InvoiceSeries/InvoiceNumerator. The fields are owned by the GR localisation and will be populated by an upcoming change in ReceiptCommandProcessorGR / InvoiceCommandProcessorGR; this commit only adds the schema so subsequent commits can build on it. Azure Table Storage entity + mapper updated. InMemory works via the generic repository without changes. SQLite/MySQL/EF QueueGR repositories all throw NotImplementedException today and are intentionally left alone. Refs fiskaltrust/market-gr#172 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
LifecycleCommandProcessorGR now takes AsyncLazy<IConfigurationRepository> and, on InitialOperationReceipt0x4001, seeds queueGR.InvoiceSeries from CashBoxIdentification when not already set. Mirrors the activation-time series seeding that QueueES does in LifecycleCommandProcessorES.cs:24-30. After this commit InvoiceSeries is guaranteed to be populated for any newly-activated GR queue; existing queues will pick it up the next time they go through activation, or via the seed-from-journal migration coming in a later commit. Refs fiskaltrust/market-gr#172 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Moves the per-receipt aa assignment off the generic ftReceiptNumerator
(which advances for every successful receipt across all markets,
including QueueGR NoOp cases like daily closings) and onto a dedicated
InvoiceNumerator on ftQueueGR. This is the actual fix for myDATA
receiving sequential aa values with gaps after non-submitted receipts.
Flow:
1. ReceiptCommandProcessorGR / InvoiceCommandProcessorGR read
queueGR.InvoiceSeries + InvoiceNumerator, reserve aa = N+1 in
memory, and attach a ProposedInvoiceCounter to ftStateData.GR
before invoking the SCU.
2. AADEFactory consumes ftStateData.GR.ProposedInvoiceCounter when
present; the legacy ftReceiptIdentification parse is kept as a
fallback so existing tests/callers keep working.
3. After the SCU returns, the country processor commits only if
State.Success AND the doc was submitted under our reserved
(series, aa) — detected via the "{series}-{aa}" suffix MyDataSCU
appends to ftReceiptIdentification only on successful SendInvoices.
This naturally excludes handwritten and mydataoverride paths,
which overwrite series/aa on the doc.
4. MyDataSCU's sandbox GovernmentApi-stash now merges into existing
ftStateData instead of replacing it, so the proposed counter is
preserved alongside the AADE response payload.
QueueGRBootstrapper plumbs CreateConfigurationRepository() into the new
processor dependencies, mirroring the QueueESBootstrapper wiring.
Refs fiskaltrust/market-gr#172
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…shes
Previously every AADE error was funnelled into SetErrorAndLog (State.Error),
including error 233 ("UID has already been sent"). 233 specifically means
the deterministic (issuer, branch, date, type, series, aa) uid is already
on file — almost always because our own previous attempt was accepted by
AADE but we never durably recorded it (process kill or network blip
between the AADE 200 OK and the storage commit). Treating 233 the same as
a real rejection meant the receipt's retry kept hitting 233 forever and
the queue never advanced past it.
This commit detects 233 explicitly and treats it as success-equivalent:
appends the "{series}-{aa}" suffix to ftReceiptIdentification (so the
country processor's reserve-then-commit logic sees a normal success and
advances InvoiceNumerator on this retry), and adds a dedicated audit
signature (AadeDuplicateResubmission, type 0x4752_2000_0000_001C) so the
duplicate is traceable in the response journal.
Phase-1 assumption is that 233 == our prior submission; the foreign-
collision case (a different system writing under the same series) is
indistinguishable from 233 alone and is left for Phase 2 follow-up:
verification via RequestTransmittedDocs to recover the prior mark and
distinguish ours-vs-theirs.
Refs fiskaltrust/market-gr#172
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
InvoiceCounterReservationTests cover the new reserve-then-commit flow
through ReceiptCommandProcessorGR:
- Success path: counter advances by one, mark recovered from the
invoiceMark signature.
- Error path: counter is not advanced.
- Override path (suffix mismatch): counter is not advanced — exercises
the safeguard against committing for handwritten / mydataoverride
submissions where the doc carries a caller-supplied (series, aa).
- Success without mark: counter still advances, LastInvoiceMark stays
null.
- Missing InvoiceSeries on the queue record: falls back to
CashBoxIdentification so a pre-seed queue still produces a valid
series rather than crashing.
Two new LifecycleCommandProcessorGR tests assert the activation seeding
behaviour: InvoiceSeries is initialised from CashBoxIdentification when
unset, and is left alone (no InsertOrUpdate write) when already set.
Refs fiskaltrust/market-gr#172
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ubmission Queues activated before this fix shipped have InvoiceNumerator=0 and have been submitting receipts with aa derived from ftReceiptNumerator. Without a migration the first post-upgrade submission would compute aa=1, which collides with documents AADE already has on file under the legacy numbering and trips error 233 indefinitely (the 233 handler advances the counter but every new receipt keeps colliding with older aa values). InvoiceCounterReservation now runs a one-time seed at the top of every reservation: if InvoiceNumerator is still 0 and ftReceiptNumerator is non-zero, copy ftReceiptNumerator into InvoiceNumerator. The first reserved aa is then ftReceiptNumerator + 1, strictly greater than any aa that could have been submitted under the old numbering. This overshoots by the count of NoOp receipts that previously advanced ftReceiptNumerator without submitting (daily closings, etc.), producing a one-off gap on the AADE side. That gap is acceptable — AADE permits non-contiguous aa, it only rejects duplicates. Test coverage: UpgradedQueue_SeedsInvoiceNumeratorFromFtReceiptNumerator verifies the migration triggers on InvoiceNumerator=0 + ftReceiptNumerator>0 and produces the expected aa. Refs fiskaltrust/market-gr#172 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
github-actions
Bot
added
market-gr
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 219d6a547d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+33
to
+35
| if (queueGR.InvoiceNumerator == 0 && request.queue.ftReceiptNumerator > 0) | ||
| { | ||
| queueGR.InvoiceNumerator = request.queue.ftReceiptNumerator; |
There was a problem hiding this comment.
The migration branch if (queueGR.InvoiceNumerator == 0 && request.queue.ftReceiptNumerator > 0) also runs on brand-new queues, not just upgraded ones. On a fresh deployment, any successful non-submitted receipts (e.g. daily/monthly/yearly closings routed through GRFallBackOperations.NoOp) increase ftReceiptNumerator; then the first real myDATA submission seeds from that value and emits aa = ftReceiptNumerator + 1, recreating the exact numbering gaps this change is trying to eliminate. This should be gated by an explicit upgrade marker/condition so new queues continue numbering from 1 regardless of prior NoOp receipts.
Useful? React with 👍 / 👎.
Member
Author
There was a problem hiding this comment.
Fixed in 11b69a6. Migration now gates on string.IsNullOrEmpty(queueGR.InvoiceSeries) rather than on the counter value — LifecycleCommandProcessorGR seeds InvoiceSeries at activation for new queues, so any queue reaching the reservation helper with InvoiceSeries still empty is definitionally pre-upgrade. Brand-new queues with prior NoOps now correctly start at aa = 1. Added NewQueueWithNoOpsBeforeFirstSubmission_StartsAtAa1 as a regression test.
Queue Test Results 43 files 43 suites 3m 34s ⏱️ Results for commit 9e913ed. ♻️ This comment has been updated with latest results. |
…tateData
ftStateData is for response-state data that is serialised back to the
merchant; using it as a side channel for in-flight coordination between
the country processor and the SCU was the wrong shape and mixed two
unrelated concerns. Other country queues (ES/FR/AT/PT) all carry their
country-specific receipt segment in ftReceiptIdentification after "#" —
QueueGR now follows the same convention.
Changes:
- QueueGR processor pre-appends "{series}-{aa}" to ftReceiptIdentification
before invoking the SCU (mirrors ES/FR/AT/PT's append pattern).
- AADEFactory parses (series, aa) from the country segment after "#",
falling back to the legacy prefix parse + ftCashBoxIdentification when
no country segment is present (back-compat for tests).
- MyDataSCU rewrites the country segment on a successful SendInvoices
(and on the 233 self-heal path) so the suffix always reflects what
AADE actually saw. Handwritten / mydataoverride paths produce a
different suffix from the reservation, which the country processor
detects via EndsWith and correctly skips the counter commit.
- The ProposedInvoiceCounter type on MiddlewareQueueGRState is removed,
along with the MyDataSCU sandbox state-merge helper (state replacement
is back to the original null-check pattern, since there is no longer
anything else on ftStateData.GR for that block to overwrite).
- QueueGR no longer ProjectReferences scu-gr/MyData. The cross-project
POCO dependency that motivated the reference is gone now that
coordination is over a plain string field.
Refs fiskaltrust/market-gr#172
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Pre-upgrade the old code read ftReceiptNumerator *before* bumping it, so after N successful submissions ftReceiptNumerator == N and the highest aa AADE has is N-1. The previous seed (InvoiceNumerator = ftReceiptNumerator) produced a reserved aa of N+1, skipping the value N — leaving a permanent one-aa gap per upgraded queue. Seeding from ftReceiptNumerator - 1 makes the first post-upgrade reserved aa exactly N, the value the old code would have submitted next. AADE's sequence stays continuous across the upgrade boundary. Edge case: if a pre-upgrade attempt crashed *after* AADE accepted but before ftReceiptNumerator was incremented, AADE has aa=N already. The tighter seed will produce reserved aa=N on the first retry, collide, and the existing 233 handler self-heals on that same round- trip (counter advances to N, next receipt at N+1). One extra AADE round-trip in that rare scenario; zero gap in the common case. Refs fiskaltrust/market-gr#172 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The 233 ("UID has already been sent") handler was a Phase-1 add-on for
crash-recovery: when a pre-existing crash between AADE's 200 and our
storage commit produces a stuck queue, the handler maps 233 to Success
so the next retry advances the counter. That capability isn't in
production today, and adding it alongside the numbering fix conflates
two changes with different risk profiles.
Moving 233 handling out of this PR. To keep the migration safe without
the handler, the InvoiceNumerator seed reverts from ftReceiptNumerator-1
(gap-free, requires 233 self-heal in the crash-recovery edge case) to
ftReceiptNumerator (one cosmetic aa skipped per upgraded queue, but
guaranteed collision-free regardless of crash-recovery state).
Removed:
- MyDataSCU's 233 detection branch in ProcessReceiptAsync.
- SignatureItemFactoryGR.AddAadeDuplicateResubmissionSignature.
- SignatureTypeGR.AadeDuplicateResubmission enum value.
- Flush-seed migration in InvoiceCounterReservation (back to
conservative seed).
The 233 self-heal + flush seed will land as a separate change.
Refs fiskaltrust/market-gr#172
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The previous migration condition (InvoiceNumerator == 0 && ftReceiptNumerator > 0) misfired on brand-new queues that had processed NoOp receipts (daily closings, etc.) before their first myDATA submission: ftReceiptNumerator advances for NoOps too, so the first real submission on a fresh queue would seed InvoiceNumerator from that value and emit aa=N+1 — recreating the very gaps this PR is meant to eliminate. The reliable upgrade discriminator is an empty InvoiceSeries. LifecycleCommandProcessorGR.InitialOperationReceipt0x4001Async now seeds InvoiceSeries from CashBoxIdentification at activation, so every new queue has it populated before any submission can reach the reservation helper. A queue that reaches the helper with InvoiceSeries still unset went through activation under the old code and never got the seed — i.e. it is genuinely pre-upgrade. Migration now only runs when InvoiceSeries is empty. Brand-new queues number from aa=1 regardless of pre-submission NoOp activity. Regression test added: NewQueueWithNoOpsBeforeFirstSubmission_StartsAtAa1 covers the codex-flagged scenario directly. Caught by Codex review on commit 219d6a5. Refs fiskaltrust/market-gr#172 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds two focused tests that close the previously-implicit coverage of
the new (series, aa) handoff from the QueueGR processor to AADEFactory:
MapToInvoicesDoc_WithCountrySuffix_ParsesSeriesAndAaFromSuffix
Verifies AADEFactory.ResolveSeriesAndAa parses (series, aa) from the
"{prefix}#{series}-{aa}" suffix the country processor now pre-appends,
rather than from the legacy hex-prefix path. Without this the new
reservation flow can't communicate the reserved values to the doc
builder.
MapToInvoicesDoc_Handwritten_OverridesSuffixBasedSeriesAndAa
Verifies the handwritten override still wins over the suffix-parsed
auto reservation: doc carries the merchant's series/aa, not ours.
This is the link that — combined with MyDataSCU's suffix rewrite —
lets the country processor detect override paths via EndsWith and
skip the InvoiceNumerator commit.
Existing AADEFactoryTests (using "ft123#" without a country suffix)
continue to exercise the legacy fallback path. Together the three
groups give end-to-end coverage of the new and legacy parsing branches
plus the handwritten interaction.
Refs fiskaltrust/market-gr#172
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ilience)
Three sequence tests that lock in the no-gaps property by exercising
multiple receipts in a row through one country processor instance:
FiveSuccessfulSubmissions_ProduceContinuousAa1Through5
Five successive POS receipts on a fresh queue submit with
aa = 1, 2, 3, 4, 5 in order, with InvoiceNumerator ending at 5.
Direct regression guard against any future change that
accidentally over-advances or under-advances the counter on the
happy path.
RetryAfterErrorReusesSameAa
POS success at aa=1, POS error attempting aa=2, retry succeeds
at aa=2 (the original failed value, not aa=3). Confirms the
reserve-then-commit semantics actually reserve in-memory only
and don't leak a counter advance on failure.
OverrideBetweenAutoReceipts_DoesNotShiftAutoSequence
POS auto at aa=1, override receipt that rewrites the suffix to
something else (simulating handwritten or mydataoverride), POS
auto at aa=2 — NOT aa=3. The override path consumes no auto
aa slot and the next auto receipt continues the sequence
uninterrupted.
Mock helpers (SetupAutoEchoSscdMock, CaptureReservedAa,
MarkAsSuccessKeepingSuffix, OverwriteSuffix) simulate MyDataSCU's
suffix-rewrite behaviour so the country processor's commit decision
exercises the same code path it would in production.
Refs fiskaltrust/market-gr#172
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes fiskaltrust/market-gr#172: QueueGR was deriving the AADE
invoiceHeader.aafrom the genericftReceiptNumerator, which advances for every successful receipt regardless of whether it was actually submitted to myDATA. QueueGR routes daily/monthly/yearly closings, zero receipts, protocol events, and other receipt cases throughGRFallBackOperations.NoOp— those advanceftReceiptNumeratorbut never reach AADE, so the next real submission seesaa = N+2instead ofN+1and the merchant's fiscal sequence has gaps.Phase 1 fix:
InvoiceSeries/InvoiceNumeratoronftQueueGR(1:1 shape match withftQueueES). Only ever advances on actual AADE submission.State.SuccessinReceiptCommandProcessorGR/InvoiceCommandProcessorGR, copying the ES pattern. Skips the commit when the caller bypasses our counter via handwritten payload ormydataoverride(detected by suffix mismatch onftReceiptIdentification).State.SuccessinMyDataSCU. Without this, a process kill between AADE's 200 OK and our storage commit leaves the queue stuck forever; with it, the next retry advances the counter and the queue self-heals.InvoiceNumeratorfromftReceiptNumeratorso the first newaalands strictly above anything AADE already has on file for the queue.InvoiceSeriesfromCashBoxIdentificationfor fresh queues (parity withLifecycleCommandProcessorES.cs:24-30).ftReceiptNumeratorsemantics are unchanged for every country, including GR.Scope notes
QueueGRrepositories alreadythrow new NotImplementedException()for GR and are intentionally left alone.MyDataSCU's sandboxGovernmentApistash onftStateDatais updated to merge rather than replace, so the country-processor'sProposedInvoiceCountersurvives the SCU call.AADEFactorykeeps the legacyftReceiptIdentification-parse path as a fallback so existing tests and direct callers continue to work without modification.scu-gr/MyDatafor the sharedMiddlewareSCUGRMyDataStatePOCO. Namespace sharing in that direction already existed implicitly (fiskaltrust.Middleware.Localization.QueueGR.Validationis declared by files in scu-gr); this makes the dependency explicit.Out of scope (Phase 2, tracked separately)
/myDataProvider/RequestTransmittedDocson 233 (to repopulate the prior submission'smark/uid/authenticationCodeon the resubmitted response).ftActionJournal.Phase-1 advance-on-233 behaviour is correct in practice (
seriesis queue-scoped, so foreign collisions imply a deployment misconfiguration). Phase 2 is operational-quality work on top, not a correctness requirement.Test plan
dotnet test scu-gr/test/.../MyData.UnitTest— 804 tests pass (no regressions).dotnet test queue/test/.../QueueGR.UnitTest— 42 tests pass (34 pre-existing + 8 new).InvoiceCounterReservationTestscovers: success advances the counter, error doesn't, handwritten/override suffix mismatch doesn't, success without mark signature still commits withLastInvoiceMark = null, emptyInvoiceSeriesfalls back toCashBoxIdentification, upgrade migration seeds fromftReceiptNumerator.LifecycleCommandProcessorGRTestscases forInvoiceSeriesseeding (fresh queue + idempotency).POS → DailyClosing → POSproducesaa = N, N+1on the AADE side.🤖 Generated with Claude Code