Harden the ddi-emitter path: allowlist, error handling, examples, lossless pass-through (#10, #11, #13, #14) - #16
Merged
Merged
Conversation
…errors (#10, #11, #14) ddi-emitter: run XLSValidator.validateSubset(…, { target: 'ddi' }) before buildDdiXml. buildDdiXml alone converts anything, so `rank`, `geopoint` and selects without choices came out as plausible but wrong DDI. Their messages now come back as a 400. ddi_client.go: - Cut the <dataDscr> children out of formtransform's <codeBook> as a token stream, not by decoding into DDIVar/DDIVarGrp and marshalling again. The structs dropped everything they don't model (notes, valrng, universe, xml:lang, …) and reordered varGrps before vars. Only the DDI namespace and the dangling `files` IDREFs are removed. - An empty <dataDscr> (a form of only notes) is an input error, not a 200 with an empty element. - ErrConverterUnavailable marks sidecar outages, bad status codes and malformed responses. All other errors describe the input and carry the library's message without a prefix. routes.go: the convert endpoint passes input errors on as the 400 message, as #3 asked for, and answers 503 (logged) when the sidecar is unavailable. Unit tests for the client and the route use a fake sidecar, so they run without docker-compose. Closes #10 Closes #11 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
#13) When populate failed, /api/examples answered 200 [] and /api/examples/{type} 404, nothing was logged, and every request took the exclusive lock and re-ran all conversions against the dead sidecar. - GetAll/GetByType return an error while the sidecar is unavailable; both routes answer 503. - populate stops at the first ErrConverterUnavailable and the failure is remembered for 30s, so requests during an outage don't hammer the sidecar or queue behind each other. - An example the converter rejects is logged and left out instead of emptying the whole list. Closes #13 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
scripts/equivalence-test.mjs only compared var/group shape (counts, IDs, intrvl, responseDomainType), so lost text, categories or instructions couldn't fail it. It now compares every <var>/<varGrp> after normalizing serializer differences only: whitespace, self-closing tags, entity spelling, `files` and namespace declarations. Checked with a shim that adds one <ivuInstr>: 3 cases fail, where the shape check passed. - New cases: note before a question (a note alone is now a 400), hint/guidance_hint (#12), relevant/required, special characters. - Defaults to the formtransform that ddi-emitter pins, not a /tmp path. - QWACBACK_DELAY_MS overrides the request spacing. 17/17 identical against this branch. New .github/workflows/test.yml runs `go test ./...` with NATS, the published schematron-worker and ddi-emitter up, fails on any skipped test, then runs the equivalence test against a qwacback built from the branch. Until now the integration and sidecar tests skipped everywhere, including the Dockerfile's `go test`. Closes #14 Co-Authored-By: Claude Opus 5.5 <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.
Closes #10, closes #11, closes #13, closes #14.
Summary
#10: enforce formtransform's allowlist.
buildDdiXmlconverts any input it gets; the allowlist lives inXLSValidator.validateSubset.ddi-emitter/server.mjsnow callsvalidateSubset(…, { target: 'ddi' })first.rank,geopointand selects without choices now get a 400 with the library's message instead of plausible-looking but wrong DDI. A form of only notes (empty<dataDscr>) is also a 400 now; it used to be a 200 with an empty element. All 9 built-in examples pass the check.#11: error detail and 503.
/api/convert/xlsform-to-ddinow includes the library's message in the 400 response ("…question "a"…"). A sidecar that is down or failing now gives a logged 503. Before, it gave a 400.converter.ErrConverterUnavailabletells server-side failures apart from input errors.#13: examples.
/api/examplesreturns 503 while the sidecar is down; before, it returned 200[], and/{type}returned 404. A failure is remembered for 30s, so requests during an outage don't each rerun every conversion while holding the lock. An example the converter rejects is logged and left out; the other examples are still served.#14: lossless pass-through and a real equivalence test.
ddi_client.gocopies the<dataDscr>children as a token stream instead of decoding them intoDDIVar/DDIVarGrpand encoding them again. Only the namespace and thefilesattributes are removed;filespoints at the<fileDscr>that isn't in the fragment.scripts/equivalence-test.mjsnow compares the full normalized<var>/<varGrp>content, not just the shape. It has 4 new cases: a note before a question, hint/guidance_hint, relevant/required, and special characters..github/workflows/test.ymlrunsgo test ./...with NATS, the published worker and the sidecar up, and fails if any test skips. It then runs the equivalence test against a qwacback built from this branch.CONVERSION_API.mddocuments the 400/503 behavior, the pass-through, and thathint/guidance_hintare currently dropped (#12, needs a formtransform fix).Verification
go vet ./...is clean.schematron-worker:v0.1.7running locally:go test ./... -vgave 111 passed, 0 skipped, 0 failed.go test.<ivuInstr>: 3 cases fail. The old shape-only check passed them.test.ymlon GitHub Actions. I ran its steps by hand on my machine.test.ymldoesn't include the pin check from #15, because that script lands there. Add the step once both are merged.🤖 Generated with Claude Code