Merge pull request #942 from georg-jung/fix/cache-tryopen-objecttype

Make GitPackCache include ObjectType

Author: AArnott

src/NerdBank.GitVersioning/ManagedGit/GitPack.cs

@@ -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;
@@ -216,7 +221,7 @@ public Stream GetObject(long offset, string objectType)
             throw;
         }
 
-        return this.cache.Add(offset, objectStream);
+        return this.cache.Add(offset, objectStream, objectType);
     }
 
     /// <summary>

src/NerdBank.GitVersioning/ManagedGit/GitPackCache.cs

@@ -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.
@@ -49,10 +49,11 @@ public abstract class GitPackCache : IDisposable
     /// A <see cref="Stream"/> which represents the object to add. This stream
     /// will be copied to the cache.
     /// </param>
+    /// <param name="objectType">The object type of the object to add 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, Stream stream, string objectType);
 
     /// <inheritdoc/>
     public void Dispose()

src/NerdBank.GitVersioning/ManagedGit/GitPackMemoryCache.cs

@@ -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, Stream, string)"/> 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, Stream stream, string objectType)
     {
         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;
     }
 
@@ -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);
             }

src/NerdBank.GitVersioning/ManagedGit/GitPackNullCache.cs

@@ -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, Stream stream, string objectType)
     {
         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;
     }
 

test/Nerdbank.GitVersioning.Tests/ManagedGit/GitPackMemoryCacheTests.cs

@@ -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, stream, "anObjectType");
+            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);