support index tag

field/composite.go

@@ -6,14 +6,12 @@ import (
 	"fmt"
 	"math"
 	"reflect"
-	"regexp"
 	"strconv"
 	"sync"
 
 	"github.com/moov-io/iso8583/encoding"
 	"github.com/moov-io/iso8583/prefix"
 	"github.com/moov-io/iso8583/sort"
-
 	"github.com/moov-io/iso8583/utils"
 )
 
@@ -174,41 +172,36 @@ func (f *Composite) Unmarshal(v interface{}) error {
 
 	// iterate over struct fields
 	for i := 0; i < dataStruct.NumField(); i++ {
-		indexOrTag, err := getFieldIndexOrTag(dataStruct.Type().Field(i))
-		if err != nil {
-			return fmt.Errorf("getting field %d index: %w", i, err)
-		}
-
-		// skip field without index
-		if indexOrTag == "" {
+		indexTag := NewIndexTag(dataStruct.Type().Field(i))
+		if indexTag.Tag == "" {
 			continue
 		}
 
-		messageField, ok := f.subfields[indexOrTag]
+		messageField, ok := f.subfields[indexTag.Tag]
 		if !ok {
 			continue
 		}
 
 		// unmarshal only subfield that has the value set
-		if _, set := f.setSubfields[indexOrTag]; !set {
+		if _, set := f.setSubfields[indexTag.Tag]; !set {
 			continue
 		}
 
 		dataField := dataStruct.Field(i)
 		switch dataField.Kind() { //nolint:exhaustive
-		case reflect.Chan, reflect.Func, reflect.Map, reflect.Pointer, reflect.UnsafePointer, reflect.Interface, reflect.Slice:
-			if dataField.IsNil() {
+		case reflect.Pointer, reflect.Interface, reflect.Slice:
+			if dataField.IsNil() && dataField.Kind() != reflect.Slice {
 				dataField.Set(reflect.New(dataField.Type().Elem()))
 			}
 
-			err = messageField.Unmarshal(dataField.Interface())
+			err := messageField.Unmarshal(dataField.Interface())
 			if err != nil {
-				return fmt.Errorf("failed to get data from field %s: %w", indexOrTag, err)
+				return fmt.Errorf("unmarshalling field %s: %w", indexTag.Tag, err)
 			}
 		default: // Native types
-			err = messageField.Unmarshal(dataField)
+			err := messageField.Unmarshal(dataField)
 			if err != nil {
-				return fmt.Errorf("failed to get data from field %s: %w", indexOrTag, err)
+				return fmt.Errorf("unmarshalling field %s: %w", indexTag.Tag, err)
 			}
 		}
 	}
@@ -251,17 +244,12 @@ func (f *Composite) Marshal(v interface{}) error {
 
 	// iterate over struct fields
 	for i := 0; i < dataStruct.NumField(); i++ {
-		indexOrTag, err := getFieldIndexOrTag(dataStruct.Type().Field(i))
-		if err != nil {
-			return fmt.Errorf("getting field %d index: %w", i, err)
-		}
-
-		// skip field without index
-		if indexOrTag == "" {
+		indexTag := NewIndexTag(dataStruct.Type().Field(i))
+		if indexTag.Tag == "" {
 			continue
 		}
 
-		messageField, ok := f.subfields[indexOrTag]
+		messageField, ok := f.subfields[indexTag.Tag]
 		if !ok {
 			continue
 		}
@@ -271,12 +259,12 @@ func (f *Composite) Marshal(v interface{}) error {
 			continue
 		}
 
-		err = messageField.Marshal(dataField.Interface())
+		err := messageField.Marshal(dataField.Interface())
 		if err != nil {
-			return fmt.Errorf("failed to set data from field %s: %w", indexOrTag, err)
+			return fmt.Errorf("marshalling field %s: %w", indexTag.Tag, err)
 		}
 
-		f.setSubfields[indexOrTag] = struct{}{}
+		f.setSubfields[indexTag.Tag] = struct{}{}
 	}
 
 	return nil
@@ -675,22 +663,3 @@ func orderedKeys(kvs map[string]Field, sorter sort.StringSlice) []string {
 	sorter(keys)
 	return keys
 }
-
-var fieldNameTagRe = regexp.MustCompile(`^F.+$`)
-
-// getFieldIndexOrTag returns index or tag of the field. First, it checks the
-// field name. If it does not match F.+ pattern, it checks value of `index`
-// tag.  If empty string, then index/tag was not found for the field.
-func getFieldIndexOrTag(field reflect.StructField) (string, error) {
-	dataFieldName := field.Name
-
-	if fieldIndex := field.Tag.Get("index"); fieldIndex != "" {
-		return fieldIndex, nil
-	}
-
-	if len(dataFieldName) > 0 && fieldNameTagRe.MatchString(dataFieldName) {
-		return dataFieldName[1:], nil
-	}
-
-	return "", nil
-}

field/composite_test.go

@@ -3,7 +3,6 @@ package field
 import (
 	"encoding/hex"
 	"fmt"
-	"reflect"
 	"strconv"
 	"sync"
 	"testing"
@@ -349,15 +348,30 @@ type SubConstructedTLVTestData struct {
 	F9F45 *Hex
 }
 
-func TestComposite_Marshal(t *testing.T) {
+func TestCompositeField_Marshal(t *testing.T) {
 	t.Run("Marshal returns an error on provision of primitive type data", func(t *testing.T) {
 		composite := NewComposite(compositeTestSpec)
 		err := composite.Marshal("primitive str")
 		require.EqualError(t, err, "data is not a pointer or nil")
 	})
+
+	t.Run("Marshal skips fields without index tag", func(t *testing.T) {
+		// the following data contains 2 fields with proper types but
+		// without index tag
+		type tlvTestData struct {
+			Date          *Hex
+			TransactionID *Hex
+		}
+		composite := NewComposite(tlvTestSpec)
+		err := composite.Marshal(&tlvTestData{
+			Date:          NewHexValue("210720"),
+			TransactionID: NewHexValue("000000000501"),
+		})
+		require.NoError(t, err)
+	})
 }
 
-func TestCompositeFieldUnmarshal(t *testing.T) {
+func TestCompositeField_Unmarshal(t *testing.T) {
 	t.Run("Unmarshal gets data for composite field", func(t *testing.T) {
 		// first, we need to populate fields of composite field
 		// we will do it by packing the field
@@ -426,6 +440,30 @@ func TestCompositeFieldUnmarshal(t *testing.T) {
 		require.Equal(t, "210720", data.Date.Value())
 		require.Equal(t, "000000000501", data.TransactionID.Value())
 	})
+
+	t.Run("Unmarshal skips fields without index tag", func(t *testing.T) {
+		// the following data contains 2 fields with proper types but
+		// without index tag
+		type tlvTestData struct {
+			Date          *Hex
+			TransactionID *Hex
+		}
+		// first, we need to populate fields of composite field
+		// we will do it by packing the field
+		composite := NewComposite(tlvTestSpec)
+		err := composite.Marshal(&TLVTestData{
+			F9A:   NewHexValue("210720"),
+			F9F02: NewHexValue("000000000501"),
+		})
+		require.NoError(t, err)
+
+		_, err = composite.Pack()
+		require.NoError(t, err)
+
+		data := &tlvTestData{}
+		err = composite.Unmarshal(data)
+		require.NoError(t, err)
+	})
 }
 
 func TestTLVPacking(t *testing.T) {
@@ -597,7 +635,7 @@ func TestCompositePacking(t *testing.T) {
 		})
 
 		require.Error(t, err)
-		require.EqualError(t, err, "failed to set data from field 1: data does not match required *String or (string, *string, int, *int) type")
+		require.EqualError(t, err, "marshalling field 1: data does not match required *String or (string, *string, int, *int) type")
 	})
 
 	t.Run("Pack returns error on failure of subfield packing", func(t *testing.T) {
@@ -747,7 +785,7 @@ func TestCompositePacking(t *testing.T) {
 		err = composite.Unmarshal(data)
 
 		require.Error(t, err)
-		require.EqualError(t, err, "failed to get data from field 1: unsupported type: expected *String, *string, or reflect.Value, got *field.Numeric")
+		require.EqualError(t, err, "unmarshalling field 1: unsupported type: expected *String, *string, or reflect.Value, got *field.Numeric")
 	})
 
 	t.Run("Unpack returns an error on failure of subfield to unpack bytes", func(t *testing.T) {
@@ -1831,70 +1869,6 @@ func TestTLVJSONConversion(t *testing.T) {
 	})
 }
 
-func TestComposite_getFieldIndexOrTag(t *testing.T) {
-	t.Run("returns index from field name", func(t *testing.T) {
-		st := reflect.ValueOf(&struct {
-			F1 string
-		}{}).Elem()
-
-		index, err := getFieldIndexOrTag(st.Type().Field(0))
-
-		require.NoError(t, err)
-		require.Equal(t, "1", index)
-	})
-
-	t.Run("returns index from field tag instead of field name when both match", func(t *testing.T) {
-		st := reflect.ValueOf(&struct {
-			F1 string `index:"AB"`
-		}{}).Elem()
-
-		index, err := getFieldIndexOrTag(st.Type().Field(0))
-
-		require.NoError(t, err)
-		require.Equal(t, "AB", index)
-	})
-
-	t.Run("returns index from field tag", func(t *testing.T) {
-		st := reflect.ValueOf(&struct {
-			Name string `index:"abcd"`
-			F    string `index:"02"`
-		}{}).Elem()
-
-		// get index from field Name
-		index, err := getFieldIndexOrTag(st.Type().Field(0))
-
-		require.NoError(t, err)
-		require.Equal(t, "abcd", index)
-
-		// get index from field F
-		index, err = getFieldIndexOrTag(st.Type().Field(1))
-
-		require.NoError(t, err)
-		require.Equal(t, "02", index)
-	})
-
-	t.Run("returns empty string when no tag and field name does not match the pattern", func(t *testing.T) {
-		st := reflect.ValueOf(&struct {
-			Name string
-		}{}).Elem()
-
-		index, err := getFieldIndexOrTag(st.Type().Field(0))
-
-		require.NoError(t, err)
-		require.Empty(t, index)
-
-		// single letter field without tag is ignored
-		st = reflect.ValueOf(&struct {
-			F string
-		}{}).Elem()
-
-		index, err = getFieldIndexOrTag(st.Type().Field(0))
-
-		require.NoError(t, err)
-		require.Empty(t, index)
-	})
-}
-
 func TestComposit_concurrency(t *testing.T) {
 	t.Run("Pack and Marshal", func(t *testing.T) {
 		// packing and marshaling

field/index_tag.go

@@ -0,0 +1,99 @@
+package field
+
+import (
+	"reflect"
+	"regexp"
+	"strconv"
+	"strings"
+)
+
+var fieldNameIndexRe = regexp.MustCompile(`^F.+$`)
+
+type IndexTag struct {
+	ID int // is -1 if index is not a number
+
+	Tag string
+	// KeepZero tells the marshaler to use zero value and set bitmap bit to
+	// 1 for this field. Default behavior is to omit the field from the
+	// message if it's zero value.
+	KeepZero bool
+}
+
+func NewIndexTag(field reflect.StructField) IndexTag {
+	// value of the key "index" in the tag
+	var value string
+
+	// keep the order of tags for now, when index tag is deprecated we can
+	// change the order
+	for _, tag := range []string{"index", "iso8583"} {
+		if value = field.Tag.Get(tag); value != "" {
+			break
+		}
+	}
+
+	// format of the value is "id[,keep_zero_value]"
+	// id is the id of the field
+	// let's parse it
+	if value != "" {
+		tag, opts := parseTag(value)
+
+		id, err := strconv.Atoi(tag)
+		if err != nil {
+			id = -1
+		}
+
+		return IndexTag{
+			ID:       id,
+			Tag:      tag,
+			KeepZero: opts.Contains("keepzero"),
+		}
+	}
+
+	dataFieldName := field.Name
+	if len(dataFieldName) > 0 && fieldNameIndexRe.MatchString(dataFieldName) {
+		indexStr := dataFieldName[1:]
+		fieldIndex, err := strconv.Atoi(indexStr)
+		if err != nil {
+			return IndexTag{
+				ID:  -1,
+				Tag: indexStr,
+			}
+		}
+
+		return IndexTag{
+			ID:  fieldIndex,
+			Tag: indexStr,
+		}
+	}
+
+	return IndexTag{
+		ID: -1,
+	}
+}
+
+type tagOptions string
+
+// parseTag splits a struct field's index tag into its id and
+// comma-separated options.
+func parseTag(tag string) (string, tagOptions) {
+	tag, opt, _ := strings.Cut(tag, ",")
+	return tag, tagOptions(opt)
+}
+
+// Contains reports whether a comma-separated list of options
+// contains a particular substr flag. substr must be surrounded by a
+// string boundary or commas.
+func (o tagOptions) Contains(optionName string) bool {
+	if len(o) == 0 {
+		return false
+	}
+	s := string(o)
+	for s != "" {
+		var name string
+		name, s, _ = strings.Cut(s, ",")
+		if name == optionName {
+			return true
+		}
+	}
+	return false
+}