Replace storage.IncompletePieceToWriter with io.Writer

It was incorrect to assume piece hashing only operates on incomplete chunk data. This actually uncovered a bug where duplicate hash checks occurred, and the redundant checks would fail due to not reading the completed data.

diff --git a/storage/interface.go b/storage/interface.go
index a1c6fcebf69b5fb0234d343c0fb0163f42fbb6a5..869556f80a547265fad99ba61d919f77e57e59c1 100644 (file)
--- a/storage/interface.go
+++ b/storage/interface.go
@@ -22,7 +22,8 @@ type TorrentImpl interface {
        Close() error
 }
 
-// Interacts with torrent piece data.
+// Interacts with torrent piece data. Optional interfaces to implement include io.WriterTo, such as
+// when a piece supports a more efficient way to write out incomplete chunks
 type PieceImpl interface {
        // These interfaces are not as strict as normally required. They can
        // assume that the parameters are appropriate for the dimensions of the
@@ -38,12 +39,6 @@ type PieceImpl interface {
        Completion() Completion
 }
 
-// Optional PieceImpl interface when pieces support a more efficient way to write incomplete chunks
-// out.
-type IncompletePieceToWriter interface {
-       WriteIncompleteTo(w io.Writer) error
-}
-
 type Completion struct {
        Complete bool
        Ok       bool

diff --git a/storage/piece_resource.go b/storage/piece_resource.go
index 7f87532042de21bb389c9b881da8a2288dc90bba..36088f32cf26718558e687baa3e8c168d5a81820 100644 (file)
--- a/storage/piece_resource.go
+++ b/storage/piece_resource.go
@@ -46,7 +46,7 @@ type PieceProvider interface {
 }
 
 type ConsecutiveChunkWriter interface {
-       WriteConsecutiveChunks(prefix string, _ io.Writer) error
+       WriteConsecutiveChunks(prefix string, _ io.Writer) (int64, error)
 }
 
 type piecePerResourcePiece struct {
@@ -54,14 +54,27 @@ type piecePerResourcePiece struct {
        rp resource.Provider
 }
 
-var _ IncompletePieceToWriter = piecePerResourcePiece{}
+var _ io.WriterTo = piecePerResourcePiece{}
 
-func (s piecePerResourcePiece) WriteIncompleteTo(w io.Writer) error {
+func (s piecePerResourcePiece) WriteTo(w io.Writer) (int64, error) {
        if ccw, ok := s.rp.(ConsecutiveChunkWriter); ok {
-               return ccw.WriteConsecutiveChunks(s.incompleteDirPath()+"/", w)
+               if s.mustIsComplete() {
+                       return ccw.WriteConsecutiveChunks(s.completedInstancePath(), w)
+               } else {
+                       return ccw.WriteConsecutiveChunks(s.incompleteDirPath()+"/", w)
+               }
        }
-       _, err := io.Copy(w, io.NewSectionReader(s.getChunks(), 0, s.mp.Length()))
-       return err
+       return io.Copy(w, io.NewSectionReader(s, 0, s.mp.Length()))
+}
+
+// Returns if the piece is complete. Ok should be true, because we are the definitive source of
+// truth here.
+func (s piecePerResourcePiece) mustIsComplete() bool {
+       completion := s.Completion()
+       if !completion.Ok {
+               panic("must know complete definitively")
+       }
+       return completion.Complete
 }
 
 func (s piecePerResourcePiece) Completion() Completion {
@@ -88,10 +101,9 @@ func (s piecePerResourcePiece) MarkNotComplete() error {
 }
 
 func (s piecePerResourcePiece) ReadAt(b []byte, off int64) (int, error) {
-       if s.Completion().Complete {
+       if s.mustIsComplete() {
                return s.completed().ReadAt(b, off)
        }
-       //panic("unexpected ReadAt of incomplete piece")
        return s.getChunks().ReadAt(b, off)
 }
 
@@ -156,8 +168,12 @@ func (s piecePerResourcePiece) getChunks() (chunks chunks) {
        return
 }
 
+func (s piecePerResourcePiece) completedInstancePath() string {
+       return path.Join("completed", s.mp.Hash().HexString())
+}
+
 func (s piecePerResourcePiece) completed() resource.Instance {
-       i, err := s.rp.NewInstance(path.Join("completed", s.mp.Hash().HexString()))
+       i, err := s.rp.NewInstance(s.completedInstancePath())
        if err != nil {
                panic(err)
        }

diff --git a/storage/sqlite/sqlite-storage.go b/storage/sqlite/sqlite-storage.go
index 7eeb8a101924804c0f83582ed3337985baabe535..9dded6b567a3d15c1ee3be8e7c10db6025d8a6c6 100644 (file)
--- a/storage/sqlite/sqlite-storage.go
+++ b/storage/sqlite/sqlite-storage.go
@@ -307,7 +307,7 @@ type provider struct {
 
 var _ storage.ConsecutiveChunkWriter = (*provider)(nil)
 
-func (p *provider) WriteConsecutiveChunks(prefix string, w io.Writer) (err error) {
+func (p *provider) WriteConsecutiveChunks(prefix string, w io.Writer) (written int64, err error) {
        p.withConn(func(conn conn) {
                err = io.EOF
                err = sqlitex.Exec(conn, `
@@ -321,7 +321,8 @@ func (p *provider) WriteConsecutiveChunks(prefix string, w io.Writer) (err error
                                r := stmt.ColumnReader(0)
                                //offset := stmt.ColumnInt64(1)
                                //log.Printf("got %v bytes at offset %v", r.Len(), offset)
-                               _, err := io.Copy(w, r)
+                               w1, err := io.Copy(w, r)
+                               written += w1
                                return err
                        },
                        len(prefix),

diff --git a/storage/wrappers.go b/storage/wrappers.go
index aa6c609b1d4d2595ad5d2adf7a8411a68052beb5..06cc12ad07cff700adb62dc9494772ae148006c3 100644 (file)
--- a/storage/wrappers.go
+++ b/storage/wrappers.go
@@ -38,14 +38,17 @@ type Piece struct {
        mip metainfo.Piece
 }
 
-func (p Piece) WriteIncompleteTo(w io.Writer) error {
-       if i, ok := p.PieceImpl.(IncompletePieceToWriter); ok {
-               return i.WriteIncompleteTo(w)
+var _ io.WriterTo = Piece{}
+
+// Why do we have this wrapper? Well PieceImpl doesn't implement io.Reader, so we can't let io.Copy
+// and friends check for io.WriterTo and fallback for us since they expect an io.Reader.
+func (p Piece) WriteTo(w io.Writer) (int64, error) {
+       if i, ok := p.PieceImpl.(io.WriterTo); ok {
+               return i.WriteTo(w)
        }
        n := p.mip.Length()
        r := io.NewSectionReader(p, 0, n)
-       _, err := io.CopyN(w, r, n)
-       return err
+       return io.CopyN(w, r, n)
 }
 
 func (p Piece) WriteAt(b []byte, off int64) (n int, err error) {

diff --git a/torrent.go b/torrent.go
index 412fad7da44750ecead326aa4de065695f1656b6..ced35bb71574de48cb8b4d41503eedacb351bac1 100644 (file)
--- a/torrent.go
+++ b/torrent.go
@@ -795,10 +795,10 @@ func (t *Torrent) hashPiece(piece pieceIndex) (ret metainfo.Hash, err error) {
        const logPieceContents = false
        if logPieceContents {
                var examineBuf bytes.Buffer
-               err = storagePiece.WriteIncompleteTo(io.MultiWriter(hash, &examineBuf))
+               _, err = storagePiece.WriteTo(io.MultiWriter(hash, &examineBuf))
                log.Printf("hashed %q with copy err %v", examineBuf.Bytes(), err)
        } else {
-               err = storagePiece.WriteIncompleteTo(hash)
+               _, err = storagePiece.WriteTo(hash)
        }
        missinggo.CopyExact(&ret, hash.Sum(nil))
        return