From f5609c0df1ea38da1ace5f187dd98e89165e9f8c Mon Sep 17 00:00:00 2001 From: Martin Hansen Date: Thu, 19 Sep 2024 09:41:47 +0200 Subject: [PATCH] fix(nordigen): nil pointer Refactor toYnabber into a single function, the nil check was on a layer too high before. Remove that pit fall by just having one function for the translation. Move the strip function into the mapper, it was a leftover from when the config value was defined on the YNAB side. We want to mangle data closet to the read point. commit-id:ca48367d --- go.mod | 2 + go.sum | 2 + reader/nordigen/mapper.go | 21 +++++++ reader/nordigen/mapper_test.go | 48 ++++++++++++++++ reader/nordigen/nordigen.go | 29 ++-------- reader/nordigen/nordigen_test.go | 96 ++++++++++++++++++++------------ ynabber.go | 10 ---- ynabber_test.go | 30 ---------- 8 files changed, 139 insertions(+), 99 deletions(-) diff --git a/go.mod b/go.mod index 0b79de4..6ca7eb4 100644 --- a/go.mod +++ b/go.mod @@ -7,3 +7,5 @@ require github.com/frieser/nordigen-go-lib/v2 v2.1.7 require github.com/kelseyhightower/envconfig v1.4.0 require github.com/carlmjohnson/versioninfo v0.22.5 + +require github.com/google/go-cmp v0.6.0 diff --git a/go.sum b/go.sum index 4aca5e1..9d7f5bd 100644 --- a/go.sum +++ b/go.sum @@ -2,5 +2,7 @@ github.com/carlmjohnson/versioninfo v0.22.5 h1:O00sjOLUAFxYQjlN/bzYTuZiS0y6fWDQj github.com/carlmjohnson/versioninfo v0.22.5/go.mod h1:QT9mph3wcVfISUKd0i9sZfVrPviHuSF+cUtLjm2WSf8= github.com/frieser/nordigen-go-lib/v2 v2.1.7 h1:n6qhksPY9iPPXBmbdnIxwWQeaMM2fsQece4BlSNmfvc= github.com/frieser/nordigen-go-lib/v2 v2.1.7/go.mod h1:NejYisqD8GvynCN0vDGw7J66slnj7jB25c8tS1tr8bw= +github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= +github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/kelseyhightower/envconfig v1.4.0 h1:Im6hONhd3pLkfDFsbRgu68RDNkGF1r3dvMUtDTo2cv8= github.com/kelseyhightower/envconfig v1.4.0/go.mod h1:cccZRl6mQpaq41TPp5QxidR+Sa3axMbJDNb//FQX6Gg= diff --git a/reader/nordigen/mapper.go b/reader/nordigen/mapper.go index bab05fd..c81e0f5 100644 --- a/reader/nordigen/mapper.go +++ b/reader/nordigen/mapper.go @@ -2,6 +2,7 @@ package nordigen import ( "fmt" + "regexp" "strconv" "strings" "time" @@ -37,6 +38,21 @@ func parseDate(t nordigen.Transaction) (time.Time, error) { return date, nil } +// payeeStripNonAlphanumeric removes all non-alphanumeric characters from payee +func payeeStripNonAlphanumeric(payee string) (x string) { + reg := regexp.MustCompile(`[^\p{L}]+`) + x = reg.ReplaceAllString(payee, " ") + return strings.TrimSpace(x) +} + +// Strip removes each string in strips from s +func strip(s string, strips []string) string { + for _, strip := range strips { + s = strings.ReplaceAll(s, strip, "") + } + return strings.TrimSpace(s) +} + // defaultMapper is generic and tries to identify the appropriate mapping func (r Reader) defaultMapper(a ynabber.Account, t nordigen.Transaction) (*ynabber.Transaction, error) { PayeeSource := r.Config.Nordigen.PayeeSource @@ -89,6 +105,11 @@ func (r Reader) defaultMapper(a ynabber.Account, t nordigen.Transaction) (*ynabb } } + // Remove elements in payee that is defined in config + if r.Config.Nordigen.PayeeStrip != nil { + payee = strip(payee, r.Config.Nordigen.PayeeStrip) + } + // Set the transaction ID according to config var id string switch TransactionID { diff --git a/reader/nordigen/mapper_test.go b/reader/nordigen/mapper_test.go index f60de10..e505789 100644 --- a/reader/nordigen/mapper_test.go +++ b/reader/nordigen/mapper_test.go @@ -48,3 +48,51 @@ func TestParseAmount(t *testing.T) { }) } } + +func TestPayeeStripNonAlphanumeric(t *testing.T) { + want := "Im just alphanumeric" + got := payeeStripNonAlphanumeric("Im just alphanumeric") + if want != got { + t.Fatalf("alphanumeric: %s != %s", want, got) + } + + want = "你好世界" + got = payeeStripNonAlphanumeric("你好世界") + if want != got { + t.Fatalf("non-english: %s != %s", want, got) + } + + want = "Im not j ust alphanumeric" + got = payeeStripNonAlphanumeric("Im! not j.ust alphanumeric42 69") + if want != got { + t.Fatalf("non-alphanumeric: %s != %s", want, got) + } +} + +func TestStrip(t *testing.T) { + type args struct { + s string + strips []string + } + tests := []struct { + name string + args args + want string + }{ + {name: "single", + args: args{s: "Im not here", strips: []string{"not "}}, + want: "Im here", + }, + {name: "multiple", + args: args{s: "Im not really here", strips: []string{"not ", "really "}}, + want: "Im here", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := strip(tt.args.s, tt.args.strips); got != tt.want { + t.Errorf("Payee.Strip() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/reader/nordigen/nordigen.go b/reader/nordigen/nordigen.go index 367d800..dc0f194 100644 --- a/reader/nordigen/nordigen.go +++ b/reader/nordigen/nordigen.go @@ -4,8 +4,6 @@ import ( "errors" "fmt" "log/slog" - "regexp" - "strings" "github.com/frieser/nordigen-go-lib/v2" "github.com/martinohansen/ynabber" @@ -33,31 +31,12 @@ func NewReader(cfg *ynabber.Config) Reader { } } -// payeeStripNonAlphanumeric removes all non-alphanumeric characters from payee -func payeeStripNonAlphanumeric(payee string) (x string) { - reg := regexp.MustCompile(`[^\p{L}]+`) - x = reg.ReplaceAllString(payee, " ") - return strings.TrimSpace(x) -} - -func (r Reader) toYnabber(a ynabber.Account, t nordigen.Transaction) (*ynabber.Transaction, error) { - transaction, err := r.Mapper(a, t) - if err != nil { - return nil, err - } - - // Execute strip method on payee if defined in config - if r.Config.Nordigen.PayeeStrip != nil { - transaction.Payee = transaction.Payee.Strip(r.Config.Nordigen.PayeeStrip) - } - - return transaction, nil -} - func (r Reader) toYnabbers(a ynabber.Account, t nordigen.AccountTransactions) ([]ynabber.Transaction, error) { + skipped := 0 + y := []ynabber.Transaction{} for _, v := range t.Transactions.Booked { - transaction, err := r.toYnabber(a, v) + transaction, err := r.Mapper(a, v) if err != nil { return nil, err } @@ -67,10 +46,12 @@ func (r Reader) toYnabbers(a ynabber.Account, t nordigen.AccountTransactions) ([ r.logger.Debug("mapped transaction", "from", v, "to", transaction) y = append(y, *transaction) } else { + skipped++ r.logger.Debug("skipping", "transaction", v) } } + r.logger.Info("read transactions", "total", len(y)+skipped, "skipped", skipped) return y, nil } diff --git a/reader/nordigen/nordigen_test.go b/reader/nordigen/nordigen_test.go index fd25e6d..153867f 100644 --- a/reader/nordigen/nordigen_test.go +++ b/reader/nordigen/nordigen_test.go @@ -1,39 +1,54 @@ package nordigen import ( - "reflect" + "log/slog" "testing" "time" "github.com/frieser/nordigen-go-lib/v2" + "github.com/google/go-cmp/cmp" "github.com/kelseyhightower/envconfig" "github.com/martinohansen/ynabber" ) -func TestToYnabber(t *testing.T) { +// getAccountTransactions returns a nordigen.AccountTransactions with a single +// transaction for testing purposes. +func getAccountTransactions(t nordigen.Transaction) nordigen.AccountTransactions { + return nordigen.AccountTransactions{ + Transactions: struct { + Booked []nordigen.Transaction `json:"booked,omitempty"` + Pending []nordigen.Transaction `json:"pending,omitempty"` + }{ + Booked: []nordigen.Transaction{t}, + }, + } +} +func TestToYnabber(t *testing.T) { + logger := slog.Default() var defaultConfig ynabber.Config _ = envconfig.Process("", &defaultConfig) type args struct { account ynabber.Account - t nordigen.Transaction + t nordigen.AccountTransactions } tests := []struct { + name string bankID string reader Reader args args - want *ynabber.Transaction + want []ynabber.Transaction wantErr bool }{ { // Tests a common Nordigen transaction from NORDEA_NDEADKKK with the // default config to highlight any breaking changes. bankID: "NORDEA_NDEADKKK", - reader: Reader{Config: &defaultConfig}, + reader: Reader{Config: &defaultConfig, logger: logger}, args: args{ account: ynabber.Account{Name: "foo", IBAN: "bar"}, - t: nordigen.Transaction{ + t: getAccountTransactions(nordigen.Transaction{ TransactionId: "H00000000000000000000", EntryReference: "", BookingDate: "2023-02-24", @@ -56,24 +71,50 @@ func TestToYnabber(t *testing.T) { RemittanceInformationUnstructuredArray: []string{""}, BankTransactionCode: "", AdditionalInformation: "VISA KØB"}, + ), }, - want: &ynabber.Transaction{ + want: []ynabber.Transaction{{ Account: ynabber.Account{Name: "foo", IBAN: "bar"}, ID: ynabber.ID("H00000000000000000000"), Date: time.Date(2023, time.February, 24, 0, 0, 0, 0, time.UTC), Payee: "Visa køb DKK HELLOFRESH Copenha Den", Memo: "Visa køb DKK 424,00 HELLOFRESH Copenha Den 23.02", - Amount: ynabber.Milliunits(10000), + Amount: ynabber.Milliunits(10000)}, + }, + wantErr: false, + }, + { + // Nordea should remove P transactions + name: "Remove P transactions", + bankID: "NORDEA_NDEADKKK", + reader: Reader{Config: &defaultConfig, logger: logger}, + args: args{ + account: ynabber.Account{Name: "foo", IBAN: "bar"}, + t: getAccountTransactions(nordigen.Transaction{ + TransactionId: "P4392858879202309260000000524", + BookingDate: "2023-09-26", + ValueDate: "2023-09-26", + TransactionAmount: struct { + Amount string `json:"amount,omitempty"` + Currency string `json:"currency,omitempty"` + }{ + Amount: "-4200.00", + Currency: "DKK", + }, + RemittanceInformationUnstructured: "Ovf. til, konto nr. 1111-222-333", + AdditionalInformation: "OVERFØRT TIL", + }), }, + want: []ynabber.Transaction{}, wantErr: false, }, { // Test transaction from SEB_KORT_AB_NO_SKHSFI21 bankID: "SEB_KORT_AB_NO_SKHSFI21", - reader: Reader{Config: &defaultConfig}, + reader: Reader{Config: &defaultConfig, logger: logger}, args: args{ account: ynabber.Account{Name: "foo", IBAN: "bar"}, - t: nordigen.Transaction{ + t: getAccountTransactions(nordigen.Transaction{ TransactionId: "foobar", EntryReference: "", BookingDate: "2023-02-24", @@ -96,54 +137,39 @@ func TestToYnabber(t *testing.T) { RemittanceInformationUnstructuredArray: []string{""}, BankTransactionCode: "PURCHASE", AdditionalInformation: "PASCAL AS"}, + ), }, - want: &ynabber.Transaction{ + want: []ynabber.Transaction{{ Account: ynabber.Account{Name: "foo", IBAN: "bar"}, ID: ynabber.ID("foobar"), Date: time.Date(2023, time.February, 24, 0, 0, 0, 0, time.UTC), Payee: "PASCAL AS", Memo: "", - Amount: ynabber.Milliunits(10000), + Amount: ynabber.Milliunits(10000)}, }, wantErr: false, }, } for _, tt := range tests { - t.Run(tt.bankID, func(t *testing.T) { + name := tt.bankID + if tt.name != "" { + name = name + "_" + tt.name + } + t.Run(name, func(t *testing.T) { // Set the BankID to the test case but keep the rest of the config // as is tt.reader.Config.Nordigen.BankID = tt.bankID - got, err := tt.reader.toYnabber(tt.args.account, tt.args.t) + got, err := tt.reader.toYnabbers(tt.args.account, tt.args.t) if (err != nil) != tt.wantErr { t.Errorf("error = %+v, wantErr %+v", err, tt.wantErr) return } - if !reflect.DeepEqual(got, tt.want) { + if cmp.Equal(got, tt.want) == false { t.Errorf("got = \n%+v, want \n%+v", got, tt.want) } }) } } - -func TestPayeeStripNonAlphanumeric(t *testing.T) { - want := "Im just alphanumeric" - got := payeeStripNonAlphanumeric("Im just alphanumeric") - if want != got { - t.Fatalf("alphanumeric: %s != %s", want, got) - } - - want = "你好世界" - got = payeeStripNonAlphanumeric("你好世界") - if want != got { - t.Fatalf("non-english: %s != %s", want, got) - } - - want = "Im not j ust alphanumeric" - got = payeeStripNonAlphanumeric("Im! not j.ust alphanumeric42 69") - if want != got { - t.Fatalf("non-alphanumeric: %s != %s", want, got) - } -} diff --git a/ynabber.go b/ynabber.go index 68012a1..8ee55de 100644 --- a/ynabber.go +++ b/ynabber.go @@ -2,7 +2,6 @@ package ynabber import ( "strconv" - "strings" "time" ) @@ -29,15 +28,6 @@ type ID string type Payee string -// Strip removes the elements from s from the payee -func (p Payee) Strip(s []string) Payee { - x := string(p) - for _, strip := range s { - x = strings.ReplaceAll(x, strip, "") - } - return Payee(strings.TrimSpace(x)) -} - type Milliunits int64 // Negate changes the sign of m to the opposite diff --git a/ynabber_test.go b/ynabber_test.go index d817803..51046da 100644 --- a/ynabber_test.go +++ b/ynabber_test.go @@ -41,33 +41,3 @@ func TestMilliunitsFromAmount(t *testing.T) { t.Fatalf("amount with no separator: %s != %s", want, got) } } - -func TestPayee_Strip(t *testing.T) { - type args struct { - s []string - } - tests := []struct { - name string - p Payee - args args - want Payee - }{ - {name: "single", - p: Payee("Im not here"), - args: args{s: []string{"not "}}, - want: "Im here", - }, - {name: "multiple", - p: Payee("Im not really here"), - args: args{s: []string{"not ", "really "}}, - want: "Im here", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := tt.p.Strip(tt.args.s); got != tt.want { - t.Errorf("Payee.Strip() = %v, want %v", got, tt.want) - } - }) - } -}