amadan

oficina / suite Public

Require complete S2 answer fields before matching revisions

Missing or null wire fields now fail the response closed to UNKNOWN instead of inventing a zero-valued item revision. Explicit numeric revision zero remains valid.

Validation: regression tests failed on original main; suite tests and vet pass. Mutating the guard to skip incomplete rows, and rejecting explicit revision zero, each failed the new tests.
Paul Campbell pushed by paul@keymail.dev ef402bdc7378bfc7ed1ee403b35ba331b81c5f85 parent 56324a5
2 files changed, +58 −5
  • s2.go +8 −5
  • s2_required_fields_test.go +50 −0
diff --git a/s2.go b/s2.go
index 4a99835..0ad865c 100644
--- a/s2.go
+++ b/s2.go
@@ -30,9 +30,9 @@ type ItemRevision struct {
}
type s2Result struct {
- ItemID string `json:"item_id"`
- Revision int64 `json:"revision"`
- Result Authorization `json:"result"`
+ ItemID *string `json:"item_id"`
+ Revision *int64 `json:"revision"`
+ Result *Authorization `json:"result"`
}
type s2Body struct {
@@ -60,7 +60,10 @@ func DecodeS2(asked []ItemRevision, status int, body []byte) []Authorization {
byPair := make(map[ItemRevision]Authorization, len(parsed.Results))
for _, r := range parsed.Results {
- pair := ItemRevision{ItemID: r.ItemID, Revision: r.Revision}
+ if r.ItemID == nil || r.Revision == nil || r.Result == nil {
+ return allUnknown(len(asked))
+ }
+ pair := ItemRevision{ItemID: *r.ItemID, Revision: *r.Revision}
if _, already := byPair[pair]; already {
// Answered twice. Two answers are not an answer, and choosing
// between them would be this client making a decision the app of
@@ -68,7 +71,7 @@ func DecodeS2(asked []ItemRevision, status int, body []byte) []Authorization {
byPair[pair] = UNKNOWN
continue
}
- byPair[pair] = known(r.Result)
+ byPair[pair] = known(*r.Result)
}
out := make([]Authorization, len(asked))
diff --git a/s2_required_fields_test.go b/s2_required_fields_test.go
new file mode 100644
index 0000000..f33ad2a
--- /dev/null
+++ b/s2_required_fields_test.go
@@ -0,0 +1,50 @@
+package suite
+
+import (
+ "fmt"
+ "testing"
+)
+
+func TestS2RequiresEveryAnswerField(t *testing.T) {
+ asked := []ItemRevision{{ItemID: "doc", Revision: 0}, {ItemID: "other", Revision: 3}}
+ valid := `{"item_id":"other","revision":3,"result":"VISIBLE"}`
+ for _, row := range []string{
+ `{"item_id":"doc","result":"VISIBLE"}`,
+ `{"item_id":"doc","revision":null,"result":"VISIBLE"}`,
+ `{"revision":0,"result":"VISIBLE"}`,
+ `{"item_id":null,"revision":0,"result":"VISIBLE"}`,
+ `{"item_id":"doc","revision":0}`,
+ `{"item_id":"doc","revision":0,"result":null}`,
+ `null`,
+ } {
+ for _, rows := range []string{row + "," + valid, valid + "," + row} {
+ t.Run(rows, func(t *testing.T) {
+ body := []byte(`{"results":[` + rows + `]}`)
+ assertResults(t, DecodeS2(asked, 200, body), []Authorization{UNKNOWN, UNKNOWN})
+ })
+ }
+ }
+}
+
+func TestS2ExplicitZeroRevisionRemainsValid(t *testing.T) {
+ for _, result := range []Authorization{VISIBLE, NOT_VISIBLE, STALE, UNKNOWN} {
+ body := []byte(fmt.Sprintf(`{"results":[{"item_id":"doc","revision":0,"result":%q}]}`, result))
+ assertResults(t, DecodeS2([]ItemRevision{{ItemID: "doc", Revision: 0}}, 200, body), []Authorization{result})
+ }
+}
+
+func TestS2IncompleteDuplicateCannotBeDiscarded(t *testing.T) {
+ valid := `{"item_id":"doc","revision":0,"result":"VISIBLE"}`
+ incomplete := `{"item_id":"doc","result":"NOT_VISIBLE"}`
+ for _, rows := range []string{valid + "," + incomplete, incomplete + "," + valid} {
+ body := []byte(`{"results":[` + rows + `]}`)
+ assertResults(t, DecodeS2([]ItemRevision{{ItemID: "doc", Revision: 0}}, 200, body), []Authorization{UNKNOWN})
+ }
+}
+
+func TestS2MissingIdentityDoesNotMatchEmptyAskedIdentity(t *testing.T) {
+ for _, row := range []string{`{"revision":0,"result":"VISIBLE"}`, `{"item_id":null,"revision":0,"result":"VISIBLE"}`} {
+ body := []byte(`{"results":[` + row + `]}`)
+ assertResults(t, DecodeS2([]ItemRevision{{ItemID: "", Revision: 0}}, 200, body), []Authorization{UNKNOWN})
+ }
+}