From 6a6fefd0cd1a2ac5d77a99ce56e22bed30236532 Mon Sep 17 00:00:00 2001 From: ionutboangiu Date: Fri, 19 May 2023 06:44:18 -0400 Subject: [PATCH] Improve cost formatting and parsing for CDRs The FormatCost function in cdr.go now accepts an additional parameter of type *config.RSRParser. This is then used to extract the value from its path as opposed to always using the value of the Cost field directly. Improved the unit test for the FormatCost function. Now it has become a table-driven test and it handles cases when the cost is retrieved from different fields other than from the CDR. --- apier/v1/cdre_amqp_it_test.go | 4 +- config/rsrparser.go | 15 +++ data/conf/samples/cdre/cgrates.json | 3 +- engine/cdr.go | 25 ++++- engine/cdr_test.go | 157 +++++++++++++++++++++++++--- 5 files changed, 181 insertions(+), 23 deletions(-) diff --git a/apier/v1/cdre_amqp_it_test.go b/apier/v1/cdre_amqp_it_test.go index ed4266e02..3871e62ba 100644 --- a/apier/v1/cdre_amqp_it_test.go +++ b/apier/v1/cdre_amqp_it_test.go @@ -165,7 +165,7 @@ func testAMQPMapAddCDRs(t *testing.T) { AnswerTime: time.Now(), RunID: utils.MetaDefault, Usage: time.Duration(30) * time.Second, - ExtraFields: map[string]string{"field_extr1": "val_extr1", "fieldextr2": "valextr2"}, + ExtraFields: map[string]string{"RawCost": "0.17"}, Cost: 1.01, }, { @@ -233,7 +233,7 @@ func testAMQPMapVerifyExport(t *testing.T) { } expCDRs := []string{ `{"Account":"1001","CGRID":"Cdr2","Category":"call","Cost":"-1.0000","Destination":"+4986517174963","OriginID":"OriginCDR2","RunID":"*default","Source":"test2","Tenant":"cgrates.org","Usage":"5s"}`, - `{"Account":"1001","CGRID":"Cdr3","Category":"call","Cost":"-1.0000","Destination":"+4986517174963","OriginID":"OriginCDR3","RunID":"*default","Source":"test2","Tenant":"cgrates.org","Usage":"30s"}`, + `{"Account":"1001","CGRID":"Cdr3","Category":"call","Cost":"0.1700","Destination":"+4986517174963","OriginID":"OriginCDR3","RunID":"*default","Source":"test2","Tenant":"cgrates.org","Usage":"30s"}`, } rcvCDRs := make([]string, 0) waiting := true diff --git a/config/rsrparser.go b/config/rsrparser.go index 526c2e5fc..7c2087674 100644 --- a/config/rsrparser.go +++ b/config/rsrparser.go @@ -19,6 +19,7 @@ along with this program. If not, see package config import ( + "errors" "fmt" "regexp" "strings" @@ -318,3 +319,17 @@ func (prsr *RSRParser) ParseDataProviderWithInterfaces(dP utils.DataProvider, se } return prsr.ParseValue(outIface) } + +// ParseDataProviderAsFloat64 retrieves a field from the provided DataProvider and attempts +// to parse it as a float64. +func (prsr *RSRParser) ParseDataProviderAsFloat64(dP utils.DataProvider, separator string) (float64, error) { + if prsr.path == "" { + // If the path is empty, we cannot retrieve any data. + return 0, errors.New("empty path in parser") + } + outIface, err := dP.FieldAsInterface(strings.Split(prsr.path, separator)) + if err != nil && (err != utils.ErrNotFound || prsr.filters.FilterRules() != "^$") { + return 0, err + } + return utils.IfaceAsFloat64(outIface) +} diff --git a/data/conf/samples/cdre/cgrates.json b/data/conf/samples/cdre/cgrates.json index 66b4f5bbd..6affc89fa 100644 --- a/data/conf/samples/cdre/cgrates.json +++ b/data/conf/samples/cdre/cgrates.json @@ -47,7 +47,8 @@ {"path": "*exp.Account", "type": "*composed", "value": "~*req.Account"}, {"path": "*exp.Destination", "type": "*composed", "value": "~*req.Destination"}, {"path": "*exp.Usage", "type": "*composed", "value": "~*req.Usage"}, - {"path": "*exp.Cost", "type": "*composed", "value": "~*req.Cost", "rounding_decimals": 4} + {"path": "*exp.Cost", "type": "*composed", "filters": ["*notstring:~*req.CGRID:Cdr3"], "value": "~*req.Cost", "rounding_decimals": 4}, + {"path": "*exp.Cost", "type": "*variable", "filters": ["*string:~*req.CGRID:Cdr3"], "value": "~*req.RawCost", "rounding_decimals": 4} ] }, "amqp_exporter_cdr": { diff --git a/engine/cdr.go b/engine/cdr.go index 30f14ade4..b54118edb 100644 --- a/engine/cdr.go +++ b/engine/cdr.go @@ -142,13 +142,19 @@ func (cdr *CDR) ComputeCGRID() { cdr.CGRID = utils.Sha1(cdr.OriginID, cdr.OriginHost) } -// FormatCost formats the cost as string on export -func (cdr *CDR) FormatCost(shiftDecimals, roundDecimals int) string { - cost := cdr.Cost +// FormatCost retrieves a cost related field from the CDR, shifts its decimal place, +// rounds it to the specified number of decimal places, and formats it as a string. +// The decimal place of the cost field is shifted by `shiftDecimals` places to the right. +// The cost field is then rounded to `roundDecimals` decimal places before it is formatted as a string. +func (cdr *CDR) FormatCost(value *config.RSRParser, shiftDecimals, roundDecimals int) (string, error) { + cost, err := cdr.FieldAsFloat64(value) + if err != nil { + return "", err + } if shiftDecimals != 0 { cost = cost * math.Pow10(shiftDecimals) } - return strconv.FormatFloat(cost, 'f', roundDecimals, 64) + return strconv.FormatFloat(cost, 'f', roundDecimals, 64), nil } // FieldAsString is used to retrieve fields as string, primary fields are const labeled @@ -161,6 +167,12 @@ func (cdr *CDR) FieldAsString(rsrPrs *config.RSRParser) (parsed string, err erro return } +// FieldAsFloat64 retrieves a field from the CDR and attempts to parse it as a float64. +// It uses the provided parser to identify and parse the field. +func (cdr *CDR) FieldAsFloat64(rsrPrs *config.RSRParser) (parsed float64, err error) { + return rsrPrs.ParseDataProviderAsFloat64(cdr.AsMapStorage(), utils.NestingSep) +} + // FieldsAsString concatenates values of multiple fields defined in template, used eg in CDR templates func (cdr *CDR) FieldsAsString(rsrFlds config.RSRParsers) string { outVal, err := rsrFlds.ParseDataProviderWithInterfaces( @@ -315,8 +327,11 @@ func (cdr *CDR) exportFieldValue(cfgCdrFld *config.FCTemplate, filterS *FilterS) var cdrVal string switch cfgCdrFld.Path { case utils.MetaExp + utils.NestingSep + utils.COST: - cdrVal = cdr.FormatCost(cfgCdrFld.CostShiftDigits, + cdrVal, err = cdr.FormatCost(rsrFld, cfgCdrFld.CostShiftDigits, cfgCdrFld.RoundingDecimals) + if err != nil { + return + } case utils.MetaExp + utils.NestingSep + utils.SetupTime: if cfgCdrFld.Layout == "" { cfgCdrFld.Layout = time.RFC3339 diff --git a/engine/cdr_test.go b/engine/cdr_test.go index c7de873a7..a2aee19ef 100644 --- a/engine/cdr_test.go +++ b/engine/cdr_test.go @@ -308,22 +308,149 @@ func TestFieldAsStringForCostDetails(t *testing.T) { } func TestFormatCost(t *testing.T) { - cdr := CDR{Cost: 1.01} - if cdr.FormatCost(0, 4) != "1.0100" { - t.Error("Unexpected format of the cost: ", cdr.FormatCost(0, 4)) + tests := []struct { + name string + cost float64 + rawCost string + parserPath string + shiftDecimals int + roundDecimals int + expectedOutput string + expectedError string + }{ + { + name: "Test1", + cost: 1.01, + rawCost: "0.17", + parserPath: "~*req.Cost", + shiftDecimals: 0, + roundDecimals: 4, + expectedOutput: "1.0100", + expectedError: "", + }, + { + name: "Test2", + cost: 1.01001, + rawCost: "0.17", + parserPath: "~*req.Cost", + shiftDecimals: 0, + roundDecimals: 4, + expectedOutput: "1.0100", + expectedError: "", + }, + { + name: "Test3", + cost: 1.01001, + rawCost: "0.17", + parserPath: "~*req.Cost", + shiftDecimals: 2, + roundDecimals: 0, + expectedOutput: "101", + expectedError: "", + }, + { + name: "Test4", + cost: 1.01001, + rawCost: "0.17", + parserPath: "~*req.Cost", + shiftDecimals: 1, + roundDecimals: 0, + expectedOutput: "10", + expectedError: "", + }, + { + name: "Test5", + cost: 1.01001, + rawCost: "0.17", + parserPath: "~*req.Cost", + shiftDecimals: 2, + roundDecimals: 3, + expectedOutput: "101.001", + expectedError: "", + }, + { + name: "Test6", + cost: 1.01, + rawCost: "0.17", + parserPath: "~*req.RawCost", + shiftDecimals: 0, + roundDecimals: 4, + expectedOutput: "0.1700", + expectedError: "", + }, + { + name: "Test7", + cost: 1.01001, + rawCost: "0.17123", + parserPath: "~*req.RawCost", + shiftDecimals: 0, + roundDecimals: 4, + expectedOutput: "0.1712", + expectedError: "", + }, + { + name: "Test8", + cost: 1.01001, + rawCost: "0.17123", + parserPath: "~*req.RawCost", + shiftDecimals: 2, + roundDecimals: 0, + expectedOutput: "17", + expectedError: "", + }, + { + name: "Test9", + cost: 1.01001, + rawCost: "0.17123", + parserPath: "~*req.RawCost", + shiftDecimals: 1, + roundDecimals: 0, + expectedOutput: "2", + expectedError: "", + }, + { + name: "Test10", + cost: 1.01001, + rawCost: "0.17123", + parserPath: "~*req.RawCost", + shiftDecimals: 2, + roundDecimals: 3, + expectedOutput: "17.123", + expectedError: "", + }, + { + name: "Test11", + cost: 1.01001, + rawCost: "invalidCost", + parserPath: "~*req.RawCost", + shiftDecimals: 2, + roundDecimals: 3, + expectedOutput: "", + expectedError: `strconv.ParseFloat: parsing "invalidCost": invalid syntax`, + }, } - cdr = CDR{Cost: 1.01001} - if cdr.FormatCost(0, 4) != "1.0100" { - t.Error("Unexpected format of the cost: ", cdr.FormatCost(0, 4)) - } - if cdr.FormatCost(2, 0) != "101" { - t.Error("Unexpected format of the cost: ", cdr.FormatCost(2, 0)) - } - if cdr.FormatCost(1, 0) != "10" { - t.Error("Unexpected format of the cost: ", cdr.FormatCost(1, 0)) - } - if cdr.FormatCost(2, 3) != "101.001" { - t.Error("Unexpected format of the cost: ", cdr.FormatCost(2, 3)) + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cdr := CDR{ + Cost: tt.cost, + ExtraFields: map[string]string{"RawCost": tt.rawCost}, + } + prsr := config.NewRSRParserMustCompile(tt.parserPath, true) + rcv, err := cdr.FormatCost(prsr, tt.shiftDecimals, tt.roundDecimals) + + if err != nil { + if tt.expectedError == "" { + t.Errorf("did not expect error, received: %v", err.Error()) + } + } else if tt.expectedError != "" { + t.Errorf("expected error: %s, received %v", tt.expectedError, err) + } + + if rcv != tt.expectedOutput { + t.Errorf("expected: <%v>, \nreceived: <%v>", tt.expectedOutput, rcv) + } + }) } }