Refactor CommitCollection

Defer the validation of invalid boundaries when querying the commit log.
Replace EmptyCommitCollection with a Enumerable.Empty<>.

8 files changed, 64 insertions, 112 deletions

diff --git a/LibGit2Sharp.Tests/CommitFixture.cs b/LibGit2Sharp.Tests/CommitFixture.cs
index b275cc27..a487a261 100644
--- a/LibGit2Sharp.Tests/CommitFixture.cs
+++ b/LibGit2Sharp.Tests/CommitFixture.cs
@@ -81,10 +81,10 @@ namespace LibGit2Sharp.Tests
         {

             using (var repo = new Repository(Constants.TestRepoPath))

             {

-                Assert.Throws<InvalidOperationException>(() => repo.Commits.QueryBy(new Filter { Since = Constants.UnknownSha}));

-                Assert.Throws<InvalidOperationException>(() => repo.Commits.QueryBy(new Filter { Since = "refs/heads/deadbeef"}));

-                Assert.Throws<InvalidOperationException>(() => repo.Commits.QueryBy(new Filter { Since = repo.Branches["deadbeef"]}));

-                Assert.Throws<InvalidOperationException>(() => repo.Commits.QueryBy(new Filter { Since = repo.Refs["refs/heads/deadbeef"] }));

+                Assert.Throws<InvalidOperationException>(() => repo.Commits.QueryBy(new Filter { Since = Constants.UnknownSha}).Count());

+                Assert.Throws<InvalidOperationException>(() => repo.Commits.QueryBy(new Filter { Since = "refs/heads/deadbeef"}).Count());

+                Assert.Throws<InvalidOperationException>(() => repo.Commits.QueryBy(new Filter { Since = repo.Branches["deadbeef"]}).Count());

+                Assert.Throws<InvalidOperationException>(() => repo.Commits.QueryBy(new Filter { Since = repo.Refs["refs/heads/deadbeef"] }).Count());

             }

         }

 

diff --git a/LibGit2Sharp/CommitCollection.cs b/LibGit2Sharp/CommitCollection.cs
index 35d3be7a..8d740b3c 100644
--- a/LibGit2Sharp/CommitCollection.cs
+++ b/LibGit2Sharp/CommitCollection.cs
@@ -2,6 +2,7 @@
 using System.Collections;
 using System.Collections.Generic;
 using System.Globalization;
+using System.Linq;
 using LibGit2Sharp.Core;
 
 namespace LibGit2Sharp
@@ -12,8 +13,8 @@ namespace LibGit2Sharp
     public class CommitCollection : IQueryableCommitCollection
     {
         private readonly Repository repo;
-        private ObjectId pushedObjectId;
-        private ObjectId hiddenObjectId;
+        private object includedIdentifier = "HEAD";
+        private object excludedIdentifier;
         private readonly GitSortOptions sortOptions;
 
         /// <summary>
@@ -21,7 +22,8 @@ namespace LibGit2Sharp
         /// The commits will be enumerated according in reverse chronological order.
         /// </summary>
         /// <param name = "repo">The repository.</param>
-        internal CommitCollection(Repository repo) : this (repo, GitSortOptions.Time)
+        internal CommitCollection(Repository repo)
+            : this(repo, GitSortOptions.Time)
         {
         }
 
@@ -52,12 +54,12 @@ namespace LibGit2Sharp
         /// <returns>An <see cref="IEnumerator{T}"/> object that can be used to iterate through the collection.</returns>
         public IEnumerator<Commit> GetEnumerator()
         {
-            if (pushedObjectId == null)
+            if ((repo.Info.IsEmpty) && PointsAtTheHead(includedIdentifier.ToString()))
             {
-                throw new InvalidOperationException();
+                return Enumerable.Empty<Commit>().GetEnumerator();
             }
 
-            return new CommitEnumerator(repo, pushedObjectId, hiddenObjectId, sortOptions);
+            return new CommitEnumerator(repo, includedIdentifier, excludedIdentifier, sortOptions);
         }
 
         /// <summary>
@@ -80,36 +82,13 @@ namespace LibGit2Sharp
         {
             Ensure.ArgumentNotNull(filter, "filter");
             Ensure.ArgumentNotNull(filter.Since, "filter.Since");
+            Ensure.ArgumentNotNullOrEmptyString(filter.Since.ToString(), "filter.Since");
 
-            string sinceIdentifier = filter.Since.ToString();
-
-            if ((repo.Info.IsEmpty) && PointsAtTheHead(sinceIdentifier))
-            {
-                return new EmptyCommitCollection(filter.SortBy);
-            }
-
-            ObjectId sinceObjectId = RetrieveCommitId(sinceIdentifier);
-            ObjectId untilObjectId = null;
-            
-            if (filter.Until != null)
-            {
-                untilObjectId = RetrieveCommitId(filter.Until.ToString());
-            }
-
-            return new CommitCollection(repo, filter.SortBy) { pushedObjectId = sinceObjectId, hiddenObjectId = untilObjectId};
-        }
-
-        private ObjectId RetrieveCommitId(string shaOrReferenceName)
-        {
-            GitObject gitObj = repo.Lookup(shaOrReferenceName);
-
-            // TODO: Should we check the type? Git-log allows TagAnnotation oid as parameter. But what about Blobs and Trees?
-            if (gitObj == null)
-            {
-                throw new InvalidOperationException(string.Format(CultureInfo.InvariantCulture, "No valid git object pointed at by '{0}' exists in the repository.", shaOrReferenceName));
-            }
-
-            return gitObj.Id;
+            return new CommitCollection(repo, filter.SortBy)
+                       {
+                           includedIdentifier = filter.Since, 
+                           excludedIdentifier = filter.Until
+                       };
         }
 
         private static bool PointsAtTheHead(string shaOrRefName)
@@ -117,23 +96,21 @@ namespace LibGit2Sharp
             return ("HEAD".Equals(shaOrRefName, StringComparison.Ordinal) || "refs/heads/master".Equals(shaOrRefName, StringComparison.Ordinal));
         }
 
-        #region Nested type: CommitEnumerator
-
         private class CommitEnumerator : IEnumerator<Commit>
         {
             private readonly Repository repo;
             private readonly RevWalkerSafeHandle handle;
             private ObjectId currentOid;
 
-            public CommitEnumerator(Repository repo, ObjectId pushedOid, ObjectId hiddenOid, GitSortOptions sortingStrategy)
+            public CommitEnumerator(Repository repo, object includedIdentifier, object excludedIdentifier, GitSortOptions sortingStrategy)
             {
                 this.repo = repo;
                 int res = NativeMethods.git_revwalk_new(out handle, repo.Handle);
                 Ensure.Success(res);
 
                 Sort(sortingStrategy);
-                Push(pushedOid);
-                Hide(hiddenOid);
+                Push(includedIdentifier);
+                Hide(excludedIdentifier);
             }
 
             #region IEnumerator<Commit> Members
@@ -160,7 +137,7 @@ namespace LibGit2Sharp
             {
                 GitOid oid;
                 var res = NativeMethods.git_revwalk_next(out oid, handle);
-                
+
                 if (res == (int)GitErrorCode.GIT_EREVWALKOVER)
                 {
                     return false;
@@ -185,7 +162,7 @@ namespace LibGit2Sharp
                 Dispose(true);
                 GC.SuppressFinalize(this);
             }
-            
+
             private void Dispose(bool disposing)
             {
                 if (handle == null || handle.IsInvalid)
@@ -196,21 +173,21 @@ namespace LibGit2Sharp
                 handle.Dispose();
             }
 
-            private void Push(ObjectId pushedOid)
+            private void Push(object identifier)
             {
-                var oid = pushedOid.Oid;
+                var oid = RetrieveCommitId(identifier).Oid;
                 int res = NativeMethods.git_revwalk_push(handle, ref oid);
                 Ensure.Success(res);
             }
 
-            private void Hide(ObjectId hiddenOid)
+            private void Hide(object identifier)
             {
-                if (hiddenOid == null)
+                if (identifier == null)
                 {
                     return;
                 }
 
-                var oid = hiddenOid.Oid;
+                var oid = RetrieveCommitId(identifier).Oid;
                 int res = NativeMethods.git_revwalk_hide(handle, ref oid);
                 Ensure.Success(res);
             }
@@ -219,8 +196,38 @@ namespace LibGit2Sharp
             {
                 NativeMethods.git_revwalk_sorting(handle, options);
             }
-        }
 
-        #endregion
+            private ObjectId RetrieveCommitId(object identifier)
+            {
+                string shaOrReferenceName = RetrieveShaOrReferenceName(identifier);
+
+                GitObject gitObj = repo.Lookup(shaOrReferenceName);
+
+                // TODO: Should we check the type? Git-log allows TagAnnotation oid as parameter. But what about Blobs and Trees?
+                if (gitObj == null)
+                {
+                    throw new InvalidOperationException(string.Format(CultureInfo.InvariantCulture, "No valid git object pointed at by '{0}' exists in the repository.", identifier));
+                }
+
+                return gitObj.Id;
+            }
+
+            private static string RetrieveShaOrReferenceName(object identifier)
+            {
+                if (identifier is string)
+                {
+                    return identifier as string;
+                }
+
+                if (identifier is ObjectId || identifier is Reference || identifier is Branch)
+                    return identifier.ToString();
+
+                if (identifier is Commit)
+                    return ((Commit)identifier).Id.Sha;
+
+                throw new InvalidOperationException(string.Format("Unexpected kind of identifier '{0}'.", identifier));
+            }
+        
+        }
     }
 }
\ No newline at end of file
diff --git a/LibGit2Sharp/EmptyCommitCollection.cs b/LibGit2Sharp/EmptyCommitCollection.cs
deleted file mode 100644
index 31d5537b..00000000
--- a/LibGit2Sharp/EmptyCommitCollection.cs
+++ /dev/null
@@ -1,53 +0,0 @@
-using System.Collections;
-using System.Collections.Generic;
-using System.Linq;
-
-namespace LibGit2Sharp
-{
-    internal class EmptyCommitCollection : IQueryableCommitCollection
-    {
-        internal EmptyCommitCollection(GitSortOptions sortedBy)
-        {
-            SortedBy = sortedBy;
-        }
-
-        /// <summary>
-        /// Returns an enumerator that iterates through the collection.
-        /// </summary>
-        /// <returns>
-        /// A <see cref="T:System.Collections.Generic.IEnumerator`1"/> that can be used to iterate through the collection.
-        /// </returns>
-        /// <filterpriority>1</filterpriority>
-        public IEnumerator<Commit> GetEnumerator()
-        {
-            return Enumerable.Empty<Commit>().GetEnumerator();
-        }
-
-        /// <summary>
-        ///  Returns the list of commits of the repository matching the specified <paramref name="filter"/>.
-        /// </summary>
-        /// <param name="filter">The options used to control which commits will be returned.</param>
-        /// <returns>A collection of commits, ready to be enumerated.</returns>
-        public ICommitCollection QueryBy(Filter filter)
-        {
-            return new EmptyCommitCollection(filter.SortBy);
-        }
-
-        /// <summary>
-        /// Returns an enumerator that iterates through a collection.
-        /// </summary>
-        /// <returns>
-        /// An <see cref="T:System.Collections.IEnumerator"/> object that can be used to iterate through the collection.
-        /// </returns>
-        /// <filterpriority>2</filterpriority>
-        IEnumerator IEnumerable.GetEnumerator()
-        {
-            return GetEnumerator();
-        }
-
-        /// <summary>
-        /// Gets the current sorting strategy applied when enumerating the collection.
-        /// </summary>
-        public GitSortOptions SortedBy { get; private set; }
-    }
-}
\ No newline at end of file
diff --git a/LibGit2Sharp/Filter.cs b/LibGit2Sharp/Filter.cs
index 5985ddf3..6e42ea47 100644
--- a/LibGit2Sharp/Filter.cs
+++ b/LibGit2Sharp/Filter.cs
@@ -11,7 +11,6 @@
         public Filter()
         {
             SortBy = GitSortOptions.Time;
-            Since = "HEAD";
         }
 
         /// <summary>
diff --git a/LibGit2Sharp/IQueryableCommitCollection.cs b/LibGit2Sharp/IQueryableCommitCollection.cs
index e4329b28..b5ea4997 100644
--- a/LibGit2Sharp/IQueryableCommitCollection.cs
+++ b/LibGit2Sharp/IQueryableCommitCollection.cs
@@ -1,6 +1,6 @@
 namespace LibGit2Sharp
 {
-    public interface IQueryableCommitCollection : ICommitCollection
+    public interface IQueryableCommitCollection : ICommitCollection //TODO: Find a name that's more explicit than IQueryableCommitCollection
     {
         /// <summary>
         ///  Returns the list of commits of the repository matching the specified <paramref name="filter"/>.
diff --git a/LibGit2Sharp/LibGit2Sharp.csproj b/LibGit2Sharp/LibGit2Sharp.csproj
index 51970404..2658dc7f 100644
--- a/LibGit2Sharp/LibGit2Sharp.csproj
+++ b/LibGit2Sharp/LibGit2Sharp.csproj
@@ -71,7 +71,6 @@
     <Compile Include="DirectReference.cs" />
     <Compile Include="Core\GitIndexTime.cs" />
     <Compile Include="Core\GitIndexEntry.cs" />
-    <Compile Include="EmptyCommitCollection.cs" />
     <Compile Include="GitObject.cs" />
     <Compile Include="GitObjectType.cs" />
     <Compile Include="GitSortOptions.cs" />
diff --git a/LibGit2Sharp/Repository.cs b/LibGit2Sharp/Repository.cs
index bd6f8849..63d77e1a 100644
--- a/LibGit2Sharp/Repository.cs
+++ b/LibGit2Sharp/Repository.cs
@@ -77,7 +77,7 @@ namespace LibGit2Sharp
         /// </summary>
         public IQueryableCommitCollection Commits
         {
-            get { return (IQueryableCommitCollection)commits.QueryBy(new Filter { Since = Head }); }
+            get { return commits; }
         }
 
         /// <summary>
diff --git a/LibGit2Sharp/RepositoryExtensions.cs b/LibGit2Sharp/RepositoryExtensions.cs
index 0b0b00dd..9d84e6e8 100644
--- a/LibGit2Sharp/RepositoryExtensions.cs
+++ b/LibGit2Sharp/RepositoryExtensions.cs
@@ -78,7 +78,7 @@
         /// <summary>
         ///   Creates a branch with the specified name. This branch will point at the commit pointed at by the <see cref="Repository.Head"/>.
         /// </summary>
-        /// <param name="repository">The <see cref="Repository"/>  being worked with.</param>
+        /// <param name="repository">The <see cref="Repository"/> being worked with.</param>
         /// <param name="branchName">The name of the branch to create.</param>
         public static Branch CreateBranch(this Repository repository, string branchName)
         {
@@ -88,7 +88,7 @@
         /// <summary>
         ///   Creates a branch with the specified name. This branch will point at the commit pointed at by the <see cref="Repository.Head"/>.
         /// </summary>
-        /// <param name="repository">The <see cref="Repository"/>  being worked with.</param>
+        /// <param name="repository">The <see cref="Repository"/> being worked with.</param>
         /// <param name="branchName">The name of the branch to create.</param>
         /// <param name="target">The canonical reference name or sha which should be pointed at by the Branch.</param>
         public static Branch CreateBranch(this Repository repository, string branchName, string target)