UploadPack: Refactor to generalize the object reachability checks
ObjectWalk#createObjectReachabilityChecker() returns the best
implementation for the repo. UploadPack can use the interface and fold
the with/without commits cases in one code path.
Change-Id: I857c11735d1d8e36c3ed8185ff11de8a62e86540
Signed-off-by: Ivan Frade <ifrade@google.com>
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/BitmappedReachabilityChecker.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/BitmappedReachabilityChecker.java
index 0251452..0d9c459 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/BitmappedReachabilityChecker.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/BitmappedReachabilityChecker.java
@@ -42,7 +42,7 @@
* @throws IOException
* if the index or the object reader cannot be opened.
*/
- public BitmappedReachabilityChecker(RevWalk walk)
+ BitmappedReachabilityChecker(RevWalk walk)
throws IOException {
this.walk = walk;
if (walk.getObjectReader().getBitmapIndex() == null) {
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/ObjectWalk.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/ObjectWalk.java
index 04a4b4c..4c7a6f5 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/ObjectWalk.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/ObjectWalk.java
@@ -160,6 +160,29 @@
}
/**
+ * Create an object reachability checker that will use bitmaps if possible.
+ *
+ * This reachability checker accepts any object as target. For checks
+ * exclusively between commits, see
+ * {@link RevWalk#createReachabilityChecker()}.
+ *
+ * @return an object reachability checker, using bitmaps if possible.
+ *
+ * @throws IOException
+ * when the index fails to load.
+ *
+ * @since 5.8
+ */
+ public ObjectReachabilityChecker createObjectReachabilityChecker()
+ throws IOException {
+ if (reader.getBitmapIndex() != null) {
+ return new BitmappedObjectReachabilityChecker(this);
+ }
+
+ return new PedestrianObjectReachabilityChecker(this);
+ }
+
+ /**
* Mark an object or commit to start graph traversal from.
* <p>
* Callers are encouraged to use
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/PedestrianObjectReachabilityChecker.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/PedestrianObjectReachabilityChecker.java
index 90cb2fa..55c5156 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/PedestrianObjectReachabilityChecker.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/PedestrianObjectReachabilityChecker.java
@@ -22,7 +22,7 @@
* Checks if all objects are reachable from certain starting points doing a
* walk.
*/
-public class PedestrianObjectReachabilityChecker implements ObjectReachabilityChecker {
+class PedestrianObjectReachabilityChecker implements ObjectReachabilityChecker {
private ObjectWalk walk;
/**
@@ -31,7 +31,7 @@
* @param walk
* ObjectWalk instance to reuse. Caller retains ownership.
*/
- public PedestrianObjectReachabilityChecker(ObjectWalk walk) {
+ PedestrianObjectReachabilityChecker(ObjectWalk walk) {
this.walk = walk;
}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
index 95e2e5a..cf504d2 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
@@ -75,11 +75,9 @@
import org.eclipse.jgit.lib.RefDatabase;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.AsyncRevObjectQueue;
-import org.eclipse.jgit.revwalk.BitmappedObjectReachabilityChecker;
import org.eclipse.jgit.revwalk.DepthWalk;
import org.eclipse.jgit.revwalk.ObjectReachabilityChecker;
import org.eclipse.jgit.revwalk.ObjectWalk;
-import org.eclipse.jgit.revwalk.PedestrianObjectReachabilityChecker;
import org.eclipse.jgit.revwalk.ReachabilityChecker;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevFlag;
@@ -1918,44 +1916,24 @@
boolean repoHasBitmaps = reader.getBitmapIndex() != null;
if (!allWantsAreCommits) {
- if (!repoHasBitmaps) {
- if (up.transferConfig.isAllowFilter()) {
- // Use allowFilter as an indication that the server
- // operator is willing to pay the cost of these
- // reachability checks.
- try (ObjectWalk objWalk = walk.toObjectWalkWithSameObjects()) {
- List<RevObject> havesAsObjs = objectIdsToRevObjects(
- objWalk, reachableFrom);
- ObjectReachabilityChecker reachabilityChecker = new PedestrianObjectReachabilityChecker(
- objWalk);
- Optional<RevObject> unreachable = reachabilityChecker
- .areAllReachable(wantsAsObjs,
- havesAsObjs.stream());
- if (unreachable.isPresent()) {
- throw new WantNotValidException(
- unreachable.get());
- }
- }
- return;
- }
-
- // If unadvertized non-commits are requested, use
- // bitmaps. If there are no bitmaps, instead of
- // incurring the expense of a manual walk, reject
- // the request.
+ if (!repoHasBitmaps && !up.transferConfig.isAllowFilter()) {
+ // Checking unadvertised non-commits without bitmaps
+ // requires an expensive manual walk. Use allowFilter as an
+ // indication that the server operator is willing to pay
+ // this cost. Reject the request otherwise.
RevObject nonCommit = wantsAsObjs
.stream()
.filter(obj -> !(obj instanceof RevCommit))
.limit(1)
.collect(Collectors.toList()).get(0);
throw new WantNotValidException(nonCommit);
-
}
+
try (ObjectWalk objWalk = walk.toObjectWalkWithSameObjects()) {
List<RevObject> havesAsObjs = objectIdsToRevObjects(objWalk,
reachableFrom);
- ObjectReachabilityChecker reachabilityChecker = new BitmappedObjectReachabilityChecker(
- objWalk);
+ ObjectReachabilityChecker reachabilityChecker = objWalk
+ .createObjectReachabilityChecker();
Optional<RevObject> unreachable = reachabilityChecker
.areAllReachable(wantsAsObjs, havesAsObjs.stream());
if (unreachable.isPresent()) {
