coreapi: Basic object API implementation #4492
Conversation
ghost
assigned
core/coreapi/interface/interface.go
Outdated
| // DataSize int | ||
| // CumulativeSize int | ||
| // } | ||
| AddLink(ctx context.Context, base Path, name string, child Path, create bool) (Node, error) //TODO: make create optional |
There was a problem hiding this comment.
I'll make this optional after the approach to optional params in #4477 is reviewed/accepted
There was a problem hiding this comment.
No need for arg names in an interface definition
core/coreapi/object.go
Outdated
| } | ||
|
|
||
| func (api *ObjectAPI) Put(context.Context, coreiface.Node) error { | ||
| return errors.New("todo") // TODO: what should this method take? Should we just redir to dag-put?f |
There was a problem hiding this comment.
I'm not even sure if we want to keep this method, it seems to me that all of it's use cases are covered by Dag.Put
There was a problem hiding this comment.
We should still implement it -- it can call DagAPI.Put() underneath
core/coreapi/interface/interface.go
Outdated
| Get(context.Context, Path) (Node, error) | ||
| Data(context.Context, Path) (io.Reader, error) | ||
| Links(context.Context, Path) ([]*Link, error) | ||
| Stat(context.Context, Path) (*ObjectStat, error) |
There was a problem hiding this comment.
I've been pondering in #4418 whether we should return a resolved Path in almost any place where we already have it resolved internally anyway. In some cases it saves a ton of additional resolve roundtrips, imagine we use the returned Path to pass it directly into another Core API function.
In addition, this is also useful in the case of functions that modify existing objects, e.g. SetData or AddLink. If I call it with /ipfs/QmFoo/a/b/c, I'd like it to return /ipfs/QmNewHash/a/b/c.
(Regardless of that, Put() should definitely return a Path.)
core/coreapi/object.go
Outdated
| type ObjectAPI CoreAPI | ||
|
|
||
| func (api *ObjectAPI) New(ctx context.Context) (coreiface.Node, error) { | ||
| node := new(dag.ProtoNode) |
There was a problem hiding this comment.
On the CLI/API, object new takes a template argument which can be unixfs-dir for a directory. We can do this here similarly to the KeyAPI.WithType() option in #4477.
core/coreapi/object.go
Outdated
| } | ||
|
|
||
| func (api *ObjectAPI) AddLink(ctx context.Context, base coreiface.Path, name string, child coreiface.Path, create bool) (coreiface.Node, error) { | ||
| rootNd, err := api.core().ResolveNode(ctx, base) |
There was a problem hiding this comment.
Let's call these variables base instead of root -- the root of /ipfs/QmFoo/a/b would be QmFoo, but the base of this operation is b
magik6k
force-pushed
the
feat/coreapi/object
branch
from
0c5d07c to
9713dff
Compare
magik6k
force-pushed
the
feat/coreapi/object
branch
2 times, most recently
from
29e178a to
2ac9768
Compare
magik6k
force-pushed
the
feat/coreapi/object
branch
from
5b58fed to
a65317d
Compare
| } | ||
|
|
||
| // ObjectStat provides information about dag nodes | ||
| type ObjectStat struct { |
There was a problem hiding this comment.
Should I wrap this into an interface for consistency?
There was a problem hiding this comment.
I think it's fine - given that the object API is quasi-frozen (it's the old merkledag/dag-pb structure), it's very unlikely this struct will ever have any functions attached to it.
|
|
||
| func ObjectNewOptions(opts ...ObjectNewOption) (*ObjectNewSettings, error) { | ||
| options := &ObjectNewSettings{ | ||
| Type: "empty", |
There was a problem hiding this comment.
The default for this should be "unixfs-dir" as in ipfs object new --help
There was a problem hiding this comment.
The default for this should be "unixfs-dir" as in ipfs object new --help
Ah, nevermind, I understand :)
|
This LGTM 👍 - pretty cool! Next let's extract coreiface to its own package, so we can get go-ipfs-api going. |
License: MIT Signed-off-by: Łukasz Magiera <[email protected]>
License: MIT Signed-off-by: Łukasz Magiera <[email protected]>
License: MIT Signed-off-by: Łukasz Magiera <[email protected]>
License: MIT Signed-off-by: Łukasz Magiera <[email protected]>
License: MIT Signed-off-by: Łukasz Magiera <[email protected]>
magik6k
force-pushed
the
feat/coreapi/object
branch
from
a65317d to
0377e49
Compare
TODO: