fix: unmarshal numeric fields regardless of them being int or float

[wernerdweight]

This fixes #243. I also updated the test data so that it contains float values.

[kreudom]

This does not work for me for the FullTrack. The fields stay zero.

I think the problem is here:

spotify/track.go

Lines 141 to 148 in f5f7cf5

	var v struct {
	SimpleTrack
	Popularity float64 `json:"popularity"`
	IsPlayable *bool `json:"is_playable"`
	LinkedFrom *LinkedFromInfo `json:"linked_from"`
	}
	if err := json.Unmarshal(data, &v); err != nil {

Because SimpleTrack implements UnmarshalJSON, the struct v also implements it. Therefore json.Unmarshal directly calls UnmarshalJSON and ignores the other fields.

[strideynet]

Could we duplicate the test data, rather than modifying the existing tests, to ensure we have a mixture of cases with floats and with integers, it looks like Spotify can't make their mind up and it'd be good to know we can handle both!

[wernerdweight]

done

[strideynet]

I wonder if longterm it makes sense for us to return these fields as floats rather than casting them back to ints. It'd be a breaking change though so perhaps worth bundling with some other fixes. I'm also a little curious if it'd make more sense to have a custom type to avoid all these unmarshallers which could be easily forgotten.

[wernerdweight]

I kept it int for now, but removed the unmarshallers in favour of a custom type (more below)

[strideynet]

Thanks for raising this - could you address my remark re: the test data ? I think it's important to prove we can handle both ints and floats. Then I'm happy to move forward with this.

[wernerdweight]

@strideynet, I rewrote it with the custom type to avoid multiple unmarshallers. It's much simpler and flexible now. That said, it introduces a breaking change for comparisons. So if you, for some reason, compare one of the fields (e.g. the follower count) to an integer variable, it will break, as the custom type is not comparable to the int.

playlist.Followers > 100 // still works
threshold := 100
playlist.Followers > threshold // doesn't work (mismatched types Numeric and int)

I still think this is a better solution than chaining the unmarshallers on top of each other, but since this is a bc-break, we/you could consider switching to floats (which would actually be a small change now that there's a custom type). Thanks

EDIT: it's a bc-break invoked by a bc-break by Spotify, so maybe not a big deal (bc-break vs. doesn't work).

[strideynet]

I still think this is a better solution than chaining the unmarshallers on top of each other, but since this is a bc-break, we/you could consider switching to floats (which would actually be a small change now that there's a custom type).

Thanks - I think I'm happy enough with this hack for now. I'll cut a release with a breaking change warning.