Skip to content

Commit 8d4cd2d

Browse files
committed
Add 'Three dots' feature in 'GetChanges' as default
Signed-off-by: David Pordomingo <David.Pordomingo.F@gmail.com>
1 parent c232785 commit 8d4cd2d

7 files changed

Lines changed: 487 additions & 68 deletions

File tree

cmd/server-test/only_pr_changes_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,4 +58,3 @@ func (suite *reviewOnlyPrChangesIntegrationSuite) TestAnalyzerErr() {
5858
suite.sendEvent(jsonReviewEvent.String())
5959
suite.GrepAndNotAll(suite.r, expectedComments, notExpectedComments)
6060
}
61-

service/git/loader.go

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import (
1313

1414
type CommitLoader interface {
1515
LoadCommits(context.Context, ...lookout.ReferencePointer) (
16-
[]*object.Commit, error)
16+
[]*object.Commit, storer.Storer, error)
1717
}
1818

1919
type LibraryCommitLoader struct {
@@ -30,31 +30,36 @@ func NewLibraryCommitLoader(l ReposCollectionHandler, s Syncer) *LibraryCommitLo
3030

3131
func (l *LibraryCommitLoader) LoadCommits(
3232
ctx context.Context, rps ...lookout.ReferencePointer) (
33-
[]*object.Commit, error) {
33+
[]*object.Commit, storer.Storer, error) {
3434

3535
if len(rps) == 0 {
36-
return nil, nil
36+
return nil, nil, nil
3737
}
3838

3939
frp := rps[0]
4040
for _, orp := range rps[1:] {
4141
if orp.InternalRepositoryURL != frp.InternalRepositoryURL {
42-
return nil, fmt.Errorf(
42+
return nil, nil, fmt.Errorf(
4343
"loading commits from multiple repositories is not supported")
4444
}
4545
}
4646

4747
if err := l.Syncer.Sync(ctx, rps...); err != nil {
48-
return nil, err
48+
return nil, nil, err
4949
}
5050

5151
r, err := l.Library.GetOrInit(ctx, frp.Repository())
5252
if err != nil {
53-
return nil, err
53+
return nil, nil, err
5454
}
5555

5656
storerCl := NewStorerCommitLoader(r.Storer)
57-
return storerCl.LoadCommits(ctx, rps...)
57+
commits, _, err := storerCl.LoadCommits(ctx, rps...)
58+
if err != nil {
59+
return nil, nil, err
60+
}
61+
62+
return commits, storerCl.Storer, nil
5863
}
5964

6065
type StorerCommitLoader struct {
@@ -68,17 +73,17 @@ func NewStorerCommitLoader(storer storer.Storer) *StorerCommitLoader {
6873
}
6974

7075
func (l *StorerCommitLoader) LoadCommits(ctx context.Context,
71-
rps ...lookout.ReferencePointer) ([]*object.Commit, error) {
76+
rps ...lookout.ReferencePointer) ([]*object.Commit, storer.Storer, error) {
7277

7378
commits := make([]*object.Commit, len(rps))
7479
for i, rp := range rps {
7580
commit, err := object.GetCommit(l.Storer, plumbing.NewHash(rp.Hash))
7681
if err != nil {
77-
return nil, err
82+
return nil, nil, err
7883
}
7984

8085
commits[i] = commit
8186
}
8287

83-
return commits, nil
88+
return commits, l.Storer, nil
8489
}

service/git/loader_test.go

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,22 +11,23 @@ import (
1111
"github.com/stretchr/testify/suite"
1212
git "gopkg.in/src-d/go-git.v4"
1313
"gopkg.in/src-d/go-git.v4/plumbing/object"
14+
"gopkg.in/src-d/go-git.v4/plumbing/storer"
1415
)
1516

1617
type MockCommitLoader struct {
1718
mock.Mock
1819
}
1920

2021
func (m *MockCommitLoader) LoadCommits(ctx context.Context,
21-
rps ...lookout.ReferencePointer) ([]*object.Commit, error) {
22+
rps ...lookout.ReferencePointer) ([]*object.Commit, storer.Storer, error) {
2223

2324
args := m.Called(ctx, rps)
2425
r0 := args.Get(0)
2526
if r0 == nil {
26-
return nil, args.Error(1)
27+
return nil, nil, args.Error(1)
2728
}
2829

29-
return r0.([]*object.Commit), args.Error(1)
30+
return r0.([]*object.Commit), nil, args.Error(1)
3031
}
3132

3233
type MockSyncer struct {
@@ -103,7 +104,7 @@ func (s *LibraryCommitLoaderTestSuite) TestErrorOnMultiRepos() {
103104

104105
cl := NewLibraryCommitLoader(&Library{}, &LibrarySyncer{})
105106

106-
commits, err := cl.LoadCommits(context.TODO(), rpsDifferentRepos...)
107+
commits, _, err := cl.LoadCommits(context.TODO(), rpsDifferentRepos...)
107108

108109
require.Nil(commits)
109110
require.Errorf(err, "loading commits from multiple repositories is not supported")
@@ -120,7 +121,7 @@ func (s *LibraryCommitLoaderTestSuite) TestErrorOnSync() {
120121

121122
cl := NewLibraryCommitLoader(&Library{}, ms)
122123

123-
commits, err := cl.LoadCommits(ctx, rpsSameRepos...)
124+
commits, _, err := cl.LoadCommits(ctx, rpsSameRepos...)
124125

125126
require.Nil(commits)
126127
require.EqualError(err, "sync mock error")
@@ -140,7 +141,7 @@ func (s *LibraryCommitLoaderTestSuite) TestErrorOnGetOrInit() {
140141

141142
cl := NewLibraryCommitLoader(ml, ms)
142143

143-
commits, err := cl.LoadCommits(ctx, rpsSameRepos...)
144+
commits, _, err := cl.LoadCommits(ctx, rpsSameRepos...)
144145

145146
require.Nil(commits)
146147
require.EqualError(err, "get or init mock error")
@@ -154,7 +155,7 @@ func (s *LibraryCommitLoaderTestSuite) TestEmpty() {
154155

155156
rps := []lookout.ReferencePointer{}
156157

157-
commits, err := cl.LoadCommits(context.TODO(), rps...)
158+
commits, _, err := cl.LoadCommits(context.TODO(), rps...)
158159

159160
require.Nil(commits)
160161
require.Nil(err)
Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
package merge_base
2+
3+
import (
4+
"io"
5+
6+
"gopkg.in/src-d/go-git.v4/plumbing"
7+
"gopkg.in/src-d/go-git.v4/plumbing/object"
8+
"gopkg.in/src-d/go-git.v4/plumbing/storer"
9+
)
10+
11+
// NewFilterCommitIter returns a CommitIter that walks the commit history,
12+
// starting at the passed commit and visiting its parents in Breadth-first order.
13+
// The commits returned by the CommitIter will validate the passed CommitFilter.
14+
// The history won't be transversed beyond a commit if isLimit is true for it.
15+
// Each commit will be visited only once.
16+
// If the commit history can not be traversed, or the Close() method is called,
17+
// the CommitIter won't return more commits.
18+
// If no isValid is passed, all ancestors of from commit will be valid.
19+
// If no isLimit is limmit, all ancestors of all commits will be visited.
20+
func NewFilterCommitIter(
21+
// REVIEWER: store argument wouldn't be needed if this were part of go-git/plumbing/object package
22+
store storer.EncodedObjectStorer,
23+
from *object.Commit,
24+
isValid *CommitFilter,
25+
isLimit *CommitFilter,
26+
) object.CommitIter {
27+
var validFilter CommitFilter
28+
if isValid == nil {
29+
validFilter = func(_ *object.Commit) bool {
30+
return true
31+
}
32+
} else {
33+
validFilter = *isValid
34+
}
35+
36+
var limitFilter CommitFilter
37+
if isLimit == nil {
38+
limitFilter = func(_ *object.Commit) bool {
39+
return false
40+
}
41+
} else {
42+
limitFilter = *isLimit
43+
}
44+
45+
return &filterCommitIter{
46+
// REVIEWER: store wouldn't be needed if this were part of go-git/plumbing/object package
47+
store: store,
48+
isValid: validFilter,
49+
isLimit: limitFilter,
50+
visited: map[plumbing.Hash]bool{},
51+
queue: []*object.Commit{from},
52+
}
53+
}
54+
55+
// CommitFilter returns a boolean for the passed Commit
56+
type CommitFilter func(*object.Commit) bool
57+
58+
// filterCommitIter implments object.CommitIter
59+
type filterCommitIter struct {
60+
// REVIEWER: store wouldn't be needed if this were part of go-git/plumbing/object package
61+
store storer.EncodedObjectStorer
62+
isValid CommitFilter
63+
isLimit CommitFilter
64+
visited map[plumbing.Hash]bool
65+
queue []*object.Commit
66+
lastErr error
67+
}
68+
69+
// Next returns the next commit of the CommitIter.
70+
// It will return io.EOF if there are no more commits to visit,
71+
// or an error if the history could not be traversed.
72+
func (w *filterCommitIter) Next() (*object.Commit, error) {
73+
var commit *object.Commit
74+
var err error
75+
for {
76+
commit, err = w.popNewFromQueue()
77+
if err != nil {
78+
return nil, w.close(err)
79+
}
80+
81+
w.visited[commit.Hash] = true
82+
83+
if !w.isLimit(commit) {
84+
// REVIEWER: first argument would be commit.s if this were part of object.Commit
85+
err = w.addToQueue(w.store, commit.ParentHashes...)
86+
if err != nil {
87+
return nil, w.close(err)
88+
}
89+
}
90+
91+
if w.isValid(commit) {
92+
return commit, nil
93+
}
94+
}
95+
}
96+
97+
// ForEach runs the passed callback over each Commit returned by the CommitIter
98+
// until the callback returns an error or there is no more commits to traverse.
99+
func (w *filterCommitIter) ForEach(cb func(*object.Commit) error) error {
100+
for {
101+
commit, err := w.Next()
102+
if err == io.EOF {
103+
break
104+
}
105+
106+
if err != nil {
107+
return err
108+
}
109+
110+
if err := cb(commit); err != nil {
111+
return err
112+
}
113+
}
114+
115+
return nil
116+
}
117+
118+
// Error returns the error that caused that the CommitIter is no longer returning commits
119+
func (w *filterCommitIter) Error() error {
120+
return w.lastErr
121+
}
122+
123+
// Close closes the CommitIter
124+
func (w *filterCommitIter) Close() {
125+
w.visited = map[plumbing.Hash]bool{}
126+
w.queue = []*object.Commit{}
127+
w.isLimit = nil
128+
w.isValid = nil
129+
}
130+
131+
// close closes the CommitIter with an error
132+
func (w *filterCommitIter) close(err error) error {
133+
w.Close()
134+
w.lastErr = err
135+
return err
136+
}
137+
138+
// popNewFromQueue returns the first new commit from the internal fifo queue, or
139+
// an io.EOF error if the queue is empty
140+
func (w *filterCommitIter) popNewFromQueue() (*object.Commit, error) {
141+
var first *object.Commit
142+
for {
143+
if len(w.queue) == 0 {
144+
if w.lastErr != nil {
145+
return nil, w.lastErr
146+
}
147+
148+
return nil, io.EOF
149+
}
150+
151+
first = w.queue[0]
152+
w.queue = w.queue[1:]
153+
if w.visited[first.Hash] {
154+
continue
155+
}
156+
157+
return first, nil
158+
}
159+
}
160+
161+
// addToQueue adds the passed commits to the internal fifo queue if they weren'r already seen
162+
// or returns an error if the passed hashes could not be used to get valid commits
163+
func (w *filterCommitIter) addToQueue(
164+
store storer.EncodedObjectStorer,
165+
hashes ...plumbing.Hash,
166+
) error {
167+
for _, hash := range hashes {
168+
if w.visited[hash] {
169+
continue
170+
}
171+
172+
commit, err := object.GetCommit(store, hash)
173+
if err != nil {
174+
return err
175+
}
176+
177+
w.queue = append(w.queue, commit)
178+
}
179+
180+
return nil
181+
}

0 commit comments

Comments
 (0)