this repo has no description
0
fork

Configure Feed

Select the types of activity you want to include in your feed.

Improve detection of in-progress application

Applying a file now rejects future text fragment applies under the
assumption that all relevant fragments were part of the file.

+98 -21
+35 -21
gitdiff/apply.go
··· 4 4 "errors" 5 5 "fmt" 6 6 "io" 7 + "sort" 7 8 ) 8 9 9 10 // Conflict indicates an apply failed due to a conflict between the patch and ··· 68 69 69 70 e, ok := err.(*ApplyError) 70 71 if !ok { 71 - e = &ApplyError{err: wrapEOF(err)} 72 + if err == io.EOF { 73 + err = io.ErrUnexpectedEOF 74 + } 75 + e = &ApplyError{err: err} 72 76 } 73 77 for _, arg := range args { 74 78 switch v := arg.(type) { ··· 91 95 applyInitial = iota 92 96 applyText 93 97 applyBinary 98 + applyFile 94 99 ) 95 100 96 101 // Applier applies changes described in fragments to source data. If changes ··· 145 150 if a.applyType != applyInitial { 146 151 return applyError(errApplyInProgress) 147 152 } 153 + defer func() { a.applyType = applyFile }() 148 154 149 - if f.IsBinary && f.BinaryFragment != nil { 150 - return a.ApplyBinaryFragment(dst, f.BinaryFragment) 155 + if f.IsBinary && len(f.TextFragments) > 0 { 156 + return applyError(errors.New("binary file contains text fragments")) 157 + } 158 + if !f.IsBinary && f.BinaryFragment != nil { 159 + return applyError(errors.New("text file contains binary fragment")) 151 160 } 152 161 153 - // TODO(bkeyes): sort fragments by start position 154 - // TODO(bkeyes): merge overlapping fragments 162 + switch { 163 + case f.BinaryFragment != nil: 164 + return a.ApplyBinaryFragment(dst, f.BinaryFragment) 155 165 156 - for i, frag := range f.TextFragments { 157 - if err := a.ApplyTextFragment(dst, frag); err != nil { 158 - return applyError(err, fragNum(i)) 166 + case len(f.TextFragments) > 0: 167 + frags := make([]*TextFragment, len(f.TextFragments)) 168 + copy(frags, f.TextFragments) 169 + 170 + sort.Slice(frags, func(i, j int) bool { 171 + return frags[i].OldPosition < frags[j].OldPosition 172 + }) 173 + 174 + // TODO(bkeyes): consider merging overlapping fragments 175 + // right now, the application fails if fragments overlap, but it should be 176 + // possible to precompute the result of applying them in order 177 + 178 + for i, frag := range frags { 179 + if err := a.ApplyTextFragment(dst, frag); err != nil { 180 + return applyError(err, fragNum(i)) 181 + } 159 182 } 160 183 } 161 184 ··· 168 191 // order of increasing start position. As a result, each fragment can be 169 192 // applied at most once before a call to Reset. 170 193 func (a *Applier) ApplyTextFragment(dst io.Writer, f *TextFragment) error { 171 - switch a.applyType { 172 - case applyInitial, applyText: 173 - default: 194 + if a.applyType != applyInitial && a.applyType != applyText { 174 195 return applyError(errApplyInProgress) 175 196 } 176 - a.applyType = applyText 197 + defer func() { a.applyType = applyText }() 177 198 178 199 // application code assumes fragment fields are consistent 179 200 if err := f.Validate(); err != nil { ··· 265 286 if a.applyType != applyInitial { 266 287 return applyError(errApplyInProgress) 267 288 } 268 - a.applyType = applyText 289 + defer func() { a.applyType = applyBinary }() 269 290 270 291 if f == nil { 271 292 return applyError(errors.New("nil fragment")) ··· 395 416 // TODO(bkeyes): consider pooling these buffers 396 417 b := make([]byte, size) 397 418 if _, err := src.ReadAt(b, offset); err != nil { 398 - return 0, delta, wrapEOF(err) 419 + return 0, delta, err 399 420 } 400 421 401 422 _, err = w.Write(b) ··· 412 433 } 413 434 return nil 414 435 } 415 - 416 - func wrapEOF(err error) error { 417 - if err == io.EOF { 418 - err = io.ErrUnexpectedEOF 419 - } 420 - return err 421 - }
+63
gitdiff/apply_test.go
··· 10 10 "testing" 11 11 ) 12 12 13 + func TestApplierInvariants(t *testing.T) { 14 + binary := &BinaryFragment{ 15 + Method: BinaryPatchLiteral, 16 + Size: 2, 17 + Data: []byte("\xbe\xef"), 18 + } 19 + 20 + text := &TextFragment{ 21 + NewPosition: 1, 22 + NewLines: 1, 23 + LinesAdded: 1, 24 + Lines: []Line{ 25 + {Op: OpAdd, Line: "new line\n"}, 26 + }, 27 + } 28 + 29 + file := &File{ 30 + TextFragments: []*TextFragment{text}, 31 + } 32 + 33 + src := bytes.NewReader(nil) 34 + dst := ioutil.Discard 35 + 36 + assertInProgress := func(t *testing.T, kind string, err error) { 37 + if !errors.Is(err, errApplyInProgress) { 38 + t.Fatalf("expected in-progress error for %s apply, but got: %v", kind, err) 39 + } 40 + } 41 + 42 + t.Run("binaryFirst", func(t *testing.T) { 43 + a := NewApplier(src) 44 + if err := a.ApplyBinaryFragment(dst, binary); err != nil { 45 + t.Fatalf("unexpected error applying fragment: %v", err) 46 + } 47 + assertInProgress(t, "text", a.ApplyTextFragment(dst, text)) 48 + assertInProgress(t, "binary", a.ApplyBinaryFragment(dst, binary)) 49 + assertInProgress(t, "file", a.ApplyFile(dst, file)) 50 + }) 51 + 52 + t.Run("textFirst", func(t *testing.T) { 53 + a := NewApplier(src) 54 + if err := a.ApplyTextFragment(dst, text); err != nil { 55 + t.Fatalf("unexpected error applying fragment: %v", err) 56 + } 57 + // additional text fragments are allowed 58 + if err := a.ApplyTextFragment(dst, text); err != nil { 59 + t.Fatalf("unexpected error applying second fragment: %v", err) 60 + } 61 + assertInProgress(t, "binary", a.ApplyBinaryFragment(dst, binary)) 62 + assertInProgress(t, "file", a.ApplyFile(dst, file)) 63 + }) 64 + 65 + t.Run("fileFirst", func(t *testing.T) { 66 + a := NewApplier(src) 67 + if err := a.ApplyFile(dst, file); err != nil { 68 + t.Fatalf("unexpected error applying file: %v", err) 69 + } 70 + assertInProgress(t, "text", a.ApplyTextFragment(dst, text)) 71 + assertInProgress(t, "binary", a.ApplyBinaryFragment(dst, binary)) 72 + assertInProgress(t, "file", a.ApplyFile(dst, file)) 73 + }) 74 + } 75 + 13 76 func TestTextFragmentApplyStrict(t *testing.T) { 14 77 tests := map[string]struct { 15 78 Files applyFiles