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

Golint: fix golint warnings in merkledag submodule #4665

Merged
merged 4 commits into from
Feb 7, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion merkledag/coding.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ func (n *ProtoNode) EncodeProtobuf(force bool) ([]byte, error) {
return n.encoded, nil
}

// Decoded decodes raw data and returns a new Node instance.
// DecodeProtobuf decodes raw data and returns a new Node instance.
func DecodeProtobuf(encoded []byte) (*ProtoNode, error) {
n := new(ProtoNode)
err := n.unmarshal(encoded)
Expand Down
6 changes: 6 additions & 0 deletions merkledag/errservice.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,28 +14,34 @@ type ErrorService struct {

var _ ipld.DAGService = (*ErrorService)(nil)

// Add returns an error.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find this description confusing, rather than say "an error" something like "returns ErrorService.Err" or maybe just "returns the error" rather than "an error".

func (cs *ErrorService) Add(ctx context.Context, nd ipld.Node) error {
return cs.Err
}

// AddMany returns an error.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

func (cs *ErrorService) AddMany(ctx context.Context, nds []ipld.Node) error {
return cs.Err
}

// Get returns an error.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Etc.

func (cs *ErrorService) Get(ctx context.Context, c *cid.Cid) (ipld.Node, error) {
return nil, cs.Err
}

// GetMany many returns an error.
func (cs *ErrorService) GetMany(ctx context.Context, cids []*cid.Cid) <-chan *ipld.NodeOption {
ch := make(chan *ipld.NodeOption)
close(ch)
return ch
}

// Remove returns an error.
func (cs *ErrorService) Remove(ctx context.Context, c *cid.Cid) error {
return cs.Err
}

// RemoveMany returns an error.
func (cs *ErrorService) RemoveMany(ctx context.Context, cids []*cid.Cid) error {
return cs.Err
}
30 changes: 20 additions & 10 deletions merkledag/merkledag.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// package merkledag implements the IPFS Merkle DAG datastructures.
// Package merkledag implements the IPFS Merkle DAG data structures.
package merkledag

import (
Expand All @@ -23,8 +23,14 @@ func init() {
ipld.Register(cid.DagCBOR, ipldcbor.DecodeBlock)
}

// contextKey is a type to use as value for the ProgressTracker contexts.
type contextKey string
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would export this type.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It has no use outside. If someone ever has a use, it can be exported then.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, your right. Sorry.


const progressContextKey contextKey = "progress"

// NewDAGService constructs a new DAGService (using the default implementation).
func NewDAGService(bs bserv.BlockService) *dagService {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does golint really complain about this?

I actually prefer to return concrete types whenever possible. It helps to deal with multiple potentially fulfilled interfaces. For example, if *dagService implements X which is a superset of DAGService, I can pass NewDAGService to something that wants an X without extra type assertions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Want me to export dagService instead?

// Note that the default implementation is also an ipld.LinkGetter.
func NewDAGService(bs bserv.BlockService) ipld.DAGService {
return &dagService{Blocks: bs}
}

Expand Down Expand Up @@ -147,8 +153,8 @@ func (sg *sesGetter) GetMany(ctx context.Context, keys []*cid.Cid) <-chan *ipld.
}

// Session returns a NodeGetter using a new session for block fetches.
func (ds *dagService) Session(ctx context.Context) ipld.NodeGetter {
return &sesGetter{bserv.NewSession(ctx, ds.Blocks)}
func (n *dagService) Session(ctx context.Context) ipld.NodeGetter {
return &sesGetter{bserv.NewSession(ctx, n.Blocks)}
}

// FetchGraph fetches all nodes that are children of the given node
Expand All @@ -159,7 +165,7 @@ func FetchGraph(ctx context.Context, root *cid.Cid, serv ipld.DAGService) error
ng = &sesGetter{bserv.NewSession(ctx, ds.Blocks)}
}

v, _ := ctx.Value("progress").(*ProgressTracker)
v, _ := ctx.Value(progressContextKey).(*ProgressTracker)
if v == nil {
return EnumerateChildrenAsync(ctx, GetLinksDirect(ng), root, cid.NewSet().Visit)
}
Expand All @@ -168,9 +174,8 @@ func FetchGraph(ctx context.Context, root *cid.Cid, serv ipld.DAGService) error
if set.Visit(c) {
v.Increment()
return true
} else {
return false
}
return false
}
return EnumerateChildrenAsync(ctx, GetLinksDirect(ng), root, visit)
}
Expand All @@ -179,8 +184,8 @@ func FetchGraph(ctx context.Context, root *cid.Cid, serv ipld.DAGService) error
// returns the indexes of any links pointing to it
func FindLinks(links []*cid.Cid, c *cid.Cid, start int) []int {
var out []int
for i, lnk_c := range links[start:] {
if c.Equals(lnk_c) {
for i, lnkC := range links[start:] {
if c.Equals(lnkC) {
out = append(out, i+start)
}
}
Expand Down Expand Up @@ -265,21 +270,26 @@ func EnumerateChildren(ctx context.Context, getLinks GetLinks, root *cid.Cid, vi
return nil
}

// ProgressTracker is used to show progress when fetching nodes.
type ProgressTracker struct {
Total int
lk sync.Mutex
}

// DeriveContext returns a new context with value "progress" derived from
// the given one.
func (p *ProgressTracker) DeriveContext(ctx context.Context) context.Context {
return context.WithValue(ctx, "progress", p)
return context.WithValue(ctx, progressContextKey, p)
}

// Increment adds one to the total progress.
func (p *ProgressTracker) Increment() {
p.lk.Lock()
defer p.lk.Unlock()
p.Total++
}

// Value returns the current progress.
func (p *ProgressTracker) Value() int {
p.lk.Lock()
defer p.lk.Unlock()
Expand Down
14 changes: 9 additions & 5 deletions merkledag/merkledag_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import (
"testing"
"time"

blocks "gx/ipfs/Qmej7nf81hi2x2tvjRBF3mcp74sQyuDH4VMYDGd1YtXjb2/go-block-format"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be grouped with cid, u, etc. (we group by standard, in-repo, then out-of-repo).


bserv "github.com/ipfs/go-ipfs/blockservice"
bstest "github.com/ipfs/go-ipfs/blockservice/test"
offline "github.com/ipfs/go-ipfs/exchange/offline"
Expand All @@ -22,7 +24,6 @@ import (
mdpb "github.com/ipfs/go-ipfs/merkledag/pb"
dstest "github.com/ipfs/go-ipfs/merkledag/test"
uio "github.com/ipfs/go-ipfs/unixfs/io"
blocks "gx/ipfs/Qmej7nf81hi2x2tvjRBF3mcp74sQyuDH4VMYDGd1YtXjb2/go-block-format"

u "gx/ipfs/QmNiJuT8Ja3hMVpBHXv3Q6dwmperaQ6JjLtpMQgMCD7xvx/go-ipfs-util"
cid "gx/ipfs/QmcZfnkapfECQGcLZaf9B79NRg7cRa9EnZh4LSbkCzwNvY/go-cid"
Expand Down Expand Up @@ -241,9 +242,10 @@ func TestFetchGraph(t *testing.T) {
// create an offline dagstore and ensure all blocks were fetched
bs := bserv.New(bsis[1].Blockstore(), offline.Exchange(bsis[1].Blockstore()))

offline_ds := NewDAGService(bs)
// we know the default dagService implements LinkGetter
offlineDS := NewDAGService(bs).(ipld.LinkGetter)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is exactly the case for having NewDAGService return the concrete type.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it is a case only used in tests (and tests in the same module). From an external user's point of view, it's not possible to know what *dagService can do because it's unexported.


err = EnumerateChildren(context.Background(), offline_ds.GetLinks, root.Cid(), func(_ *cid.Cid) bool { return true })
err = EnumerateChildren(context.Background(), offlineDS.GetLinks, root.Cid(), func(_ *cid.Cid) bool { return true })
if err != nil {
t.Fatal(err)
}
Expand All @@ -260,7 +262,9 @@ func TestEnumerateChildren(t *testing.T) {
}

set := cid.NewSet()
err = EnumerateChildren(context.Background(), ds.GetLinks, root.Cid(), set.Visit)
lg := ds.(ipld.LinkGetter)

err = EnumerateChildren(context.Background(), lg.GetLinks, root.Cid(), set.Visit)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -491,7 +495,7 @@ func TestCidRetention(t *testing.T) {
}

func TestCidRawDoesnNeedData(t *testing.T) {
srv := NewDAGService(dstest.Bserv())
srv := NewDAGService(dstest.Bserv()).(ipld.LinkGetter)
nd := NewRawNode([]byte("somedata"))

ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
Expand Down
31 changes: 26 additions & 5 deletions merkledag/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,13 @@ import (
ipld "gx/ipfs/Qme5bWv7wtjUNGsK2BNGVUFPKiuxWrsqrtvYwCLRw8YFES/go-ipld-format"
)

var ErrNotProtobuf = fmt.Errorf("expected protobuf dag node")
var ErrLinkNotFound = fmt.Errorf("no link by that name")
// Common errors
var (
ErrNotProtobuf = fmt.Errorf("expected protobuf dag node")
ErrLinkNotFound = fmt.Errorf("no link by that name")
)

// Node represents a node in the IPFS Merkle DAG.
// ProtoNode represents a node in the IPFS Merkle DAG.
// nodes have opaque data and a set of navigable links.
type ProtoNode struct {
links []*ipld.Link
Expand Down Expand Up @@ -73,12 +76,14 @@ func (n *ProtoNode) SetPrefix(prefix *cid.Prefix) {
}
}

// LinkSlice is a slice of ipld.Links
type LinkSlice []*ipld.Link

func (ls LinkSlice) Len() int { return len(ls) }
func (ls LinkSlice) Swap(a, b int) { ls[a], ls[b] = ls[b], ls[a] }
func (ls LinkSlice) Less(a, b int) bool { return ls[a].Name < ls[b].Name }

// NodeWithData builds a new Protonode with the given data.
func NodeWithData(d []byte) *ProtoNode {
return &ProtoNode{data: d}
}
Expand Down Expand Up @@ -204,15 +209,18 @@ func (n *ProtoNode) Copy() ipld.Node {
return nnode
}

// RawData returns the protobuf-encoded version of the node.
func (n *ProtoNode) RawData() []byte {
out, _ := n.EncodeProtobuf(false)
return out
}

// Data returns the data stored by this node.
func (n *ProtoNode) Data() []byte {
return n.data
}

// SetData stores data in this nodes.
func (n *ProtoNode) SetData(d []byte) {
n.encoded = nil
n.cached = nil
Expand Down Expand Up @@ -265,12 +273,14 @@ func (n *ProtoNode) Stat() (*ipld.NodeStat, error) {
}, nil
}

// Loggable implements the ipfs/go-log.Loggable interface.
func (n *ProtoNode) Loggable() map[string]interface{} {
return map[string]interface{}{
"node": n.String(),
}
}

// UnmarshalJSON reads the node fields from a JSON-encoded byte slice.
func (n *ProtoNode) UnmarshalJSON(b []byte) error {
s := struct {
Data []byte `json:"data"`
Expand All @@ -287,6 +297,7 @@ func (n *ProtoNode) UnmarshalJSON(b []byte) error {
return nil
}

// MarshalJSON returns a JSON representation of the node.
func (n *ProtoNode) MarshalJSON() ([]byte, error) {
out := map[string]interface{}{
"data": n.data,
Expand All @@ -296,6 +307,8 @@ func (n *ProtoNode) MarshalJSON() ([]byte, error) {
return json.Marshal(out)
}

// Cid returns the node's Cid, calculated according to its prefix
// and raw data contents.
func (n *ProtoNode) Cid() *cid.Cid {
if n.encoded != nil && n.cached != nil {
return n.cached
Expand All @@ -316,6 +329,7 @@ func (n *ProtoNode) Cid() *cid.Cid {
return c
}

// String prints the node's Cid.
func (n *ProtoNode) String() string {
return n.Cid().String()
}
Expand All @@ -332,18 +346,24 @@ func (n *ProtoNode) Multihash() mh.Multihash {
return n.cached.Hash()
}

// Links returns the node links.
func (n *ProtoNode) Links() []*ipld.Link {
return n.links
}

// SetLinks replaces the node links with the given ones.
func (n *ProtoNode) SetLinks(links []*ipld.Link) {
n.links = links
}

// Resolve is an alias for ResolveLink.
func (n *ProtoNode) Resolve(path []string) (interface{}, []string, error) {
return n.ResolveLink(path)
}

// ResolveLink consumes the first element of the path and obtains the link
// corresponding to it from the node. It returns the link
// and the path without the consumed element.
func (n *ProtoNode) ResolveLink(path []string) (*ipld.Link, []string, error) {
if len(path) == 0 {
return nil, nil, fmt.Errorf("end of path, no more links to resolve")
Expand All @@ -357,9 +377,10 @@ func (n *ProtoNode) ResolveLink(path []string) (*ipld.Link, []string, error) {
return lnk, path[1:], nil
}

// Tree returns the link names of the ProtoNode.
// ProtoNodes are only ever one path deep, so anything different than an empty
// string for p results in nothing. The depth parameter is ignored.
func (n *ProtoNode) Tree(p string, depth int) []string {
// ProtoNodes are only ever one path deep, anything below that results in
// nothing
if p != "" {
return nil
}
Expand Down
8 changes: 8 additions & 0 deletions merkledag/raw.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
ipld "gx/ipfs/Qme5bWv7wtjUNGsK2BNGVUFPKiuxWrsqrtvYwCLRw8YFES/go-ipld-format"
)

// RawNode represents a node which only contains data.
type RawNode struct {
blocks.Block
}
Expand Down Expand Up @@ -52,22 +53,27 @@ func NewRawNodeWPrefix(data []byte, prefix cid.Prefix) (*RawNode, error) {
return &RawNode{blk}, nil
}

// Links returns nil.
func (rn *RawNode) Links() []*ipld.Link {
return nil
}

// ResolveLink returns an error.
func (rn *RawNode) ResolveLink(path []string) (*ipld.Link, []string, error) {
return nil, nil, ErrLinkNotFound
}

// Resolve returns an error.
func (rn *RawNode) Resolve(path []string) (interface{}, []string, error) {
return nil, nil, ErrLinkNotFound
}

// Tree returns nil.
func (rn *RawNode) Tree(p string, depth int) []string {
return nil
}

// Copy performs a deep copy of this node and returns it as an ipld.Node
func (rn *RawNode) Copy() ipld.Node {
copybuf := make([]byte, len(rn.RawData()))
copy(copybuf, rn.RawData())
Expand All @@ -80,10 +86,12 @@ func (rn *RawNode) Copy() ipld.Node {
return &RawNode{nblk}
}

// Size returns the size of this node
func (rn *RawNode) Size() (uint64, error) {
return uint64(len(rn.RawData())), nil
}

// Stat returns some Stats about this node.
func (rn *RawNode) Stat() (*ipld.NodeStat, error) {
return &ipld.NodeStat{
CumulativeSize: len(rn.RawData()),
Expand Down
6 changes: 6 additions & 0 deletions merkledag/rwservice.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,26 +16,32 @@ type ComboService struct {

var _ ipld.DAGService = (*ComboService)(nil)

// Add writes a new node using the Write DAGService.
func (cs *ComboService) Add(ctx context.Context, nd ipld.Node) error {
return cs.Write.Add(ctx, nd)
}

// AddMany adds nodes using the Write DAGService.
func (cs *ComboService) AddMany(ctx context.Context, nds []ipld.Node) error {
return cs.Write.AddMany(ctx, nds)
}

// Get fetches a node using the Read DAGService.
func (cs *ComboService) Get(ctx context.Context, c *cid.Cid) (ipld.Node, error) {
return cs.Read.Get(ctx, c)
}

// GetMany fetches nodes using the Read DAGService.
func (cs *ComboService) GetMany(ctx context.Context, cids []*cid.Cid) <-chan *ipld.NodeOption {
return cs.Read.GetMany(ctx, cids)
}

// Remove deletes a node using the Write DAGService.
func (cs *ComboService) Remove(ctx context.Context, c *cid.Cid) error {
return cs.Write.Remove(ctx, c)
}

// RemoveMany deletes nodes using the Write DAGService.
func (cs *ComboService) RemoveMany(ctx context.Context, cids []*cid.Cid) error {
return cs.Write.RemoveMany(ctx, cids)
}
Loading