Support number/string agnostic JSON decoding

[dacohen]

For certain applications, it's helpful to be able to decode a currency object from either a JSON string or number.

This PR uses the stdlib json.Number, instead of string, to handle either case gracefully.

[bojanz]

Thanks!

This is definitely worth a try. The tests aren't passing though, we need to make sure MarshalJSON() still returns a string, while Unmarshal can accept both a string and a number.

[dacohen]

I reverted MarshalJSON to use a string as before. However, because the current contract expects that an InvalidNumberError will be returned if the number field isn't a valid number, json.Number won't work, because it returns it's own error.

Instead, we can use json.RawMessage, and attempt to unmarshal it into a string. If this isn't successful, we can use the raw value, and let SetString handle any errors, similar to how it worked previously.

[bojanz]

Using json.RawMessage makes sense, but I don't think we can unmarshal to a string after that, that will still fail if the input was a number.

I suggest something like this:

    aux := struct {
        Number       json.RawMessage `json:"number"`
        CurrencyCode string          `json:"currency"`
    }{}
    if err := json.Unmarshal(data, &aux); err != nil {
        return err
    }
    var jsonNumber json.Number
    if err := json.Unmarshal(aux.Number, &jsonNumber); err != nil {
        if strings.HasPrefix(err.Error(), "json: invalid number") {
            return InvalidNumberError{strings.Trim(string(aux.Number), `"`)}
        } else {
            return err
        }
    }
    number := apd.Decimal{}
    if _, _, err := number.SetString(jsonNumber.String()); err != nil {
        return InvalidNumberError{jsonNumber.String()}
    }
    if aux.CurrencyCode == "" || !IsValid(aux.CurrencyCode) {
        return InvalidCurrencyCodeError{aux.CurrencyCode}
    }

[dacohen]

I added the test case you suggested, and all the tests appear to pass for me.

json.RawMessage is holding the raw bytes that represent that value in JSON. That means that the value will either be "3.45" (if its a string), or 3.45 (if its a number).

Currently, the PR does this:

var auxNumber string
if err = json.Unmarshal(aux.Number, &auxNumber); err != nil {
  auxNumber = string(aux.Number)
}

In the first case, Unmarshal doesn't return an error, because we're correctly unmarshaling a string into a string. In the second case, it returns an error because of the type mismatch, but then we set auxNumber to the raw value (3.45 in this case). Accordingly, if aux.Number is either a string or a number, the value of auxNumber will always be the same, valid string.

My only concern with your suggested solution is that it depends on the stability of the error text from the encoding/json library, which I think is generally an anti-pattern in Go. However, I'll defer to you if you think this is more readable.

Thanks for your help on this!

[bojanz]

Sorry for coming back to this so late. Your version is definitely cleaner, so proceeding. Thank you!