ipld / go-ipld-prime

Golang interfaces for the IPLD Data Model, with core Codecs included, IPLD Schemas support, and some handy functional transforms tools.
MIT License
132 stars 49 forks source link

node/bindnode: should typed link nodes implement schema.TypedLinkNode? #272

Open mvdan opened 2 years ago

mvdan commented 2 years ago

See https://pkg.go.dev/github.com/ipld/go-ipld-prime@master/schema#TypedLinkNode.

For example, take this IPLD schema:

type Target struct {
    foo String
}

type Root struct {
    target &Target
}

Then imagine I use that schema with bindnode as follows:

ts := ipld.LoadSchema(...)
type Root struct {
    Target datamodel.Link
}
type Target struct {
    Foo string
}
proto := bindnode.Prototype((*Root)(nil), ts.TypeByName("Root"))

If I then build a node and reach for the target link node, should that link have a LinkTargetNodePrototype method? I outline some options below.

Note that none of this affects "untyped" links, such as Link or &Any - it only affects specific &SomeType schema links.

Option 1

Don't expose the method. This is what we currently do. However, note that this is a pretty significant downside to bindnode when compared to gengo codegen, as the latter has this method. And, when doing traversals that cross blocks, the lack of this method means that we fall back to basicnode.Prototype.Any, losing any ability to take advantage of schemas.

Option 2

Expose the method, inferring the Go type. Note how, in the example above, the Root Go type does not reference the Target Go type at all. bindnode does not see Target at all, so it can't use that Go type in LinkTargetNodePrototype.

This fixes the problem of losing the schema when crossing block boundaries, but still means that one of the main advantages of bindnode - being in control of the Go types - is lost.

Option 3

Expose the method, using the original Go type (provided or inferred). This would require providing bindnode with the Target Go type in some way. Note that the methods need to work with "typed links" anywhere, including struct fields, list elements, or just standalone link types. Here are some ideas:

Idea 3.1

Attach the "target Go type" to the parent Go type itself. For example:

type TargetLink struct {
    datamodel.Link
    _type *Target
}

type Root struct {
    Target TargetLink
}

The main upside is that we keep all the information in one place - the Go types. The main downside is the added complexity/verbosity for Go types using typed links.

Idea 3.2

Separately provide bindnode with the knowledge of the Target type. For instance, by adding:

bindnode.RegisterTypedLink((*Target)(nil), ts.TypeByName("Target"))

This is similar to what some other libraries do, to globally register a type that should back some form of indirection - an interface, a link, a sum type, etc. The main upside is not having to mess with the original Go types. The main downside is that the relationship with the Target Go type is separate, so one can easily forget to add it or keep it up to date.

Idea 3.3

Like idea 3.1, but with generics:

// in package bindnode
type TypedLink[TargetType any, LinkType interface{datamodel.Link | cid.Cid}] struct {
    LinkType
    _type *TargetType
}

type Root struct {
    Target bindnode.TypedLink[Target, datamodel.Link]
}

Essentially, using type parameters to hide the complexity of the struct holding the link and the type hint.

--

I don't think we want option 1 forever; it gives up too many advantages of schemas.

I similarly don't think we want option 2. It would put using bindnode with custom Go types at an unfair disadvantage.

As for implementing 3, I dislike "globally registering" types that way. Bindnode currently works without a global registry of the relationships between Go types and Schema types, and I'd like to keep it that way - the relationship is simply encoded in prototype and node vaules that you hold. So I think I'm discarding option 3.2.

I think option 3.3 is the best I've come up with so far. I don't think we can require Go 1.18+ for bindnode users until at least a year from now, so until that happens, I might go ahead with option 3.1 as a temporary solution, phasing it out in favor of 3.3 in the future.

cc @warpfork @rvagg @willscott @hannahhoward @aschmahmann also FYI @masih as I think we'd need this for IPRPC if we want to properly support typed links without codegen.

rvagg commented 2 years ago

So .. &Foo is meant to be a hint to the runtime about what to find at a link, this is information that the link itself carries, and should exist separate to where it exists in the object hierarchy, it's a disembodied link that tells you what it believes you should find when you follow it and leaves it to the runtime to enforce that or not.

With that lens, I'd like to see something along the lines of 3.1 and 3.3(ish), but maybe thinking a bit bigger than bindnode - could we introduce an interface for datamodel.Link so that this type hinting pattern can exist more broadly in ipld-prime?

type TypedLink interface {
  Type() <something here, Prototype maybe?>
}

Because ideally, it'd be awesome for this to be a concern of the link loader and prototype chooser -- "oh, you're a TypedLink? Great, you tell me what prototype to reach for!", possibly with the option to override that with a custom prototype chooser.

rvagg commented 2 years ago

^ further to that thought, consider the following two patterns, the first is the existing and the second is a possibility if we push this type hint into the link. Maybe we can have both? It just seems to me that this is more often a property of the link, not an adjacent concern, so if we can provide options to tie the what in with the link itself then it might make some interesting patterns possible?

lnk := cidlink.Link{Cid: cid})
pbNode := linksys.Load(ipld.LinkContext{}, lnk, dagpb.Type.PBNode)
lnk := cidlink.Link{Cid: cid, Type: dagpb.Type.PBNode})
pbNode := linksys.Load(ipld.LinkContext{}, lnk)
mvdan commented 2 years ago

@rvagg I think the TypedLink interface you're imagining is what already exists, and what I am trying to implement: https://pkg.go.dev/github.com/ipld/go-ipld-prime@master/schema#TypedLinkNode

Essentially, solutions 3.1, 3.2, and 3.3 are about making bindnode aware of what Go type should be used for the link target prototype when building nodes.

rvagg commented 2 years ago

ok then, of course it does, don't mind me

BigLep commented 2 years ago

2022-05-10 conversation: next step is to determine an answer to the question. We don't expect to prioritize the work in the short term.