From cc9ca71a166914969b795aeb887a882aa53a2fec Mon Sep 17 00:00:00 2001 From: Matthew DeVore Date: Fri, 15 Mar 2019 18:57:04 -0700 Subject: [PATCH] Put filter spec information in a dedicated object This increases type-safety and is ground work for support of the "tree:" filter. Change-Id: Id19eacdcdaddb9132064c642f6d554b1060efe9f Signed-off-by: Matthew DeVore --- .../jgit/transport/ProtocolV0ParserTest.java | 2 +- .../jgit/transport/ProtocolV2ParserTest.java | 4 +- .../eclipse/jgit/transport/FetchRequest.java | 20 +++---- .../jgit/transport/FetchV0Request.java | 21 ++++---- .../jgit/transport/FetchV2Request.java | 19 +++---- .../eclipse/jgit/transport/FilterSpec.java | 52 ++++++++++++++++--- .../jgit/transport/ProtocolV0Parser.java | 2 +- .../jgit/transport/ProtocolV2Parser.java | 2 +- .../eclipse/jgit/transport/UploadPack.java | 10 ++-- 9 files changed, 89 insertions(+), 43 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ProtocolV0ParserTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ProtocolV0ParserTest.java index 2c98c84ae..017f98079 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ProtocolV0ParserTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ProtocolV0ParserTest.java @@ -193,7 +193,7 @@ public class ProtocolV0ParserTest { assertThat(request.getWantIds(), hasOnlyObjectIds("4624442d68ee402a94364191085b77137618633e", "f900c8326a43303685c46b279b9f70411bff1a4b")); - assertEquals(13000, request.getFilterBlobLimit()); + assertEquals(13000, request.getFilterSpec().getBlobLimit()); } } diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ProtocolV2ParserTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ProtocolV2ParserTest.java index dafa81ecd..36c39568e 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ProtocolV2ParserTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ProtocolV2ParserTest.java @@ -232,7 +232,7 @@ public class ProtocolV2ParserTest { ProtocolV2Parser parser = new ProtocolV2Parser( ConfigBuilder.start().allowFilter().done()); FetchV2Request request = parser.parseFetchRequest(pckIn); - assertEquals(0, request.getFilterBlobLimit()); + assertEquals(0, request.getFilterSpec().getBlobLimit()); } @Test @@ -243,7 +243,7 @@ public class ProtocolV2ParserTest { ProtocolV2Parser parser = new ProtocolV2Parser( ConfigBuilder.start().allowFilter().done()); FetchV2Request request = parser.parseFetchRequest(pckIn); - assertEquals(15, request.getFilterBlobLimit()); + assertEquals(15, request.getFilterSpec().getBlobLimit()); } @Test diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchRequest.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchRequest.java index 40ba3a3ad..4dd7d6ed8 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchRequest.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchRequest.java @@ -62,7 +62,7 @@ abstract class FetchRequest { final Set clientShallowCommits; - final long filterBlobLimit; + final FilterSpec filterSpec; final Set clientCapabilities; @@ -82,8 +82,8 @@ abstract class FetchRequest { * how deep to go in the tree * @param clientShallowCommits * commits the client has without history - * @param filterBlobLimit - * to exclude blobs on certain conditions + * @param filterSpec + * the filter spec * @param clientCapabilities * capabilities sent in the request * @param deepenNotRefs @@ -96,13 +96,14 @@ abstract class FetchRequest { * agent as reported by the client in the request body */ FetchRequest(@NonNull Set wantIds, int depth, - @NonNull Set clientShallowCommits, long filterBlobLimit, + @NonNull Set clientShallowCommits, + @NonNull FilterSpec filterSpec, @NonNull Set clientCapabilities, int deepenSince, @NonNull List deepenNotRefs, @Nullable String agent) { this.wantIds = requireNonNull(wantIds); this.depth = depth; this.clientShallowCommits = requireNonNull(clientShallowCommits); - this.filterBlobLimit = filterBlobLimit; + this.filterSpec = requireNonNull(filterSpec); this.clientCapabilities = requireNonNull(clientCapabilities); this.deepenSince = deepenSince; this.deepenNotRefs = requireNonNull(deepenNotRefs); @@ -137,10 +138,11 @@ abstract class FetchRequest { } /** - * @return the blob limit set in a "filter" line (-1 if not set) + * @return the filter spec given in a "filter" line */ - long getFilterBlobLimit() { - return filterBlobLimit; + @NonNull + FilterSpec getFilterSpec() { + return filterSpec; } /** @@ -191,4 +193,4 @@ abstract class FetchRequest { String getAgent() { return agent; } -} \ No newline at end of file +} diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchV0Request.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchV0Request.java index 05f4a8155..231ab9f2c 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchV0Request.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchV0Request.java @@ -42,6 +42,8 @@ */ package org.eclipse.jgit.transport; +import static java.util.Objects.requireNonNull; + import java.util.Collection; import java.util.Collections; import java.util.HashSet; @@ -57,10 +59,11 @@ import org.eclipse.jgit.lib.ObjectId; final class FetchV0Request extends FetchRequest { FetchV0Request(@NonNull Set wantIds, int depth, - @NonNull Set clientShallowCommits, long filterBlobLimit, + @NonNull Set clientShallowCommits, + @NonNull FilterSpec filterSpec, @NonNull Set clientCapabilities, @Nullable String agent) { - super(wantIds, depth, clientShallowCommits, filterBlobLimit, - clientCapabilities, 0, Collections.emptyList(), agent); + super(wantIds, depth, clientShallowCommits, filterSpec, + clientCapabilities, 0, Collections.emptyList(), agent); } static final class Builder { @@ -71,7 +74,7 @@ final class FetchV0Request extends FetchRequest { final Set clientShallowCommits = new HashSet<>(); - long filterBlobLimit = -1; + FilterSpec filterSpec = FilterSpec.NO_FILTER; final Set clientCaps = new HashSet<>(); @@ -129,18 +132,18 @@ final class FetchV0Request extends FetchRequest { } /** - * @param filterBlobLim - * blob limit set in a "filter" line + * @param filter + * the filter set in a filter line * @return this builder */ - Builder setFilterBlobLimit(long filterBlobLim) { - filterBlobLimit = filterBlobLim; + Builder setFilterSpec(@NonNull FilterSpec filter) { + filterSpec = requireNonNull(filter); return this; } FetchV0Request build() { return new FetchV0Request(wantIds, depth, clientShallowCommits, - filterBlobLimit, clientCaps, agent); + filterSpec, clientCaps, agent); } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchV2Request.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchV2Request.java index 04b6dc3bd..6c2426909 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchV2Request.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchV2Request.java @@ -77,11 +77,12 @@ public final class FetchV2Request extends FetchRequest { @NonNull Set wantIds, @NonNull Set clientShallowCommits, int deepenSince, @NonNull List deepenNotRefs, int depth, - long filterBlobLimit, + @NonNull FilterSpec filterSpec, boolean doneReceived, @NonNull Set clientCapabilities, @Nullable String agent, @NonNull List serverOptions) { - super(wantIds, depth, clientShallowCommits, filterBlobLimit, - clientCapabilities, deepenSince, deepenNotRefs, agent); + super(wantIds, depth, clientShallowCommits, filterSpec, + clientCapabilities, deepenSince, + deepenNotRefs, agent); this.peerHas = requireNonNull(peerHas); this.wantedRefs = requireNonNull(wantedRefs); this.doneReceived = doneReceived; @@ -149,7 +150,7 @@ public final class FetchV2Request extends FetchRequest { int deepenSince; - long filterBlobLimit = -1; + FilterSpec filterSpec = FilterSpec.NO_FILTER; boolean doneReceived; @@ -268,12 +269,12 @@ public final class FetchV2Request extends FetchRequest { } /** - * @param filterBlobLim - * set in a "filter" line + * @param filter + * spec set in a "filter" line * @return this builder */ - Builder setFilterBlobLimit(long filterBlobLim) { - filterBlobLimit = filterBlobLim; + Builder setFilterSpec(@NonNull FilterSpec filter) { + filterSpec = requireNonNull(filter); return this; } @@ -322,7 +323,7 @@ public final class FetchV2Request extends FetchRequest { FetchV2Request build() { return new FetchV2Request(peerHas, wantedRefs, wantIds, clientShallowCommits, deepenSince, deepenNotRefs, - depth, filterBlobLimit, doneReceived, clientCapabilities, + depth, filterSpec, doneReceived, clientCapabilities, agent, Collections.unmodifiableList(serverOptions)); } } 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 041ea2886..39e412b56 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/FilterSpec.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/FilterSpec.java @@ -49,24 +49,31 @@ import org.eclipse.jgit.errors.PackProtocolException; import org.eclipse.jgit.internal.JGitText; /** - * Utility code for dealing with filter lines. + * Represents either a filter specified in a protocol "filter" line, or a + * placeholder to indicate no filtering. + * + * @since 5.4 */ -final class FilterSpec { +public final class FilterSpec { + + private final long blobLimit; - private FilterSpec() {} + private FilterSpec(long blobLimit) { + this.blobLimit = blobLimit; + } - /* + /** * Process the content of "filter" line from the protocol. It has a shape * like "blob:none" or "blob:limit=N", with limit a positive number. * * @param filterLine * the content of the "filter" line in the protocol - * @return N, the limit, defaulting to 0 if "none" + * @return a FilterSpec representing the given filter * @throws PackProtocolException * invalid filter because due to unrecognized format or * negative/non-numeric filter. */ - static long parseFilterLine(String filterLine) + public static FilterSpec fromFilterLine(String filterLine) throws PackProtocolException { long blobLimit = -1; @@ -92,7 +99,40 @@ final class FilterSpec { JGitText.get().invalidFilter, filterLine)); } + return new FilterSpec(blobLimit); + } + + /** + * @param blobLimit + * the blob limit in a "blob:[limit]" or "blob:none" filter line + * @return a filter spec which filters blobs above a certain size + */ + static FilterSpec withBlobLimit(long blobLimit) { + if (blobLimit < 0) { + throw new IllegalArgumentException( + "blobLimit cannot be negative: " + blobLimit); //$NON-NLS-1$ + } + return new FilterSpec(blobLimit); + } + + /** + * A placeholder that indicates no filtering. + */ + public static final FilterSpec NO_FILTER = new FilterSpec(-1); + + /** + * @return -1 if this filter does not filter blobs based on size, or a + * non-negative integer representing the max size of blobs to allow + */ + public long getBlobLimit() { return blobLimit; } + /** + * @return true if this filter doesn't filter out anything + */ + public boolean isNoOp() { + return blobLimit == -1; + } + } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ProtocolV0Parser.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ProtocolV0Parser.java index 53630fc31..396327aab 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ProtocolV0Parser.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ProtocolV0Parser.java @@ -130,7 +130,7 @@ final class ProtocolV0Parser { } filterReceived = true; - reqBuilder.setFilterBlobLimit(FilterSpec.parseFilterLine(arg)); + reqBuilder.setFilterSpec(FilterSpec.fromFilterLine(arg)); continue; } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ProtocolV2Parser.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ProtocolV2Parser.java index c0f105add..cb04ff69a 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ProtocolV2Parser.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ProtocolV2Parser.java @@ -207,7 +207,7 @@ final class ProtocolV2Parser { JGitText.get().tooManyFilters); } filterReceived = true; - reqBuilder.setFilterBlobLimit(FilterSpec.parseFilterLine( + reqBuilder.setFilterSpec(FilterSpec.fromFilterLine( line.substring(OPTION_FILTER.length() + 1))); } else { throw new PackProtocolException(MessageFormat 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 dce06d305..6eb683791 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java @@ -1536,7 +1536,7 @@ public class UploadPack { if (currentRequest == null) { throw new RequestNotYetReadException(); } - return currentRequest.getFilterBlobLimit(); + return currentRequest.getFilterSpec().getBlobLimit(); } /** @@ -2098,11 +2098,11 @@ public class UploadPack { accumulator); try { pw.setIndexDisabled(true); - if (req.getFilterBlobLimit() >= 0) { - pw.setFilterBlobLimit(req.getFilterBlobLimit()); - pw.setUseCachedPacks(false); - } else { + if (req.getFilterSpec().isNoOp()) { pw.setUseCachedPacks(true); + } else { + pw.setFilterBlobLimit(req.getFilterSpec().getBlobLimit()); + pw.setUseCachedPacks(false); } pw.setUseBitmaps( req.getDepth() == 0