From 4cd954856eb8bdbe651338e7a1e3a61dd13d4fa2 Mon Sep 17 00:00:00 2001 From: Matthew DeVore Date: Mon, 18 Mar 2019 09:45:59 -0700 Subject: [PATCH] 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 --- .../internal/storage/pack/PackWriter.java | 15 +++++--- .../transport/BasePackFetchConnection.java | 18 +++++---- .../eclipse/jgit/transport/FilterSpec.java | 16 ++++++++ .../org/eclipse/jgit/transport/Transport.java | 38 +++++++++++++++---- .../eclipse/jgit/transport/UploadPack.java | 23 ++++++++--- 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 1e3d74ab5..83e2a4494 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 d54ae10f6..a72944530 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 39e412b56..7e77419de 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 621c2ea56..b6307ad6b 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 6eb683791..8ebaec132 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(