fix(advice): an empty token must not be used as a guard
None of the 47 existing readings have a token - they all predate it. A
reading read from the database therefore has an EMPTY token, and the
guard was checking `expected_token is not None`, which is TRUE for an
empty string. So the write was conditioned on a token that does not
exist, matched nothing, and **the advice would never have been written
for any of the existing readings**.
Verified against the live database, before the fix:
write with expected_token='' -> 0 (0 = the advice would NEVER be written)
After:
write for a pre-token live reading: 1 (must be 1)
An empty token is now not a usable guard - there is no token to match -
so the write falls through to the gravity check instead. A non-empty
token is still the primary guard.
There is a test for it, with a reading inserted the way the existing
ones are - directly, with no token.
178 tests (up from 177).
This commit is contained in:
@@ -263,10 +263,15 @@ class BeerRepository:
|
|||||||
return 0
|
return 0
|
||||||
|
|
||||||
query: dict = {"_id": {"$in": doc_id}}
|
query: dict = {"_id": {"$in": doc_id}}
|
||||||
if expected_token is not None:
|
if expected_token:
|
||||||
# The editing token is the PRIMARY guard: it changes on every
|
# The editing token is the PRIMARY guard: it changes on every
|
||||||
# edit, so it catches a stale generation whatever else
|
# edit, so it catches a stale generation whatever else
|
||||||
# changed - the gravity, the comment, or nothing.
|
# changed - the gravity, the comment, or nothing.
|
||||||
|
#
|
||||||
|
# An EMPTY token is not a usable guard: it means the reading
|
||||||
|
# predates the token, so there is no token to match.
|
||||||
|
# Querying on it would match nothing, and the advice would
|
||||||
|
# never be written for the existing readings.
|
||||||
query["advice_token"] = str(expected_token)
|
query["advice_token"] = str(expected_token)
|
||||||
elif expected_sg is not None:
|
elif expected_sg is not None:
|
||||||
# No token - a direct call with a gravity to check against.
|
# No token - a direct call with a gravity to check against.
|
||||||
|
|||||||
@@ -710,6 +710,41 @@ def test_the_token_is_the_primary_guard(repo):
|
|||||||
assert after.get("advice") is None
|
assert after.get("advice") is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_advice_is_written_for_a_reading_that_predates_the_token(repo):
|
||||||
|
"""None of the existing readings have a token.
|
||||||
|
|
||||||
|
They all predate it, so a reading read from the database has an EMPTY
|
||||||
|
token. An empty token is not a usable guard - there is no token to
|
||||||
|
match - so the write must not be conditioned on it, or the advice
|
||||||
|
would never be written for any of them.
|
||||||
|
"""
|
||||||
|
# Inserted directly, the way the readings already in the database
|
||||||
|
# are - no token.
|
||||||
|
repo.readings.insert_one({
|
||||||
|
"_id": "l78ZidBJ3QaYn98tvB88x4uM",
|
||||||
|
"beer_run_id": "1",
|
||||||
|
"beer_type": "Ale",
|
||||||
|
"sg": "1042",
|
||||||
|
"date": "2024-09-01",
|
||||||
|
"final_reading": "False",
|
||||||
|
"comment": "a comment",
|
||||||
|
})
|
||||||
|
|
||||||
|
run = repo.run("1")
|
||||||
|
assert run.readings[0].advice_token == "", "the reading should have no token"
|
||||||
|
|
||||||
|
# The advice flow passes the token it read, which is empty.
|
||||||
|
written = repo.set_reading_advice("l78ZidBJ3QaYn98tvB88x4uM",
|
||||||
|
"THE ADVICE", True,
|
||||||
|
expected_sg=None,
|
||||||
|
expected_token=run.readings[0].advice_token)
|
||||||
|
assert written == 1, "the advice was never written for a pre-token reading"
|
||||||
|
|
||||||
|
after = repo.readings.find_one({"beer_run_id": "1"})
|
||||||
|
assert after["advice"] == "THE ADVICE"
|
||||||
|
assert after["comment"] == "a comment"
|
||||||
|
|
||||||
|
|
||||||
def test_a_reply_cut_off_at_the_token_ceiling_is_marked(repo):
|
def test_a_reply_cut_off_at_the_token_ceiling_is_marked(repo):
|
||||||
"""A reply that stopped at the token limit is incomplete.
|
"""A reply that stopped at the token limit is incomplete.
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user