Expose the ObjectInserter that created an ObjectReader

We've found in Gerrit Code Review that it is common to pass around
both an ObjectReader (or more commonly a RevWalk wrapping one) and an
ObjectInserter. These code paths often assume that the ObjectReader
can read back any objects created by the ObjectInserter without
flushing. However, we previously had no way to enforce that constraint
programmatically, leading to hard-to-spot problems.

Provide a solution by exposing the ObjectInserter that created an
ObjectReader, when known. Callers can either continue passing both
objects and check:
  reader.getCreatedFromInserter() == inserter
or they can just pass around ObjectReader and extract the inserter
when it's needed (checking that it's not null at usage time).

Change-Id: Ibbf5d1968b506f6b47030ab1b046ffccb47352ea

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsInserterTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsInserterTest.java
index 11a092468c7a3fb127df703fde984c537c2af0f1..74790f72c5818cc518c888a0d412ea83b4210627 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsInserterTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsInserterTest.java
@@ -45,6 +45,7 @@ package org.eclipse.jgit.internal.storage.dfs;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 
 import java.io.IOException;
@@ -97,6 +98,7 @@ public class DfsInserterTest {
                assertEquals(0, db.getObjectDatabase().listPacks().size());
 
                ObjectReader reader = ins.newReader();
+               assertSame(ins, reader.getCreatedFromInserter());
                assertEquals("foo", readString(reader.open(id1)));
                assertEquals("bar", readString(reader.open(id2)));
                assertEquals(0, db.getObjectDatabase().listPacks().size());
@@ -118,6 +120,7 @@ public class DfsInserterTest {
                assertEquals(0, db.getObjectDatabase().listPacks().size());
 
                ObjectReader reader = ins.newReader();
+               assertSame(ins, reader.getCreatedFromInserter());
                assertTrue(Arrays.equals(data, readStream(reader.open(id1))));
                assertEquals(0, db.getObjectDatabase().listPacks().size());
                ins.flush();
@@ -136,6 +139,7 @@ public class DfsInserterTest {
                assertEquals(1, db.getObjectDatabase().listPacks().size());
 
                ObjectReader reader = ins.newReader();
+               assertSame(ins, reader.getCreatedFromInserter());
                assertEquals("foo", readString(reader.open(id1)));
                assertEquals("bar", readString(reader.open(id2)));
                assertEquals(1, db.getObjectDatabase().listPacks().size());
@@ -154,6 +158,7 @@ public class DfsInserterTest {
                assertFalse(abbr1.equals(abbr2));
 
                ObjectReader reader = ins.newReader();
+               assertSame(ins, reader.getCreatedFromInserter());
                Collection<ObjectId> objs;
                objs = reader.resolve(AbbreviatedObjectId.fromString(abbr1));
                assertEquals(1, objs.size());

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsInserter.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsInserter.java
index e5ae9800fa9fe393581aef0a51614bb112258ce4..f5673e83d14bd01b4dacaaa1b91164ab8c626ad4 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsInserter.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsInserter.java
@@ -600,6 +600,11 @@ public class DfsInserter extends ObjectInserter {
                        return ctx.getShallowCommits();
                }
 
+               @Override
+               public ObjectInserter getCreatedFromInserter() {
+                       return DfsInserter.this;
+               }
+
                @Override
                public void close() {
                        ctx.close();

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectoryInserter.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectoryInserter.java
index fb411722be49df196b38501294f0f6c7484e0e70..9820e0ea393401ec9cfaec4cd9979cf54c78f772 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectoryInserter.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectoryInserter.java
@@ -136,7 +136,7 @@ class ObjectDirectoryInserter extends ObjectInserter {
 
        @Override
        public ObjectReader newReader() {
-               return new WindowCursor(db);
+               return new WindowCursor(db, this);
        }
 
        @Override

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/WindowCursor.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/WindowCursor.java
index a555e10d4735e0b7ad5271448aacfc83f3fd26ea..a2c0561ae19169e00fae46b9b253f0404c03eac5 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/WindowCursor.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/WindowCursor.java
@@ -53,6 +53,7 @@ import java.util.Set;
 import java.util.zip.DataFormatException;
 import java.util.zip.Inflater;
 
+import org.eclipse.jgit.annotations.Nullable;
 import org.eclipse.jgit.errors.IncorrectObjectTypeException;
 import org.eclipse.jgit.errors.MissingObjectException;
 import org.eclipse.jgit.errors.StoredObjectRepresentationNotAvailableException;
@@ -69,6 +70,7 @@ import org.eclipse.jgit.lib.BitmapIndex.BitmapBuilder;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.InflaterCache;
 import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectInserter;
 import org.eclipse.jgit.lib.ObjectLoader;
 import org.eclipse.jgit.lib.ObjectReader;
 import org.eclipse.jgit.lib.ProgressMonitor;
@@ -84,10 +86,20 @@ final class WindowCursor extends ObjectReader implements ObjectReuseAsIs {
 
        private DeltaBaseCache baseCache;
 
+       @Nullable
+       private final ObjectInserter createdFromInserter;
+
        final FileObjectDatabase db;
 
        WindowCursor(FileObjectDatabase db) {
                this.db = db;
+               this.createdFromInserter = null;
+       }
+
+       WindowCursor(FileObjectDatabase db,
+                       @Nullable ObjectDirectoryInserter createdFromInserter) {
+               this.db = db;
+               this.createdFromInserter = createdFromInserter;
        }
 
        DeltaBaseCache getDeltaBaseCache() {
@@ -329,6 +341,12 @@ final class WindowCursor extends ObjectReader implements ObjectReuseAsIs {
                return WindowCache.getStreamFileThreshold();
        }
 
+       @Override
+       @Nullable
+       public ObjectInserter getCreatedFromInserter() {
+               return createdFromInserter;
+       }
+
        /** Release the current window cursor. */
        @Override
        public void close() {

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectInserter.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectInserter.java
index d9da65f03fe79de80bd8f80489567fa6f48030e1..4c51279d05c5967e0d9065f93740002223a919ac 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectInserter.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectInserter.java
@@ -143,7 +143,18 @@ public abstract class ObjectInserter implements AutoCloseable {
                }
 
                public ObjectReader newReader() {
-                       return delegate().newReader();
+                       final ObjectReader dr = delegate().newReader();
+                       return new ObjectReader.Filter() {
+                               @Override
+                               protected ObjectReader delegate() {
+                                       return dr;
+                               }
+
+                               @Override
+                               public ObjectInserter getCreatedFromInserter() {
+                                       return ObjectInserter.Filter.this;
+                               }
+                       };
                }
 
                public void flush() throws IOException {
@@ -398,6 +409,9 @@ public abstract class ObjectInserter implements AutoCloseable {
         * visible to the repository. The returned reader should only be used from
         * the same thread as the inserter. Objects written by this inserter may not
         * be visible to {@code this.newReader().newReader()}.
+        * <p>
+        * The returned reader should return this inserter instance from {@link
+        * ObjectReader#getCreatedFromInserter()}.
         *
         * @since 3.5
         * @return reader for any object, including an object recently inserted by

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectReader.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectReader.java
index 99661a8d66180ed8f86d35d2b431c7fb08c9e261..b23145d7983d04827665541b5edfe16e6a09aff3 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectReader.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectReader.java
@@ -50,6 +50,7 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Set;
 
+import org.eclipse.jgit.annotations.Nullable;
 import org.eclipse.jgit.errors.IncorrectObjectTypeException;
 import org.eclipse.jgit.errors.MissingObjectException;
 import org.eclipse.jgit.internal.storage.pack.ObjectReuseAsIs;
@@ -421,6 +422,17 @@ public abstract class ObjectReader implements AutoCloseable {
                return null;
        }
 
+       /**
+        * @return the {@link ObjectInserter} from which this reader was created
+        *         using {@code inserter.newReader()}, or null if this reader was not
+        *         created from an inserter.
+        * @since 4.4
+        */
+       @Nullable
+       public ObjectInserter getCreatedFromInserter() {
+               return null;
+       }
+
        /**
         * Release any resources used by this reader.
         * <p>
@@ -524,6 +536,12 @@ public abstract class ObjectReader implements AutoCloseable {
                        return delegate().getBitmapIndex();
                }
 
+               @Override
+               @Nullable
+               public ObjectInserter getCreatedFromInserter() {
+                       return delegate().getCreatedFromInserter();
+               }
+
                @Override
                public void close() {
                        delegate().close();