From 497c276a34ce879f4f870d8f2911dbc5ed51e77c Mon Sep 17 00:00:00 2001 From: Gareth Kirwan Date: Sun, 1 Dec 2024 15:30:09 +0700 Subject: [PATCH] Config: AssetEnabled upgrade should respect assetTypes Previously we ignored the field and just turned on everything. I think that was because we couldn't get at the old value. In either case, we have the option to do better, and respect the assetEnabled value --- config/versions/v4.go | 61 +++++++++++++++++++++++++----- config/versions/v4_test.go | 77 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 129 insertions(+), 9 deletions(-) create mode 100644 config/versions/v4_test.go diff --git a/config/versions/v4.go b/config/versions/v4.go index b5642a87a00..4fe41e6e126 100644 --- a/config/versions/v4.go +++ b/config/versions/v4.go @@ -3,11 +3,19 @@ package versions import ( "bytes" "context" + "errors" + "fmt" + "strings" "github.com/buger/jsonparser" ) -// Version4 implements ExchangeVersion to set AssetEnabed: true for any exchange missing it +var ( + errUpgradingAssetTypes = errors.New("error upgrading assetTypes") + errUpgradingCurrencyPairs = errors.New("error upgrading currencyPairs.pairs") +) + +// Version4 is an Exchange upgrade to move currencyPairs.assetTypes to currencyPairs.pairs.*.assetEnabled type Version4 struct { } @@ -18,20 +26,55 @@ func init() { // Exchanges returns all exchanges: "*" func (v *Version4) Exchanges() []string { return []string{"*"} } -// UpgradeExchange sets AssetEnabed: true for any exchange missing it +// UpgradeExchange sets AssetEnabled: true for all assets listed in assetTypes, and false for any with no field func (v *Version4) UpgradeExchange(_ context.Context, e []byte) ([]byte, error) { - cb := func(k []byte, v []byte, _ jsonparser.ValueType, _ int) error { - if _, err := jsonparser.GetBoolean(v, "assetEnabled"); err != nil { - e, err = jsonparser.Set(e, []byte(`true`), "currencyPairs", "pairs", string(k), "assetEnabled") + toEnable := map[string]bool{} + + assetTypesFn := func(asset []byte, valueType jsonparser.ValueType, _ int, _ error) { + if valueType == jsonparser.String { + toEnable[string(asset)] = true + } + } + _, err := jsonparser.ArrayEach(e, assetTypesFn, "currencyPairs", "assetTypes") + if err != nil && !errors.Is(err, jsonparser.KeyPathNotFoundError) { + return e, fmt.Errorf("%w: %w", errUpgradingAssetTypes, err) + } + + assetEnabledFn := func(assetBytes []byte, v []byte, _ jsonparser.ValueType, _ int) error { + asset := string(assetBytes) + if toEnable[asset] { + e, err = jsonparser.Set(e, []byte(`true`), "currencyPairs", "pairs", asset, "assetEnabled") return err } - return nil + _, err = jsonparser.GetBoolean(v, "assetEnabled") + if errors.Is(err, jsonparser.KeyPathNotFoundError) { + e, err = jsonparser.Set(e, []byte(`false`), "currencyPairs", "pairs", asset, "assetEnabled") + } + return err + } + if err = jsonparser.ObjectEach(bytes.Clone(e), assetEnabledFn, "currencyPairs", "pairs"); err != nil { + return e, fmt.Errorf("%w: %w", errUpgradingCurrencyPairs, err) } - err := jsonparser.ObjectEach(bytes.Clone(e), cb, "currencyPairs", "pairs") + e = jsonparser.Delete(e, "currencyPairs", "assetTypes") return e, err } -// DowngradeExchange doesn't do anything for this version, because it's a lossy downgrade to disable all assets +// DowngradeExchange moves AssetEnabled assets into AssetType field func (v *Version4) DowngradeExchange(_ context.Context, e []byte) ([]byte, error) { - return e, nil + assetTypes := []string{} + + assetEnabledFn := func(asset []byte, v []byte, _ jsonparser.ValueType, _ int) error { + if b, err := jsonparser.GetBoolean(v, "assetEnabled"); err == nil { + if b { + assetTypes = append(assetTypes, fmt.Sprintf("%q", asset)) + } + e = jsonparser.Delete(e, "currencyPairs", "pairs", string(asset), "assetEnabled") + } + return nil + } + err := jsonparser.ObjectEach(bytes.Clone(e), assetEnabledFn, "currencyPairs", "pairs") + if err == nil { + e, err = jsonparser.Set(e, []byte(`[`+strings.Join(assetTypes, ",")+`]`), "currencyPairs", "assetTypes") + } + return e, err } diff --git a/config/versions/v4_test.go b/config/versions/v4_test.go new file mode 100644 index 00000000000..f754f3e9d3b --- /dev/null +++ b/config/versions/v4_test.go @@ -0,0 +1,77 @@ +package versions + +import ( + "bytes" + "context" + "testing" + + "github.com/buger/jsonparser" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestVersion4ExchangeType(t *testing.T) { + t.Parallel() + assert.Implements(t, (*ExchangeVersion)(nil), new(Version4)) +} + +func TestVersion4Exchanges(t *testing.T) { + t.Parallel() + assert.Equal(t, []string{"*"}, new(Version4).Exchanges()) +} + +func TestVersion4Upgrade(t *testing.T) { + t.Parallel() + + _, err := new(Version4).UpgradeExchange(context.Background(), []byte{}) + require.ErrorIs(t, err, errUpgradingAssetTypes) + + _, err = new(Version4).UpgradeExchange(context.Background(), []byte(`{}`)) + require.ErrorIs(t, err, errUpgradingCurrencyPairs) + + in := []byte(`{"name":"Cracken","currencyPairs":{"assetTypes":["spot"],"pairs":{"spot":{"enabled":"BTC-AUD","available":"BTC-AUD"},"futures":{"assetEnabled":true},"options":{}}}}`) + out, err := new(Version4).UpgradeExchange(context.Background(), in) + require.NoError(t, err) + require.NotEmpty(t, out) + + _, _, _, err = jsonparser.Get(out, "currencyPairs", "assetTypes") //nolint:dogsled // Ignored return values really not needed + assert.ErrorIs(t, err, jsonparser.KeyPathNotFoundError, "assetTypes should be removed") + + e, err := jsonparser.GetBoolean(out, "currencyPairs", "pairs", "spot", "assetEnabled") + require.NoError(t, err, "Must find assetEnabled for spot") + assert.True(t, e, "assetEnabled should be set to true") + + e, err = jsonparser.GetBoolean(out, "currencyPairs", "pairs", "futures", "assetEnabled") + require.NoError(t, err, "Must find assetEnabled for futures") + assert.True(t, e, "assetEnabled should be set to true") + + e, err = jsonparser.GetBoolean(out, "currencyPairs", "pairs", "options", "assetEnabled") + require.NoError(t, err, "Must find assetEnabled for options") + assert.False(t, e, "assetEnabled should be set to false") + + out2, err := new(Version4).UpgradeExchange(context.Background(), out) + require.NoError(t, err, "Must not error on re-upgrading") + assert.Equal(t, out, out2, "Should not affect an already upgraded config") +} + +func TestVersion4Downgrade(t *testing.T) { + t.Parallel() + + in := []byte(`{"name":"Cracken","currencyPairs":{"pairs":{"spot":{"enabled":"BTC-AUD","available":"BTC-AUD","assetEnabled":true},"futures":{"assetEnabled":false},"options":{},"options_combo":{"assetEnabled":true}}}}`) + out, err := new(Version4).DowngradeExchange(context.Background(), in) + require.NoError(t, err) + require.NotEmpty(t, out) + + v, vT, _, err := jsonparser.Get(out, "currencyPairs", "assetTypes") + require.NoError(t, err, "assetTypes must be found") + require.Equal(t, jsonparser.Array, vT, "assetTypes must be an array") + require.Equal(t, `["spot","options_combo"]`, string(v), "assetTypes must be correct") + + assetEnabledFn := func(k []byte, v []byte, _ jsonparser.ValueType, _ int) error { + _, err = jsonparser.GetBoolean(v, "assetEnabled") + require.ErrorIsf(t, err, jsonparser.KeyPathNotFoundError, "assetEnabled must be removed from %s", k) + return nil + } + err = jsonparser.ObjectEach(bytes.Clone(out), assetEnabledFn, "currencyPairs", "pairs") + require.NoError(t, err, "Must not error visiting currencyPairs") +}