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

bindnode unions force downshift into anonymous types #210

Closed
warpfork opened this issue Jul 26, 2021 · 0 comments
Closed

bindnode unions force downshift into anonymous types #210

warpfork opened this issue Jul 26, 2021 · 0 comments
Assignees

Comments

@warpfork
Copy link
Collaborator

Summary

bindnode is shaping up to be really awesome in a lot of ways, but unions are going to need more work.

Currently, the bindnode implementation of unions leans heavily on reflect.StructOf to create anonymous types. This works fine for supporting all the ipld.Node contracts, but produces a very nearly completely unusable result if you want to use bindnode.Unwrap, because the anonymous types can't be navigated with normal golang code (they can only be navigated with more reflection).

We're going to need to add support for some other ways to have bindnode work with unions that are more friendly to being worked with when wrapped and unwrapped.


Problem example

Here is an example of the problem one will currently encounter:

We can set up a union, and a member type for it, like this:

type MyUnionType struct {
	Index int         // very nearly the only shape `bindnode` currently supports
	Value interface{} // very nearly the only shape `bindnode` currently supports
}
func init() {
	TypeSystem.Accumulate(schema.SpawnUnion("MyUnionType",
		[]schema.TypeName{
			"MyExpectedMember",
		},
		schema.SpawnUnionRepresentationKeyed(map[string]schema.TypeName{
			"myExpectedMember": "MyExpectedMember",
		})))
}

type MyExpectedMember struct { // example content, details not relevant
	Name     string
	Metadata map[string]string
}
func init() {
	TypeSystem.Accumulate(schema.SpawnStruct("MyExpectedMember",
		[]schema.StructField{
			schema.SpawnStructField("name", "String", false, false),
			schema.SpawnStructField("metadata", "Map__String__String", false, false),
		},
		schema.SpawnStructRepresentationMap(nil)))
}

(Some minor details elided, but so far, so good.)

Now we try to use it. Let's say we want to deserialize something, so first we get a NodePrototype:

np := bindnode.Prototype((*MyUnionType)(nil), TypeSystem.TypeByName("MyUnionType"))

Then create a NodeBuilder, and let's feed some json into that:

nb := np.Representation().NewBuilder()
err := json.Decode(nb, strings.NewReader(`{"myExpectedMember":{"name":"foo","metadata":{}}}`))

Still fine. Now, next, we'd like to use bindnode.Unwrap, and navigate the data using the golang native form:

n := bindnode.Unwrap(nb.Build()).(*MyUnionType)
_ = n.Value.(*MyExpectedMember)  // panics!

And now we've hit the trouble. We'll receive an error something like:

panic: interface conversion: interface {} is struct { Name string; Metadata struct { Keys []string; Values map[string]string } }, not *mypackage.MyExpectedMember

This is a pretty sizable bummer. I can't currently think of any way to work around with this, either, short of going back to reflection. That's not the UX we want. So we're going to need some more code in bindnode.


Possible actions

approach#1

We could teach bindnode to recognize the golangoid pattern of struct { A *MemberA; B *MemberB } (and this could work either with or without a tag int field).

This is probably the easiest option, since it puts the type info there in golang. (I think this could be done without any changes to the public API of bindnode.) It's also probably typically regarded as a fairly highly usable option in terms of golang syntax -- although it does result in some wasted memory, for unions with large numbers of members; and yes, it can certainly be used wrong (no compile-time way to force than no more than one field can be non-nil).

approach#2

If we want to keep an Value interface{} field, we'll need to be able to register some mapping between schema type names and the golang types to instantiate for them.

This seems like a bit larger diff. I don't think we currently have that information.

Whether or not we do this approach first/soon, it might be worth thinking about the possibility -- leaving room for that information flow might affect public API.

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

2 participants