Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

int64 id value changed while decoding #202

Open
gadelkareem opened this issue Mar 28, 2021 · 12 comments
Open

int64 id value changed while decoding #202

gadelkareem opened this issue Mar 28, 2021 · 12 comments

Comments

@gadelkareem
Copy link

Using an id of 1616867757123793000, the jsonapi decoded the value into 1616867757123792896. Any idea what's wrong?
ex:

type r struct {
	ID int64 `jsonapi:"primary,r" json:"id"`
}

json

{"id":"1616867757123793000"}

after decode:

1616867757123792896
@quetzyg
Copy link
Contributor

quetzyg commented Mar 28, 2021

I've seen this happen before with Go 1.15.x. From the convo in golang/go#36657, this should have been sorted in version 1.16.x.

Disregard the previous.

@gadelkareem
Copy link
Author

@quetzyg I just upgraded to go version go1.16.2 darwin/amd64 but I am getting the same result.

@aren55555
Copy link
Contributor

aren55555 commented Apr 1, 2021

@gadelkareem it looks like you are decoding JSON not JSON API; not sure if this was intended or not. A JSON API representation would look something more like:

{
  
    "data": {
      "id": "1",
      ...
    }
}

To further help I've tried to replicate what you are seeing with a couple tests:

package main

import (
	"encoding/json"
	"fmt"
	"strings"
	"testing"

	"github.com/google/jsonapi"
)

type model struct {
	ID int64 `jsonapi:"primary,r" json:"id"`
}

const data = `{"id":"1616867757123793000"}`

func TestJSONAPI(t *testing.T) {
	m := &model{}
	if err := jsonapi.UnmarshalPayload(strings.NewReader(data), m); err != nil {
		t.Fatal(err)
	}
	fmt.Printf("%#v", m)
}

func TestJSON(t *testing.T) {
	m := &model{}
	if err := json.Unmarshal([]byte(data), m); err != nil {
		t.Fatal(err)
	}
	fmt.Printf("%#v", m)
}

Running the tests via go test -v ./... produces:

=== RUN   TestJSONAPI
    bug_test.go:21: data is not a jsonapi representation of '*main.model'
--- FAIL: TestJSONAPI (0.00s)
=== RUN   TestJSON
    bug_test.go:29: json: cannot unmarshal string into Go struct field model.id of type int64
--- FAIL: TestJSON (0.00s)

If you are using JSON, is it possible you are setting the value before decoding? More verbose code/examples would help in debugging.

@gadelkareem
Copy link
Author

@aren55555 I gave a wrong example, sorry for that. Here is the correct request I am sending

curl 'http://localhost:8081/api/v1/users/34/notifications' \
  -X 'PATCH' \
  -H 'Connection: keep-alive' \
  -H 'sec-ch-ua: " Not;A Brand";v="99", "Google Chrome";v="91", "Chromium";v="91"' \
  -H 'Accept: application/vnd.api+json' \
  -H 'X-Requested-With: XMLHttpRequest' \
  -H 'sec-ch-ua-mobile: ?0' \
  -H 'Authorization: Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpIjozNCwidCI6MCwiciI6ZmFsc2UsImV4cCI6MTYxNzI2NDAzOSwibmJmIjoxNjE3MjYzNzM5LCJpYXQiOjE2MTcyNjMxMzl9.oUbXkNofs3LNy7x34ADR7AEZf_BrNDNhyFZZ64ewENk' \
  -H 'User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/91.0.4464.5 Safari/537.36' \
  -H 'Content-Type: application/vnd.api+json' \
  -H 'Origin: http://localhost:8080' \
  -H 'Sec-Fetch-Site: same-site' \
  -H 'Sec-Fetch-Mode: cors' \
  -H 'Sec-Fetch-Dest: empty' \
  -H 'Referer: http://localhost:8080/' \
  -H 'Accept-Language: en-GB,en-US;q=0.9,en;q=0.8' \
  -H 'Cookie: i18n_redirected=en; cookielawinfo-checkbox-necessary=yes; cookielawinfo-checkbox-non-necessary=yes; _ga=GA1.1.703225094.1614008206' \
  --data-raw '{"data":{"type":"notifications","id":"1616867757123793000","attributes":{"read_receipt":true}}}' \
  --compressed

The debug output for that ID is 1616867757123792896

@aren55555
Copy link
Contributor

aren55555 commented Apr 1, 2021

@gadelkareem just a heads up that you pasted in your Authorization token into the cURL, you may want to redact it!

I've managed to replicate this with a unit test:

package main

import (
	"strings"
	"testing"

	"github.com/google/jsonapi"
)

type model struct {
	ID          int64 `jsonapi:"primary,notifications"`
	ReadReceipt bool  `jsonapi:"attr,read_receipt"`
}

const data = `{
	"data": {
			"type": "notifications",
			"id": "1616867757123793000",
			"attributes": {
					"read_receipt": true
			}
	}
}`

func TestJSONAPI(t *testing.T) {
	m := &model{}
	if err := jsonapi.UnmarshalPayload(strings.NewReader(data), m); err != nil {
		t.Fatal(err)
	}
	if got, want := m.ID, int64(1616867757123793000); got != want {
		t.Fatalf("got %v, want %v", got, want)
	}
}

Running go test -v ./...:

=== RUN   TestJSONAPI
    bug_test.go:31: got 1616867757123792896, want 1616867757123793000
--- FAIL: TestJSONAPI (0.00s)

My gut feeling hypothesis is that this likely has something to do with the string => int64 parsing. I'll have to look more deeply into this.

@gadelkareem
Copy link
Author

Thanks @aren55555

@quetzyg
Copy link
Contributor

quetzyg commented Apr 3, 2021

From what I can tell, this happens because the current approach to JSON decoding transforms any numeric value into a float64 by default.

if err := json.NewDecoder(in).Decode(payload); err != nil {
	return err
}

In order to preserve the literal value when unmarshalling, we should tell the decoder to save the value as string with UseNumber(), like:

decoder := json.NewDecoder(in)
decoder.UseNumber()
if err := decoder.Decode(payload); err != nil {
	return err
}

The handleNumeric() function (and probably some others) will have to be updated in order to handle the json.Number type, instead of float64.

Using the JSON decoder with default settings we get:

2021/04/03 16:44:02 

JSON PAYLOAD: &{{"data":{"attributes":{"computed_at":1615734000,"decimal":3,"fractional":1415926535897932,"name":"Pi","periodic":false,"value":3.141592653589793},"id":"314","type":"null-string-id"}} %!s(int64=0) %!s(int=-1)}

2021/04/03 16:44:02 

UNMARSHALLED PAYLOAD: &{null-string-id 314  map[computed_at:1.615734e+09 decimal:3 fractional:1.415926535897932e+15 name:Pi periodic:false value:3.141592653589793] map[] <nil> <nil>}

Hint: See the differences in computed_at and fractional values.

Telling the JSON decoder to preserve numbers we get:

2021/04/03 16:45:35 

JSON PAYLOAD: &{{"data":{"attributes":{"computed_at":1615734000,"decimal":3,"fractional":1415926535897932,"name":"Pi","periodic":false,"value":3.141592653589793},"id":"314","type":"null-string-id"}} %!s(int64=0) %!s(int=-1)}

2021/04/03 16:45:35 

UNMARSHALLED PAYLOAD: &{null-string-id 314  map[computed_at:1615734000 decimal:3 fractional:1415926535897932 name:Pi periodic:false value:3.141592653589793] map[] <nil> <nil>}

@aren55555 I'm able to help out, if you want.

@aren55555
Copy link
Contributor

@quetzyg if you want to take a swipe at it and submit a well tested PR I can take a look at this earliest on Sunday (not at a computer today) or later during the work week. LMK as I plan to fix either this or the rfc3339 thing this weekend.

@quetzyg
Copy link
Contributor

quetzyg commented Apr 3, 2021

I'll take a stab at this in the next few hours.

@aren55555
Copy link
Contributor

@quetzyg I encountered this float64 induced weirdness when writing the TestMarshal_Times tests in #204. Notice in TestMarshal_Times/default_byValue and TestMarshal_Times/default_byPointer I'm pulling values of float64 from the JSON map and casting them to int64.

Curious if you were able to make any progress with json.Number that I could take a look at?

@quetzyg
Copy link
Contributor

quetzyg commented Apr 5, 2021

I've made some progress, but haven't finished yet. I can open a draft PR with what I have done so far, if you want?

@quetzyg
Copy link
Contributor

quetzyg commented Apr 26, 2021

Mind checking the PR that fixes this issue @aren55555 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants