fix(golang): address the second review of the operator exports

- The catalog reader and se_query_term's reader share one parser each for
  an operator's name, fidelity and index. A name is one of eq, lt, lte,
  gt, gte, between and matches, the set the engine's OpSpec::parse
  accepts. The catalog took any non-empty name, so "like" or "Eq" parsed
  and failed later as unknown_operator from Cipher.QueryTerm, and
  se_query_term took any operator and any index, which the catalog
  refused. Both now refuse the same values, and a test feeds every name
  the engine writes through both.
- The query fixture test checks that query_operators.json was recorded
  under queryPlan, so a fixture regenerated under another plan fails by
  name rather than as mismatched term bytes.

Author: coderdan

languages/golang/encrypt/checker.go

@@ -212,12 +212,9 @@ func targetOperators(value any) ([]record.TargetOperator, error) {
 			}
 			switch f.Key {
 			case "operator":
-				op.Name = s
+				op.Name, err = parseOperatorName(s)
 			case "fidelity":
-				op.Fidelity = record.Fidelity(s)
-				if op.Fidelity != record.Exact && op.Fidelity != record.Probabilistic {
-					return nil, fmt.Errorf("operator %d: unknown fidelity %q", i, s)
-				}
+				op.Fidelity, err = parseFidelity(s)
 			case "arity":
 				switch s {
 				case "one":
@@ -227,15 +224,13 @@ func targetOperators(value any) ([]record.TargetOperator, error) {
 					return nil, fmt.Errorf("operator %d: unknown arity %q", i, s)
 				}
 			case "index":
-				switch o := record.Output(s); o {
-				case record.Equality, record.Match, record.Ore, record.Ope:
-					op.Index = o
-				default:
-					return nil, fmt.Errorf("operator %d: unknown index %q", i, s)
-				}
+				op.Index, err = parseOperatorIndex(s)
 			default:
 				return nil, fmt.Errorf("operator %d: unknown key %q", i, f.Key)
 			}
+			if err != nil {
+				return nil, fmt.Errorf("operator %d: %v", i, err)
+			}
 		}
 		if len(seen) != 4 || op.Name == "" {
 			return nil, fmt.Errorf("operator %d: not every key, or no name", i)
@@ -245,6 +240,38 @@ func targetOperators(value any) ([]record.TargetOperator, error) {
 	return operators, nil
 }
 
+// parseOperatorName reads an operator's name as the engine writes it: one
+// of eq, lt, lte, gt, gte, between and matches, the names the engine's
+// OpSpec::parse accepts. The catalog (se_targets) and se_query_term are
+// read through this, parseFidelity and parseOperatorIndex, so the two
+// readers refuse the same values.
+func parseOperatorName(s string) (string, error) {
+	switch s {
+	case "eq", "lt", "lte", "gt", "gte", "between", "matches":
+		return s, nil
+	}
+	return "", fmt.Errorf("unknown operator %q", s)
+}
+
+// parseFidelity reads an operator's fidelity: exact or probabilistic.
+func parseFidelity(s string) (record.Fidelity, error) {
+	switch f := record.Fidelity(s); f {
+	case record.Exact, record.Probabilistic:
+		return f, nil
+	}
+	return "", fmt.Errorf("unknown fidelity %q", s)
+}
+
+// parseOperatorIndex reads the index that answers an operator: eq, match,
+// ore or ope.
+func parseOperatorIndex(s string) (record.Output, error) {
+	switch o := record.Output(s); o {
+	case record.Equality, record.Match, record.Ore, record.Ope:
+		return o, nil
+	}
+	return "", fmt.Errorf("unknown index %q", s)
+}
+
 func targetString(f vcvalue.Field) (string, error) {
 	s, ok := f.Value.(string)
 	if !ok {

languages/golang/encrypt/query.go

@@ -147,7 +147,8 @@ func (cph *Cipher) QueryTerm(ctx context.Context, plan *record.Plan, field, oper
 
 // parseQueryTerm reads se_query_term's object: exactly the keys operator,
 // fidelity, index, domain (a string or null) and term (bytes, or a list of
-// two for between).
+// two for between). The operator, fidelity and index take only the values
+// the engine writes, as the catalog reader's do.
 func parseQueryTerm(decoded any) (QueryTerm, error) {
 	obj, ok := decoded.(vcvalue.Object)
 	if !ok {
@@ -163,18 +164,20 @@ func parseQueryTerm(decoded any) (QueryTerm, error) {
 		var err error
 		switch f.Key {
 		case "operator":
-			t.Operator, err = targetString(f)
+			var s string
+			if s, err = targetString(f); err == nil {
+				t.Operator, err = parseOperatorName(s)
+			}
 		case "fidelity":
 			var s string
-			s, err = targetString(f)
-			t.Fidelity = record.Fidelity(s)
-			if err == nil && t.Fidelity != record.Exact && t.Fidelity != record.Probabilistic {
-				err = fmt.Errorf("unknown fidelity %q", s)
+			if s, err = targetString(f); err == nil {
+				t.Fidelity, err = parseFidelity(s)
 			}
 		case "index":
 			var s string
-			s, err = targetString(f)
-			t.Index = record.Output(s)
+			if s, err = targetString(f); err == nil {
+				t.Index, err = parseOperatorIndex(s)
+			}
 		case "domain":
 			t.Domain, err = targetOptionalString(f)
 		case "term":

languages/golang/encrypt/query_test.go

@@ -7,10 +7,12 @@ import (
 	"errors"
 	"os"
 	"path/filepath"
+	"reflect"
 	"testing"
 
 	"github.com/cipherstash/stack/languages/golang/encrypt"
 	"github.com/cipherstash/stack/languages/golang/internal/record"
+	"github.com/cipherstash/vitaminc/bindings/go/vcvalue"
 )
 
 // The query fixture holds the guest's query exports to the Rust results:
@@ -25,6 +27,7 @@ type queryFixture struct {
 	KeySource struct {
 		Seed string `json:"seed"`
 	} `json:"key_source"`
+	Plan    json.RawMessage `json:"plan"`
 	Queries []struct {
 		Field    string          `json:"field"`
 		Operator string          `json:"operator"`
@@ -68,9 +71,38 @@ func readQueryFixture(t *testing.T) queryFixture {
 	if err := json.Unmarshal(raw, &f); err != nil {
 		t.Fatal(err)
 	}
+	// The terms are derived under queryPlan: a fixture recorded under any
+	// other plan fails here, by name, rather than as mismatched bytes.
+	var recorded any
+	if err := json.Unmarshal(f.Plan, &recorded); err != nil {
+		t.Fatal(err)
+	}
+	if want := wireJSON(queryPlan.Wire()); !reflect.DeepEqual(recorded, want) {
+		t.Fatalf("the fixture's plan is not queryPlan; update queryPlan to match it:\nfixture:   %v\nqueryPlan: %v", recorded, want)
+	}
 	return f
 }
 
+// wireJSON is a plan's wire form as encoding/json decodes the same JSON:
+// objects as maps, lists as []any.
+func wireJSON(v any) any {
+	switch v := v.(type) {
+	case vcvalue.Object:
+		out := make(map[string]any, len(v))
+		for _, f := range v {
+			out[f.Key] = wireJSON(f.Value)
+		}
+		return out
+	case []any:
+		out := make([]any, len(v))
+		for i, item := range v {
+			out[i] = wireJSON(item)
+		}
+		return out
+	}
+	return v
+}
+
 // queryPlan is the fixture's declaration: an email with equality, an age
 // with equality and ORE, a name with match, all under users.
 var queryPlan = &record.Plan{Context: []string{"users"}, Fields: []record.Field{

languages/golang/encrypt/targets_test.go

@@ -75,6 +75,8 @@ func TestParseTargetsReadsEachEntry(t *testing.T) {
 		"unknown arity":       withOperators([]any{operator("eq", "exact", "three", "eq")}),
 		"unknown op index":    withOperators([]any{operator("eq", "exact", "one", "json")}),
 		"no op name":          withOperators([]any{operator("", "exact", "one", "eq")}),
+		"unknown operator":    withOperators([]any{operator("like", "exact", "one", "eq")}),
+		"operator case":       withOperators([]any{operator("Eq", "exact", "one", "eq")}),
 		"op key missing":      withOperators([]any{vcvalue.Object{{Key: "operator", Value: "eq"}}}),
 		"op unknown key":      withOperators([]any{append(operator("eq", "exact", "one", "eq"), vcvalue.Field{Key: "case", Value: "insensitive"})}),
 		"op key twice":        withOperators([]any{append(operator("eq", "exact", "one", "eq"), vcvalue.Field{Key: "index", Value: "eq"})}),
@@ -97,3 +99,35 @@ func withOperators(value any) vcvalue.Object {
 		{Key: "name", Value: "TextEq"}, {Key: "indexes", Value: []any{}}, {Key: "producible", Value: true}, {Key: "operators", Value: value},
 	}}}}
 }
+
+// se_query_term's object is read with the catalog's own parsers: an
+// operator, fidelity or index the catalog would refuse is refused here too,
+// and every name the engine writes is read by both.
+func TestParseQueryTermTakesOnlyWhatTheEngineWrites(t *testing.T) {
+	term := func(operator, fidelity, index string) vcvalue.Object {
+		return vcvalue.Object{
+			{Key: "operator", Value: operator}, {Key: "fidelity", Value: fidelity}, {Key: "index", Value: index},
+			{Key: "domain", Value: nil}, {Key: "term", Value: []byte{1, 2}},
+		}
+	}
+	for _, name := range []string{"eq", "lt", "lte", "gt", "gte", "between", "matches"} {
+		got, err := parseQueryTerm(term(name, "exact", "ore"))
+		if err != nil || got.Operator != name || got.Fidelity != record.Exact || got.Index != record.Ore {
+			t.Errorf("%s: %+v, %v", name, got, err)
+		}
+		if _, err := parseTargets(withOperators([]any{operator(name, "exact", "one", "ore")})); err != nil {
+			t.Errorf("catalog %s: %v", name, err)
+		}
+	}
+	for name, bad := range map[string]vcvalue.Object{
+		"unknown operator": term("like", "exact", "eq"),
+		"operator case":    term("Eq", "exact", "eq"),
+		"unknown fidelity": term("eq", "approximate", "eq"),
+		"unknown index":    term("eq", "exact", "json"),
+		"misspelled index": term("eq", "exact", "equality"),
+	} {
+		if _, err := parseQueryTerm(bad); err == nil {
+			t.Errorf("%s: read, want refused", name)
+		}
+	}
+}

packages/stack-encrypt/tests/ui/op_at_path_not_a_leaf.stderr

@@ -9,12 +9,12 @@ error[E0277]: `Contains` is not a leaf operator, so it cannot be applied at a pa
   = help: the trait `Leaf` is not implemented for `Contains`
   = note: `op::AtPath` applies `op::Eq`, `Lt`, `Lte`, `Gt`, `Gte`, `Between` or `Matches` at a path; `op::Contains` and `op::ContainedBy` compare whole documents, and an accessor does not nest
   = help: the following other types implement trait `Leaf`:
-            Between
-            Gt
-            Gte
-            Lt
-            Lte
+            stack_encrypt::op::Between
             stack_encrypt::op::Eq
+            stack_encrypt::op::Gt
+            stack_encrypt::op::Gte
+            stack_encrypt::op::Lt
+            stack_encrypt::op::Lte
             stack_encrypt::op::Matches<C>
   = note: required for `AtPath<Contains>` to implement `Operator`
 note: required by a bound in `operator`
@@ -34,12 +34,12 @@ error[E0277]: `AtPath<stack_encrypt::op::Eq>` is not a leaf operator, so it cann
    = help: the trait `Leaf` is not implemented for `AtPath<stack_encrypt::op::Eq>`
    = note: `op::AtPath` applies `op::Eq`, `Lt`, `Lte`, `Gt`, `Gte`, `Between` or `Matches` at a path; `op::Contains` and `op::ContainedBy` compare whole documents, and an accessor does not nest
    = help: the following other types implement trait `Leaf`:
-             Between
-             Gt
-             Gte
-             Lt
-             Lte
+             stack_encrypt::op::Between
              stack_encrypt::op::Eq
+             stack_encrypt::op::Gt
+             stack_encrypt::op::Gte
+             stack_encrypt::op::Lt
+             stack_encrypt::op::Lte
              stack_encrypt::op::Matches<C>
    = note: required for `AtPath<AtPath<stack_encrypt::op::Eq>>` to implement `Operator`
 note: required by a bound in `operator`

packages/stack-encrypt/tests/ui/op_between_takes_a_pair.stderr

@@ -22,7 +22,7 @@ note: required by a bound in `plan::chain::<impl StackCipher<K>>::query`
    |         F<Q>: Input<Op::Arity>,
    |               ^^^^^^^^^^^^^^^^ required by this bound in `plan::chain::<impl StackCipher<K>>::query`
 
-error[E0599]: the method `on` exists for struct `QueryBuilder<'_, Between, u32, FakeDataKeySource>`, but its trait bounds were not satisfied
+error[E0599]: the method `on` exists for struct `QueryBuilder<'_, stack_encrypt::op::Between, u32, FakeDataKeySource>`, but its trait bounds were not satisfied
   --> tests/ui/op_between_takes_a_pair.rs:12:47
    |
 12 |     let _ = cipher.query(op::Between, &40u32).on(&age_plan);
@@ -39,7 +39,7 @@ error[E0271]: type mismatch resolving `<(u32, {integer}) as Input<One>>::Plainte
    |
    = note: expected type `u32`
              found tuple `(u32, {integer})`
-   = note: required for `ValuePlan<u32, Indexed<stack_encrypt::Ore>>` to implement `Chooses<Lt, (u32, {integer}), Whole>`
+   = note: required for `ValuePlan<u32, Indexed<stack_encrypt::Ore>>` to implement `Chooses<stack_encrypt::op::Lt, (u32, {integer}), Whole>`
 
 error[E0277]: `(u32, {integer})` is not ordered: the ordering operators are answered over the integer types only
   --> tests/ui/op_between_takes_a_pair.rs:13:48
@@ -59,5 +59,5 @@ error[E0277]: `(u32, {integer})` is not ordered: the ordering operators are answ
              u16
              u32
            and $N others
-   = note: required for `Lt` to implement `DefaultIndex<(u32, {integer})>`
-   = note: required for `ValuePlan<u32, Indexed<stack_encrypt::Ore>>` to implement `Chooses<Lt, (u32, {integer}), Whole>`
+   = note: required for `stack_encrypt::op::Lt` to implement `DefaultIndex<(u32, {integer})>`
+   = note: required for `ValuePlan<u32, Indexed<stack_encrypt::Ore>>` to implement `Chooses<stack_encrypt::op::Lt, (u32, {integer}), Whole>`

packages/stack-encrypt/tests/ui/op_json_not_answered.stderr

@@ -16,7 +16,7 @@ error[E0277]: `Contains` has no default index given `std::string::String`
              `stack_encrypt::op::Matches<<DefaultMatch as stack_encrypt::op::MatchCase>::Case>` implements `DefaultIndex<S>`
    = note: required for `ValuePlan<std::string::String, Indexed<stack_encrypt::Equality>>` to implement `Chooses<Contains, std::string::String, Whole>`
 
-error[E0277]: `(&str, &str)` is not what a query of arity `Pathed<One>` takes
+error[E0277]: `(&str, &str)` is not what a query of arity `Pathed<stack_encrypt::op::One>` takes
   --> tests/ui/op_json_not_answered.rs:15:36
    |
 15 |         .query(op::AtPath(op::Eq), &("$.address.city", "Sydney"))
@@ -25,13 +25,13 @@ error[E0277]: `(&str, &str)` is not what a query of arity `Pathed<One>` takes
    |          required by a bound introduced by this call
    |
    = note: an operator of arity `One` takes one value, `op::Between` (arity `Pair`) takes a pair, `&(lo, hi)`, and `op::AtPath` (arity `Pathed`) takes no input yet: it arrives with the JSON index
-help: the trait `Input<Pathed<One>>` is not implemented for `(&str, &str)`
+help: the trait `Input<Pathed<stack_encrypt::op::One>>` is not implemented for `(&str, &str)`
       but trait `Input<Pair>` is implemented for it
   --> src/op/mod.rs
    |
    | impl<S: 'static> Input<Pair> for (S, S) {
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   = help: for that trait implementation, expected `Pair`, found `Pathed<One>`
+   = help: for that trait implementation, expected `Pair`, found `Pathed<stack_encrypt::op::One>`
 note: required by a bound in `plan::chain::<impl StackCipher<K>>::query`
   --> src/plan/chain.rs
    |
@@ -51,4 +51,4 @@ error[E0599]: the method `on` exists for struct `QueryBuilder<'_, AtPath<stack_e
    | |_________-^^
    |
    = note: the following trait bounds were not satisfied:
-           `(&str, &str): Input<Pathed<One>>`
+           `(&str, &str): Input<Pathed<stack_encrypt::op::One>>`