Repository navigation
fix(modifiers): read a stored field with no value as null at the end of the read - #113
Merged
Merged
Conversation
…r saved again keeps the fallback
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
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.
Why
#111 made a
@Localized()field read with no locale answernull, which DMS'sfiles/route-answerstest needs. It produced thatnullinside the modifier chain, and two problems were accepted for 0.3.1 (listed on #111):@Localized()receivednullinunlock. A customunlock, ortoJSONfor the row, could throw.EncryptionModifier.unlock's published parameter widened toLockedType | null.This PR produces the
nullat the end of the read instead, so no modifier ever receives it.Summary
nullwhen its modifiers find no value, or when it storesnull.undefined.unlockreceivesundefinedornull. That covers the read,set's previous-value unlock andtestValue. As a side effect, the first assignment to a stacked field such as@Localized() @Encrypted()on a row that stores nothing no longer throws. It threw onmaintoo.nullunder a locale is stored as no text, as""already was. A row read with no text answersnull, so saving that answer again (GET, then PUT, or thelock()helper) stores nothing, and thefallbackLocalestill answers. The round-1 review found that this write-back blocked the fallback once reads answerednull.docs/3.table-modifiers.mdand the shipped skill say all of this.Behaviour change, minor release (0.4.0)
localize("de")on a field with nodetext and nofallbackLocaleundefined(left out of JSON)nullundefinednull@Encrypted()field stored asnullnullnull(no longer decrypted)@Localized()field (#111)nullnullundefinedundefinednullassigned under a locale{ de: null }Evidence
Each new test fails on
main(3b3ff72, or on 74a193b for the write-back tests) and passes on this branch.Finding 2. The test types its override from
Parameters<EncryptionModifier["unlock"]>[0]and readsstored[1]:Finding 1, and the
localize("de")change (src/tests/end_of_read_null.test.ts):Write-back of a
nullread (round-1 review finding). Before is 74a193b, the first commit of this branch:Stored
nullon an@Encrypted()field (round-1 review finding; 0.3.2 handled it insideunlock):Stacked field and
testValue(round-2 review findings; the assignment also throws onmain):#111's own test still passes:✔ answers null for a field the request did not send.DMS
files/route-answersagainst the packed head (ca69b81), linked into a fresh dmsmainclone (ffcee020) through apnpm.overridesfile:tarball. A load tracer confirmed the patcheddist/modifiers/common.jswas the only copy loaded:Other checks:
pnpm run checkspasses all 11 checks.antelopejs-verify journey --planpasses all 73 journeys.Merge Danger
Door: two-way. No stored schema changes. Rows written by 0.3.x read the same, except where the table above says otherwise.
Blast Radius: consumers.
=== undefinedto detect a missing locale now getsnull. Hence the minor release."field": null.Attention
@Localized() @Encrypted()field, anullread posted back under a locale is encrypted before the localization modifier sees it. It is stored as encryptednullunder that locale, and it still blocks thefallbackLocale. No AntelopeJS repo stacks the two outside tests and verify playgrounds. Fixing it would need a write-side rule fornullin every two-way modifier, so it is left for a follow-up.