Move apply methods to an Applier type

This removes the distinction between "strict" and "fuzzy" application
by allowing future methods on Applier that control settings. It also
avoids state tracking in the text fragment apply signature by moving it
into the Applier type.

While in practice, an Applier will be used once and discarded, the
capability is provided to reset it for multiple uses.

gitdiff/apply.go

@@ -1,11 +1,9 @@
 package gitdiff
 
 import (
-	"bytes"
 	"errors"
 	"fmt"
 	"io"
-	"io/ioutil"
 )
 
 // Conflict indicates an apply failed due to a conflict between the patch and
@@ -85,70 +83,101 @@ func applyError(err error, args ...interface{}) error {
 	return e
 }
 
-// ApplyStrict writes data from src to dst, modifying it as described by the
-// fragments in the file. For text files, each fragment, including all context
-// lines, must exactly match src at the expected line number.
+var (
+	errApplyInProgress = errors.New("gitdiff: incompatible apply in progress")
+)
+
+const (
+	applyInitial = iota
+	applyText
+	applyBinary
+)
+
+// Applier applies changes described in fragments to source data. If changes
+// are described in multiple fragments, those fragments must be applied in
+// order, usually by calling ApplyFile.
 //
-// If the apply fails, ApplyStrict returns an *ApplyError wrapping the cause.
-// Partial data may be written to dst in this case.
-func (f *File) ApplyStrict(dst io.Writer, src io.Reader) error {
-	// TODO(bkeyes): take an io.ReaderAt and avoid this!
-	data, err := ioutil.ReadAll(src)
-	if err != nil {
-		return applyError(err)
-	}
+// By default, Applier operates in "strict" mode, where fragment content and
+// positions must exactly match those of the source.
+//
+// If an error occurs while applying, methods on Applier return instances of
+// *ApplyError that annotate the wrapped error with additional information
+// when available. If the error is because of a conflict between a fragment and
+// the source, the wrapped error will be a *Conflict.
+//
+// While an Applier can apply both text and binary fragments, only one fragment
+// type can be used without resetting the Applier. The first fragment applied
+// sets the type for the Applier. Mixing fragment types or mixing
+// fragment-level and file-level applies results in an error.
+type Applier struct {
+	src       io.ReaderAt
+	lineSrc   LineReaderAt
+	nextLine  int64
+	applyType int
+}
 
-	if f.IsBinary {
-		if f.BinaryFragment != nil {
-			return f.BinaryFragment.Apply(dst, bytes.NewReader(data))
+// NewApplier creates an Applier that reads data from src. If src is a
+// LineReaderAt, it is used directly to apply text fragments.
+func NewApplier(src io.ReaderAt) *Applier {
+	a := new(Applier)
+	a.Reset(src)
+	return a
+}
+
+// Reset resets the input and internal state of the Applier. If src is nil, the
+// existing source is reused.
+func (a *Applier) Reset(src io.ReaderAt) {
+	if src != nil {
+		a.src = src
+		if lineSrc, ok := src.(LineReaderAt); ok {
+			a.lineSrc = lineSrc
+		} else {
+			a.lineSrc = &lineReaderAt{r: src}
 		}
-		_, err = dst.Write(data)
-		return applyError(err)
+	}
+	a.nextLine = 0
+	a.applyType = applyInitial
+}
+
+// ApplyFile applies the changes in all of the fragments of f and writes the
+// result to dst.
+func (a *Applier) ApplyFile(dst io.Writer, f *File) error {
+	if a.applyType != applyInitial {
+		return applyError(errApplyInProgress)
 	}
 
-	// TODO(bkeyes): check for this conflict case
-	// &Conflict{"cannot create new file from non-empty src"}
+	if f.IsBinary && f.BinaryFragment != nil {
+		return a.ApplyBinaryFragment(dst, f.BinaryFragment)
+	}
 
-	lra := NewLineReaderAt(bytes.NewReader(data))
+	// TODO(bkeyes): sort fragments by start position
+	// TODO(bkeyes): merge overlapping fragments
 
-	var next int64
 	for i, frag := range f.TextFragments {
-		next, err = frag.ApplyStrict(dst, lra, next)
-		if err != nil {
+		if err := a.ApplyTextFragment(dst, frag); err != nil {
 			return applyError(err, fragNum(i))
 		}
 	}
 
-	// TODO(bkeyes): extract this to a utility
-	buf := make([][]byte, 64)
-	for {
-		n, err := lra.ReadLinesAt(buf, next)
-		if err != nil && err != io.EOF {
-			return applyError(err, lineNum(next+int64(n)))
-		}
-
-		for i := 0; i < n; i++ {
-			if _, err := dst.Write(buf[n]); err != nil {
-				return applyError(err, lineNum(next+int64(n)))
-			}
-		}
-
-		next += int64(n)
-		if n < len(buf) {
-			return nil
-		}
-	}
+	return applyError(a.Flush(dst))
 }
 
-// ApplyStrict copies from src to dst, from line start through then end of the
-// fragment, modifying the data as described by the fragment.  The fragment,
-// including all context lines, must exactly match src at the expected line
-// number. ApplyStrict returns the number of the next unprocessed line in src
-// and any error. When the error is not non-nil, partial data may be written.
-func (f *TextFragment) ApplyStrict(dst io.Writer, src LineReaderAt, start int64) (next int64, err error) {
+// ApplyTextFragment applies the changes in the fragment f and writes unwritten
+// data before the start of the fragment and the result to dst. If multiple
+// text fragments apply to the same source, ApplyTextFragment must be called in
+// order of increasing start position. As a result, each fragment can be
+// applied at most once before a call to Reset.
+func (a *Applier) ApplyTextFragment(dst io.Writer, f *TextFragment) error {
+	switch a.applyType {
+	case applyInitial, applyText:
+	default:
+		return applyError(errApplyInProgress)
+	}
+	a.applyType = applyText
+
 	// application code assumes fragment fields are consistent
 	if err := f.Validate(); err != nil {
-		return start, applyError(err)
+		return applyError(err)
 	}
 
 	// lines are 0-indexed, positions are 1-indexed (but new files have position = 0)
@@ -158,24 +187,35 @@ func (f *TextFragment) ApplyStrict(dst io.Writer, src LineReaderAt, start int64)
 	}
 	fragEnd := fragStart + f.OldLines
 
+	start := a.nextLine
 	if fragStart < start {
-		return start, applyError(&Conflict{"fragment overlaps with an applied fragment"})
+		return applyError(&Conflict{"fragment overlaps with an applied fragment"})
+	}
+
+	if f.OldPosition == 0 {
+		ok, err := isLen(a.src, 0)
+		if err != nil {
+			return applyError(err)
+		}
+		if !ok {
+			return applyError(&Conflict{"cannot create new file from non-empty src"})
+		}
 	}
 
 	preimage := make([][]byte, fragEnd-start)
-	n, err := src.ReadLinesAt(preimage, start)
+	n, err := a.lineSrc.ReadLinesAt(preimage, start)
 	switch {
 	case err == nil:
 	case err == io.EOF && n == len(preimage): // last line of frag has no newline character
 	default:
-		return start, applyError(err, lineNum(start+int64(n)))
+		return applyError(err, lineNum(start+int64(n)))
 	}
 
 	// copy leading data before the fragment starts
 	for i, line := range preimage[:fragStart-start] {
 		if _, err := dst.Write(line); err != nil {
-			next = start + int64(i)
-			return next, applyError(err, lineNum(next))
+			a.nextLine = start + int64(i)
+			return applyError(err, lineNum(a.nextLine))
 		}
 	}
 	preimage = preimage[fragStart-start:]
@@ -184,14 +224,15 @@ func (f *TextFragment) ApplyStrict(dst io.Writer, src LineReaderAt, start int64)
 	used := int64(0)
 	for i, line := range f.Lines {
 		if err := applyTextLine(dst, line, preimage, used); err != nil {
-			next = fragStart + used
-			return next, applyError(err, lineNum(next), fragLineNum(i))
+			a.nextLine = fragStart + used
+			return applyError(err, lineNum(a.nextLine), fragLineNum(i))
 		}
 		if line.Old() {
 			used++
 		}
 	}
-	return fragStart + used, nil
+	a.nextLine = fragStart + used
+	return nil
 }
 
 func applyTextLine(dst io.Writer, line Line, preimage [][]byte, i int64) (err error) {
@@ -201,34 +242,53 @@ func applyTextLine(dst io.Writer, line Line, preimage [][]byte, i int64) (err er
 	if line.New() {
 		_, err = io.WriteString(dst, line.Line)
 	}
-	return
+	return err
 }
 
-// Apply writes data from src to dst, modifying it as described by the
-// fragment.
-//
-// Unlike text fragments, binary fragments do not distinguish between strict
-// and non-strict application.
-func (f *BinaryFragment) Apply(dst io.Writer, src io.ReaderAt) error {
+// Flush writes any data following the last applied fragment to dst.
+func (a *Applier) Flush(dst io.Writer) (err error) {
+	switch a.applyType {
+	case applyInitial:
+		_, err = copyFrom(dst, a.src, 0)
+	case applyText:
+		_, err = copyLinesFrom(dst, a.lineSrc, a.nextLine)
+	case applyBinary:
+		// nothing to flush, binary apply "consumes" full source
+	}
+	return err
+}
+
+// ApplyBinaryFragment applies the changes in the fragment f and writes the
+// result to dst. At most one binary fragment can be applied before a call to
+// Reset.
+func (a *Applier) ApplyBinaryFragment(dst io.Writer, f *BinaryFragment) error {
+	if a.applyType != applyInitial {
+		return applyError(errApplyInProgress)
+	}
+	a.applyType = applyText
+
+	if f == nil {
+		return applyError(errors.New("nil fragment"))
+	}
+
 	switch f.Method {
 	case BinaryPatchLiteral:
 		if _, err := dst.Write(f.Data); err != nil {
 			return applyError(err)
 		}
 	case BinaryPatchDelta:
-		if err := applyBinaryDeltaFragment(dst, src, f.Data); err != nil {
+		if err := applyBinaryDeltaFragment(dst, a.src, f.Data); err != nil {
 			return applyError(err)
 		}
 	default:
 		return applyError(fmt.Errorf("unsupported binary patch method: %v", f.Method))
 	}
-
 	return nil
 }
 
 func applyBinaryDeltaFragment(dst io.Writer, src io.ReaderAt, frag []byte) error {
 	srcSize, delta := readBinaryDeltaSize(frag)
-	if err := checkBinarySrcSize(srcSize, src); err != nil {
+	if err := checkBinarySrcSize(src, srcSize); err != nil {
 		return err
 	}
 
@@ -342,20 +402,15 @@ func applyBinaryDeltaCopy(w io.Writer, op byte, delta []byte, src io.ReaderAt) (
 	return size, delta, err
 }
 
-func checkBinarySrcSize(size int64, src io.ReaderAt) error {
-	start := size
-	if start > 0 {
-		start--
-	}
-	var b [2]byte
-	n, err := src.ReadAt(b[:], start)
-	if err == io.EOF && (size == 0 && n == 0) || (size > 0 && n == 1) {
-		return nil
-	}
-	if err != nil && err != io.EOF {
+func checkBinarySrcSize(r io.ReaderAt, size int64) error {
+	ok, err := isLen(r, size)
+	if err != nil {
 		return err
 	}
-	return &Conflict{"fragment src size does not match actual src size"}
+	if !ok {
+		return &Conflict{"fragment src size does not match actual src size"}
+	}
+	return nil
 }
 
 func wrapEOF(err error) error {

gitdiff/apply_test.go

@@ -57,14 +57,13 @@ func TestTextFragmentApplyStrict(t *testing.T) {
 			},
 			Err: &Conflict{},
 		},
-		// TODO(bkeyes): this check has moved to the file level (probably)
-		// "errorNewFile": {
-		// 	Files: applyFiles{
-		// 		Src:   "text_fragment_error.src",
-		// 		Patch: "text_fragment_error_new_file.patch",
-		// 	},
-		// 	Err: &Conflict{},
-		// },
+		"errorNewFile": {
+			Files: applyFiles{
+				Src:   "text_fragment_error.src",
+				Patch: "text_fragment_error_new_file.patch",
+			},
+			Err: &Conflict{},
+		},
 	}
 
 	for name, test := range tests {
@@ -82,10 +81,10 @@ func TestTextFragmentApplyStrict(t *testing.T) {
 				t.Fatalf("patch should contain exactly one fragment, but it has %d", len(files[0].TextFragments))
 			}
 
-			frag := files[0].TextFragments[0]
+			applier := NewApplier(bytes.NewReader(src))
 
 			var dst bytes.Buffer
-			_, err = frag.ApplyStrict(&dst, NewLineReaderAt(bytes.NewReader(src)), 0)
+			err = applier.ApplyTextFragment(&dst, files[0].TextFragments[0])
 			if test.Err != nil {
 				checkApplyError(t, test.Err, err)
 				return
@@ -153,13 +152,10 @@ func TestBinaryFragmentApply(t *testing.T) {
 				t.Fatalf("patch should contain exactly one file, but it has %d", len(files))
 			}
 
-			frag := files[0].BinaryFragment
-			if frag == nil {
-				t.Fatalf("patch should contain a binary fragment, but it was nil")
-			}
+			applier := NewApplier(bytes.NewReader(src))
 
 			var dst bytes.Buffer
-			err = frag.Apply(&dst, bytes.NewReader(src))
+			err = applier.ApplyBinaryFragment(&dst, files[0].BinaryFragment)
 			if test.Err != nil {
 				checkApplyError(t, test.Err, err)
 				return

gitdiff/gitdiff.go

@@ -1,6 +1,7 @@
 package gitdiff
 
 import (
+	"errors"
 	"fmt"
 	"os"
 )
@@ -64,6 +65,10 @@ func (f *TextFragment) Header() string {
 // Validate checks that the fragment is self-consistent and appliable. Validate
 // returns an error if and only if the fragment is invalid.
 func (f *TextFragment) Validate() error {
+	if f == nil {
+		return errors.New("nil fragment")
+	}
+
 	var (
 		oldLines, newLines                     int64
 		leadingContext, trailingContext        int64
@@ -117,7 +122,7 @@ func (f *TextFragment) Validate() error {
 
 	// if a file is being created, it can only contain additions
 	if f.OldPosition == 0 && f.OldLines != 0 {
-		return fmt.Errorf("file creation fragment contains context or deletion lines")
+		return errors.New("file creation fragment contains context or deletion lines")
 	}
 
 	return nil

gitdiff/io.go

@@ -24,11 +24,6 @@ type LineReaderAt interface {
 	ReadLinesAt(lines [][]byte, offset int64) (n int, err error)
 }
 
-// NewLineReaderAt creates a LineReaderAt from an io.ReaderAt.
-func NewLineReaderAt(r io.ReaderAt) LineReaderAt {
-	return &lineReaderAt{r: r}
-}
-
 type lineReaderAt struct {
 	r     io.ReaderAt
 	index []int64
@@ -129,3 +124,82 @@ func lookupLines(index []int64, start, n int64) (size int64, offset int64) {
 	}
 	return
 }
+
+func isLen(r io.ReaderAt, n int64) (bool, error) {
+	off := n - 1
+	if off < 0 {
+		off = 0
+	}
+
+	var b [2]byte
+	nr, err := r.ReadAt(b[:], off)
+	if err == io.EOF {
+		return (n == 0 && nr == 0) || (n > 0 && nr == 1), nil
+	}
+	return false, err
+}
+
+// copyFrom writes bytes starting from offset off in src to dst stopping at the
+// end of src or at the first error. copyFrom returns the number of bytes
+// written and any error.
+func copyFrom(dst io.Writer, src io.ReaderAt, off int64) (written int64, err error) {
+	buf := make([]byte, 32*1024) // stolen from io.Copy
+	for {
+		nr, rerr := src.ReadAt(buf, off)
+		if nr > 0 {
+			nw, werr := dst.Write(buf[0:nr])
+			if nw > 0 {
+				written += int64(nw)
+			}
+			if werr != nil {
+				err = werr
+				break
+			}
+			if nr != nw {
+				err = io.ErrShortWrite
+				break
+			}
+		}
+		if rerr != nil {
+			if rerr != io.EOF {
+				err = rerr
+			}
+			break
+		}
+	}
+	return written, err
+}
+
+// copyLinesFrom writes lines starting from line off in src to dst stopping at
+// the end of src or at the first error. copyLinesFrom returns the number of
+// lines written and any error.
+func copyLinesFrom(dst io.Writer, src LineReaderAt, off int64) (written int64, err error) {
+	buf := make([][]byte, 32)
+ReadLoop:
+	for {
+		nr, rerr := src.ReadLinesAt(buf, off)
+		if nr > 0 {
+			for _, line := range buf[0:nr] {
+				nw, werr := dst.Write(line)
+				if nw > 0 {
+					written++
+				}
+				if werr != nil {
+					err = werr
+					break ReadLoop
+				}
+				if len(line) != nw {
+					err = io.ErrShortWrite
+					break ReadLoop
+				}
+			}
+		}
+		if rerr != nil {
+			if rerr != io.EOF {
+				err = rerr
+			}
+			break
+		}
+	}
+	return written, err
+}