Skip to content

Commit 1356d70

Browse files
AMSadekAMSadek
committed
Add two pack methods with different parameter list. Better handling for null and invalid parameters. Add test cases for null and invalid parameters
1 parent cbf521e commit 1356d70

File tree

3 files changed

+232
-47
lines changed

3 files changed

+232
-47
lines changed

‎LibGit2Sharp.Tests/PackBuilderFixture.cs

Lines changed: 120 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -38,30 +38,31 @@ internal void TestIfSameRepoAfterPacking(Action<PackBuilder> buildingDelegate)
3838
Directory.CreateDirectory(mrrRepoPathPackDirPath);
3939
Directory.CreateDirectory(mrrRepoPathPackDirPath + "/pack");
4040

41-
PackBuilderOptions packBuilderOptions = new PackBuilderOptions();
42-
packBuilderOptions.MaximumNumberOfThreads = 0;
43-
packBuilderOptions.PackFilePath = mrrRepoPathPackDirPath + "/pack";
41+
PackBuilderOptions packBuilderOptions = new PackBuilderOptions(mrrRepoPathPackDirPath + "/pack");
4442

45-
Repository orgRepo = new Repository(orgRepoPath);
46-
47-
PackBuilderResults results = orgRepo.ObjectDatabase.Pack(packBuilderOptions, buildingDelegate);
48-
49-
// written objects count is the same as in objects database
50-
Assert.Equal(orgRepo.ObjectDatabase.Count(), results.WrittenObjectsCount);
51-
52-
// loading a repo from the written pack file.
53-
Repository mrrRepo = new Repository(mrrRepoPath);
43+
using (Repository orgRepo = new Repository(orgRepoPath))
44+
{
45+
PackBuilderResults results;
46+
if (buildingDelegate != null)
47+
results = orgRepo.ObjectDatabase.Pack(packBuilderOptions, buildingDelegate);
48+
else
49+
results = orgRepo.ObjectDatabase.Pack(packBuilderOptions);
5450

55-
// make sure the objects of the original repo are the same as the ones in the mirror repo
56-
// doing that by making sure the count is the same, and the set differnce is empty
57-
Assert.True(mrrRepo.ObjectDatabase.Count() == orgRepo.ObjectDatabase.Count() && !mrrRepo.ObjectDatabase.Except(orgRepo.ObjectDatabase).Any());
51+
// written objects count is the same as in objects database
52+
Assert.Equal(orgRepo.ObjectDatabase.Count(), results.WrittenObjectsCount);
5853

59-
Assert.Equal(orgRepo.Commits.Count(), mrrRepo.Commits.Count());
60-
Assert.Equal(orgRepo.Branches.Count(), mrrRepo.Branches.Count());
61-
Assert.Equal(orgRepo.Refs.Count(), mrrRepo.Refs.Count());
54+
// loading a repo from the written pack file.
55+
using (Repository mrrRepo = new Repository(mrrRepoPath))
56+
{
57+
// make sure the objects of the original repo are the same as the ones in the mirror repo
58+
// doing that by making sure the count is the same, and the set differnce is empty
59+
Assert.True(mrrRepo.ObjectDatabase.Count() == orgRepo.ObjectDatabase.Count() && !mrrRepo.ObjectDatabase.Except(orgRepo.ObjectDatabase).Any());
6260

63-
orgRepo.Dispose();
64-
mrrRepo.Dispose();
61+
Assert.Equal(orgRepo.Commits.Count(), mrrRepo.Commits.Count());
62+
Assert.Equal(orgRepo.Branches.Count(), mrrRepo.Branches.Count());
63+
Assert.Equal(orgRepo.Refs.Count(), mrrRepo.Refs.Count());
64+
}
65+
}
6566
}
6667

6768
internal void TestBuildDelegate(PackBuilder builder)
@@ -74,5 +75,104 @@ internal void TestBuildDelegate(PackBuilder builder)
7475
}
7576
}
7677
}
78+
79+
[Fact]
80+
public void ExceptionIfPathDoesNotExist()
81+
{
82+
PackBuilderOptions pbo;
83+
84+
Assert.Throws<DirectoryNotFoundException>(() =>
85+
{
86+
pbo = new PackBuilderOptions("aaa");
87+
});
88+
}
89+
90+
[Fact]
91+
public void ExceptionIfPathEqualsNull()
92+
{
93+
PackBuilderOptions pbo;
94+
95+
Assert.Throws<ArgumentNullException>(() =>
96+
{
97+
pbo = new PackBuilderOptions(null);
98+
});
99+
}
100+
101+
[Fact]
102+
public void ExceptionIfOptionsEqualsNull()
103+
{
104+
string orgRepoPath = SandboxPackBuilderTestRepo();
105+
106+
using (Repository orgRepo = new Repository(orgRepoPath))
107+
{
108+
Assert.Throws<ArgumentNullException>(() =>
109+
{
110+
orgRepo.ObjectDatabase.Pack(null);
111+
});
112+
}
113+
}
114+
115+
[Fact]
116+
public void ExceptionIfBuildDelegateEqualsNull()
117+
{
118+
string orgRepoPath = SandboxPackBuilderTestRepo();
119+
PackBuilderOptions packBuilderOptions = new PackBuilderOptions(orgRepoPath);
120+
121+
using (Repository orgRepo = new Repository(orgRepoPath))
122+
{
123+
Assert.Throws<ArgumentNullException>(() =>
124+
{
125+
orgRepo.ObjectDatabase.Pack(packBuilderOptions, null);
126+
});
127+
}
128+
}
129+
130+
[Fact]
131+
public void ExceptionIfNegativeNumberOfThreads()
132+
{
133+
string orgRepoPath = SandboxPackBuilderTestRepo();
134+
PackBuilderOptions packBuilderOptions = new PackBuilderOptions(orgRepoPath);
135+
136+
Assert.Throws<ArgumentException>(() =>
137+
{
138+
packBuilderOptions.MaximumNumberOfThreads = -1;
139+
});
140+
}
141+
142+
[Fact]
143+
public void ExceptionIfAddNullObjectID()
144+
{
145+
string orgRepoPath = SandboxPackBuilderTestRepo();
146+
PackBuilderOptions packBuilderOptions = new PackBuilderOptions(orgRepoPath);
147+
148+
using (Repository orgRepo = new Repository(orgRepoPath))
149+
{
150+
Assert.Throws<ArgumentNullException>(() =>
151+
{
152+
orgRepo.ObjectDatabase.Pack(packBuilderOptions, (PackBuilder builder) =>
153+
{
154+
builder.Add(null);
155+
});
156+
});
157+
}
158+
}
159+
160+
[Fact]
161+
public void ExceptionIfAddRecursivelyNullObjectID()
162+
{
163+
string orgRepoPath = SandboxPackBuilderTestRepo();
164+
PackBuilderOptions packBuilderOptions = new PackBuilderOptions(orgRepoPath);
165+
166+
using (Repository orgRepo = new Repository(orgRepoPath))
167+
{
168+
Assert.Throws<ArgumentNullException>(() =>
169+
{
170+
orgRepo.ObjectDatabase.Pack(packBuilderOptions, (PackBuilder builder) =>
171+
{
172+
builder.AddRecursively(null);
173+
});
174+
});
175+
}
176+
}
77177
}
78178
}

‎LibGit2Sharp/ObjectDatabase.cs

Lines changed: 40 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -645,44 +645,62 @@ public virtual MergeTreeResult MergeCommits(Commit ours, Commit theirs, MergeTre
645645
}
646646

647647
/// <summary>
648-
/// Pack the objects in the <see cref="ObjectDatabase"/> and write a packfile and indexfile for it.
648+
/// Pack the objects in the <see cref="ObjectDatabase"/> and write a pack (.pack) and index (.idx) files for it.
649+
/// </summary>
650+
/// <param name="options">Packing options</param>
651+
/// This method will invoke the default action of packing all objects in an arbitrary order.
652+
/// <returns>Packing results</returns>
653+
public virtual PackBuilderResults Pack(PackBuilderOptions options)
654+
{
655+
return InternalPack(options, (PackBuilder builder) =>
656+
{
657+
foreach (GitObject obj in this)
658+
{
659+
builder.Add(obj.Id);
660+
}
661+
});
662+
}
663+
664+
/// <summary>
665+
/// Pack the objects in the <see cref="ObjectDatabase"/> and write a pack (.pack) and index (.idx) files for it.
666+
/// </summary>
667+
/// <param name="options">Packing options</param>
668+
/// <param name="buildDelegate">Packing action</param>
669+
/// <returns>Packing results</returns>
670+
public virtual PackBuilderResults Pack(PackBuilderOptions options, Action<PackBuilder> buildDelegate)
671+
{
672+
return InternalPack(options, buildDelegate);
673+
}
674+
675+
/// <summary>
676+
/// Pack the objects in the <see cref="ObjectDatabase"/> and write a pack (.pack) and index (.idx) files for it. For internal use only.
649677
/// </summary>
650678
/// <param name="options">Packing options</param>
651679
/// <param name="buildDelegate">Packing action. If null is passed, the pack method will invoke the default action of packing
652680
/// all objects in an arbitrary order.</param>
653681
/// <returns>Packing results</returns>
654-
public virtual PackBuilderResults Pack(PackBuilderOptions options, Action<PackBuilder> buildDelegate)
682+
private PackBuilderResults InternalPack(PackBuilderOptions options, Action<PackBuilder> buildDelegate)
655683
{
656684
Ensure.ArgumentNotNull(options, "options");
685+
Ensure.ArgumentNotNull(buildDelegate, "buildDelegate");
657686

658687
PackBuilderResults results = new PackBuilderResults();
659-
PackBuilder builder = new PackBuilder(repo);
660-
661-
// set pre-build options
662-
builder.SetMaximumNumberOfThreads(options.MaximumNumberOfThreads);
663688

664-
if (buildDelegate == null)
665-
{
666-
// if no build delegate is provided, just insert all objects one by one.
667-
foreach (GitObject obj in this)
668-
{
669-
builder.Add(obj.Id);
670-
}
671-
}
672-
else
689+
using (PackBuilder builder = new PackBuilder(repo))
673690
{
691+
// set pre-build options
692+
builder.SetMaximumNumberOfThreads(options.MaximumNumberOfThreads);
693+
674694
// call the provided action
675695
buildDelegate(builder);
676-
}
677696

678-
// writing the pack file
679-
builder.Write(options.PackFilePath);
697+
// writing the pack and index files
698+
builder.Write(options.PackDirectoryPath);
680699

681-
// adding the results to the PackBuilderResults object
682-
results.WrittenObjectsCount = builder.WrittenObjectsCount;
700+
// adding the results to the PackBuilderResults object
701+
results.WrittenObjectsCount = builder.WrittenObjectsCount;
702+
}
683703

684-
// dispose and return
685-
builder.Dispose();
686704
return results;
687705
}
688706
}

‎LibGit2Sharp/PackBuilder.cs

Lines changed: 72 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System;
2+
using System.IO;
23
using LibGit2Sharp.Core;
34
using LibGit2Sharp.Core.Handles;
45

@@ -83,7 +84,7 @@ internal long ObjectsCount
8384
}
8485

8586
/// <summary>
86-
/// Get the number of objects the packbuilder has already written out.
87+
/// Number of objects the packbuilder has already written out.
8788
/// This is only correct after the packfile has been written.
8889
/// </summary>
8990
internal long WrittenObjectsCount
@@ -105,14 +106,80 @@ internal PackBuilderSafeHandle Handle
105106
}
106107
}
107108

108-
public sealed class PackBuilderResults
109+
/// <summary>
110+
/// The results of pack process of the <see cref="ObjectDatabase"/>.
111+
/// </summary>
112+
public struct PackBuilderResults
109113
{
110-
public long WrittenObjectsCount { get; set; }
114+
/// <summary>
115+
/// Number of objects the packbuilder has already written out.
116+
/// </summary>
117+
public long WrittenObjectsCount { get; internal set; }
111118
}
112119

120+
/// <summary>
121+
/// Packing options of the <see cref="ObjectDatabase"/>.
122+
/// </summary>
113123
public sealed class PackBuilderOptions
114124
{
115-
public string PackFilePath { get; set; }
116-
public int MaximumNumberOfThreads { get; set; }
125+
/// <summary>
126+
/// Constructor
127+
/// </summary>
128+
/// <param name="packDirectory">Directory path to write the pack and index files to it</param>
129+
/// The default value for maximum number of threads to spawn is 0 which ensures using all available CPUs.
130+
/// <exception cref="ArgumentNullException">if packDirectory is null or empty</exception>
131+
/// <exception cref="DirectoryNotFoundException">if packDirectory doesn't exist</exception>
132+
public PackBuilderOptions (string packDirectory)
133+
{
134+
PackDirectoryPath = packDirectory;
135+
MaximumNumberOfThreads = 0;
136+
}
137+
138+
/// <summary>
139+
/// Directory path to write the pack and index files to it.
140+
/// </summary>
141+
public string PackDirectoryPath
142+
{
143+
set
144+
{
145+
Ensure.ArgumentNotNullOrEmptyString(value, "packDirectory");
146+
147+
if (!Directory.Exists(value))
148+
{
149+
throw new DirectoryNotFoundException("The Directory " + value + " does not exist.");
150+
}
151+
152+
path = value;
153+
}
154+
get
155+
{
156+
return path;
157+
}
158+
}
159+
160+
/// <summary>
161+
/// Maximum number of threads to spawn.
162+
/// The default value is 0 which ensures using all available CPUs.
163+
/// </summary>
164+
public int MaximumNumberOfThreads
165+
{
166+
set
167+
{
168+
if (value < 0)
169+
{
170+
throw new ArgumentException("Argument can not be negative", "MaximumNumberOfThreads");
171+
}
172+
173+
nThreads = value;
174+
}
175+
get
176+
{
177+
return nThreads;
178+
}
179+
180+
}
181+
182+
private string path;
183+
private int nThreads;
117184
}
118185
}

0 commit comments

Comments
 (0)