go-ipld-prime icon indicating copy to clipboard operation
go-ipld-prime copied to clipboard

Encoding and subsequent decoding of an ipld schema'd struct needs to work

Open willscott opened this issue 3 years ago • 2 comments

Suppose you have a struct define as

	ts.Accumulate(schema.SpawnStruct("Example", []schema.StructField{
		schema.SpawnStructField("Optional", "String", true, false),
	}, schema.SpawnStructRepresentationMap(map[string]string{})))

and you serialize it:

node := _Example {
  Optional: _String__Maybe{v:schema.Maybe_Abscent}
}
linkPrototype.Build(ctx, ipld.LinkContext{}, node, storer)

This will serialize (with the link prototype being dagjson) to

{
  "Optional": null
}

which.. is wrong?

on attempting to decode it, it will then fail.

willscott avatar Jun 08 '21 20:06 willscott

From my node-schema talks with @warpfork, I've come to understand that a struct's MapIterator will only skip optional fields which are absent when one is using the node's representation. It seems to be by design that, without the representation view, absent fields are included.

I agree it seems inconsistent that the non-repr node then does not allow assigning null. I think it should be a property that marshaling a node with a well-designed codec, and unmarshaling back into the same node's prototype, should not error. I'm less sure that it should give exactly the same data in memory, but at least decoding shouldn't error if encoding worked.

I think, in practice, everyone is expected to use representation nodes when encoding, and representation prototypes when decoding. A few weeks ago I argued that this should be done by default by all codecs, but Eric mentioned that maybe one does want to encode or decode a node without its repr view, e.g. for debugging purposes. Though I do personally think that the codecs should, by default, choose the repr view.

mvdan avatar Jun 08 '21 21:06 mvdan

https://github.com/ipld/go-ipld-prime/issues/191 is also now about this.

Re: unmarshalling back into the same node prototype: yeah, I agree that no matter what, we should get this to be more symmetrical. I'm afraid unmarshalling symmetrically isn't generally possible though: if there was a field that's optional nullable (both) then the coersion of absent to null was a lossy one.

warpfork avatar Jun 09 '21 00:06 warpfork