skip references pointing to objects that are not commits

Fixes #745

On all iterators where references are used only hash references
are used. But we're not taking into account that hash references
can point to any kind of object, not only commit objects, which
are the only objects we care about for the references table.

This fix was already applied on the new commits iterator that
walks the history skipping unreachable commits, but it was not
consistent with the rest of the iterators, which only check if the
reference was a hash reference or not. Now this behavior is
consistent in all iterators.

`isIgnoredReference` function has been added to abstract all this
logic away so if we ever need to change the logic of this we will
only have to replace it in one place instead of being scattered
all over the codebase.

Signed-off-by: Miguel Molina <[email protected]>

Author: erizocosmico

commits.go

@@ -230,12 +230,7 @@ func (i *commitIter) loadNextRef() (err error) {
 			return err
 		}
 
-		if i.ref.Type() != plumbing.HashReference {
-			i.ref = nil
-			continue
-		}
-
-		obj, err := i.repo.Object(plumbing.AnyObject, i.ref.Hash())
+		ignored, err := isIgnoredReference(i.repo.Repository, i.ref)
 		if err != nil {
 			if i.skipGitErrors {
 				continue
@@ -244,7 +239,7 @@ func (i *commitIter) loadNextRef() (err error) {
 			return err
 		}
 
-		if obj.Type() != plumbing.CommitObject {
+		if ignored {
 			continue
 		}
 

integration_test.go

@@ -594,7 +594,7 @@ func TestMissingHeadRefs(t *testing.T) {
 
 	rows, err := sql.RowIterToRows(iter)
 	require.NoError(err)
-	require.Len(rows, 56)
+	require.Len(rows, 54)
 }
 
 func BenchmarkQueries(b *testing.B) {

ref_commits.go

@@ -306,7 +306,16 @@ func (i *refCommitsRowIter) next() (sql.Row, error) {
 					return nil, err
 				}
 
-				if ref.Type() != plumbing.HashReference {
+				ignored, err := isIgnoredReference(i.repo.Repository, ref)
+				if err != nil {
+					if i.skipGitErrors {
+						continue
+					}
+
+					return nil, err
+				}
+
+				if ignored {
 					continue
 				}
 			} else {

references.go

@@ -6,6 +6,7 @@ import (
 	"io"
 	"strings"
 
+	git "gopkg.in/src-d/go-git.v4"
 	"gopkg.in/src-d/go-mysql-server.v0/sql"
 
 	"gopkg.in/src-d/go-git.v4/plumbing"
@@ -299,7 +300,16 @@ func (i *refRowIter) next() (sql.Row, error) {
 			return nil, err
 		}
 
-		if o.Type() != plumbing.HashReference {
+		ignored, err := isIgnoredReference(i.repo.Repository, o)
+		if err != nil {
+			if i.skipGitErrors {
+				continue
+			}
+
+			return nil, err
+		}
+
+		if ignored {
 			continue
 		}
 
@@ -338,3 +348,16 @@ func referenceToRow(repositoryID string, c *plumbing.Reference) sql.Row {
 		hash,
 	)
 }
+
+func isIgnoredReference(repo *git.Repository, ref *plumbing.Reference) (bool, error) {
+	if ref.Type() != plumbing.HashReference {
+		return true, nil
+	}
+
+	obj, err := repo.Object(plumbing.AnyObject, ref.Hash())
+	if err != nil {
+		return false, err
+	}
+
+	return obj.Type() != plumbing.CommitObject, nil
+}

squash_iterator.go

@@ -474,7 +474,16 @@ func (i *squashRefIter) Advance() error {
 			}
 		}
 
-		if ref.Type() != plumbing.HashReference {
+		ignored, err := isIgnoredReference(i.repo.Repository, ref)
+		if err != nil {
+			if i.skipGitErrors {
+				continue
+			}
+
+			return err
+		}
+
+		if ignored {
 			continue
 		}
 
@@ -715,11 +724,16 @@ func (i *squashRepoRefsIter) Advance() error {
 			}
 		}
 
-		if ref.Type() != plumbing.HashReference {
-			logrus.WithFields(logrus.Fields{
-				"type": ref.Type(),
-				"ref":  ref.Name(),
-			}).Debug("ignoring reference, it's not a hash reference")
+		ignored, err := isIgnoredReference(i.repos.Repository().Repository, ref)
+		if err != nil {
+			if i.skipGitErrors {
+				continue
+			}
+
+			return err
+		}
+
+		if ignored {
 			continue
 		}
 
@@ -859,7 +873,16 @@ func (i *squashRemoteRefsIter) Advance() error {
 			}
 		}
 
-		if ref.Type() != plumbing.HashReference {
+		ignored, err := isIgnoredReference(i.Repository().Repository, ref)
+		if err != nil {
+			if i.skipGitErrors {
+				continue
+			}
+
+			return err
+		}
+
+		if ignored {
 			continue
 		}