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

[http] Strange behaviour around sending reference types #72

Open
dignifiedquire opened this issue Mar 8, 2018 · 11 comments
Open

[http] Strange behaviour around sending reference types #72

dignifiedquire opened this issue Mar 8, 2018 · 11 comments

Comments

@dignifiedquire
Copy link
Member

dignifiedquire commented Mar 8, 2018

I am trying to implement a command which has Type either *big.Int or big.Cid.

  1. When using no Type and only MakeEncoder and send big.NewInt(0) I end up with the error unexpected type float64 (expected interface {})
  2. When I try to wrap the result in something like this:
type result struct {
  myint *big.Int
  cid *cid.Cid
}

and use Type: &result{}, I end up with result being nil when sending result{myint: big.NewInt(0)}

@whyrusleeping
Copy link
Member

unexported fields can't be json marshaled. Other packages can't read fields of your struct if you don't give them that ability.

@dignifiedquire
Copy link
Member Author

That explains case 2, but what about case 1, float vs big.Int?

@whyrusleeping
Copy link
Member

I don't know what youre expecting to happen. What types does json have? If you don't tell the commands lib what type to use, youre going to get json types. json has floats, json doesnt have 'big integers'

@Kubuxu
Copy link
Member

Kubuxu commented Mar 10, 2018

@dignifiedquire your struct result has two unexported fields: myint and cid. You either have to export them or implement manual marshalling. See go-cid as an example: https://github.com/ipfs/go-cid/blob/master/cid.go#L379-L415

@dignifiedquire
Copy link
Member Author

Yeah, I got that, but it is super surprising and not at all clear that a json encoder is used from the current docs :/

@dignifiedquire
Copy link
Member Author

also the first issue still is there, cmds lib shouldn't freak out on a bigInt when getting a float just because json is unable to represent that properly

@whyrusleeping
Copy link
Member

@dignifiedquire use your brain. a big int encodes to json as a float. Then, without telling the commands lib how to intepret whatever response it gets, you just generically try to decode the thing. So the json unmarshaler, without any type hints, says "this is a float" and decodes it as such.

This is the whole reason we have the type field on the commands.

@dignifiedquire
Copy link
Member Author

dignifiedquire commented Mar 17, 2018 via email

@Stebalien
Copy link
Member

Ideally, we'd be using CBOR or something but we'd need to add support for that.

@whyrusleeping
Copy link
Member

@dignifiedquire I don't think youre understanding this here. The explicit contract of the library is "SET THE TYPE ON THE COMMAND". I guess we could check to see if its unset and do panic("youre being pretty dumb right now")

@keks
Copy link
Contributor

keks commented Apr 14, 2018

@whyrusleeping @dignifiedquire Actually JSON doesn't use floats but Numbers, and big.Int implements json.Marshaler and json.Unmarshaler so you can actually do it that way if you expect big.Int. It's just that Go parses JSON Numbers into float64 if no type indication is given.
So to accomodate this you'll have to write your own option type (like you did) that implements json.(Unm|M)arshaler:

type Result struct {
Int *big.Int
Cid *cid.Cid
isInt bool // to decide which one is used
}

func (r *Result) JSONMarshal() ([]byte, error) {
  if r.isInt {
    return json.Marshal(r.Int)
  } else {
    return json.Marshal(r.Cid)
  }
}

func (r *Result) JSONUnmarshal(data []byte) error {
  if data[0] >= '0' && data[0] <= '9' {
    r.isInt = true
    r.Int = big.NewInt(0)
    return json.Unmarshal(data, r.Int)
  } else {
    r.isInt = false
    r.Cid = cid.NewCid() // or sth like that
    return json.Unmarshal(data, r.Cid)
  }
}

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

5 participants