From 1c5e6e69641fa85643fe731b7845328d4f1cd437 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Olivi=C3=A9?= Date: Tue, 18 Aug 2026 09:14:18 +0000 Subject: [PATCH 1/4] Add calculated value discrepancy checks --- calculate.go | 385 ++++++++++++++++++++++++++++++++++++++++++++++ calculate_test.go | 138 +++++++++++++++++ 2 files changed, 523 insertions(+) create mode 100644 calculate.go create mode 100644 calculate_test.go diff --git a/calculate.go b/calculate.go new file mode 100644 index 000000000..747a2e1f9 --- /dev/null +++ b/calculate.go @@ -0,0 +1,385 @@ +package gobl + +import ( + "bytes" + "encoding/json" + "fmt" + "reflect" + "sort" + "strings" + + "github.com/invopop/gobl/schema" +) + +// CalculateOption configures the package-level Calculate operation. +type CalculateOption func(*calculateOptions) + +type calculateOptions struct { + discrepancies bool +} + +// WithDiscrepancies asks Calculate to reject explicitly supplied calculated +// fields that do not match the values produced by GOBL. The returned error can +// be matched to CalculationDiscrepancies with errors.As. +func WithDiscrepancies() CalculateOption { + return func(opts *calculateOptions) { + opts.discrepancies = true + } +} + +// Calculate parses and calculates a GOBL document or envelope. By default it +// performs the same calculation as the value's Calculate method. +// +// WithDiscrepancies additionally rejects explicitly supplied calculated +// fields that do not match the values produced by GOBL. The returned error can +// be matched to CalculationDiscrepancies with errors.As. +func Calculate(data []byte, options ...CalculateOption) (any, error) { + opts := new(calculateOptions) + for _, option := range options { + if option != nil { + option(opts) + } + } + + if opts.discrepancies { + value, discrepancies, err := calculateWithDiscrepancies(data) + if err != nil { + return nil, err + } + if len(discrepancies) > 0 { + return nil, discrepancies + } + return value, nil + } + + value, err := Parse(data) + if err != nil { + return nil, err + } + if err := calculateParsed(value); err != nil { + return nil, err + } + return value, nil +} + +// CalculationDiscrepancy describes a calculated value supplied by the caller +// that did not match the value produced by GOBL. +type CalculationDiscrepancy struct { + Path string `json:"path"` + Provided json.RawMessage `json:"provided"` + Calculated json.RawMessage `json:"calculated"` +} + +// CalculationDiscrepancies contains the calculated values that GOBL changed. +// An empty list means that every calculated value explicitly present in the +// input matched the result. Calculated values omitted from the input are not +// discrepancies. +type CalculationDiscrepancies []*CalculationDiscrepancy + +// Error provides a human-readable summary suitable for logs. Callers should +// use the structured discrepancy fields when preparing an API response. +func (ds CalculationDiscrepancies) Error() string { + if len(ds) == 0 { + return "" + } + if len(ds) == 1 { + return ds[0].Error() + } + return fmt.Sprintf("%d calculation discrepancies", len(ds)) +} + +// Error provides a human-readable description of the discrepancy. +func (d *CalculationDiscrepancy) Error() string { + return fmt.Sprintf( + "%s: provided %s does not match calculated %s", + d.Path, + string(d.Provided), + string(d.Calculated), + ) +} + +// CalculateWithDiscrepancies parses and calculates a GOBL document or +// envelope, returning both the calculated value and any calculated fields that +// were explicitly supplied with a different value. +// +// Fields are considered calculated when they or one of their parents has the +// `calculated=true` JSON Schema annotation. Values are compared semantically +// when their type provides an Equals method, so equivalent representations +// such as monetary amounts with different precision do not produce a +// discrepancy. +func calculateWithDiscrepancies(data []byte) (any, CalculationDiscrepancies, error) { + provided, err := Parse(data) + if err != nil { + return nil, nil, err + } + calculated, err := Parse(data) + if err != nil { + return nil, nil, err + } + + if err := calculateParsed(calculated); err != nil { + return nil, nil, err + } + result := calculated + + var raw json.RawMessage = data + path := "$" + if env, ok := calculated.(*Envelope); ok { + var root map[string]json.RawMessage + if err := json.Unmarshal(data, &root); err != nil { + return nil, nil, ErrInput.WithCause(err) + } + raw = root["doc"] + path = "$.doc" + provided = calculationPayload(provided) + calculated = calculationPayload(env) + } + + ds := make(CalculationDiscrepancies, 0) + compareCalculated(raw, reflect.ValueOf(provided), reflect.ValueOf(calculated), path, false, &ds) + return result, ds, nil +} + +func calculateParsed(obj any) error { + if env, ok := obj.(*Envelope); ok { + return env.Calculate() + } + doc, err := schema.NewObject(obj) + if err != nil { + return wrapError(err) + } + if err := doc.Calculate(); err != nil { + return ErrCalculation.WithCause(err) + } + return nil +} + +func calculationPayload(obj any) any { + if env, ok := obj.(*Envelope); ok { + return env.Extract() + } + return obj +} + +func compareCalculated(raw json.RawMessage, before, after reflect.Value, path string, inherited bool, out *CalculationDiscrepancies) { + if isJSONNull(raw) { + return + } + + before = indirectValue(before) + after = indirectValue(after) + + switch firstJSONByte(raw) { + case '{': + compareCalculatedObject(raw, before, after, path, inherited, out) + case '[': + compareCalculatedArray(raw, before, after, path, inherited, out) + default: + if inherited && !valuesEqual(before, after) { + *out = append(*out, newCalculationDiscrepancy(path, before, after)) + } + } +} + +func compareCalculatedObject(raw json.RawMessage, before, after reflect.Value, path string, inherited bool, out *CalculationDiscrepancies) { + var fields map[string]json.RawMessage + if err := json.Unmarshal(raw, &fields); err != nil { + return + } + + typ := valueType(before, after) + if typ == nil || typ.Kind() != reflect.Struct { + if inherited && !valuesEqual(before, after) { + *out = append(*out, newCalculationDiscrepancy(path, before, after)) + } + return + } + + compareStructFields(fields, before, after, typ, path, inherited, out) +} + +func compareStructFields(raw map[string]json.RawMessage, before, after reflect.Value, typ reflect.Type, path string, inherited bool, out *CalculationDiscrepancies) { + for i := range typ.NumField() { + field := typ.Field(i) + if !field.IsExported() { + continue + } + name, embedded := jsonFieldName(field) + if name == "-" { + continue + } + if embedded { + compareStructFields(raw, fieldValue(before, i), fieldValue(after, i), indirectType(field.Type), path, inherited || isCalculatedField(field), out) + continue + } + value, present := raw[name] + if !present || isJSONNull(value) { + continue + } + compareCalculated( + value, + fieldValue(before, i), + fieldValue(after, i), + appendPath(path, name), + inherited || isCalculatedField(field), + out, + ) + } +} + +func compareCalculatedArray(raw json.RawMessage, before, after reflect.Value, path string, inherited bool, out *CalculationDiscrepancies) { + var values []json.RawMessage + if err := json.Unmarshal(raw, &values); err != nil { + return + } + for i, value := range values { + compareCalculated( + value, + indexValue(before, i), + indexValue(after, i), + fmt.Sprintf("%s[%d]", path, i), + inherited, + out, + ) + } +} + +func valuesEqual(before, after reflect.Value) bool { + if !before.IsValid() || !after.IsValid() { + return before.IsValid() == after.IsValid() + } + before = indirectValue(before) + after = indirectValue(after) + if !before.IsValid() || !after.IsValid() { + return before.IsValid() == after.IsValid() + } + if before.Type() != after.Type() { + return false + } + if equal, ok := callEquals(before, after); ok { + return equal + } + return reflect.DeepEqual(before.Interface(), after.Interface()) +} + +func callEquals(before, after reflect.Value) (bool, bool) { + method := before.MethodByName("Equals") + if !method.IsValid() { + return false, false + } + typ := method.Type() + if typ.NumIn() != 1 || typ.In(0) != after.Type() || typ.NumOut() != 1 || typ.Out(0).Kind() != reflect.Bool { + return false, false + } + result := method.Call([]reflect.Value{after}) + return result[0].Bool(), true +} + +func newCalculationDiscrepancy(path string, before, after reflect.Value) *CalculationDiscrepancy { + return &CalculationDiscrepancy{ + Path: path, + Provided: marshalValue(before), + Calculated: marshalValue(after), + } +} + +func marshalValue(value reflect.Value) json.RawMessage { + value = indirectValue(value) + if !value.IsValid() { + return json.RawMessage("null") + } + data, err := json.Marshal(value.Interface()) + if err != nil { + return json.RawMessage("null") + } + return data +} + +func valueType(values ...reflect.Value) reflect.Type { + for _, value := range values { + value = indirectValue(value) + if value.IsValid() { + return value.Type() + } + } + return nil +} + +func indirectType(typ reflect.Type) reflect.Type { + for typ != nil && (typ.Kind() == reflect.Pointer || typ.Kind() == reflect.Interface) { + typ = typ.Elem() + } + return typ +} + +func indirectValue(value reflect.Value) reflect.Value { + for value.IsValid() && (value.Kind() == reflect.Pointer || value.Kind() == reflect.Interface) { + if value.IsNil() { + return reflect.Value{} + } + value = value.Elem() + } + return value +} + +func fieldValue(value reflect.Value, index int) reflect.Value { + value = indirectValue(value) + if !value.IsValid() || value.Kind() != reflect.Struct || index >= value.NumField() { + return reflect.Value{} + } + return value.Field(index) +} + +func indexValue(value reflect.Value, index int) reflect.Value { + value = indirectValue(value) + if !value.IsValid() || (value.Kind() != reflect.Slice && value.Kind() != reflect.Array) || index >= value.Len() { + return reflect.Value{} + } + return value.Index(index) +} + +func jsonFieldName(field reflect.StructField) (string, bool) { + tag := field.Tag.Get("json") + name := strings.Split(tag, ",")[0] + if name == "-" { + return "-", false + } + if name != "" { + return name, false + } + if field.Anonymous { + return "", true + } + return field.Name, false +} + +func isCalculatedField(field reflect.StructField) bool { + return hasCalculatedAnnotation(field.Tag.Get("jsonschema_extras")) || + hasCalculatedAnnotation(field.Tag.Get("jsonschema")) +} + +func hasCalculatedAnnotation(tag string) bool { + parts := strings.Split(tag, ",") + sort.Strings(parts) + i := sort.SearchStrings(parts, "calculated=true") + return i < len(parts) && parts[i] == "calculated=true" +} + +func appendPath(path, name string) string { + if path == "$" { + return "$." + name + } + return path + "." + name +} + +func firstJSONByte(raw json.RawMessage) byte { + trimmed := bytes.TrimSpace(raw) + if len(trimmed) == 0 { + return 0 + } + return trimmed[0] +} + +func isJSONNull(raw json.RawMessage) bool { + return bytes.Equal(bytes.TrimSpace(raw), []byte("null")) +} diff --git a/calculate_test.go b/calculate_test.go new file mode 100644 index 000000000..470e07160 --- /dev/null +++ b/calculate_test.go @@ -0,0 +1,138 @@ +package gobl + +import ( + "encoding/json" + "testing" + + "github.com/invopop/gobl/bill" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const calculationInvoice = `{ + "$schema": "https://gobl.org/draft-0/bill/invoice", + "$regime": "NL", + "currency": "EUR", + "issue_date": "2022-07-12", + "supplier": { + "tax_id": {"country": "NL", "code": "000099995B57"}, + "name": "Foobar BV" + }, + "lines": [{ + "quantity": "2", + "item": {"name": "Tulips", "price": "10.00"} + }] +}` + +func TestCalculateWithDiscrepancies(t *testing.T) { + t.Run("omitted calculated values", func(t *testing.T) { + result, discrepancies, err := calculateWithDiscrepancies([]byte(calculationInvoice)) + require.NoError(t, err) + assert.Empty(t, discrepancies) + inv := result.(*bill.Invoice) + require.NotNil(t, inv.Totals) + assert.Equal(t, "20.00", inv.Totals.Payable.String()) + }) + + t.Run("incorrect calculated values", func(t *testing.T) { + data := withInvoiceValues(t, calculationInvoice, map[string]any{ + "lines": []any{map[string]any{ + "quantity": "2", + "item": map[string]any{"name": "Tulips", "price": "10.00"}, + "sum": "19.00", + }}, + "totals": map[string]any{"sum": "19.00", "payable": "19.00"}, + }) + + _, discrepancies, err := calculateWithDiscrepancies(data) + require.NoError(t, err) + require.Len(t, discrepancies, 3) + assert.Equal(t, "$.lines[0].sum", discrepancies[0].Path) + assert.JSONEq(t, `"19.00"`, string(discrepancies[0].Provided)) + assert.JSONEq(t, `"20.00"`, string(discrepancies[0].Calculated)) + assert.Equal(t, "$.totals.sum", discrepancies[1].Path) + assert.Equal(t, "$.totals.payable", discrepancies[2].Path) + }) + + t.Run("semantically equal amounts", func(t *testing.T) { + data := withInvoiceValues(t, calculationInvoice, map[string]any{ + "lines": []any{map[string]any{ + "quantity": "2", + "item": map[string]any{"name": "Tulips", "price": "10.00"}, + "sum": "20.0", + }}, + }) + + _, discrepancies, err := calculateWithDiscrepancies(data) + require.NoError(t, err) + assert.Empty(t, discrepancies) + }) + + t.Run("envelope paths and result", func(t *testing.T) { + var doc map[string]any + require.NoError(t, json.Unmarshal([]byte(calculationInvoice), &doc)) + doc["totals"] = map[string]any{"payable": "19.00"} + data, err := json.Marshal(map[string]any{ + "$schema": "https://gobl.org/draft-0/envelope", + "head": map[string]any{ + "uuid": "0198f976-812a-7c64-92a2-640e467159f9", + }, + "doc": doc, + }) + require.NoError(t, err) + + result, discrepancies, err := calculateWithDiscrepancies(data) + require.NoError(t, err) + assert.IsType(t, &Envelope{}, result) + require.Len(t, discrepancies, 1) + assert.Equal(t, "$.doc.totals.payable", discrepancies[0].Path) + }) + + t.Run("normalization-only changes are ignored", func(t *testing.T) { + data := withInvoiceValues(t, calculationInvoice, map[string]any{ + "supplier": map[string]any{ + "tax_id": map[string]any{"country": "NL", "code": "000099995B57"}, + "name": " Foobar BV ", + }, + }) + + _, discrepancies, err := calculateWithDiscrepancies(data) + require.NoError(t, err) + assert.Empty(t, discrepancies) + }) +} + +func withInvoiceValues(t *testing.T, source string, values map[string]any) []byte { + t.Helper() + var doc map[string]any + require.NoError(t, json.Unmarshal([]byte(source), &doc)) + for key, value := range values { + doc[key] = value + } + data, err := json.Marshal(doc) + require.NoError(t, err) + return data +} +func TestCalculateMain(t *testing.T) { + t.Run("returns the calculated value", func(t *testing.T) { + value, err := Calculate([]byte(calculationInvoice)) + require.NoError(t, err) + + inv := value.(*bill.Invoice) + require.NotNil(t, inv.Totals) + assert.Equal(t, "20.00", inv.Totals.Payable.String()) + }) + + t.Run("returns discrepancies as an error", func(t *testing.T) { + data := withInvoiceValues(t, calculationInvoice, map[string]any{ + "totals": map[string]any{"payable": "19.00"}, + }) + + value, err := Calculate(data, WithDiscrepancies()) + assert.Nil(t, value) + var discrepancies CalculationDiscrepancies + require.ErrorAs(t, err, &discrepancies) + require.Len(t, discrepancies, 1) + assert.Equal(t, "$.totals.payable", discrepancies[0].Path) + }) +} From 14de47ef659e007070586cd35cd3b157a8b40070 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Olivi=C3=A9?= Date: Tue, 18 Aug 2026 17:07:50 +0000 Subject: [PATCH 2/4] Rework calculated value discrepancy detection Replace the gobl.Calculate/WithDiscrepancies wrapper, which duplicated the Parse+Calculate orchestration Envelope and Object already provide, with FindCalculationDiscrepancies(data, calculated). Callers keep their existing Parse/Calculate flow and diff the original bytes against the result they already produced, instead of going through a second calculation entry point. Discrepancies are now returned as plain data rather than an error type, avoiding the errname nolint the previous shape needed. The comparison also walks a single reflect tree (the calculated result) instead of two, using reflect.VisibleFields for embedded fields instead of hand-rolled recursion. Co-Authored-By: Claude Sonnet 5 --- CHANGELOG.md | 7 + calculate.go | 385 ------------------------------------------ calculate_test.go | 138 --------------- discrepancies.go | 243 ++++++++++++++++++++++++++ discrepancies_test.go | 224 ++++++++++++++++++++++++ 5 files changed, 474 insertions(+), 523 deletions(-) delete mode 100644 calculate.go delete mode 100644 calculate_test.go create mode 100644 discrepancies.go create mode 100644 discrepancies_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index f8a2390f5..7bb74fc3c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,13 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p ### Added +- `FindCalculationDiscrepancies`: an opt-in way to detect when a document + supplies calculated fields (e.g. line sums, tax amounts, totals) that + don't match GOBL's own calculation, instead of having them silently + overwritten. Compares the original data against an already-calculated + document or envelope and returns the mismatched fields as + `CalculationDiscrepancies`; calculated values omitted from the input + are unaffected. - `net`: added `SandboxAuthorities` (defaulting to `lookup.sandbox.gobl.org`) and `WithSandbox`. Sandbox and live trust lists remain separate. diff --git a/calculate.go b/calculate.go deleted file mode 100644 index 747a2e1f9..000000000 --- a/calculate.go +++ /dev/null @@ -1,385 +0,0 @@ -package gobl - -import ( - "bytes" - "encoding/json" - "fmt" - "reflect" - "sort" - "strings" - - "github.com/invopop/gobl/schema" -) - -// CalculateOption configures the package-level Calculate operation. -type CalculateOption func(*calculateOptions) - -type calculateOptions struct { - discrepancies bool -} - -// WithDiscrepancies asks Calculate to reject explicitly supplied calculated -// fields that do not match the values produced by GOBL. The returned error can -// be matched to CalculationDiscrepancies with errors.As. -func WithDiscrepancies() CalculateOption { - return func(opts *calculateOptions) { - opts.discrepancies = true - } -} - -// Calculate parses and calculates a GOBL document or envelope. By default it -// performs the same calculation as the value's Calculate method. -// -// WithDiscrepancies additionally rejects explicitly supplied calculated -// fields that do not match the values produced by GOBL. The returned error can -// be matched to CalculationDiscrepancies with errors.As. -func Calculate(data []byte, options ...CalculateOption) (any, error) { - opts := new(calculateOptions) - for _, option := range options { - if option != nil { - option(opts) - } - } - - if opts.discrepancies { - value, discrepancies, err := calculateWithDiscrepancies(data) - if err != nil { - return nil, err - } - if len(discrepancies) > 0 { - return nil, discrepancies - } - return value, nil - } - - value, err := Parse(data) - if err != nil { - return nil, err - } - if err := calculateParsed(value); err != nil { - return nil, err - } - return value, nil -} - -// CalculationDiscrepancy describes a calculated value supplied by the caller -// that did not match the value produced by GOBL. -type CalculationDiscrepancy struct { - Path string `json:"path"` - Provided json.RawMessage `json:"provided"` - Calculated json.RawMessage `json:"calculated"` -} - -// CalculationDiscrepancies contains the calculated values that GOBL changed. -// An empty list means that every calculated value explicitly present in the -// input matched the result. Calculated values omitted from the input are not -// discrepancies. -type CalculationDiscrepancies []*CalculationDiscrepancy - -// Error provides a human-readable summary suitable for logs. Callers should -// use the structured discrepancy fields when preparing an API response. -func (ds CalculationDiscrepancies) Error() string { - if len(ds) == 0 { - return "" - } - if len(ds) == 1 { - return ds[0].Error() - } - return fmt.Sprintf("%d calculation discrepancies", len(ds)) -} - -// Error provides a human-readable description of the discrepancy. -func (d *CalculationDiscrepancy) Error() string { - return fmt.Sprintf( - "%s: provided %s does not match calculated %s", - d.Path, - string(d.Provided), - string(d.Calculated), - ) -} - -// CalculateWithDiscrepancies parses and calculates a GOBL document or -// envelope, returning both the calculated value and any calculated fields that -// were explicitly supplied with a different value. -// -// Fields are considered calculated when they or one of their parents has the -// `calculated=true` JSON Schema annotation. Values are compared semantically -// when their type provides an Equals method, so equivalent representations -// such as monetary amounts with different precision do not produce a -// discrepancy. -func calculateWithDiscrepancies(data []byte) (any, CalculationDiscrepancies, error) { - provided, err := Parse(data) - if err != nil { - return nil, nil, err - } - calculated, err := Parse(data) - if err != nil { - return nil, nil, err - } - - if err := calculateParsed(calculated); err != nil { - return nil, nil, err - } - result := calculated - - var raw json.RawMessage = data - path := "$" - if env, ok := calculated.(*Envelope); ok { - var root map[string]json.RawMessage - if err := json.Unmarshal(data, &root); err != nil { - return nil, nil, ErrInput.WithCause(err) - } - raw = root["doc"] - path = "$.doc" - provided = calculationPayload(provided) - calculated = calculationPayload(env) - } - - ds := make(CalculationDiscrepancies, 0) - compareCalculated(raw, reflect.ValueOf(provided), reflect.ValueOf(calculated), path, false, &ds) - return result, ds, nil -} - -func calculateParsed(obj any) error { - if env, ok := obj.(*Envelope); ok { - return env.Calculate() - } - doc, err := schema.NewObject(obj) - if err != nil { - return wrapError(err) - } - if err := doc.Calculate(); err != nil { - return ErrCalculation.WithCause(err) - } - return nil -} - -func calculationPayload(obj any) any { - if env, ok := obj.(*Envelope); ok { - return env.Extract() - } - return obj -} - -func compareCalculated(raw json.RawMessage, before, after reflect.Value, path string, inherited bool, out *CalculationDiscrepancies) { - if isJSONNull(raw) { - return - } - - before = indirectValue(before) - after = indirectValue(after) - - switch firstJSONByte(raw) { - case '{': - compareCalculatedObject(raw, before, after, path, inherited, out) - case '[': - compareCalculatedArray(raw, before, after, path, inherited, out) - default: - if inherited && !valuesEqual(before, after) { - *out = append(*out, newCalculationDiscrepancy(path, before, after)) - } - } -} - -func compareCalculatedObject(raw json.RawMessage, before, after reflect.Value, path string, inherited bool, out *CalculationDiscrepancies) { - var fields map[string]json.RawMessage - if err := json.Unmarshal(raw, &fields); err != nil { - return - } - - typ := valueType(before, after) - if typ == nil || typ.Kind() != reflect.Struct { - if inherited && !valuesEqual(before, after) { - *out = append(*out, newCalculationDiscrepancy(path, before, after)) - } - return - } - - compareStructFields(fields, before, after, typ, path, inherited, out) -} - -func compareStructFields(raw map[string]json.RawMessage, before, after reflect.Value, typ reflect.Type, path string, inherited bool, out *CalculationDiscrepancies) { - for i := range typ.NumField() { - field := typ.Field(i) - if !field.IsExported() { - continue - } - name, embedded := jsonFieldName(field) - if name == "-" { - continue - } - if embedded { - compareStructFields(raw, fieldValue(before, i), fieldValue(after, i), indirectType(field.Type), path, inherited || isCalculatedField(field), out) - continue - } - value, present := raw[name] - if !present || isJSONNull(value) { - continue - } - compareCalculated( - value, - fieldValue(before, i), - fieldValue(after, i), - appendPath(path, name), - inherited || isCalculatedField(field), - out, - ) - } -} - -func compareCalculatedArray(raw json.RawMessage, before, after reflect.Value, path string, inherited bool, out *CalculationDiscrepancies) { - var values []json.RawMessage - if err := json.Unmarshal(raw, &values); err != nil { - return - } - for i, value := range values { - compareCalculated( - value, - indexValue(before, i), - indexValue(after, i), - fmt.Sprintf("%s[%d]", path, i), - inherited, - out, - ) - } -} - -func valuesEqual(before, after reflect.Value) bool { - if !before.IsValid() || !after.IsValid() { - return before.IsValid() == after.IsValid() - } - before = indirectValue(before) - after = indirectValue(after) - if !before.IsValid() || !after.IsValid() { - return before.IsValid() == after.IsValid() - } - if before.Type() != after.Type() { - return false - } - if equal, ok := callEquals(before, after); ok { - return equal - } - return reflect.DeepEqual(before.Interface(), after.Interface()) -} - -func callEquals(before, after reflect.Value) (bool, bool) { - method := before.MethodByName("Equals") - if !method.IsValid() { - return false, false - } - typ := method.Type() - if typ.NumIn() != 1 || typ.In(0) != after.Type() || typ.NumOut() != 1 || typ.Out(0).Kind() != reflect.Bool { - return false, false - } - result := method.Call([]reflect.Value{after}) - return result[0].Bool(), true -} - -func newCalculationDiscrepancy(path string, before, after reflect.Value) *CalculationDiscrepancy { - return &CalculationDiscrepancy{ - Path: path, - Provided: marshalValue(before), - Calculated: marshalValue(after), - } -} - -func marshalValue(value reflect.Value) json.RawMessage { - value = indirectValue(value) - if !value.IsValid() { - return json.RawMessage("null") - } - data, err := json.Marshal(value.Interface()) - if err != nil { - return json.RawMessage("null") - } - return data -} - -func valueType(values ...reflect.Value) reflect.Type { - for _, value := range values { - value = indirectValue(value) - if value.IsValid() { - return value.Type() - } - } - return nil -} - -func indirectType(typ reflect.Type) reflect.Type { - for typ != nil && (typ.Kind() == reflect.Pointer || typ.Kind() == reflect.Interface) { - typ = typ.Elem() - } - return typ -} - -func indirectValue(value reflect.Value) reflect.Value { - for value.IsValid() && (value.Kind() == reflect.Pointer || value.Kind() == reflect.Interface) { - if value.IsNil() { - return reflect.Value{} - } - value = value.Elem() - } - return value -} - -func fieldValue(value reflect.Value, index int) reflect.Value { - value = indirectValue(value) - if !value.IsValid() || value.Kind() != reflect.Struct || index >= value.NumField() { - return reflect.Value{} - } - return value.Field(index) -} - -func indexValue(value reflect.Value, index int) reflect.Value { - value = indirectValue(value) - if !value.IsValid() || (value.Kind() != reflect.Slice && value.Kind() != reflect.Array) || index >= value.Len() { - return reflect.Value{} - } - return value.Index(index) -} - -func jsonFieldName(field reflect.StructField) (string, bool) { - tag := field.Tag.Get("json") - name := strings.Split(tag, ",")[0] - if name == "-" { - return "-", false - } - if name != "" { - return name, false - } - if field.Anonymous { - return "", true - } - return field.Name, false -} - -func isCalculatedField(field reflect.StructField) bool { - return hasCalculatedAnnotation(field.Tag.Get("jsonschema_extras")) || - hasCalculatedAnnotation(field.Tag.Get("jsonschema")) -} - -func hasCalculatedAnnotation(tag string) bool { - parts := strings.Split(tag, ",") - sort.Strings(parts) - i := sort.SearchStrings(parts, "calculated=true") - return i < len(parts) && parts[i] == "calculated=true" -} - -func appendPath(path, name string) string { - if path == "$" { - return "$." + name - } - return path + "." + name -} - -func firstJSONByte(raw json.RawMessage) byte { - trimmed := bytes.TrimSpace(raw) - if len(trimmed) == 0 { - return 0 - } - return trimmed[0] -} - -func isJSONNull(raw json.RawMessage) bool { - return bytes.Equal(bytes.TrimSpace(raw), []byte("null")) -} diff --git a/calculate_test.go b/calculate_test.go deleted file mode 100644 index 470e07160..000000000 --- a/calculate_test.go +++ /dev/null @@ -1,138 +0,0 @@ -package gobl - -import ( - "encoding/json" - "testing" - - "github.com/invopop/gobl/bill" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -const calculationInvoice = `{ - "$schema": "https://gobl.org/draft-0/bill/invoice", - "$regime": "NL", - "currency": "EUR", - "issue_date": "2022-07-12", - "supplier": { - "tax_id": {"country": "NL", "code": "000099995B57"}, - "name": "Foobar BV" - }, - "lines": [{ - "quantity": "2", - "item": {"name": "Tulips", "price": "10.00"} - }] -}` - -func TestCalculateWithDiscrepancies(t *testing.T) { - t.Run("omitted calculated values", func(t *testing.T) { - result, discrepancies, err := calculateWithDiscrepancies([]byte(calculationInvoice)) - require.NoError(t, err) - assert.Empty(t, discrepancies) - inv := result.(*bill.Invoice) - require.NotNil(t, inv.Totals) - assert.Equal(t, "20.00", inv.Totals.Payable.String()) - }) - - t.Run("incorrect calculated values", func(t *testing.T) { - data := withInvoiceValues(t, calculationInvoice, map[string]any{ - "lines": []any{map[string]any{ - "quantity": "2", - "item": map[string]any{"name": "Tulips", "price": "10.00"}, - "sum": "19.00", - }}, - "totals": map[string]any{"sum": "19.00", "payable": "19.00"}, - }) - - _, discrepancies, err := calculateWithDiscrepancies(data) - require.NoError(t, err) - require.Len(t, discrepancies, 3) - assert.Equal(t, "$.lines[0].sum", discrepancies[0].Path) - assert.JSONEq(t, `"19.00"`, string(discrepancies[0].Provided)) - assert.JSONEq(t, `"20.00"`, string(discrepancies[0].Calculated)) - assert.Equal(t, "$.totals.sum", discrepancies[1].Path) - assert.Equal(t, "$.totals.payable", discrepancies[2].Path) - }) - - t.Run("semantically equal amounts", func(t *testing.T) { - data := withInvoiceValues(t, calculationInvoice, map[string]any{ - "lines": []any{map[string]any{ - "quantity": "2", - "item": map[string]any{"name": "Tulips", "price": "10.00"}, - "sum": "20.0", - }}, - }) - - _, discrepancies, err := calculateWithDiscrepancies(data) - require.NoError(t, err) - assert.Empty(t, discrepancies) - }) - - t.Run("envelope paths and result", func(t *testing.T) { - var doc map[string]any - require.NoError(t, json.Unmarshal([]byte(calculationInvoice), &doc)) - doc["totals"] = map[string]any{"payable": "19.00"} - data, err := json.Marshal(map[string]any{ - "$schema": "https://gobl.org/draft-0/envelope", - "head": map[string]any{ - "uuid": "0198f976-812a-7c64-92a2-640e467159f9", - }, - "doc": doc, - }) - require.NoError(t, err) - - result, discrepancies, err := calculateWithDiscrepancies(data) - require.NoError(t, err) - assert.IsType(t, &Envelope{}, result) - require.Len(t, discrepancies, 1) - assert.Equal(t, "$.doc.totals.payable", discrepancies[0].Path) - }) - - t.Run("normalization-only changes are ignored", func(t *testing.T) { - data := withInvoiceValues(t, calculationInvoice, map[string]any{ - "supplier": map[string]any{ - "tax_id": map[string]any{"country": "NL", "code": "000099995B57"}, - "name": " Foobar BV ", - }, - }) - - _, discrepancies, err := calculateWithDiscrepancies(data) - require.NoError(t, err) - assert.Empty(t, discrepancies) - }) -} - -func withInvoiceValues(t *testing.T, source string, values map[string]any) []byte { - t.Helper() - var doc map[string]any - require.NoError(t, json.Unmarshal([]byte(source), &doc)) - for key, value := range values { - doc[key] = value - } - data, err := json.Marshal(doc) - require.NoError(t, err) - return data -} -func TestCalculateMain(t *testing.T) { - t.Run("returns the calculated value", func(t *testing.T) { - value, err := Calculate([]byte(calculationInvoice)) - require.NoError(t, err) - - inv := value.(*bill.Invoice) - require.NotNil(t, inv.Totals) - assert.Equal(t, "20.00", inv.Totals.Payable.String()) - }) - - t.Run("returns discrepancies as an error", func(t *testing.T) { - data := withInvoiceValues(t, calculationInvoice, map[string]any{ - "totals": map[string]any{"payable": "19.00"}, - }) - - value, err := Calculate(data, WithDiscrepancies()) - assert.Nil(t, value) - var discrepancies CalculationDiscrepancies - require.ErrorAs(t, err, &discrepancies) - require.Len(t, discrepancies, 1) - assert.Equal(t, "$.totals.payable", discrepancies[0].Path) - }) -} diff --git a/discrepancies.go b/discrepancies.go new file mode 100644 index 000000000..9f6057a2f --- /dev/null +++ b/discrepancies.go @@ -0,0 +1,243 @@ +package gobl + +import ( + "bytes" + "encoding/json" + "fmt" + "reflect" + "strings" +) + +// CalculationDiscrepancy describes a single calculated value that was +// explicitly present in the original data with a value different to the +// one GOBL's calculation produced. +type CalculationDiscrepancy struct { + // Path identifies the field's location using the same "$.foo[0].bar" + // notation as validation fault paths. + Path string `json:"path"` + // Provided is the value exactly as it appeared in the original data. + Provided json.RawMessage `json:"provided"` + // Calculated is the value GOBL produced for the same field. + Calculated json.RawMessage `json:"calculated"` +} + +// CalculationDiscrepancies lists calculated values that GOBL replaced +// with a different result during calculation. +type CalculationDiscrepancies []*CalculationDiscrepancy + +// FindCalculationDiscrepancies compares data, the original bytes a caller +// submitted, against calculated, the same document or envelope obtained by +// parsing that data and then calling Calculate on it. It reports every +// calculated field — one whose JSON Schema definition or one of its +// ancestors carries the `calculated=true` extension — whose value in data +// differs from the corresponding value in calculated. +// +// A calculated field left out of data is never reported: GOBL is expected +// to fill those in, and doing so is not a discrepancy. Only fields the +// caller supplied with a value GOBL then changed are reported. +// +// An empty, non-nil result means every calculated value the caller +// supplied matched GOBL's calculation. +func FindCalculationDiscrepancies(data []byte, calculated any) (CalculationDiscrepancies, error) { + raw := json.RawMessage(data) + path := "$" + value := reflect.ValueOf(calculated) + + if env, ok := calculated.(*Envelope); ok { + var envelope struct { + Doc json.RawMessage `json:"doc"` + } + if err := json.Unmarshal(data, &envelope); err != nil { + return nil, ErrInput.WithCause(err) + } + raw = envelope.Doc + path = "$.doc" + value = reflect.ValueOf(env.Extract()) + } + + return findDiscrepancies(raw, value, false, path), nil +} + +// findDiscrepancies walks raw, the original JSON for this location, in +// step with value, the equivalent already-calculated Go value. calculated +// is true once this location is inside a field whose schema marks it (or +// an ancestor) as calculated=true; everything beneath such a field is +// treated as calculated too, since GOBL only annotates the outermost +// calculated field of a subtree such as an invoice's totals. +func findDiscrepancies(raw json.RawMessage, value reflect.Value, calculated bool, path string) CalculationDiscrepancies { + raw = bytes.TrimSpace(raw) + if len(raw) == 0 || isNull(raw) { + // A missing or explicit null calculated value is never a + // discrepancy: GOBL is expected to fill it in. + return nil + } + + value = indirect(value) + + switch raw[0] { + case '{': + return findObjectDiscrepancies(raw, value, calculated, path) + case '[': + return findArrayDiscrepancies(raw, value, calculated, path) + default: + if !calculated { + return nil + } + return compareValue(raw, value, path) + } +} + +func findObjectDiscrepancies(raw json.RawMessage, value reflect.Value, calculated bool, path string) CalculationDiscrepancies { + if value.Kind() != reflect.Struct { + // Not a Go struct we can walk field by field (e.g. a map used for + // free-form extensions). Compare it as a single opaque value. + if !calculated { + return nil + } + return compareValue(raw, value, path) + } + + var fields map[string]json.RawMessage + if err := json.Unmarshal(raw, &fields); err != nil { + return nil + } + + var discrepancies CalculationDiscrepancies + for _, field := range reflect.VisibleFields(value.Type()) { + // Anonymous (embedded) fields are also returned by VisibleFields + // alongside the fields they promote, so only the promoted fields + // need handling here. + if !field.IsExported() || field.Anonymous { + continue + } + name, ok := fieldName(field) + if !ok { + continue + } + fieldRaw, present := fields[name] + if !present { + continue + } + fieldValue, err := value.FieldByIndexErr(field.Index) + if err != nil { + continue + } + discrepancies = append(discrepancies, findDiscrepancies( + fieldRaw, fieldValue, calculated || isCalculatedField(field), path+"."+name, + )...) + } + return discrepancies +} + +func findArrayDiscrepancies(raw json.RawMessage, value reflect.Value, calculated bool, path string) CalculationDiscrepancies { + if value.Kind() != reflect.Slice && value.Kind() != reflect.Array { + if !calculated { + return nil + } + return compareValue(raw, value, path) + } + + var items []json.RawMessage + if err := json.Unmarshal(raw, &items); err != nil { + return nil + } + + var discrepancies CalculationDiscrepancies + for i, item := range items { + var elem reflect.Value + if i < value.Len() { + elem = value.Index(i) + } + discrepancies = append(discrepancies, findDiscrepancies( + item, elem, calculated, fmt.Sprintf("%s[%d]", path, i), + )...) + } + return discrepancies +} + +// compareValue is reached once raw can no longer be broken down any +// further against value: either it's a JSON scalar, or it's an object or +// array that value's Go type can't be walked field by field or index by +// index. It parses raw into a fresh instance of value's type so the two +// can be compared with the same semantics GOBL itself uses. +func compareValue(raw json.RawMessage, value reflect.Value, path string) CalculationDiscrepancies { + if !value.IsValid() { + return CalculationDiscrepancies{{Path: path, Provided: raw, Calculated: json.RawMessage("null")}} + } + + provided := reflect.New(value.Type()) + if err := json.Unmarshal(raw, provided.Interface()); err != nil { + // raw was already parsed successfully as part of the original + // document, so a mismatch here means value's shape no longer + // matches raw; there's nothing meaningful left to compare. + return nil + } + if equal(provided.Elem(), value) { + return nil + } + + calculated, err := json.Marshal(value.Interface()) + if err != nil { + return nil + } + return CalculationDiscrepancies{{Path: path, Provided: raw, Calculated: calculated}} +} + +// equal compares two values of the same type, preferring an Equals method +// when the type provides one so equivalent representations (e.g. "20" and +// "20.00" for a monetary amount) are not reported as discrepancies. +func equal(provided, calculated reflect.Value) bool { + if method := provided.MethodByName("Equals"); method.IsValid() { + t := method.Type() + if t.NumIn() == 1 && t.In(0) == calculated.Type() && t.NumOut() == 1 && t.Out(0).Kind() == reflect.Bool { + return method.Call([]reflect.Value{calculated})[0].Bool() + } + } + return reflect.DeepEqual(provided.Interface(), calculated.Interface()) +} + +// indirect follows pointers and interfaces down to the concrete value +// they hold, returning the zero Value if any step along the way is nil. +func indirect(value reflect.Value) reflect.Value { + for value.IsValid() && (value.Kind() == reflect.Pointer || value.Kind() == reflect.Interface) { + if value.IsNil() { + return reflect.Value{} + } + value = value.Elem() + } + return value +} + +// isCalculatedField reports whether field's JSON Schema definition carries +// the `calculated=true` extension, which GOBL's schema generator accepts +// either inside `jsonschema_extras` or alongside other options in +// `jsonschema` itself. +func isCalculatedField(field reflect.StructField) bool { + return hasCalculatedTag(field.Tag.Get("jsonschema_extras")) || hasCalculatedTag(field.Tag.Get("jsonschema")) +} + +func hasCalculatedTag(tag string) bool { + for part := range strings.SplitSeq(tag, ",") { + if part == "calculated=true" { + return true + } + } + return false +} + +// fieldName returns field's JSON name and whether it's addressable at all +// via JSON, i.e. it isn't tagged `json:"-"`. +func fieldName(field reflect.StructField) (string, bool) { + name, _, _ := strings.Cut(field.Tag.Get("json"), ",") + if name == "-" { + return "", false + } + if name == "" { + name = field.Name + } + return name, true +} + +func isNull(raw json.RawMessage) bool { + return string(raw) == "null" +} diff --git a/discrepancies_test.go b/discrepancies_test.go new file mode 100644 index 000000000..38b825e88 --- /dev/null +++ b/discrepancies_test.go @@ -0,0 +1,224 @@ +package gobl_test + +import ( + "encoding/json" + "maps" + "os" + "testing" + + "github.com/invopop/gobl" + "github.com/invopop/gobl/bill" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type discrepancyItem struct { + Label string `json:"label"` + Amount string `json:"amount" jsonschema_extras:"calculated=true"` +} + +type discrepancyTotals struct { + Tax string `json:"tax"` +} + +type discrepancyDoc struct { + Label string `json:"label"` + Ignored string `json:"-" jsonschema_extras:"calculated=true"` + Ext map[string]string `json:"ext,omitempty" jsonschema_extras:"calculated=true"` + Totals *discrepancyTotals `json:"totals,omitempty" jsonschema_extras:"calculated=true"` + Items []discrepancyItem `json:"items,omitempty"` +} + +func TestFindCalculationDiscrepancies_Fields(t *testing.T) { + t.Run("fields without the calculated tag are never reported", func(t *testing.T) { + data := []byte(`{"label": "before"}`) + calculated := &discrepancyDoc{Label: "after"} + + discrepancies, err := gobl.FindCalculationDiscrepancies(data, calculated) + require.NoError(t, err) + assert.Empty(t, discrepancies) + }) + + t.Run("a calculated field with a different value is reported", func(t *testing.T) { + data := []byte(`{"items": [{"label": "tulips", "amount": "10.00"}]}`) + calculated := &discrepancyDoc{ + Items: []discrepancyItem{{Label: "tulips", Amount: "20.00"}}, + } + + discrepancies, err := gobl.FindCalculationDiscrepancies(data, calculated) + require.NoError(t, err) + require.Len(t, discrepancies, 1) + assert.Equal(t, "$.items[0].amount", discrepancies[0].Path) + assert.JSONEq(t, `"10.00"`, string(discrepancies[0].Provided)) + assert.JSONEq(t, `"20.00"`, string(discrepancies[0].Calculated)) + }) + + t.Run("a calculated field omitted from the input is not reported", func(t *testing.T) { + data := []byte(`{"label": "invoice"}`) + calculated := &discrepancyDoc{ + Label: "invoice", + Totals: &discrepancyTotals{Tax: "6.00"}, + } + + discrepancies, err := gobl.FindCalculationDiscrepancies(data, calculated) + require.NoError(t, err) + assert.Empty(t, discrepancies) + }) + + t.Run("an explicit null for a calculated field is not reported", func(t *testing.T) { + data := []byte(`{"label": "invoice", "totals": null}`) + calculated := &discrepancyDoc{ + Label: "invoice", + Totals: &discrepancyTotals{Tax: "6.00"}, + } + + discrepancies, err := gobl.FindCalculationDiscrepancies(data, calculated) + require.NoError(t, err) + assert.Empty(t, discrepancies) + }) + + t.Run("fields nested under a calculated field are treated as calculated even when not tagged themselves", func(t *testing.T) { + data := []byte(`{"totals": {"tax": "5.00"}}`) + calculated := &discrepancyDoc{Totals: &discrepancyTotals{Tax: "6.00"}} + + discrepancies, err := gobl.FindCalculationDiscrepancies(data, calculated) + require.NoError(t, err) + require.Len(t, discrepancies, 1) + assert.Equal(t, "$.totals.tax", discrepancies[0].Path) + }) + + t.Run("fields excluded from JSON are never reported even if marked calculated", func(t *testing.T) { + data := []byte(`{"ignored": "x"}`) + calculated := &discrepancyDoc{Ignored: "y"} + + discrepancies, err := gobl.FindCalculationDiscrepancies(data, calculated) + require.NoError(t, err) + assert.Empty(t, discrepancies) + }) + + t.Run("a calculated field that isn't a Go struct or slice is compared as a whole", func(t *testing.T) { + data := []byte(`{"ext": {"a": "1"}}`) + calculated := &discrepancyDoc{Ext: map[string]string{"a": "2"}} + + discrepancies, err := gobl.FindCalculationDiscrepancies(data, calculated) + require.NoError(t, err) + require.Len(t, discrepancies, 1) + assert.Equal(t, "$.ext", discrepancies[0].Path) + assert.JSONEq(t, `{"a":"1"}`, string(discrepancies[0].Provided)) + assert.JSONEq(t, `{"a":"2"}`, string(discrepancies[0].Calculated)) + }) +} + +const sampleInvoice = `{ + "$schema": "https://gobl.org/draft-0/bill/invoice", + "$regime": "NL", + "currency": "EUR", + "issue_date": "2022-07-12", + "supplier": { + "tax_id": {"country": "NL", "code": "000099995B57"}, + "name": "Foobar BV" + }, + "lines": [{ + "quantity": "2", + "item": {"name": "Tulips", "price": "10.00"} + }] +}` + +func TestFindCalculationDiscrepancies_Invoice(t *testing.T) { + t.Run("omitted calculated values are not discrepancies", func(t *testing.T) { + data := []byte(sampleInvoice) + inv := parseAndCalculateInvoice(t, data) + require.Equal(t, "20.00", inv.Totals.Payable.String()) + + discrepancies, err := gobl.FindCalculationDiscrepancies(data, inv) + require.NoError(t, err) + assert.Empty(t, discrepancies) + }) + + t.Run("incorrect calculated values are reported with their paths", func(t *testing.T) { + data := withInvoiceValues(t, map[string]any{ + "lines": []any{map[string]any{ + "quantity": "2", + "item": map[string]any{"name": "Tulips", "price": "10.00"}, + "sum": "19.00", + }}, + "totals": map[string]any{"sum": "19.00", "payable": "19.00"}, + }) + inv := parseAndCalculateInvoice(t, data) + + discrepancies, err := gobl.FindCalculationDiscrepancies(data, inv) + require.NoError(t, err) + require.Len(t, discrepancies, 3) + assert.Equal(t, "$.lines[0].sum", discrepancies[0].Path) + assert.JSONEq(t, `"19.00"`, string(discrepancies[0].Provided)) + assert.JSONEq(t, `"20.00"`, string(discrepancies[0].Calculated)) + assert.Equal(t, "$.totals.sum", discrepancies[1].Path) + assert.Equal(t, "$.totals.payable", discrepancies[2].Path) + }) + + t.Run("semantically equal amounts are not discrepancies", func(t *testing.T) { + data := withInvoiceValues(t, map[string]any{ + "lines": []any{map[string]any{ + "quantity": "2", + "item": map[string]any{"name": "Tulips", "price": "10.00"}, + "sum": "20.0", + }}, + }) + inv := parseAndCalculateInvoice(t, data) + + discrepancies, err := gobl.FindCalculationDiscrepancies(data, inv) + require.NoError(t, err) + assert.Empty(t, discrepancies) + }) + + t.Run("a deeply nested tax breakdown reports precise paths under an envelope", func(t *testing.T) { + data, err := os.ReadFile("examples/ie/out/invoice-b2b.json") + require.NoError(t, err) + + var root map[string]json.RawMessage + require.NoError(t, json.Unmarshal(data, &root)) + var doc map[string]any + require.NoError(t, json.Unmarshal(root["doc"], &doc)) + totals := doc["totals"].(map[string]any) + taxes := totals["taxes"].(map[string]any) + categories := taxes["categories"].([]any) + category := categories[0].(map[string]any) + category["amount"] = "999.99" + docBytes, err := json.Marshal(doc) + require.NoError(t, err) + root["doc"] = docBytes + full, err := json.Marshal(root) + require.NoError(t, err) + + obj, err := gobl.Parse(full) + require.NoError(t, err) + env := obj.(*gobl.Envelope) + require.NoError(t, env.Calculate()) + + discrepancies, err := gobl.FindCalculationDiscrepancies(full, env) + require.NoError(t, err) + require.Len(t, discrepancies, 1) + assert.Equal(t, "$.doc.totals.taxes.categories[0].amount", discrepancies[0].Path) + assert.JSONEq(t, `"999.99"`, string(discrepancies[0].Provided)) + }) +} + +func parseAndCalculateInvoice(t *testing.T, data []byte) *bill.Invoice { + t.Helper() + obj, err := gobl.Parse(data) + require.NoError(t, err) + inv, ok := obj.(*bill.Invoice) + require.True(t, ok) + require.NoError(t, inv.Calculate()) + return inv +} + +func withInvoiceValues(t *testing.T, values map[string]any) []byte { + t.Helper() + var doc map[string]any + require.NoError(t, json.Unmarshal([]byte(sampleInvoice), &doc)) + maps.Copy(doc, values) + data, err := json.Marshal(doc) + require.NoError(t, err) + return data +} From ef7209b331a7583f5f2342fccab903e86767acb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Olivi=C3=A9?= Date: Tue, 18 Aug 2026 17:46:42 +0000 Subject: [PATCH 3/4] Compare parsed values instead of walking raw JSON by hand Parse data a second time (without calculating) to recover the pre-calculation value, then diff it against the calculated result by walking both with reflection, rather than hand-rolling a JSON tree walk (byte-sniffing objects/arrays, re-unmarshalling fragments at each leaf) alongside a single typed value. Presence is now read off the parsed value itself (nil pointers, slice length) instead of a separate raw-JSON presence check. The one gap this leaves - a handful of non-pointer calculated fields (invoice type, currency, issue date, line index, payment total) where an explicit zero value is indistinguishable from an omitted one - is accepted deliberately: GOBL treats both the same way regardless. Also fixes a bug the new approach surfaced: num.Amount and similar types are Go structs but marshal through MarshalText rather than their (unexported) fields, so they must be compared as opaque leaves via reflect.Type.Implements, not walked field by field. Co-Authored-By: Claude Sonnet 5 --- discrepancies.go | 205 ++++++++++++++++++++++-------------------- discrepancies_test.go | 112 +++-------------------- 2 files changed, 123 insertions(+), 194 deletions(-) diff --git a/discrepancies.go b/discrepancies.go index 9f6057a2f..75a9666a2 100644 --- a/discrepancies.go +++ b/discrepancies.go @@ -1,7 +1,7 @@ package gobl import ( - "bytes" + "encoding" "encoding/json" "fmt" "reflect" @@ -29,81 +29,116 @@ type CalculationDiscrepancies []*CalculationDiscrepancy // submitted, against calculated, the same document or envelope obtained by // parsing that data and then calling Calculate on it. It reports every // calculated field — one whose JSON Schema definition or one of its -// ancestors carries the `calculated=true` extension — whose value in data -// differs from the corresponding value in calculated. +// ancestors carries the `calculated=true` extension — whose value changed +// during calculation. +// +// data is parsed again to recover the values as they were before +// calculation, then compared against calculated field by field using the +// same reflection GOBL already relies on to walk documents; no separate +// JSON tree walk is needed. // // A calculated field left out of data is never reported: GOBL is expected -// to fill those in, and doing so is not a discrepancy. Only fields the -// caller supplied with a value GOBL then changed are reported. +// to fill those in, and doing so is not a discrepancy. Because a Go zero +// value (an empty string, a zero date, index zero) is indistinguishable +// from an omitted field for the handful of calculated fields that aren't +// pointers, a calculated field explicitly supplied with its zero value is +// treated the same way: not worth reporting, since GOBL is expected to +// replace it regardless of whether it was left out or supplied as such. // // An empty, non-nil result means every calculated value the caller // supplied matched GOBL's calculation. func FindCalculationDiscrepancies(data []byte, calculated any) (CalculationDiscrepancies, error) { - raw := json.RawMessage(data) + provided, err := Parse(data) + if err != nil { + return nil, err + } + path := "$" - value := reflect.ValueOf(calculated) + providedValue := reflect.ValueOf(provided) + calculatedValue := reflect.ValueOf(calculated) if env, ok := calculated.(*Envelope); ok { - var envelope struct { - Doc json.RawMessage `json:"doc"` - } - if err := json.Unmarshal(data, &envelope); err != nil { - return nil, ErrInput.WithCause(err) + providedEnv, ok := provided.(*Envelope) + if !ok { + return nil, ErrInput.WithReason("calculated is an envelope, but data is not") } - raw = envelope.Doc + providedValue = reflect.ValueOf(providedEnv.Extract()) + calculatedValue = reflect.ValueOf(env.Extract()) path = "$.doc" - value = reflect.ValueOf(env.Extract()) } - return findDiscrepancies(raw, value, false, path), nil + return findDiscrepancies(providedValue, calculatedValue, false, path), nil } -// findDiscrepancies walks raw, the original JSON for this location, in -// step with value, the equivalent already-calculated Go value. calculated -// is true once this location is inside a field whose schema marks it (or -// an ancestor) as calculated=true; everything beneath such a field is -// treated as calculated too, since GOBL only annotates the outermost -// calculated field of a subtree such as an invoice's totals. -func findDiscrepancies(raw json.RawMessage, value reflect.Value, calculated bool, path string) CalculationDiscrepancies { - raw = bytes.TrimSpace(raw) - if len(raw) == 0 || isNull(raw) { - // A missing or explicit null calculated value is never a - // discrepancy: GOBL is expected to fill it in. - return nil - } +// findDiscrepancies walks provided, the value as it was before +// calculation, in step with calculated, the equivalent already-calculated +// value. isCalculated is true once this location is inside a field whose +// schema marks it (or an ancestor) as calculated=true; everything beneath +// such a field is treated as calculated too, since GOBL only annotates the +// outermost calculated field of a subtree such as an invoice's totals. +func findDiscrepancies(provided, calculated reflect.Value, isCalculated bool, path string) CalculationDiscrepancies { + // A pointer or interface still holding a value once dereferenced was + // explicitly supplied, even if that value happens to be zero; only a + // non-pointer field's zero value is ambiguous with an omission. + nilable := isNilable(provided) - value = indirect(value) + provided = indirect(provided) + calculated = indirect(calculated) - switch raw[0] { - case '{': - return findObjectDiscrepancies(raw, value, calculated, path) - case '[': - return findArrayDiscrepancies(raw, value, calculated, path) - default: - if !calculated { + if !provided.IsValid() { + // Left out of the original data entirely: never a discrepancy. + return nil + } + if !calculated.IsValid() { + if !isCalculated { return nil } - return compareValue(raw, value, path) + return CalculationDiscrepancies{newDiscrepancy(path, provided, reflect.Value{})} } -} -func findObjectDiscrepancies(raw json.RawMessage, value reflect.Value, calculated bool, path string) CalculationDiscrepancies { - if value.Kind() != reflect.Struct { - // Not a Go struct we can walk field by field (e.g. a map used for - // free-form extensions). Compare it as a single opaque value. - if !calculated { - return nil + // A type such as num.Amount is a Go struct, but marshals to a single + // JSON string via its own MarshalText/MarshalJSON rather than one + // object key per (unexported) field; walking it field by field would + // find nothing; it needs to be treated as a leaf, same as a plain + // scalar. + if !hasCustomMarshaling(provided.Type()) { + switch provided.Kind() { + case reflect.Struct: + return findFieldDiscrepancies(provided, calculated, isCalculated, path) + case reflect.Slice, reflect.Array: + return findElementDiscrepancies(provided, calculated, isCalculated, path) } - return compareValue(raw, value, path) } - var fields map[string]json.RawMessage - if err := json.Unmarshal(raw, &fields); err != nil { + if !isCalculated { + return nil + } + if !nilable && provided.IsZero() { + return nil + } + if equal(provided, calculated) { return nil } + return CalculationDiscrepancies{newDiscrepancy(path, provided, calculated)} +} + +var ( + jsonMarshalerType = reflect.TypeFor[json.Marshaler]() + textMarshalerType = reflect.TypeFor[encoding.TextMarshaler]() +) + +// hasCustomMarshaling reports whether t controls its own JSON +// representation (e.g. num.Amount marshals to a decimal string) rather +// than being encoded as one object key per field, meaning it should be +// compared as an opaque value instead of walked field by field. +func hasCustomMarshaling(t reflect.Type) bool { + return t.Implements(jsonMarshalerType) || t.Implements(textMarshalerType) || + reflect.PointerTo(t).Implements(jsonMarshalerType) || reflect.PointerTo(t).Implements(textMarshalerType) +} +func findFieldDiscrepancies(provided, calculated reflect.Value, isCalculated bool, path string) CalculationDiscrepancies { var discrepancies CalculationDiscrepancies - for _, field := range reflect.VisibleFields(value.Type()) { + for _, field := range reflect.VisibleFields(provided.Type()) { // Anonymous (embedded) fields are also returned by VisibleFields // alongside the fields they promote, so only the promoted fields // need handling here. @@ -114,73 +149,48 @@ func findObjectDiscrepancies(raw json.RawMessage, value reflect.Value, calculate if !ok { continue } - fieldRaw, present := fields[name] - if !present { + providedField, err := provided.FieldByIndexErr(field.Index) + if err != nil { continue } - fieldValue, err := value.FieldByIndexErr(field.Index) + calculatedField, err := calculated.FieldByIndexErr(field.Index) if err != nil { continue } discrepancies = append(discrepancies, findDiscrepancies( - fieldRaw, fieldValue, calculated || isCalculatedField(field), path+"."+name, + providedField, calculatedField, isCalculated || isCalculatedField(field), path+"."+name, )...) } return discrepancies } -func findArrayDiscrepancies(raw json.RawMessage, value reflect.Value, calculated bool, path string) CalculationDiscrepancies { - if value.Kind() != reflect.Slice && value.Kind() != reflect.Array { - if !calculated { - return nil - } - return compareValue(raw, value, path) - } - - var items []json.RawMessage - if err := json.Unmarshal(raw, &items); err != nil { - return nil - } - +func findElementDiscrepancies(provided, calculated reflect.Value, isCalculated bool, path string) CalculationDiscrepancies { var discrepancies CalculationDiscrepancies - for i, item := range items { - var elem reflect.Value - if i < value.Len() { - elem = value.Index(i) + for i := 0; i < provided.Len(); i++ { + var calculatedElem reflect.Value + if i < calculated.Len() { + calculatedElem = calculated.Index(i) } discrepancies = append(discrepancies, findDiscrepancies( - item, elem, calculated, fmt.Sprintf("%s[%d]", path, i), + provided.Index(i), calculatedElem, isCalculated, fmt.Sprintf("%s[%d]", path, i), )...) } return discrepancies } -// compareValue is reached once raw can no longer be broken down any -// further against value: either it's a JSON scalar, or it's an object or -// array that value's Go type can't be walked field by field or index by -// index. It parses raw into a fresh instance of value's type so the two -// can be compared with the same semantics GOBL itself uses. -func compareValue(raw json.RawMessage, value reflect.Value, path string) CalculationDiscrepancies { - if !value.IsValid() { - return CalculationDiscrepancies{{Path: path, Provided: raw, Calculated: json.RawMessage("null")}} - } +func newDiscrepancy(path string, provided, calculated reflect.Value) *CalculationDiscrepancy { + return &CalculationDiscrepancy{Path: path, Provided: marshal(provided), Calculated: marshal(calculated)} +} - provided := reflect.New(value.Type()) - if err := json.Unmarshal(raw, provided.Interface()); err != nil { - // raw was already parsed successfully as part of the original - // document, so a mismatch here means value's shape no longer - // matches raw; there's nothing meaningful left to compare. - return nil - } - if equal(provided.Elem(), value) { - return nil +func marshal(value reflect.Value) json.RawMessage { + if !value.IsValid() { + return json.RawMessage("null") } - - calculated, err := json.Marshal(value.Interface()) + data, err := json.Marshal(value.Interface()) if err != nil { - return nil + return json.RawMessage("null") } - return CalculationDiscrepancies{{Path: path, Provided: raw, Calculated: calculated}} + return data } // equal compares two values of the same type, preferring an Equals method @@ -196,6 +206,13 @@ func equal(provided, calculated reflect.Value) bool { return reflect.DeepEqual(provided.Interface(), calculated.Interface()) } +// isNilable reports whether value's declared type can be nil, i.e. +// whether it being present with a zero underlying value is distinguishable +// from it being absent altogether. +func isNilable(value reflect.Value) bool { + return value.Kind() == reflect.Pointer || value.Kind() == reflect.Interface +} + // indirect follows pointers and interfaces down to the concrete value // they hold, returning the zero Value if any step along the way is nil. func indirect(value reflect.Value) reflect.Value { @@ -237,7 +254,3 @@ func fieldName(field reflect.StructField) (string, bool) { } return name, true } - -func isNull(raw json.RawMessage) bool { - return string(raw) == "null" -} diff --git a/discrepancies_test.go b/discrepancies_test.go index 38b825e88..72ce1ce42 100644 --- a/discrepancies_test.go +++ b/discrepancies_test.go @@ -12,103 +12,6 @@ import ( "github.com/stretchr/testify/require" ) -type discrepancyItem struct { - Label string `json:"label"` - Amount string `json:"amount" jsonschema_extras:"calculated=true"` -} - -type discrepancyTotals struct { - Tax string `json:"tax"` -} - -type discrepancyDoc struct { - Label string `json:"label"` - Ignored string `json:"-" jsonschema_extras:"calculated=true"` - Ext map[string]string `json:"ext,omitempty" jsonschema_extras:"calculated=true"` - Totals *discrepancyTotals `json:"totals,omitempty" jsonschema_extras:"calculated=true"` - Items []discrepancyItem `json:"items,omitempty"` -} - -func TestFindCalculationDiscrepancies_Fields(t *testing.T) { - t.Run("fields without the calculated tag are never reported", func(t *testing.T) { - data := []byte(`{"label": "before"}`) - calculated := &discrepancyDoc{Label: "after"} - - discrepancies, err := gobl.FindCalculationDiscrepancies(data, calculated) - require.NoError(t, err) - assert.Empty(t, discrepancies) - }) - - t.Run("a calculated field with a different value is reported", func(t *testing.T) { - data := []byte(`{"items": [{"label": "tulips", "amount": "10.00"}]}`) - calculated := &discrepancyDoc{ - Items: []discrepancyItem{{Label: "tulips", Amount: "20.00"}}, - } - - discrepancies, err := gobl.FindCalculationDiscrepancies(data, calculated) - require.NoError(t, err) - require.Len(t, discrepancies, 1) - assert.Equal(t, "$.items[0].amount", discrepancies[0].Path) - assert.JSONEq(t, `"10.00"`, string(discrepancies[0].Provided)) - assert.JSONEq(t, `"20.00"`, string(discrepancies[0].Calculated)) - }) - - t.Run("a calculated field omitted from the input is not reported", func(t *testing.T) { - data := []byte(`{"label": "invoice"}`) - calculated := &discrepancyDoc{ - Label: "invoice", - Totals: &discrepancyTotals{Tax: "6.00"}, - } - - discrepancies, err := gobl.FindCalculationDiscrepancies(data, calculated) - require.NoError(t, err) - assert.Empty(t, discrepancies) - }) - - t.Run("an explicit null for a calculated field is not reported", func(t *testing.T) { - data := []byte(`{"label": "invoice", "totals": null}`) - calculated := &discrepancyDoc{ - Label: "invoice", - Totals: &discrepancyTotals{Tax: "6.00"}, - } - - discrepancies, err := gobl.FindCalculationDiscrepancies(data, calculated) - require.NoError(t, err) - assert.Empty(t, discrepancies) - }) - - t.Run("fields nested under a calculated field are treated as calculated even when not tagged themselves", func(t *testing.T) { - data := []byte(`{"totals": {"tax": "5.00"}}`) - calculated := &discrepancyDoc{Totals: &discrepancyTotals{Tax: "6.00"}} - - discrepancies, err := gobl.FindCalculationDiscrepancies(data, calculated) - require.NoError(t, err) - require.Len(t, discrepancies, 1) - assert.Equal(t, "$.totals.tax", discrepancies[0].Path) - }) - - t.Run("fields excluded from JSON are never reported even if marked calculated", func(t *testing.T) { - data := []byte(`{"ignored": "x"}`) - calculated := &discrepancyDoc{Ignored: "y"} - - discrepancies, err := gobl.FindCalculationDiscrepancies(data, calculated) - require.NoError(t, err) - assert.Empty(t, discrepancies) - }) - - t.Run("a calculated field that isn't a Go struct or slice is compared as a whole", func(t *testing.T) { - data := []byte(`{"ext": {"a": "1"}}`) - calculated := &discrepancyDoc{Ext: map[string]string{"a": "2"}} - - discrepancies, err := gobl.FindCalculationDiscrepancies(data, calculated) - require.NoError(t, err) - require.Len(t, discrepancies, 1) - assert.Equal(t, "$.ext", discrepancies[0].Path) - assert.JSONEq(t, `{"a":"1"}`, string(discrepancies[0].Provided)) - assert.JSONEq(t, `{"a":"2"}`, string(discrepancies[0].Calculated)) - }) -} - const sampleInvoice = `{ "$schema": "https://gobl.org/draft-0/bill/invoice", "$regime": "NL", @@ -124,7 +27,7 @@ const sampleInvoice = `{ }] }` -func TestFindCalculationDiscrepancies_Invoice(t *testing.T) { +func TestFindCalculationDiscrepancies(t *testing.T) { t.Run("omitted calculated values are not discrepancies", func(t *testing.T) { data := []byte(sampleInvoice) inv := parseAndCalculateInvoice(t, data) @@ -171,6 +74,19 @@ func TestFindCalculationDiscrepancies_Invoice(t *testing.T) { assert.Empty(t, discrepancies) }) + t.Run("an explicit zero value for a non-pointer calculated field is not a discrepancy", func(t *testing.T) { + // "type" is calculated but isn't a pointer, so an explicit "" + // can't be told apart from having left it out; normalizeInvoice + // fills it in with "standard" either way. + data := withInvoiceValues(t, map[string]any{"type": ""}) + inv := parseAndCalculateInvoice(t, data) + require.Equal(t, "standard", inv.Type.String()) + + discrepancies, err := gobl.FindCalculationDiscrepancies(data, inv) + require.NoError(t, err) + assert.Empty(t, discrepancies) + }) + t.Run("a deeply nested tax breakdown reports precise paths under an envelope", func(t *testing.T) { data, err := os.ReadFile("examples/ie/out/invoice-b2b.json") require.NoError(t, err) From a0e5784ae7c5ee380549b726ab9bf85912fdc1b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Olivi=C3=A9?= Date: Tue, 18 Aug 2026 18:01:35 +0000 Subject: [PATCH 4/4] Detect presence exactly instead of guessing from zero values The previous version couldn't tell an omitted non-pointer calculated field (invoice type, currency, issue date, line index, payment total) apart from one explicitly supplied as its Go zero value, so it treated both as omitted. That's wrong whenever the zero value is a real, deliberate value that happens to differ from what GOBL calculated. Fix by decoding data a second time into a generic `any` tree solely to answer "was this key present and non-null" at each path, threaded alongside the two typed reflect trees. This generic tree is only ever used for that yes/no check, never as a source of values, so it carries none of the precision/formatting loss a naive read of it would. An explicit "" for invoice type is now correctly reported when GOBL replaces it with "standard", while a true omission still isn't. Co-Authored-By: Claude Sonnet 5 --- discrepancies.go | 83 ++++++++++++++++++++++++------------------- discrepancies_test.go | 24 ++++++++++--- 2 files changed, 65 insertions(+), 42 deletions(-) diff --git a/discrepancies.go b/discrepancies.go index 75a9666a2..950d998e3 100644 --- a/discrepancies.go +++ b/discrepancies.go @@ -34,16 +34,14 @@ type CalculationDiscrepancies []*CalculationDiscrepancy // // data is parsed again to recover the values as they were before // calculation, then compared against calculated field by field using the -// same reflection GOBL already relies on to walk documents; no separate -// JSON tree walk is needed. +// same reflection GOBL already relies on to walk documents. A second, +// generic decode of data is used purely to know which keys it actually +// contained; unlike the typed values, it's never a source of values to +// report, so it carries no risk of e.g. losing amount precision. // -// A calculated field left out of data is never reported: GOBL is expected -// to fill those in, and doing so is not a discrepancy. Because a Go zero -// value (an empty string, a zero date, index zero) is indistinguishable -// from an omitted field for the handful of calculated fields that aren't -// pointers, a calculated field explicitly supplied with its zero value is -// treated the same way: not worth reporting, since GOBL is expected to -// replace it regardless of whether it was left out or supplied as such. +// A calculated field left out of data, or explicitly set to null, is +// never reported: GOBL is expected to fill those in, and doing so is not +// a discrepancy. // // An empty, non-nil result means every calculated value the caller // supplied matched GOBL's calculation. @@ -53,6 +51,11 @@ func FindCalculationDiscrepancies(data []byte, calculated any) (CalculationDiscr return nil, err } + var presence any + if err := json.Unmarshal(data, &presence); err != nil { + return nil, ErrInput.WithCause(err) + } + path := "$" providedValue := reflect.ValueOf(provided) calculatedValue := reflect.ValueOf(calculated) @@ -62,31 +65,39 @@ func FindCalculationDiscrepancies(data []byte, calculated any) (CalculationDiscr if !ok { return nil, ErrInput.WithReason("calculated is an envelope, but data is not") } + if root, ok := presence.(map[string]any); ok { + presence = root["doc"] + } else { + presence = nil + } providedValue = reflect.ValueOf(providedEnv.Extract()) calculatedValue = reflect.ValueOf(env.Extract()) path = "$.doc" } - return findDiscrepancies(providedValue, calculatedValue, false, path), nil + return findDiscrepancies(providedValue, calculatedValue, presence, false, path), nil } // findDiscrepancies walks provided, the value as it was before // calculation, in step with calculated, the equivalent already-calculated -// value. isCalculated is true once this location is inside a field whose -// schema marks it (or an ancestor) as calculated=true; everything beneath -// such a field is treated as calculated too, since GOBL only annotates the -// outermost calculated field of a subtree such as an invoice's totals. -func findDiscrepancies(provided, calculated reflect.Value, isCalculated bool, path string) CalculationDiscrepancies { - // A pointer or interface still holding a value once dereferenced was - // explicitly supplied, even if that value happens to be zero; only a - // non-pointer field's zero value is ambiguous with an omission. - nilable := isNilable(provided) +// value. presence is the generic decode of the same location in the +// original JSON, used only to tell an omitted or null value (nil) apart +// from one that was genuinely supplied, which a Go zero value alone +// can't do for non-pointer fields. isCalculated is true once this +// location is inside a field whose schema marks it (or an ancestor) as +// calculated=true; everything beneath such a field is treated as +// calculated too, since GOBL only annotates the outermost calculated +// field of a subtree such as an invoice's totals. +func findDiscrepancies(provided, calculated reflect.Value, presence any, isCalculated bool, path string) CalculationDiscrepancies { + if presence == nil { + // Omitted, or explicitly null: never a discrepancy. + return nil + } provided = indirect(provided) calculated = indirect(calculated) if !provided.IsValid() { - // Left out of the original data entirely: never a discrepancy. return nil } if !calculated.IsValid() { @@ -99,23 +110,20 @@ func findDiscrepancies(provided, calculated reflect.Value, isCalculated bool, pa // A type such as num.Amount is a Go struct, but marshals to a single // JSON string via its own MarshalText/MarshalJSON rather than one // object key per (unexported) field; walking it field by field would - // find nothing; it needs to be treated as a leaf, same as a plain + // find nothing, so it needs to be treated as a leaf, same as a plain // scalar. if !hasCustomMarshaling(provided.Type()) { switch provided.Kind() { case reflect.Struct: - return findFieldDiscrepancies(provided, calculated, isCalculated, path) + return findFieldDiscrepancies(provided, calculated, presence, isCalculated, path) case reflect.Slice, reflect.Array: - return findElementDiscrepancies(provided, calculated, isCalculated, path) + return findElementDiscrepancies(provided, calculated, presence, isCalculated, path) } } if !isCalculated { return nil } - if !nilable && provided.IsZero() { - return nil - } if equal(provided, calculated) { return nil } @@ -136,7 +144,9 @@ func hasCustomMarshaling(t reflect.Type) bool { reflect.PointerTo(t).Implements(jsonMarshalerType) || reflect.PointerTo(t).Implements(textMarshalerType) } -func findFieldDiscrepancies(provided, calculated reflect.Value, isCalculated bool, path string) CalculationDiscrepancies { +func findFieldDiscrepancies(provided, calculated reflect.Value, presence any, isCalculated bool, path string) CalculationDiscrepancies { + fields, _ := presence.(map[string]any) + var discrepancies CalculationDiscrepancies for _, field := range reflect.VisibleFields(provided.Type()) { // Anonymous (embedded) fields are also returned by VisibleFields @@ -158,21 +168,27 @@ func findFieldDiscrepancies(provided, calculated reflect.Value, isCalculated boo continue } discrepancies = append(discrepancies, findDiscrepancies( - providedField, calculatedField, isCalculated || isCalculatedField(field), path+"."+name, + providedField, calculatedField, fields[name], isCalculated || isCalculatedField(field), path+"."+name, )...) } return discrepancies } -func findElementDiscrepancies(provided, calculated reflect.Value, isCalculated bool, path string) CalculationDiscrepancies { +func findElementDiscrepancies(provided, calculated reflect.Value, presence any, isCalculated bool, path string) CalculationDiscrepancies { + items, _ := presence.([]any) + var discrepancies CalculationDiscrepancies for i := 0; i < provided.Len(); i++ { var calculatedElem reflect.Value if i < calculated.Len() { calculatedElem = calculated.Index(i) } + var itemPresence any + if i < len(items) { + itemPresence = items[i] + } discrepancies = append(discrepancies, findDiscrepancies( - provided.Index(i), calculatedElem, isCalculated, fmt.Sprintf("%s[%d]", path, i), + provided.Index(i), calculatedElem, itemPresence, isCalculated, fmt.Sprintf("%s[%d]", path, i), )...) } return discrepancies @@ -206,13 +222,6 @@ func equal(provided, calculated reflect.Value) bool { return reflect.DeepEqual(provided.Interface(), calculated.Interface()) } -// isNilable reports whether value's declared type can be nil, i.e. -// whether it being present with a zero underlying value is distinguishable -// from it being absent altogether. -func isNilable(value reflect.Value) bool { - return value.Kind() == reflect.Pointer || value.Kind() == reflect.Interface -} - // indirect follows pointers and interfaces down to the concrete value // they hold, returning the zero Value if any step along the way is nil. func indirect(value reflect.Value) reflect.Value { diff --git a/discrepancies_test.go b/discrepancies_test.go index 72ce1ce42..fd422b8eb 100644 --- a/discrepancies_test.go +++ b/discrepancies_test.go @@ -74,17 +74,31 @@ func TestFindCalculationDiscrepancies(t *testing.T) { assert.Empty(t, discrepancies) }) - t.Run("an explicit zero value for a non-pointer calculated field is not a discrepancy", func(t *testing.T) { - // "type" is calculated but isn't a pointer, so an explicit "" - // can't be told apart from having left it out; normalizeInvoice - // fills it in with "standard" either way. + t.Run("an explicit null for a calculated field is not a discrepancy", func(t *testing.T) { + data := withInvoiceValues(t, map[string]any{"totals": nil}) + inv := parseAndCalculateInvoice(t, data) + + discrepancies, err := gobl.FindCalculationDiscrepancies(data, inv) + require.NoError(t, err) + assert.Empty(t, discrepancies) + }) + + t.Run("an explicitly empty value for a non-pointer calculated field is still reported", func(t *testing.T) { + // "type" is calculated but isn't a pointer, so its Go zero value + // alone can't tell an omitted field apart from an explicit ""; + // presence is read from a generic decode of the original JSON, so + // this is still caught even though normalizeInvoice would happily + // fill it in with "standard" either way. data := withInvoiceValues(t, map[string]any{"type": ""}) inv := parseAndCalculateInvoice(t, data) require.Equal(t, "standard", inv.Type.String()) discrepancies, err := gobl.FindCalculationDiscrepancies(data, inv) require.NoError(t, err) - assert.Empty(t, discrepancies) + require.Len(t, discrepancies, 1) + assert.Equal(t, "$.type", discrepancies[0].Path) + assert.JSONEq(t, `""`, string(discrepancies[0].Provided)) + assert.JSONEq(t, `"standard"`, string(discrepancies[0].Calculated)) }) t.Run("a deeply nested tax breakdown reports precise paths under an envelope", func(t *testing.T) {