Expose and pass around the FilterSpec object rather than the raw blob limit

Use the FilterSpec object so that less code has to know about the make-up of
FilterSpecs. When fields are added to FilterSpec, these pieces of code won't
need updating again.

Change-Id: I2b9e59a9926ff112faf62a3fa2d33c961a1779e5
Signed-off-by: Matthew DeVore <matvore@gmail.com>

5 files changed, 84 insertions, 26 deletions

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java
index 1e3d74ab57..83e2a44940 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java
@@ -44,6 +44,7 @@
 
 package org.eclipse.jgit.internal.storage.pack;
 
+import static java.util.Objects.requireNonNull;
 import static org.eclipse.jgit.internal.storage.pack.StoredObjectRepresentation.PACK_DELTA;
 import static org.eclipse.jgit.internal.storage.pack.StoredObjectRepresentation.PACK_WHOLE;
 import static org.eclipse.jgit.lib.Constants.OBJECT_ID_LENGTH;
@@ -119,6 +120,7 @@ import org.eclipse.jgit.revwalk.RevTag;
 import org.eclipse.jgit.revwalk.RevTree;
 import org.eclipse.jgit.storage.pack.PackConfig;
 import org.eclipse.jgit.storage.pack.PackStatistics;
+import org.eclipse.jgit.transport.FilterSpec;
 import org.eclipse.jgit.transport.ObjectCountCallback;
 import org.eclipse.jgit.transport.WriteAbortedException;
 import org.eclipse.jgit.util.BlockList;
@@ -303,7 +305,7 @@ public class PackWriter implements AutoCloseable {
 
 	private ObjectCountCallback callback;
 
-	private long filterBlobLimit = -1;
+	private FilterSpec filterSpec = FilterSpec.NO_FILTER;
 
 	/**
 	 * Create writer for specified repository.
@@ -641,10 +643,11 @@ public class PackWriter implements AutoCloseable {
 	}
 
 	/**
-	 * @param bytes exclude blobs of size greater than this
+	 * @param filter the filter which indicates what and what not this writer
+	 *            should include
 	 */
-	public void setFilterBlobLimit(long bytes) {
-		filterBlobLimit = bytes;
+	public void setFilterSpec(@NonNull FilterSpec filter) {
+		filterSpec = requireNonNull(filter);
 	}
 
 	/**
@@ -2079,10 +2082,10 @@ public class PackWriter implements AutoCloseable {
 
 		// Check if this object needs to be rejected, doing the cheaper
 		// checks first.
-		boolean reject = filterBlobLimit >= 0 &&
+		boolean reject = filterSpec.getBlobLimit() >= 0 &&
 			type == OBJ_BLOB &&
 			!want.contains(src) &&
-			reader.getObjectSize(src, OBJ_BLOB) > filterBlobLimit;
+			reader.getObjectSize(src, OBJ_BLOB) > filterSpec.getBlobLimit();
 		if (!reject) {
 			addObject(src, type, pathHashCode);
 		}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackFetchConnection.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackFetchConnection.java
index d54ae10f69..a729445309 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackFetchConnection.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackFetchConnection.java
@@ -245,8 +245,12 @@ public abstract class BasePackFetchConnection extends BasePackConnection
 
 	private PacketLineOut pckState;
 
-	/** If not -1, the maximum blob size to be sent to the server. */
-	private final long filterBlobLimit;
+	/**
+	 * Either FilterSpec.NO_FILTER for a filter that doesn't filter
+	 * anything, or a filter that indicates what and what not to send to the
+	 * server.
+	 */
+	private final FilterSpec filterSpec;
 
 	/**
 	 * Create a new connection to fetch using the native git transport.
@@ -268,7 +272,7 @@ public abstract class BasePackFetchConnection extends BasePackConnection
 
 		includeTags = transport.getTagOpt() != TagOpt.NO_TAGS;
 		thinPack = transport.isFetchThin();
-		filterBlobLimit = transport.getFilterBlobLimit();
+		filterSpec = transport.getFilterSpec();
 
 		if (local != null) {
 			walk = new RevWalk(local);
@@ -521,10 +525,8 @@ public abstract class BasePackFetchConnection extends BasePackConnection
 		if (first) {
 			return false;
 		}
-		if (filterBlobLimit == 0) {
-			p.writeString(OPTION_FILTER + " blob:none"); //$NON-NLS-1$
-		} else if (filterBlobLimit > 0) {
-			p.writeString(OPTION_FILTER + " blob:limit=" + filterBlobLimit); //$NON-NLS-1$
+		if (!filterSpec.isNoOp()) {
+			p.writeString(filterSpec.filterLine());
 		}
 		p.end();
 		outNeedsEnd = false;
@@ -566,7 +568,7 @@ public abstract class BasePackFetchConnection extends BasePackConnection
 					OPTION_MULTI_ACK_DETAILED));
 		}
 
-		if (filterBlobLimit >= 0 && !wantCapability(line, OPTION_FILTER)) {
+		if (!filterSpec.isNoOp() && !wantCapability(line, OPTION_FILTER)) {
 			throw new PackProtocolException(uri,
 					JGitText.get().filterRequiresCapability);
 		}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/FilterSpec.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/FilterSpec.java
index 39e412b56f..7e77419dec 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/FilterSpec.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/FilterSpec.java
@@ -45,6 +45,7 @@ package org.eclipse.jgit.transport;
 
 import java.text.MessageFormat;
 
+import org.eclipse.jgit.annotations.Nullable;
 import org.eclipse.jgit.errors.PackProtocolException;
 import org.eclipse.jgit.internal.JGitText;
 
@@ -135,4 +136,19 @@ public final class FilterSpec {
 		return blobLimit == -1;
 	}
 
+	/**
+	 * @return the filter line which describes this spec, e.g. "filter blob:limit=42"
+	 */
+	@Nullable
+	public String filterLine() {
+		if (blobLimit == 0) {
+			return GitProtocolConstants.OPTION_FILTER + " blob:none"; //$NON-NLS-1$
+		}
+
+		if (blobLimit > 0) {
+			return GitProtocolConstants.OPTION_FILTER + " blob:limit=" + blobLimit; //$NON-NLS-1$
+		}
+
+		return null;
+	}
 }
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/Transport.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/Transport.java
index 621c2ea56c..b6307ad6bb 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/Transport.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/Transport.java
@@ -47,6 +47,7 @@
 package org.eclipse.jgit.transport;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
+import static java.util.Objects.requireNonNull;
 
 import java.io.BufferedReader;
 import java.io.IOException;
@@ -70,6 +71,7 @@ import java.util.Map;
 import java.util.Vector;
 import java.util.concurrent.CopyOnWriteArrayList;
 
+import org.eclipse.jgit.annotations.NonNull;
 import org.eclipse.jgit.api.errors.AbortedByHookException;
 import org.eclipse.jgit.errors.NotSupportedException;
 import org.eclipse.jgit.errors.TransportException;
@@ -790,7 +792,7 @@ public abstract class Transport implements AutoCloseable {
 	/** Should refs no longer on the source be pruned from the destination? */
 	private boolean removeDeletedRefs;
 
-	private long filterBlobLimit = -1;
+	private FilterSpec filterSpec = FilterSpec.NO_FILTER;
 
 	/** Timeout in seconds to wait before aborting an IO read or write. */
 	private int timeout;
@@ -1067,20 +1069,42 @@ public abstract class Transport implements AutoCloseable {
 	}
 
 	/**
-	 * @return the last value passed to {@link #setFilterBlobLimit}, or -1 if
-	 *         it was never invoked.
+	 * @return the blob limit value set with {@link #setFilterBlobLimit} or
+	 *         {@link #setFilterSpec(FilterSpec)}, or -1 if no blob limit value
+	 *         was set
 	 * @since 5.0
+	 * @deprecated Use {@link #getFilterSpec()} instead
 	 */
-	public long getFilterBlobLimit() {
-		return filterBlobLimit;
+	@Deprecated
+	public final long getFilterBlobLimit() {
+		return filterSpec.getBlobLimit();
 	}
 
 	/**
 	 * @param bytes exclude blobs of size greater than this
 	 * @since 5.0
+	 * @deprecated Use {@link #setFilterSpec(FilterSpec)} instead
 	 */
-	public void setFilterBlobLimit(long bytes) {
-		filterBlobLimit = bytes;
+	@Deprecated
+	public final void setFilterBlobLimit(long bytes) {
+		setFilterSpec(FilterSpec.withBlobLimit(bytes));
+	}
+
+	/**
+	 * @return the last filter spec set with {@link #setFilterSpec(FilterSpec)},
+	 *         or {@link FilterSpec#NO_FILTER} if it was never invoked.
+	 * @since 5.4
+	 */
+	public final FilterSpec getFilterSpec() {
+		return filterSpec;
+	}
+
+	/**
+	 * @param filter a new filter to use for this transport
+	 * @since 5.4
+	 */
+	public final void setFilterSpec(@NonNull FilterSpec filter) {
+		filterSpec = requireNonNull(filter);
 	}
 
 	/**
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 6eb6837912..8ebaec132f 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
@@ -1526,17 +1526,30 @@ public class UploadPack {
 	}
 
 	/**
-	 * Returns the filter blob limit for the current request. Valid only after
-	 * calling recvWants(). A limit -1 means no limit.
+	 * Deprecated synonym for {@code getFilterSpec().getBlobLimit()}.
 	 *
 	 * @return filter blob limit requested by the client, or -1 if no limit
 	 * @since 5.3
+	 * @deprecated Use {@link #getFilterSpec()} instead
 	 */
-	public long getFilterBlobLimit() {
+	@Deprecated
+	public final long getFilterBlobLimit() {
+		return getFilterSpec().getBlobLimit();
+	}
+
+	/**
+	 * Returns the filter spec for the current request. Valid only after
+	 * calling recvWants(). This may be a no-op filter spec, but it won't be
+	 * null.
+	 *
+	 * @return filter requested by the client
+	 * @since 5.4
+	 */
+	public final FilterSpec getFilterSpec() {
 		if (currentRequest == null) {
 			throw new RequestNotYetReadException();
 		}
-		return currentRequest.getFilterSpec().getBlobLimit();
+		return currentRequest.getFilterSpec();
 	}
 
 	/**
@@ -2101,7 +2114,7 @@ public class UploadPack {
 			if (req.getFilterSpec().isNoOp()) {
 				pw.setUseCachedPacks(true);
 			} else {
-				pw.setFilterBlobLimit(req.getFilterSpec().getBlobLimit());
+				pw.setFilterSpec(req.getFilterSpec());
 				pw.setUseCachedPacks(false);
 			}
 			pw.setUseBitmaps(