From 026f88fda23b3554b5c16a6c97a20ef4f1a0f991 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/v3.go | 54 ++++++++++++++++++++++++++++----- config/versions/v3_test.go | 61 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+), 8 deletions(-) create mode 100644 config/versions/v3_test.go diff --git a/config/versions/v3.go b/config/versions/v3.go index ed5cf7f60ec..41123f56aa4 100644 --- a/config/versions/v3.go +++ b/config/versions/v3.go @@ -3,6 +3,9 @@ package versions import ( "bytes" "context" + "errors" + "fmt" + "strings" "github.com/buger/jsonparser" ) @@ -18,20 +21,55 @@ func init() { // Exchanges returns all exchanges: "*" func (v *Version3) Exchanges() []string { return []string{"*"} } -// UpgradeExchange sets AssetEnabed: true for any exchange missing it +// UpgradeExchange sets AssetEnabed: true for all assets listed in assetTypes, and false for any with no field func (v *Version3) 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(v []byte, vT jsonparser.ValueType, _ int, _ error) { + if vT == jsonparser.String { + toEnable[string(v)] = true + } + } + _, err := jsonparser.ArrayEach(e, assetTypesFn, "currencyPairs", "assetTypes") + if err != nil && !errors.Is(err, jsonparser.KeyPathNotFoundError) { + return e, err + } + + assetEnabledFn := func(kBytes []byte, v []byte, _ jsonparser.ValueType, _ int) error { + k := string(kBytes) + if toEnable[k] { + e, err = jsonparser.Set(e, []byte(`true`), "currencyPairs", "pairs", k, "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", k, "assetEnabled") + } + return err + } + err = jsonparser.ObjectEach(bytes.Clone(e), assetEnabledFn, "currencyPairs", "pairs") + if err == nil { + e = jsonparser.Delete(e, "currencyPairs", "assetTypes") } - err := jsonparser.ObjectEach(bytes.Clone(e), cb, "currencyPairs", "pairs") 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 *Version3) DowngradeExchange(_ context.Context, e []byte) ([]byte, error) { - return e, nil + assetTypes := []string{} + + assetEnabledFn := func(k []byte, v []byte, _ jsonparser.ValueType, _ int) error { + if b, err := jsonparser.GetBoolean(v, "assetEnabled"); err == nil { + if b { + assetTypes = append(assetTypes, fmt.Sprintf("%q", k)) + } + e = jsonparser.Delete(e, "currencyPairs", "pairs", string(k), "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/v3_test.go b/config/versions/v3_test.go new file mode 100644 index 00000000000..ed47ca69592 --- /dev/null +++ b/config/versions/v3_test.go @@ -0,0 +1,61 @@ +package versions + +import ( + "bytes" + "context" + "testing" + + "github.com/buger/jsonparser" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestVersion3Upgrade(t *testing.T) { + t.Parallel() + + in := []byte(`{"name":"Cracken","currencyPairs":{"assetTypes":["spot"],"pairs":{"spot":{"enabled":"BTC-AUD","available":"BTC-AUD"},"futures":{"assetEnabled":true},"options":{}}}}`) + out, err := new(Version3).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 must 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(Version3).UpgradeExchange(context.Background(), out) + require.NoError(t, err, "Must not error on re-upgrading") + assert.Equal(t, out, out2, "Should not effect an already upgraded config") +} + +func TestVersion3Downgrade(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(Version3).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") +}