amadan

oficina / suite Public

Merge branch 'fix/s2-required-fields' into main

Require explicit S2 response identity revision and result
claude pushed by paul@keymail.dev bbc335ce78730c7c9dae3fcf5de7897a3d985311 parent 56324a5parent ddfb3f2
3 files changed, +60 −7
  • mutations.py +2 −2
  • s2.go +8 −5
  • s2_required_fields_test.go +50 −0

CI passed — run details

CI log 
=== gate ===
ok  	amadan.net/oficina/suite	0.005s
ok  	amadan.net/oficina/suite/conformance	0.004s
diff --git a/mutations.py b/mutations.py
index 219c06f..c9993f2 100644
--- a/mutations.py
+++ b/mutations.py
@@ -14,8 +14,8 @@ MUTATIONS = [
"\tif status < 200 || status > 299 {\n\t\treturn nil\n\t}"),
("12a: the revision is ignored when matching answers to questions", "s2.go",
- "\t\tpair := ItemRevision{ItemID: r.ItemID, Revision: r.Revision}",
- "\t\tpair := ItemRevision{ItemID: r.ItemID}"),
+ "\t\tpair := ItemRevision{ItemID: *r.ItemID, Revision: *r.Revision}",
+ "\t\tpair := ItemRevision{ItemID: *r.ItemID}"),
("12b: STALE is dropped from the closed vocabulary", "s2.go",
"\tcase VISIBLE, NOT_VISIBLE, STALE, UNKNOWN:",
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})
+ }
+}