Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make GitPackCache include ObjectType #942

Merged
merged 3 commits into from
Sep 14, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions src/NerdBank.GitVersioning/ManagedGit/GitPack.cs
Original file line number Diff line number Diff line change
Expand Up @@ -178,9 +178,14 @@ public Stream GetObject(long offset, string objectType)
}
#endif

if (this.cache.TryOpen(offset, out Stream? stream))
if (this.cache.TryOpen(offset, out (Stream ContentStream, string ObjectType)? hit))
{
return stream!;
if (hit.Value.ObjectType != objectType)
{
throw new GitException($"An object of type {objectType} could not be located at offset {offset}.") { ErrorCode = GitException.ErrorCodes.ObjectNotFound };
}

return hit.Value.ContentStream;
}

GitPackObjectType packObjectType;
Expand Down Expand Up @@ -216,7 +221,7 @@ public Stream GetObject(long offset, string objectType)
throw;
}

return this.cache.Add(offset, objectStream);
return this.cache.Add(offset, objectType, objectStream);
}

/// <summary>
Expand Down
9 changes: 5 additions & 4 deletions src/NerdBank.GitVersioning/ManagedGit/GitPackCache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,14 @@ public abstract class GitPackCache : IDisposable
/// <param name="offset">
/// The offset of the Git object in the Git pack.
/// </param>
/// <param name="stream">
/// A <see cref="Stream"/> which will be set to the cached Git object.
/// <param name="hit">
/// A (<see cref="Stream"/>, ContentType) tuple which will be set to the cached data if found.
/// </param>
/// <returns>
/// <see langword="true"/> if the object was found in cache; otherwise,
/// <see langword="false"/>.
/// </returns>
public abstract bool TryOpen(long offset, [NotNullWhen(true)] out Stream? stream);
public abstract bool TryOpen(long offset, [NotNullWhen(true)] out (Stream ContentStream, string ObjectType)? hit);

/// <summary>
/// Gets statistics about the cache usage.
Expand All @@ -45,14 +45,15 @@ public abstract class GitPackCache : IDisposable
/// <param name="offset">
/// The offset of the Git object in the Git pack.
/// </param>
/// <param name="objectType">The object type of the object to add to the cache.</param>
/// <param name="stream">
/// A <see cref="Stream"/> which represents the object to add. This stream
/// will be copied to the cache.
/// </param>
/// <returns>
/// A <see cref="Stream"/> which represents the cached entry.
/// </returns>
public abstract Stream Add(long offset, Stream stream);
public abstract Stream Add(long offset, string objectType, Stream stream);
AArnott marked this conversation as resolved.
Show resolved Hide resolved

/// <inheritdoc/>
public void Dispose()
Expand Down
20 changes: 10 additions & 10 deletions src/NerdBank.GitVersioning/ManagedGit/GitPackMemoryCache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,33 +21,33 @@ namespace Nerdbank.GitVersioning.ManagedGit;
/// twice, it is read from the <see cref="MemoryStream"/>, rather than the underlying <see cref="Stream"/>.
/// </para>
/// <para>
/// <see cref="Add(long, Stream)"/> and <see cref="TryOpen(long, out Stream?)"/> return <see cref="Stream"/>
/// <see cref="Add(long, string, Stream)"/> and <see cref="TryOpen(long, out ValueTuple{Stream, string}?)"/> return <see cref="Stream"/>
/// objects which may operate on the same underlying <see cref="Stream"/>, but independently maintain
/// their state.
/// </para>
/// </summary>
public class GitPackMemoryCache : GitPackCache
{
private readonly Dictionary<long, GitPackMemoryCacheStream> cache = new Dictionary<long, GitPackMemoryCacheStream>();
private readonly Dictionary<long, (GitPackMemoryCacheStream, string)> cache = new();

/// <inheritdoc/>
public override Stream Add(long offset, Stream stream)
public override Stream Add(long offset, string objectType, Stream stream)
{
var cacheStream = new GitPackMemoryCacheStream(stream);
this.cache.Add(offset, cacheStream);
this.cache.Add(offset, (cacheStream, objectType));
return new GitPackMemoryCacheViewStream(cacheStream);
}

/// <inheritdoc/>
public override bool TryOpen(long offset, [NotNullWhen(true)] out Stream? stream)
public override bool TryOpen(long offset, [NotNullWhen(true)] out (Stream ContentStream, string ObjectType)? hit)
{
if (this.cache.TryGetValue(offset, out GitPackMemoryCacheStream? cacheStream))
if (this.cache.TryGetValue(offset, out (GitPackMemoryCacheStream Stream, string ObjectType) tuple))
{
stream = new GitPackMemoryCacheViewStream(cacheStream!);
hit = (new GitPackMemoryCacheViewStream(tuple.Stream), tuple.ObjectType);
return true;
}

stream = null;
hit = null;
return false;
}

Expand All @@ -64,8 +64,8 @@ protected override void Dispose(bool disposing)
{
while (this.cache.Count > 0)
{
long key = this.cache.Keys.First();
GitPackMemoryCacheStream? stream = this.cache[key];
var key = this.cache.Keys.First();
(GitPackMemoryCacheStream? stream, _) = this.cache[key];
stream.Dispose();
this.cache.Remove(key);
}
Expand Down
6 changes: 3 additions & 3 deletions src/NerdBank.GitVersioning/ManagedGit/GitPackNullCache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,15 @@ public class GitPackNullCache : GitPackCache
public static GitPackNullCache Instance { get; } = new GitPackNullCache();

/// <inheritdoc/>
public override Stream Add(long offset, Stream stream)
public override Stream Add(long offset, string objectType, Stream stream)
{
return stream;
}

/// <inheritdoc/>
public override bool TryOpen(long offset, [NotNullWhen(true)] out Stream? stream)
public override bool TryOpen(long offset, [NotNullWhen(true)] out (Stream ContentStream, string ObjectType)? hit)
{
stream = null;
hit = null;
return false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,13 @@ public void StreamsAreIndependent()
{
var cache = new GitPackMemoryCache();

Stream stream1 = cache.Add(0, stream);
Assert.True(cache.TryOpen(0, out Stream stream2));
Stream stream1 = cache.Add(0, "anObjectType", stream);
Assert.True(cache.TryOpen(0, out (Stream ContentStream, string ObjectType)? hit));
Assert.True(hit.HasValue);
Assert.Equal("anObjectType", hit.Value.ObjectType);

using (stream1)
using (stream2)
using (Stream stream2 = hit.Value.ContentStream)
{
stream1.Seek(5, SeekOrigin.Begin);
Assert.Equal(5, stream1.Position);
Expand Down