Proposal: use Go's native types for getting/setting ISO 8583 message data

[alovak]

Background:

Introduce a new way to handle ISO 8583 message data using Go's native types, while retaining the current mechanism for compatibility and flexibility.

Proposed Changes:

Currently, we use field.String, field.Numerci, and other field types to work with ISO 8583 message data:

// define a struct for the authorization request
type AuthorizationRequest struct {
	MTI                  *field.String  `index:"0"`
	PrimaryAccountNumber *field.String  `index:"2"`
	ProcessingCode       *field.String  `index:"3"`
	AmountTransaction    *field.Numeric `index:"4"`
	TransmissionDateTime *field.String  `index:"7"`
	STAN                 *field.String  `index:"11"`
	LocalTransactionTime *field.String  `index:"12"`
	LocalTransactionDate *field.String  `index:"13"`
	ExpirationDate       *field.String  `index:"14"`
	// ...
}

// usage
request := &AuthorizationRequest{
	MTI:                  field.NewStringValue("0100"),
	PrimaryAccountNumber: field.NewStringValue(data.CardNumber),
	ProcessingCode:       field.NewStringValue("200000"),
	AmountTransaction:    field.NewNumericValue(data.Amount), // 0.10 cents
	TransmissionDateTime: field.NewStringValue(data.CreatedAt.Format(TransmissionDateTimeFormat)),
	STAN:                 field.NewStringValue(data.STAN),
	LocalTransactionTime: field.NewStringValue(data.Acceptor.LocalTime.Format(spec.LocalTimeFormat)),
	LocalTransactionDate: field.NewStringValue(data.Acceptor.LocalTime.Format(spec.LocalDateFormat)),
}

message := iso8583.NewMessage(Spec)

err := message.Marshal(request)
// handle error
packed, err := message.Pack()
// handle error
// send packed message to the server

Allow to use use Go's native types like this:

// define a struct for the authorization request
type AuthorizationRequest struct {
	MTI                  string `index:"0"`
	PrimaryAccountNumber string `index:"2"`
	ProcessingCode       string `index:"3"`
	AmountTransaction    int    `index:"4"`
	TransmissionDateTime string `index:"7"`
	STAN                 string `index:"11"`
	LocalTransactionTime string `index:"12"`
	LocalTransactionDate string `index:"13"`
	ExpirationDate       string `index:"14"`
	// ...
}

// usage
request := &AuthorizationRequest{
	MTI:                  "0100",
	PrimaryAccountNumber: data.CardNumber,
	ProcessingCode:       "200000",
	AmountTransaction:    data.Amount,
	TransmissionDateTime: data.CreatedAt.Format(TransmissionDateTimeFormat),
	STAN:                 data.STAN,
	LocalTransactionTime: data.Acceptor.LocalTime.Format(spec.LocalTimeFormat),
	LocalTransactionDate: data.Acceptor.LocalTime.Format(spec.LocalDateFormat),
}

message := iso8583.NewMessage(Spec)

err := message.Marshal(request)
// handle error
packed, err := message.Pack()
// handle error
// send packed message to the server

The message field such as String will be capable to handle string, int types. The same is for Numeric. If the string type is used for the value, the Marshal method will try to convert it into int.

Motivation:

• Simplicity: leverage Go's native types, making the code cleaner and more intuitive.
• Reduced Overhead: No need for additional wrappers or custom field types for basic data types.

Concerns

In the current approach, when a struct field is set with a pointer to an instance of field.String or Numeric, Binary, etc., the marshaller understands that a value has been assigned to the message field, even if the actual value inside the field.String is 0 or an empty string "". This provides clarity that the field has been intentionally populated, even if with a zero value:

request := &AuthorizationRequest{
	// ...
	ProcessingCode: field.NewStringValue(""),
	// ...
}

However, when directly using the string type, if we don't provide a value (resulting in the default zero value of ""), our intention becomes ambiguous:

request := &AuthorizationRequest{
	// ...
	// ProcessingCode: "", absence of the field is the same as setting its value to ""
	// ...
}

• Did we intentionally set the field to an empty string?
• Or did we intend to skip this field entirely?

The ambiguity here can lead to unintended outcomes, making it crucial to define how the marshaller should interpret such scenarios.

To address this ambiguity, I propose skipping all zero values by default. Practically speaking, it's infrequent to utilize zero values for message fields, barring some exceptions such as transaction amounts, which can be 0.

For developers who wish to prevent fields with zero values from being skipped, we suggest several alternatives:

1. Employ the keepzero tag within the field tag for iso8583:

type AuthorizationRequest struct {
	// ...
	AmountTransaction int `index:"4,keepzero"`
}

2. For fields like amounts, consider using a string type with a value of "0", ensuring the field remains present.

[wadearnold]

I enjoy the simplicity

Are their any particular types such as binary or hex that we will need to expose in the future for something like EMV that would break this pattern?

[adamdecaf]

We might be able to detect of the field was set with Go ast/parser tools, but that's likely too complex. Is this going to be a change limited to the code reading index struct tags to handle more types? Or would it need to expand elsewhere in iso8583?

[alovak]

@wadearnold

Are their any particular types such as binary or hex that we will need to expose in the future for something like EMV that would break this pattern?

The underlying value for both field.Binary and field.Hex is []byte. So, in the struct for either of these fields we will use []byte:

type AuthorizationRequest struct {
	// ...
	SomeEncryptedData []byte `index:"99"`
}

We can do some convenient things to work with hex like this:

// in the spec, field 99 is `field.Binary`

type AuthorizationRequest struct {
	// ...
	SomeEncryptedData string `index:"99"` 
}

err := message.Marshal(&AuthorizationRequest{
	// this is a hex and it will be decoded into []byte when value is assigned to the message field
	SomeEncryptedData: "16ddf39d3d8a4a99" 
})

[alovak]

@adamdecaf

We might be able to detect of the field was set with Go ast/parser tools, but that's likely too complex. Is this going to be a change limited to the code reading index struct tags to handle more types? Or would it need to expand elsewhere in iso8583?

The change will impact Marshal/Unmarshal methods of the Message and the fields: Composite,String, Numeric, Binary, Hex.

This is how the change inside simple field will look like: https://github.com/moov-io/iso8583/pull/273/files#diff-79ac174d65bca7ada39659ad4ae9fb2ae82b4532ca22f9392010bad81638708cR129

This is the change in the Message (and similar will have to be done in Composite): https://github.com/moov-io/iso8583/pull/273/files#diff-2ce052ca28a77c3b54be26bfc568dddef91bf239253d0ad537609af1731032d4R376

[adamdecaf]

The Marshal/Unmarshal changes make sense to me. It's a nice cleanup.

We're just going to assume string -> []byte is hex, binary string, or raw bytes? (In that order)

[alovak]

We're just going to assume string -> []byte is hex, binary string, or raw bytes? (In that order)

I think we can limit string to only hex. The rest will return an error. If we support other not trivial conversions, that may lead to confusion. I'm not sure 100% about this, though.

[wadearnold]

It doesn't seem we are adding complex validation or anything fancy in the type conversations. It makes the code more readable. Approved

[alovak]

Great! Thanks for the review @adamdecaf @wadearnold!
I created a set of tasks to implement the proposal https://github.com/moov-io/iso8583/milestone/1.