diff --git a/internal/interpreter/function_exprs.go b/internal/interpreter/function_exprs.go index 667ef21e..48e785bd 100644 --- a/internal/interpreter/function_exprs.go +++ b/internal/interpreter/function_exprs.go @@ -60,13 +60,23 @@ func meta( return "", err } + // Check the cache first, so that we don't query the store again + // for a (account, key) pair we already fetched + if value, ok := s.CachedAccountsMeta[*account][*key]; ok { + return value, nil + } + + // Note: we do not cache negative lookups, so a key that is absent from + // the store is queried again (and errors again) on every meta() call meta, fetchMetaErr := s.Store.GetAccountsMetadata(s.ctx, MetadataQuery{ *account: []string{*key}, }) if fetchMetaErr != nil { return "", QueryMetadataError{WrappedError: fetchMetaErr} } - s.CachedAccountsMeta = meta + // Merge the fetched metadata into the cache instead of replacing it, + // so that previously cached entries are preserved + s.CachedAccountsMeta.Merge(meta) // body accountMeta := s.CachedAccountsMeta[*account] diff --git a/internal/utils/__snapshots__/pretty_csv_test.snap b/internal/utils/__snapshots__/pretty_csv_test.snap index 18e0527e..e9944c04 100755 --- a/internal/utils/__snapshots__/pretty_csv_test.snap +++ b/internal/utils/__snapshots__/pretty_csv_test.snap @@ -12,3 +12,11 @@ | b | 12345 | | very-very-very-long-key | | --- + +[TestPrettyCsvRaggedRows - 1] +| Account | Asset  | Balance | +| alice | EUR/2 | 1 | +| bob | | | +| charlie | USD/1234 | | +| dave | BTC | 3 | +--- diff --git a/internal/utils/pretty_csv.go b/internal/utils/pretty_csv.go index 1703b56f..82520aa8 100644 --- a/internal/utils/pretty_csv.go +++ b/internal/utils/pretty_csv.go @@ -8,7 +8,8 @@ import ( "github.com/formancehq/numscript/internal/ansi" ) -// Fails if the header is shorter than any of the rows +// Rows shorter than the header are padded with empty cells; +// cells beyond the header length are ignored for padding purposes func CsvPretty( header []string, rows [][]string, @@ -34,9 +35,10 @@ func CsvPretty( maxLen := len(fieldName) for _, row := range rows { - // panics if row[fieldIndex] is out of bounds - // thus we must never have unlabeled cols - maxLen = max(maxLen, len(row[fieldIndex])) + // rows shorter than the header are treated as having empty cells + if fieldIndex < len(row) { + maxLen = max(maxLen, len(row[fieldIndex])) + } } maxLengths[fieldIndex] = maxLen @@ -61,10 +63,15 @@ func CsvPretty( // -- Print rows for _, row := range rows { var partialRow []string - for index, fieldName := range row { + for index := range header { + // missing cells in ragged rows are rendered as empty strings + var fieldValue string + if index < len(row) { + fieldValue = row[index] + } partialRow = append(partialRow, fmt.Sprintf("| %-*s ", maxLengths[index], - fieldName, + fieldValue, )) } partialRow = append(partialRow, "|") diff --git a/internal/utils/pretty_csv_test.go b/internal/utils/pretty_csv_test.go index 010456b7..5d850b21 100644 --- a/internal/utils/pretty_csv_test.go +++ b/internal/utils/pretty_csv_test.go @@ -19,6 +19,21 @@ func TestPrettyCsv(t *testing.T) { snaps.MatchSnapshot(t, out) } +func TestPrettyCsvRaggedRows(t *testing.T) { + // rows shorter (or longer) than the header must not panic; + // missing cells are rendered as empty strings + out := utils.CsvPretty([]string{ + "Account", "Asset", "Balance", + }, [][]string{ + {"alice", "EUR/2", "1"}, + {"bob"}, + {"charlie", "USD/1234"}, + {"dave", "BTC", "3", "extra-cell"}, + }, true) + + snaps.MatchSnapshot(t, out) +} + func TestPrettyCsvMap(t *testing.T) { out := utils.CsvPrettyMap("Name", "Value", map[string]string{ "a": "0", diff --git a/numscript_test.go b/numscript_test.go index aef0c1da..dc8334e7 100644 --- a/numscript_test.go +++ b/numscript_test.go @@ -539,6 +539,110 @@ send [USD/2 10] ( } +func TestMetaFunctionCachesQueries(t *testing.T) { + parseResult := numscript.Parse(`vars { + string $a = meta(@acc1, "key1") + string $b = meta(@acc2, "key2") + string $a_again = meta(@acc1, "key1") +} +set_tx_meta("a", $a) +set_tx_meta("b", $b) +set_tx_meta("a_again", $a_again) +`) + + require.Empty(t, parseResult.GetParsingErrors(), "There should not be parsing errors") + + store := scopedMetaStore{ + meta: interpreter.AccountsMetadata{ + "acc1": {"key1": "value1"}, + "acc2": {"key2": "value2"}, + }, + } + + res, err := parseResult.Run(context.Background(), numscript.VariablesMap{}, &store) + require.Nil(t, err) + + // merging the second query into the cache must not clobber + // the previously cached acc1 metadata + require.Equal(t, + numscript.Metadata{ + "a": interpreter.String("value1"), + "b": interpreter.String("value2"), + "a_again": interpreter.String("value1"), + }, + res.Metadata) + + // acc1's "key1" is cached after the first meta() call, + // so the third meta() call must not query the store again + require.Equal(t, + []numscript.MetadataQuery{ + {"acc1": {"key1"}}, + {"acc2": {"key2"}}, + }, + store.GetMetadataCalls) +} + +func TestMetaFunctionMissingKeyStillErrors(t *testing.T) { + parseResult := numscript.Parse(`vars { + string $a = meta(@acc1, "key1") + string $b = meta(@acc1, "missing_key") +} +set_tx_meta("a", $a) +set_tx_meta("b", $b) +`) + + require.Empty(t, parseResult.GetParsingErrors(), "There should not be parsing errors") + + store := scopedMetaStore{ + meta: interpreter.AccountsMetadata{ + "acc1": {"key1": "value1"}, + }, + } + + // even though acc1's metadata is (partially) cached, + // a key that is absent from the store must still error + _, err := parseResult.Run(context.Background(), numscript.VariablesMap{}, &store) + require.NotNil(t, err) + + notFoundErr, ok := err.(interpreter.MetadataNotFound) + require.True(t, ok, "expected a MetadataNotFound error, got: %v", err) + require.Equal(t, "acc1", notFoundErr.Account) + require.Equal(t, "missing_key", notFoundErr.Key) +} + +// A store that, unlike StaticStore, only returns the queried +// (account, key) pairs, and records the metadata queries it receives +type scopedMetaStore struct { + meta interpreter.AccountsMetadata + GetMetadataCalls []numscript.MetadataQuery +} + +func (s *scopedMetaStore) GetBalances(ctx context.Context, q interpreter.BalanceQuery) (interpreter.Balances, error) { + return interpreter.StaticStore{}.GetBalances(ctx, q) +} + +func (s *scopedMetaStore) GetAccountsMetadata(_ context.Context, q interpreter.MetadataQuery) (interpreter.AccountsMetadata, error) { + s.GetMetadataCalls = append(s.GetMetadataCalls, q) + + out := interpreter.AccountsMetadata{} + for account, keys := range q { + for _, key := range keys { + value, ok := s.meta[account][key] + if !ok { + continue + } + + accountMeta, ok := out[account] + if !ok { + accountMeta = interpreter.AccountMetadata{} + out[account] = accountMeta + } + accountMeta[key] = value + } + } + return out, nil +} + type ObservableStore struct { StaticStore interpreter.StaticStore GetBalancesCalls []numscript.BalanceQuery