modules/lfstransfer: modify transfer.Backend interface

Make a few changes to make the transfer backend suit Gitea better.

Merge Upload into one method:
  Makes things simpler since out content store verifies size & hashes
  itself

Change Download return into io.ReadCloser:
  Removes dependency on filesystem

Add size: int64 arguments wherever appropriate
  In arguments for Upload and Verify, and in return for Download

Author: ConcurrentCrab

Diff for: modules/lfstransfer/transfer/backend.go

@@ -2,7 +2,6 @@ package transfer
 
 import (
 	"io"
-	"io/fs"
 )
 
 const (
@@ -15,10 +14,9 @@ const (
 // Backend is a Git LFS backend.
 type Backend interface {
 	Batch(op string, pointers []BatchItem, args Args) ([]BatchItem, error)
-	StartUpload(oid string, r io.Reader, args Args) (io.Closer, error)
-	FinishUpload(state io.Closer, args Args) error
-	Verify(oid string, args Args) (Status, error)
-	Download(oid string, args Args) (fs.File, error)
+	Upload(oid string, size int64, r io.Reader, args Args) error
+	Verify(oid string, size int64, args Args) (Status, error)
+	Download(oid string, args Args) (io.ReadCloser, int64, error)
 	LockBackend(args Args) LockBackend
 }
 

Diff for: modules/lfstransfer/transfer/hash.go

@@ -2,6 +2,7 @@ package transfer
 
 import (
 	"encoding/hex"
+	"fmt"
 	"hash"
 	"io"
 )
@@ -40,3 +41,39 @@ func (h *HashingReader) Read(p []byte) (int, error) {
 	h.hash.Write(p[:n])
 	return n, err
 }
+
+var _ io.Reader = (*VerifyingReader)(nil)
+
+// VerifyingReader is a reader that hashes the data it reads.
+// At EOF, it compares the OID and size with expected values and replaces EOF
+// with an error in the case of mismatch.
+type VerifyingReader struct {
+	r            *HashingReader
+	expectedOid  string
+	expectedSize int64
+}
+
+// NewVerifyingReader creates a new VerifyingReader.
+func NewVerifyingReader(r *HashingReader, oid string, size int64) *VerifyingReader {
+	return &VerifyingReader{
+		r:            r,
+		expectedOid:  oid,
+		expectedSize: size,
+	}
+}
+
+// Read reads data from the underlying HashingReader.
+// At EOF, it compares results and returns error if OID or size mismatch
+func (v *VerifyingReader) Read(p []byte) (int, error) {
+	n, err := v.r.Read(p)
+	if err == io.EOF {
+		// stream consumed, now check for mismatch
+		if v.r.Size() != v.expectedSize {
+			err = fmt.Errorf("%w: invalid object size, expected %v, got %v", ErrCorruptData, v.expectedSize, v.r.Size())
+		}
+		if v.r.Oid() != v.expectedOid {
+			err = fmt.Errorf("%w: invalid object ID, expected %v, got %v", ErrCorruptData, v.expectedOid, v.r.Oid())
+		}
+	}
+	return n, err
+}

Diff for: modules/lfstransfer/transfer/processor.go

@@ -1,7 +1,6 @@
 package transfer
 
 import (
-	"crypto/sha256"
 	"errors"
 	"fmt"
 	"io"
@@ -161,29 +160,13 @@ func (p *Processor) PutObject(oid string) (Status, error) {
 	if err != nil {
 		return nil, err
 	}
-	r := p.handler.Reader()
-	rdr := NewHashingReader(r, sha256.New())
-	state, err := p.backend.StartUpload(oid, rdr, args)
+	rdr := io.Reader(p.handler.Reader())
+	// we don't need to verify data here because the backend already does
+	//rdr = NewVerifyingReader(NewHashingReader(rdr, sha256.New()), oid, expectedSize)
+	err = p.backend.Upload(oid, expectedSize, rdr, args)
 	if err != nil {
 		return nil, err
 	}
-	defer state.Close() // nolint: errcheck
-	actualSize := rdr.Size()
-	if actualSize != expectedSize {
-		err := fmt.Errorf("invalid size, expected %d, got %d", expectedSize, actualSize)
-		if actualSize > expectedSize {
-			err = fmt.Errorf("%w: %s", ErrExtraData, err)
-		} else {
-			err = fmt.Errorf("%w: %s", ErrMissingData, err)
-		}
-		return nil, err
-	}
-	if actualOid := rdr.Oid(); actualOid != oid {
-		return nil, fmt.Errorf("%w: %s", ErrCorruptData, fmt.Sprintf("invalid object ID, expected %s, got %s", oid, actualOid))
-	}
-	if err := p.backend.FinishUpload(state, args); err != nil {
-		return nil, err
-	}
 	return SuccessStatus(), nil
 }
 
@@ -197,7 +180,11 @@ func (p *Processor) VerifyObject(oid string) (Status, error) {
 	if err != nil {
 		return nil, fmt.Errorf("%w: %s", ErrParseError, err)
 	}
-	return p.backend.Verify(oid, args)
+	size, err := SizeFromArgs(args)
+	if err != nil {
+		return nil, fmt.Errorf("%w: %s", ErrParseError, err)
+	}
+	return p.backend.Verify(oid, size, args)
 }
 
 // GetObject writes an object ID to the transfer protocol.
@@ -210,18 +197,14 @@ func (p *Processor) GetObject(oid string) (Status, error) {
 	if err != nil {
 		return nil, fmt.Errorf("%w: %s", ErrParseError, err)
 	}
-	r, err := p.backend.Download(oid, args)
+	r, size, err := p.backend.Download(oid, args)
 	if errors.Is(err, fs.ErrNotExist) {
 		return NewStatus(StatusNotFound, fmt.Sprintf("object %s not found", oid)), nil
 	}
 	if err != nil {
 		return nil, err
 	}
-	info, err := r.Stat()
-	if err != nil {
-		return nil, err
-	}
-	return NewSuccessStatusWithReader(r, fmt.Sprintf("size=%d", info.Size())), nil
+	return NewSuccessStatusWithReader(r, fmt.Sprintf("size=%d", size)), nil
 }
 
 // Lock writes a lock to the transfer protocol.