pbdagreader: use FSNode instead of protobuf structure
Focus on the UnixFS layer and avoid explicit references to protocol buffers format (used to serialize objects of that layer). Use the `unixfs.FSNode` structure which it abstracts from the `unixfs.pb.Data` format. Replace `PBDagReader` field `ftpb.Data` with `ft.FSNode`, renaming it to `file` (which is the type of UnixFS object represented in the reader) and changing its comment removing the "cached" reference, as this structure is not used here as a cache (`PBDagReader` doesn't modify the DAG, it's read-only). Also, removed unused `ProtoNode` field to avoid confusions, as it would normally be present if the `FSNode` was in fact used as a cache of the contents of the `ProtoNode`. An example of the advantage of shifting the focus from the format to the UnixFS layer is dropping the of use `len(pb.Blocksizes)` in favor of the more clear `NumChildren()` abstraction. Added `BlockSize()` accessor. License: MIT Signed-off-by: Lucas Molas <schomatis@gmail.com>
Lucas Molas committed
Jul 4, 2018 at 23:12 UTC
acb9e23163e79097d4644fba251d67315b0ed790
4 files changed
+36
-41
unixfs/archive/tar/writer.go
+8
-9
@@ -16,7 +16,6 @@ import (
16
upb "github.com/ipfs/go-ipfs/unixfs/pb"
17
18
ipld "gx/ipfs/QmWi2BYBL5gJ3CiAiQchg6rn1A8iBsrWy51EYxvHVjFvLb/go-ipld-format"
19
- proto "gx/ipfs/QmZ4Qi3GaRbjcx28Sme5eMH7RQjGkt8wHxt2a65oLaeFEV/gogo-protobuf/proto"
19
)
20
21
// Writer is a utility structure that helps to write
@@ -57,12 +56,12 @@ func (w *Writer) writeDir(nd *mdag.ProtoNode, fpath string) error {
56
})
57
}
58
60
-func (w *Writer) writeFile(nd *mdag.ProtoNode, pb *upb.Data, fpath string) error {
61
- if err := writeFileHeader(w.TarW, fpath, pb.GetFilesize()); err != nil {
59
+func (w *Writer) writeFile(nd *mdag.ProtoNode, fsNode *ft.FSNode, fpath string) error {
60
+ if err := writeFileHeader(w.TarW, fpath, fsNode.FileSize()); err != nil {
61
return err
62
}
63
65
- dagr := uio.NewPBFileReader(w.ctx, nd, pb, w.Dag)
64
+ dagr := uio.NewPBFileReader(w.ctx, nd, fsNode, w.Dag)
65
if _, err := dagr.WriteTo(w.TarW); err != nil {
66
return err
67
}
@@ -74,12 +73,12 @@ func (w *Writer) writeFile(nd *mdag.ProtoNode, pb *upb.Data, fpath string) error
73
func (w *Writer) WriteNode(nd ipld.Node, fpath string) error {
74
switch nd := nd.(type) {
75
case *mdag.ProtoNode:
77
- pb := new(upb.Data)
78
- if err := proto.Unmarshal(nd.Data(), pb); err != nil {
76
+ fsNode, err := ft.FSNodeFromBytes(nd.Data())
77
+ if err != nil {
78
return err
79
}
80
82
- switch pb.GetType() {
81
+ switch fsNode.GetType() {
82
case upb.Data_Metadata:
83
fallthrough
84
case upb.Data_Directory, upb.Data_HAMTShard:
@@ -87,9 +86,9 @@ func (w *Writer) WriteNode(nd ipld.Node, fpath string) error {
86
case upb.Data_Raw:
87
fallthrough
88
case upb.Data_File:
90
- return w.writeFile(nd, pb, fpath)
89
+ return w.writeFile(nd, fsNode, fpath)
90
case upb.Data_Symlink:
92
- return writeSymlinkHeader(w.TarW, string(pb.GetData()), fpath)
91
+ return writeSymlinkHeader(w.TarW, string(fsNode.GetData()), fpath)
92
default:
93
return ft.ErrUnrecognizedType
94
}
unixfs/io/dagreader.go
+4
-5
@@ -11,7 +11,6 @@ import (
11
ftpb "github.com/ipfs/go-ipfs/unixfs/pb"
12
13
ipld "gx/ipfs/QmWi2BYBL5gJ3CiAiQchg6rn1A8iBsrWy51EYxvHVjFvLb/go-ipld-format"
14
- proto "gx/ipfs/QmZ4Qi3GaRbjcx28Sme5eMH7RQjGkt8wHxt2a65oLaeFEV/gogo-protobuf/proto"
14
)
15
16
// Common errors
@@ -45,17 +44,17 @@ func NewDagReader(ctx context.Context, n ipld.Node, serv ipld.NodeGetter) (DagRe
44
case *mdag.RawNode:
45
return NewBufDagReader(n.RawData()), nil
46
case *mdag.ProtoNode:
48
- pb := new(ftpb.Data)
49
- if err := proto.Unmarshal(n.Data(), pb); err != nil {
47
+ fsNode, err := ft.FSNodeFromBytes(n.Data())
48
+ if err != nil {
49
return nil, err
50
}
51
53
- switch pb.GetType() {
52
+ switch fsNode.GetType() {
53
case ftpb.Data_Directory, ftpb.Data_HAMTShard:
54
// Dont allow reading directories
55
return nil, ErrIsDir
56
case ftpb.Data_File, ftpb.Data_Raw:
58
- return NewPBFileReader(ctx, n, pb, serv), nil
57
+ return NewPBFileReader(ctx, n, fsNode, serv), nil
58
case ftpb.Data_Metadata:
59
if len(n.Links()) == 0 {
60
return nil, errors.New("incorrectly formatted metadata object")
unixfs/io/pbdagreader.go
+18
-27
@@ -11,7 +11,6 @@ import (
11
ftpb "github.com/ipfs/go-ipfs/unixfs/pb"
12
13
ipld "gx/ipfs/QmWi2BYBL5gJ3CiAiQchg6rn1A8iBsrWy51EYxvHVjFvLb/go-ipld-format"
14
- proto "gx/ipfs/QmZ4Qi3GaRbjcx28Sme5eMH7RQjGkt8wHxt2a65oLaeFEV/gogo-protobuf/proto"
14
cid "gx/ipfs/QmapdYm1b22Frv3k17fqrBYTFRxwiaVJkB299Mfn33edeB/go-cid"
15
)
16
@@ -19,11 +18,8 @@ import (
18
type PBDagReader struct {
19
serv ipld.NodeGetter
20
22
- // the node being read
23
- node *mdag.ProtoNode
24
-
25
- // cached protobuf structure from node.Data
26
- pbdata *ftpb.Data
21
+ // UnixFS file (it should be of type `Data_File` or `Data_Raw` only).
22
+ file *ft.FSNode
23
24
// the current data buffer to be read from
25
// will either be a bytes.Reader or a child DagReader
@@ -51,18 +47,17 @@ type PBDagReader struct {
47
var _ DagReader = (*PBDagReader)(nil)
48
49
// NewPBFileReader constructs a new PBFileReader.
54
-func NewPBFileReader(ctx context.Context, n *mdag.ProtoNode, pb *ftpb.Data, serv ipld.NodeGetter) *PBDagReader {
50
+func NewPBFileReader(ctx context.Context, n *mdag.ProtoNode, file *ft.FSNode, serv ipld.NodeGetter) *PBDagReader {
51
fctx, cancel := context.WithCancel(ctx)
52
curLinks := getLinkCids(n)
53
return &PBDagReader{
58
- node: n,
54
serv: serv,
60
- buf: NewBufDagReader(pb.GetData()),
55
+ buf: NewBufDagReader(file.GetData()),
56
promises: make([]*ipld.NodePromise, len(curLinks)),
57
links: curLinks,
58
ctx: fctx,
59
cancel: cancel,
65
- pbdata: pb,
60
+ file: file,
61
}
62
}
63
@@ -105,21 +100,20 @@ func (dr *PBDagReader) precalcNextBuf(ctx context.Context) error {
100
101
switch nxt := nxt.(type) {
102
case *mdag.ProtoNode:
108
- pb := new(ftpb.Data)
109
- err = proto.Unmarshal(nxt.Data(), pb)
103
+ fsNode, err := ft.FSNodeFromBytes(nxt.Data())
104
if err != nil {
105
return fmt.Errorf("incorrectly formatted protobuf: %s", err)
106
}
107
114
- switch pb.GetType() {
108
+ switch fsNode.GetType() {
109
case ftpb.Data_Directory, ftpb.Data_HAMTShard:
110
// A directory should not exist within a file
111
return ft.ErrInvalidDirLocation
112
case ftpb.Data_File:
119
- dr.buf = NewPBFileReader(dr.ctx, nxt, pb, dr.serv)
113
+ dr.buf = NewPBFileReader(dr.ctx, nxt, fsNode, dr.serv)
114
return nil
115
case ftpb.Data_Raw:
122
- dr.buf = NewBufDagReader(pb.GetData())
116
+ dr.buf = NewBufDagReader(fsNode.GetData())
117
return nil
118
case ftpb.Data_Metadata:
119
return errors.New("shouldnt have had metadata object inside file")
@@ -146,7 +140,7 @@ func getLinkCids(n ipld.Node) []*cid.Cid {
140
141
// Size return the total length of the data from the DAG structured file.
142
func (dr *PBDagReader) Size() uint64 {
149
- return dr.pbdata.GetFilesize()
143
+ return dr.file.FileSize()
144
}
145
146
// Read reads data from the DAG structured file
@@ -244,17 +238,14 @@ func (dr *PBDagReader) Seek(offset int64, whence int) (int64, error) {
238
return offset, nil
239
}
240
247
- // Grab cached protobuf object (solely to make code look cleaner)
248
- pb := dr.pbdata
249
-
241
// left represents the number of bytes remaining to seek to (from beginning)
242
left := offset
252
- if int64(len(pb.Data)) >= offset {
243
+ if int64(len(dr.file.GetData())) >= offset {
244
// Close current buf to close potential child dagreader
245
if dr.buf != nil {
246
dr.buf.Close()
247
}
257
- dr.buf = NewBufDagReader(pb.GetData()[offset:])
248
+ dr.buf = NewBufDagReader(dr.file.GetData()[offset:])
249
250
// start reading links from the beginning
251
dr.linkPosition = 0
@@ -263,15 +254,15 @@ func (dr *PBDagReader) Seek(offset int64, whence int) (int64, error) {
254
}
255
256
// skip past root block data
266
- left -= int64(len(pb.Data))
257
+ left -= int64(len(dr.file.GetData()))
258
259
// iterate through links and find where we need to be
269
- for i := 0; i < len(pb.Blocksizes); i++ {
270
- if pb.Blocksizes[i] > uint64(left) {
260
+ for i := 0; i < dr.file.NumChildren(); i++ {
261
+ if dr.file.BlockSize(i) > uint64(left) {
262
dr.linkPosition = i
263
break
264
} else {
274
- left -= int64(pb.Blocksizes[i])
265
+ left -= int64(dr.file.BlockSize(i))
266
}
267
}
268
@@ -303,14 +294,14 @@ func (dr *PBDagReader) Seek(offset int64, whence int) (int64, error) {
294
noffset := dr.offset + offset
295
return dr.Seek(noffset, io.SeekStart)
296
case io.SeekEnd:
306
- noffset := int64(dr.pbdata.GetFilesize()) - offset
297
+ noffset := int64(dr.file.FileSize()) - offset
298
n, err := dr.Seek(noffset, io.SeekStart)
299
300
// Return negative number if we can't figure out the file size. Using io.EOF
301
// for this seems to be good(-enough) solution as it's only returned by
302
// precalcNextBuf when we step out of file range.
303
// This is needed for gateway to function properly
313
- if err == io.EOF && *dr.pbdata.Type == ftpb.Data_File {
304
+ if err == io.EOF && dr.file.GetType() == ftpb.Data_File {
305
return -1, nil
306
}
307
return n, err
unixfs/unixfs.go
+6
@@ -195,6 +195,12 @@ func (n *FSNode) RemoveBlockSize(i int) {
195
n.format.Blocksizes = append(n.format.Blocksizes[:i], n.format.Blocksizes[i+1:]...)
196
}
197
198
+// BlockSize returns the block size indexed by `i`.
199
+// TODO: Evaluate if this function should be bounds checking.
200
+func (n *FSNode) BlockSize(i int) uint64 {
201
+ return n.format.Blocksizes[i]
202
+}
203
+
204
// GetBytes marshals this node as a protobuf message.
205
func (n *FSNode) GetBytes() ([]byte, error) {
206
return proto.Marshal(&n.format)