Move objectType to GitPackCache's value

Otherwise, we would have I/O operations for every request for a wrong objectType. By having it as part of the value we can use the cache and aquire the information that the objectType does not match from the cache.

Author: georg-jung

src/NerdBank.GitVersioning/ManagedGit/GitPack.cs

@@ -178,9 +178,14 @@ public Stream GetObject(long offset, string objectType)
         }
 #endif
 
-        if (this.cache.TryOpen(offset, objectType, 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;

src/NerdBank.GitVersioning/ManagedGit/GitPackCache.cs

@@ -22,15 +22,14 @@ 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 retrieve.</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, string objectType, [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.

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, string, Stream)"/> and <see cref="TryOpen(long, string, 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, string), GitPackMemoryCacheStream> cache = new Dictionary<(long, string), GitPackMemoryCacheStream>();
+    private readonly Dictionary<long, (GitPackMemoryCacheStream, string)> cache = new();
 
     /// <inheritdoc/>
     public override Stream Add(long offset, string objectType, Stream stream)
     {
         var cacheStream = new GitPackMemoryCacheStream(stream);
-        this.cache.Add((offset, objectType), cacheStream);
+        this.cache.Add(offset, (cacheStream, objectType));
         return new GitPackMemoryCacheViewStream(cacheStream);
     }
 
     /// <inheritdoc/>
-    public override bool TryOpen(long offset, string objectType, [NotNullWhen(true)] out Stream? stream)
+    public override bool TryOpen(long offset, [NotNullWhen(true)] out (Stream ContentStream, string ObjectType)? hit)
     {
-        if (this.cache.TryGetValue((offset, objectType), 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;
     }
 
@@ -65,7 +65,7 @@ protected override void Dispose(bool disposing)
             while (this.cache.Count > 0)
             {
                 var key = this.cache.Keys.First();
-                GitPackMemoryCacheStream? stream = this.cache[key];
+                (GitPackMemoryCacheStream? stream, _) = this.cache[key];
                 stream.Dispose();
                 this.cache.Remove(key);
             }

src/NerdBank.GitVersioning/ManagedGit/GitPackNullCache.cs

@@ -25,9 +25,9 @@ public override Stream Add(long offset, string objectType, Stream stream)
     }
 
     /// <inheritdoc/>
-    public override bool TryOpen(long offset, string objectType, [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

@@ -21,10 +21,12 @@ public void StreamsAreIndependent()
             var cache = new GitPackMemoryCache();
 
             Stream stream1 = cache.Add(0, "anObjectType", stream);
-            Assert.True(cache.TryOpen(0, "anObjectType", out Stream stream2));
+            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);