From 519cb1e91b06fbc82b7e87431ac7485bf3c9d91b Mon Sep 17 00:00:00 2001 From: Demetr Starshov Date: Wed, 6 May 2020 17:36:34 -0700 Subject: Moving transport/internal -> internal/transport Moving transport related internal classes into dedicated subpackage in o/e/j/internal package. Signed-off-by: Demetr Starshov Change-Id: I21ed029d359f5f7d8298f102efbb4b1dcdf404ad --- .../connectivity/FullConnectivityChecker.java | 201 +++++++++++++++++++++ .../transport/http/DelegatingSSLSocketFactory.java | 101 +++++++++++ .../org/eclipse/jgit/transport/ReceivePack.java | 2 +- .../jgit/transport/http/JDKHttpConnection.java | 2 +- .../internal/DelegatingSSLSocketFactory.java | 101 ----------- .../internal/FullConnectivityChecker.java | 201 --------------------- 6 files changed, 304 insertions(+), 304 deletions(-) create mode 100644 org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/connectivity/FullConnectivityChecker.java create mode 100644 org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/http/DelegatingSSLSocketFactory.java delete mode 100644 org.eclipse.jgit/src/org/eclipse/jgit/transport/internal/DelegatingSSLSocketFactory.java delete mode 100644 org.eclipse.jgit/src/org/eclipse/jgit/transport/internal/FullConnectivityChecker.java (limited to 'org.eclipse.jgit/src') diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/connectivity/FullConnectivityChecker.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/connectivity/FullConnectivityChecker.java new file mode 100644 index 0000000000..b76e3a3caa --- /dev/null +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/connectivity/FullConnectivityChecker.java @@ -0,0 +1,201 @@ +/* + * Copyright (c) 2019, Google LLC and others + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Distribution License v. 1.0 which is available at + * http://www.eclipse.org/org/documents/edl-v10.php. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +package org.eclipse.jgit.internal.transport.connectivity; + +import java.io.IOException; +import java.util.Set; + +import org.eclipse.jgit.errors.MissingObjectException; +import org.eclipse.jgit.internal.JGitText; +import org.eclipse.jgit.lib.Constants; +import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.ObjectIdSubclassMap; +import org.eclipse.jgit.lib.ProgressMonitor; +import org.eclipse.jgit.revwalk.ObjectWalk; +import org.eclipse.jgit.revwalk.RevBlob; +import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevFlag; +import org.eclipse.jgit.revwalk.RevObject; +import org.eclipse.jgit.revwalk.RevSort; +import org.eclipse.jgit.revwalk.RevTree; +import org.eclipse.jgit.transport.ConnectivityChecker; +import org.eclipse.jgit.transport.ReceiveCommand; +import org.eclipse.jgit.transport.ReceiveCommand.Result; + +/** + * A connectivity checker that uses the entire reference database to perform + * reachability checks when checking the connectivity of objects. If + * info.isCheckObjects() is set it will also check that objects referenced by + * deltas are either provided or reachable as well. + */ +public final class FullConnectivityChecker implements ConnectivityChecker { + @Override + public void checkConnectivity(ConnectivityCheckInfo connectivityCheckInfo, + Set haves, ProgressMonitor pm) + throws MissingObjectException, IOException { + pm.beginTask(JGitText.get().countingObjects, + ProgressMonitor.UNKNOWN); + try (ObjectWalk ow = new ObjectWalk(connectivityCheckInfo.getRepository())) { + if (!markStartAndKnownNodes(connectivityCheckInfo, ow, haves, + pm)) { + return; + } + checkCommitTree(connectivityCheckInfo, ow, pm); + checkObjects(connectivityCheckInfo, ow, pm); + } finally { + pm.endTask(); + } + } + + /** + * @param connectivityCheckInfo + * Source for connectivity check. + * @param ow + * Walk which can also check blobs. + * @param haves + * Set of references known for client. + * @param pm + * Monitor to publish progress to. + * @return true if at least one new node was marked. + * @throws IOException + * an error occurred during connectivity checking. + */ + private boolean markStartAndKnownNodes( + ConnectivityCheckInfo connectivityCheckInfo, + ObjectWalk ow, + Set haves, ProgressMonitor pm) + throws IOException { + boolean markTrees = connectivityCheckInfo + .isCheckObjects() + && !connectivityCheckInfo.getParser().getBaseObjectIds() + .isEmpty(); + if (connectivityCheckInfo.isCheckObjects()) { + ow.sort(RevSort.TOPO); + if (!connectivityCheckInfo.getParser().getBaseObjectIds() + .isEmpty()) { + ow.sort(RevSort.BOUNDARY, true); + } + } + boolean hasInteresting = false; + + for (ReceiveCommand cmd : connectivityCheckInfo.getCommands()) { + if (cmd.getResult() != Result.NOT_ATTEMPTED) { + continue; + } + if (cmd.getType() == ReceiveCommand.Type.DELETE) { + continue; + } + if (haves.contains(cmd.getNewId())) { + continue; + } + ow.markStart(ow.parseAny(cmd.getNewId())); + pm.update(1); + hasInteresting = true; + } + if (!hasInteresting) { + return false; + } + for (ObjectId have : haves) { + RevObject o = ow.parseAny(have); + ow.markUninteresting(o); + pm.update(1); + + if (markTrees) { + o = ow.peel(o); + if (o instanceof RevCommit) { + o = ((RevCommit) o).getTree(); + } + if (o instanceof RevTree) { + ow.markUninteresting(o); + } + } + } + return true; + } + + /** + * @param connectivityCheckInfo + * Source for connectivity check. + * @param ow + * Walk which can also check blobs. + * @param pm + * Monitor to publish progress to. + * @throws IOException + * an error occurred during connectivity checking. + */ + private void checkCommitTree(ConnectivityCheckInfo connectivityCheckInfo, + ObjectWalk ow, + ProgressMonitor pm) throws IOException { + RevCommit c; + ObjectIdSubclassMap newObjectIds = connectivityCheckInfo + .getParser() + .getNewObjectIds(); + while ((c = ow.next()) != null) { + pm.update(1); + if (connectivityCheckInfo.isCheckObjects() + && !c.has(RevFlag.UNINTERESTING) + && !newObjectIds.contains(c)) { + throw new MissingObjectException(c, Constants.TYPE_COMMIT); + } + } + } + + /** + * @param connectivityCheckInfo + * Source for connectivity check. + * @param ow + * Walk which can also check blobs. + * @param pm + * Monitor to publish progress to. + * @throws IOException + * an error occurred during connectivity checking. + * + */ + private void checkObjects(ConnectivityCheckInfo connectivityCheckInfo, + ObjectWalk ow, + ProgressMonitor pm) throws IOException { + RevObject o; + ObjectIdSubclassMap newObjectIds = connectivityCheckInfo + .getParser() + .getNewObjectIds(); + + while ((o = ow.nextObject()) != null) { + pm.update(1); + if (o.has(RevFlag.UNINTERESTING)) { + continue; + } + + if (connectivityCheckInfo.isCheckObjects()) { + if (newObjectIds.contains(o)) { + continue; + } + throw new MissingObjectException(o, o.getType()); + + } + + if (o instanceof RevBlob + && !connectivityCheckInfo.getRepository().getObjectDatabase() + .has(o)) { + throw new MissingObjectException(o, Constants.TYPE_BLOB); + } + } + + if (connectivityCheckInfo.isCheckObjects()) { + for (ObjectId id : connectivityCheckInfo.getParser() + .getBaseObjectIds()) { + o = ow.parseAny(id); + if (!o.has(RevFlag.UNINTERESTING)) { + throw new MissingObjectException(o, o.getType()); + } + } + } + } +} diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/http/DelegatingSSLSocketFactory.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/http/DelegatingSSLSocketFactory.java new file mode 100644 index 0000000000..5aab61ad05 --- /dev/null +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/http/DelegatingSSLSocketFactory.java @@ -0,0 +1,101 @@ +/* + * Copyright (c) 2020 Thomas Wolf + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Distribution License v. 1.0 which is available at + * https://www.eclipse.org/org/documents/edl-v10.php. + * + * SPDX-License-Identifier: BSD-3-Clause + */ +package org.eclipse.jgit.internal.transport.http; + +import java.io.IOException; +import java.net.InetAddress; +import java.net.Socket; + +import javax.net.ssl.SSLSocket; +import javax.net.ssl.SSLSocketFactory; + +/** + * An {@link SSLSocketFactory} that delegates to another factory and allows + * configuring the created socket via {@link #configure(SSLSocket)} before it is + * returned. + */ +public abstract class DelegatingSSLSocketFactory extends SSLSocketFactory { + + private final SSLSocketFactory delegate; + + /** + * Creates a new {@link DelegatingSSLSocketFactory} based on the given + * delegate. + * + * @param delegate + * {@link SSLSocketFactory} to delegate to + */ + public DelegatingSSLSocketFactory(SSLSocketFactory delegate) { + this.delegate = delegate; + } + + @Override + public SSLSocket createSocket() throws IOException { + return prepare(delegate.createSocket()); + } + + @Override + public SSLSocket createSocket(String host, int port) throws IOException { + return prepare(delegate.createSocket(host, port)); + } + + @Override + public SSLSocket createSocket(String host, int port, + InetAddress localAddress, int localPort) throws IOException { + return prepare( + delegate.createSocket(host, port, localAddress, localPort)); + } + + @Override + public SSLSocket createSocket(InetAddress host, int port) + throws IOException { + return prepare(delegate.createSocket(host, port)); + } + + @Override + public SSLSocket createSocket(InetAddress host, int port, + InetAddress localAddress, int localPort) throws IOException { + return prepare( + delegate.createSocket(host, port, localAddress, localPort)); + } + + @Override + public SSLSocket createSocket(Socket socket, String host, int port, + boolean autoClose) throws IOException { + return prepare(delegate.createSocket(socket, host, port, autoClose)); + } + + @Override + public String[] getDefaultCipherSuites() { + return delegate.getDefaultCipherSuites(); + } + + @Override + public String[] getSupportedCipherSuites() { + return delegate.getSupportedCipherSuites(); + } + + private SSLSocket prepare(Socket socket) throws IOException { + SSLSocket sslSocket = (SSLSocket) socket; + configure(sslSocket); + return sslSocket; + } + + /** + * Configure the newly created socket. + * + * @param socket + * to configure + * @throws IOException + * if the socket cannot be configured + */ + protected abstract void configure(SSLSocket socket) throws IOException; + +} diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java index 8a8c1ae0ba..49413e54f3 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java @@ -48,6 +48,7 @@ import org.eclipse.jgit.internal.JGitText; import org.eclipse.jgit.internal.storage.file.PackLock; import org.eclipse.jgit.internal.submodule.SubmoduleValidator; import org.eclipse.jgit.internal.submodule.SubmoduleValidator.SubmoduleValidationException; +import org.eclipse.jgit.internal.transport.connectivity.FullConnectivityChecker; import org.eclipse.jgit.internal.transport.parser.FirstCommand; import org.eclipse.jgit.lib.AnyObjectId; import org.eclipse.jgit.lib.BatchRefUpdate; @@ -72,7 +73,6 @@ import org.eclipse.jgit.transport.ConnectivityChecker.ConnectivityCheckInfo; import org.eclipse.jgit.transport.PacketLineIn.InputOverLimitIOException; import org.eclipse.jgit.transport.ReceiveCommand.Result; import org.eclipse.jgit.transport.RefAdvertiser.PacketLineOutRefAdvertiser; -import org.eclipse.jgit.transport.internal.FullConnectivityChecker; import org.eclipse.jgit.util.io.InterruptTimer; import org.eclipse.jgit.util.io.LimitedInputStream; import org.eclipse.jgit.util.io.TimeoutInputStream; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/http/JDKHttpConnection.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/http/JDKHttpConnection.java index 925c4e2f84..3b0bae21ef 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/http/JDKHttpConnection.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/http/JDKHttpConnection.java @@ -32,7 +32,7 @@ import javax.net.ssl.SSLSocket; import javax.net.ssl.TrustManager; import org.eclipse.jgit.annotations.NonNull; -import org.eclipse.jgit.transport.internal.DelegatingSSLSocketFactory; +import org.eclipse.jgit.internal.transport.http.DelegatingSSLSocketFactory; import org.eclipse.jgit.util.HttpSupport; /** * A {@link org.eclipse.jgit.transport.http.HttpConnection} which simply diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/internal/DelegatingSSLSocketFactory.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/internal/DelegatingSSLSocketFactory.java deleted file mode 100644 index d25ecd459d..0000000000 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/internal/DelegatingSSLSocketFactory.java +++ /dev/null @@ -1,101 +0,0 @@ -/* - * Copyright (c) 2020 Thomas Wolf - * - * This program and the accompanying materials are made available under the - * terms of the Eclipse Distribution License v. 1.0 which is available at - * https://www.eclipse.org/org/documents/edl-v10.php. - * - * SPDX-License-Identifier: BSD-3-Clause - */ -package org.eclipse.jgit.transport.internal; - -import java.io.IOException; -import java.net.InetAddress; -import java.net.Socket; - -import javax.net.ssl.SSLSocket; -import javax.net.ssl.SSLSocketFactory; - -/** - * An {@link SSLSocketFactory} that delegates to another factory and allows - * configuring the created socket via {@link #configure(SSLSocket)} before it is - * returned. - */ -public abstract class DelegatingSSLSocketFactory extends SSLSocketFactory { - - private final SSLSocketFactory delegate; - - /** - * Creates a new {@link DelegatingSSLSocketFactory} based on the given - * delegate. - * - * @param delegate - * {@link SSLSocketFactory} to delegate to - */ - public DelegatingSSLSocketFactory(SSLSocketFactory delegate) { - this.delegate = delegate; - } - - @Override - public SSLSocket createSocket() throws IOException { - return prepare(delegate.createSocket()); - } - - @Override - public SSLSocket createSocket(String host, int port) throws IOException { - return prepare(delegate.createSocket(host, port)); - } - - @Override - public SSLSocket createSocket(String host, int port, - InetAddress localAddress, int localPort) throws IOException { - return prepare( - delegate.createSocket(host, port, localAddress, localPort)); - } - - @Override - public SSLSocket createSocket(InetAddress host, int port) - throws IOException { - return prepare(delegate.createSocket(host, port)); - } - - @Override - public SSLSocket createSocket(InetAddress host, int port, - InetAddress localAddress, int localPort) throws IOException { - return prepare( - delegate.createSocket(host, port, localAddress, localPort)); - } - - @Override - public SSLSocket createSocket(Socket socket, String host, int port, - boolean autoClose) throws IOException { - return prepare(delegate.createSocket(socket, host, port, autoClose)); - } - - @Override - public String[] getDefaultCipherSuites() { - return delegate.getDefaultCipherSuites(); - } - - @Override - public String[] getSupportedCipherSuites() { - return delegate.getSupportedCipherSuites(); - } - - private SSLSocket prepare(Socket socket) throws IOException { - SSLSocket sslSocket = (SSLSocket) socket; - configure(sslSocket); - return sslSocket; - } - - /** - * Configure the newly created socket. - * - * @param socket - * to configure - * @throws IOException - * if the socket cannot be configured - */ - protected abstract void configure(SSLSocket socket) throws IOException; - -} diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/internal/FullConnectivityChecker.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/internal/FullConnectivityChecker.java deleted file mode 100644 index 60d8f452ba..0000000000 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/internal/FullConnectivityChecker.java +++ /dev/null @@ -1,201 +0,0 @@ -/* - * Copyright (c) 2019, Google LLC and others - * - * This program and the accompanying materials are made available under the - * terms of the Eclipse Distribution License v. 1.0 which is available at - * http://www.eclipse.org/org/documents/edl-v10.php. - * - * SPDX-License-Identifier: BSD-3-Clause - */ - -package org.eclipse.jgit.transport.internal; - -import java.io.IOException; -import java.util.Set; - -import org.eclipse.jgit.errors.MissingObjectException; -import org.eclipse.jgit.internal.JGitText; -import org.eclipse.jgit.lib.Constants; -import org.eclipse.jgit.lib.ObjectId; -import org.eclipse.jgit.lib.ObjectIdSubclassMap; -import org.eclipse.jgit.lib.ProgressMonitor; -import org.eclipse.jgit.revwalk.ObjectWalk; -import org.eclipse.jgit.revwalk.RevBlob; -import org.eclipse.jgit.revwalk.RevCommit; -import org.eclipse.jgit.revwalk.RevFlag; -import org.eclipse.jgit.revwalk.RevObject; -import org.eclipse.jgit.revwalk.RevSort; -import org.eclipse.jgit.revwalk.RevTree; -import org.eclipse.jgit.transport.ConnectivityChecker; -import org.eclipse.jgit.transport.ReceiveCommand; -import org.eclipse.jgit.transport.ReceiveCommand.Result; - -/** - * A connectivity checker that uses the entire reference database to perform - * reachability checks when checking the connectivity of objects. If - * info.isCheckObjects() is set it will also check that objects referenced by - * deltas are either provided or reachable as well. - */ -public final class FullConnectivityChecker implements ConnectivityChecker { - @Override - public void checkConnectivity(ConnectivityCheckInfo connectivityCheckInfo, - Set haves, ProgressMonitor pm) - throws MissingObjectException, IOException { - pm.beginTask(JGitText.get().countingObjects, - ProgressMonitor.UNKNOWN); - try (ObjectWalk ow = new ObjectWalk(connectivityCheckInfo.getRepository())) { - if (!markStartAndKnownNodes(connectivityCheckInfo, ow, haves, - pm)) { - return; - } - checkCommitTree(connectivityCheckInfo, ow, pm); - checkObjects(connectivityCheckInfo, ow, pm); - } finally { - pm.endTask(); - } - } - - /** - * @param connectivityCheckInfo - * Source for connectivity check. - * @param ow - * Walk which can also check blobs. - * @param haves - * Set of references known for client. - * @param pm - * Monitor to publish progress to. - * @return true if at least one new node was marked. - * @throws IOException - * an error occurred during connectivity checking. - */ - private boolean markStartAndKnownNodes( - ConnectivityCheckInfo connectivityCheckInfo, - ObjectWalk ow, - Set haves, ProgressMonitor pm) - throws IOException { - boolean markTrees = connectivityCheckInfo - .isCheckObjects() - && !connectivityCheckInfo.getParser().getBaseObjectIds() - .isEmpty(); - if (connectivityCheckInfo.isCheckObjects()) { - ow.sort(RevSort.TOPO); - if (!connectivityCheckInfo.getParser().getBaseObjectIds() - .isEmpty()) { - ow.sort(RevSort.BOUNDARY, true); - } - } - boolean hasInteresting = false; - - for (ReceiveCommand cmd : connectivityCheckInfo.getCommands()) { - if (cmd.getResult() != Result.NOT_ATTEMPTED) { - continue; - } - if (cmd.getType() == ReceiveCommand.Type.DELETE) { - continue; - } - if (haves.contains(cmd.getNewId())) { - continue; - } - ow.markStart(ow.parseAny(cmd.getNewId())); - pm.update(1); - hasInteresting = true; - } - if (!hasInteresting) { - return false; - } - for (ObjectId have : haves) { - RevObject o = ow.parseAny(have); - ow.markUninteresting(o); - pm.update(1); - - if (markTrees) { - o = ow.peel(o); - if (o instanceof RevCommit) { - o = ((RevCommit) o).getTree(); - } - if (o instanceof RevTree) { - ow.markUninteresting(o); - } - } - } - return true; - } - - /** - * @param connectivityCheckInfo - * Source for connectivity check. - * @param ow - * Walk which can also check blobs. - * @param pm - * Monitor to publish progress to. - * @throws IOException - * an error occurred during connectivity checking. - */ - private void checkCommitTree(ConnectivityCheckInfo connectivityCheckInfo, - ObjectWalk ow, - ProgressMonitor pm) throws IOException { - RevCommit c; - ObjectIdSubclassMap newObjectIds = connectivityCheckInfo - .getParser() - .getNewObjectIds(); - while ((c = ow.next()) != null) { - pm.update(1); - if (connectivityCheckInfo.isCheckObjects() - && !c.has(RevFlag.UNINTERESTING) - && !newObjectIds.contains(c)) { - throw new MissingObjectException(c, Constants.TYPE_COMMIT); - } - } - } - - /** - * @param connectivityCheckInfo - * Source for connectivity check. - * @param ow - * Walk which can also check blobs. - * @param pm - * Monitor to publish progress to. - * @throws IOException - * an error occurred during connectivity checking. - * - */ - private void checkObjects(ConnectivityCheckInfo connectivityCheckInfo, - ObjectWalk ow, - ProgressMonitor pm) throws IOException { - RevObject o; - ObjectIdSubclassMap newObjectIds = connectivityCheckInfo - .getParser() - .getNewObjectIds(); - - while ((o = ow.nextObject()) != null) { - pm.update(1); - if (o.has(RevFlag.UNINTERESTING)) { - continue; - } - - if (connectivityCheckInfo.isCheckObjects()) { - if (newObjectIds.contains(o)) { - continue; - } - throw new MissingObjectException(o, o.getType()); - - } - - if (o instanceof RevBlob - && !connectivityCheckInfo.getRepository().getObjectDatabase() - .has(o)) { - throw new MissingObjectException(o, Constants.TYPE_BLOB); - } - } - - if (connectivityCheckInfo.isCheckObjects()) { - for (ObjectId id : connectivityCheckInfo.getParser() - .getBaseObjectIds()) { - o = ow.parseAny(id); - if (!o.has(RevFlag.UNINTERESTING)) { - throw new MissingObjectException(o, o.getType()); - } - } - } - } -} -- cgit v1.2.3 From 9075beefb1bcde3eea9ccee7e34a74a0f61e7ea2 Mon Sep 17 00:00:00 2001 From: Demetr Starshov Date: Wed, 6 May 2020 17:53:36 -0700 Subject: ReceivePack: adding IterativeConnectivityChecker Introduce an IterativeConnectivityChecker which runs a connectivity check with a filtered set of references, and falls back to using the full set of advertised references. It uses references during first check attempt: - References that are ancestors of an incoming commits (e.g., pushing a commit onto an existing branch or pushing a new branch based on another branch) - Additional list of references we know client can be interested in (e.g. list of open changes for Gerrit) We tested it inside Google and it improves connectivity for certain topologies. For example connectivity counts for chromium.googlesource.com/chromium/src: percentile_50: 1923 (was: 22777) percentile_90: 23272 (was: 353003) percentile_99: 345522 (was: 353435) This saved ~2 seconds on every push to this repository. Signed-off-by: Demetr Starshov Change-Id: I6543c2e10ed04622ca795b195665133e690d3b10 --- org.eclipse.jgit.test/META-INF/MANIFEST.MF | 1 + .../IterativeConnectivityCheckerTest.java | 258 +++++++++++++++++++++ .../connectivity/IterativeConnectivityChecker.java | 152 ++++++++++++ 3 files changed, 411 insertions(+) create mode 100644 org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/transport/connectivity/IterativeConnectivityCheckerTest.java create mode 100644 org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/connectivity/IterativeConnectivityChecker.java (limited to 'org.eclipse.jgit/src') diff --git a/org.eclipse.jgit.test/META-INF/MANIFEST.MF b/org.eclipse.jgit.test/META-INF/MANIFEST.MF index 6bd06e11b6..eda4505264 100644 --- a/org.eclipse.jgit.test/META-INF/MANIFEST.MF +++ b/org.eclipse.jgit.test/META-INF/MANIFEST.MF @@ -42,6 +42,7 @@ Import-Package: com.googlecode.javaewah;version="[1.1.6,2.0.0)", org.eclipse.jgit.internal.storage.pack;version="[5.8.0,5.9.0)", org.eclipse.jgit.internal.storage.reftable;version="[5.8.0,5.9.0)", org.eclipse.jgit.internal.storage.reftree;version="[5.8.0,5.9.0)", + org.eclipse.jgit.internal.transport.connectivity;version="[5.8.0,5.9.0)", org.eclipse.jgit.internal.transport.http;version="[5.8.0,5.9.0)", org.eclipse.jgit.internal.transport.parser;version="[5.8.0,5.9.0)", org.eclipse.jgit.junit;version="[5.8.0,5.9.0)", diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/transport/connectivity/IterativeConnectivityCheckerTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/transport/connectivity/IterativeConnectivityCheckerTest.java new file mode 100644 index 0000000000..e75dd22591 --- /dev/null +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/transport/connectivity/IterativeConnectivityCheckerTest.java @@ -0,0 +1,258 @@ +/* + * Copyright (c) 2019, Google LLC and others + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Distribution License v. 1.0 which is available at + * http://www.eclipse.org/org/documents/edl-v10.php. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +package org.eclipse.jgit.internal.transport.connectivity; + +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.verify; + +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; + +import org.eclipse.jgit.errors.MissingObjectException; +import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription; +import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository; +import org.eclipse.jgit.junit.TestRepository; +import org.eclipse.jgit.lib.Constants; +import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.ProgressMonitor; +import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.transport.PackParser; +import org.eclipse.jgit.transport.ReceiveCommand; +import org.eclipse.jgit.transport.ConnectivityChecker; +import org.eclipse.jgit.transport.ConnectivityChecker.ConnectivityCheckInfo; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnit; +import org.mockito.junit.MockitoRule; + +public class IterativeConnectivityCheckerTest { + @Rule + public MockitoRule rule = MockitoJUnit.rule(); + + private ObjectId branchHeadObjectId; + + private ObjectId openRewiewObjectId; + + private ObjectId newCommitObjectId; + private ObjectId otherHaveObjectId = ObjectId + .fromString("DEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEF"); + + private Set advertisedHaves; + + @Mock + private ConnectivityChecker connectivityCheckerDelegate; + + @Mock + private ProgressMonitor pm; + + @Mock + private PackParser parser; + + private RevCommit branchHeadCommitObject; + private RevCommit openReviewCommitObject; + private RevCommit newCommitObject; + + private ConnectivityCheckInfo connectivityCheckInfo; + private IterativeConnectivityChecker connectivityChecker; + + private TestRepository tr; + + @Before + public void setUp() throws Exception { + tr = new TestRepository<>( + new InMemoryRepository(new DfsRepositoryDescription("test"))); + connectivityChecker = new IterativeConnectivityChecker( + connectivityCheckerDelegate); + connectivityCheckInfo = new ConnectivityCheckInfo(); + connectivityCheckInfo.setParser(parser); + connectivityCheckInfo.setRepository(tr.getRepository()); + connectivityCheckInfo.setWalk(tr.getRevWalk()); + + branchHeadCommitObject = tr.commit().create(); + branchHeadObjectId = branchHeadCommitObject.getId(); + + openReviewCommitObject = tr.commit().create(); + openRewiewObjectId = openReviewCommitObject.getId(); + + advertisedHaves = wrap(branchHeadObjectId, openRewiewObjectId, + otherHaveObjectId); + } + + @Test + public void testSuccessfulNewBranchBasedOnOld() throws Exception { + createNewCommit(branchHeadCommitObject); + connectivityCheckInfo.setCommands( + Collections.singletonList(createNewBrachCommand())); + + connectivityChecker.checkConnectivity(connectivityCheckInfo, + advertisedHaves, pm); + + verify(connectivityCheckerDelegate).checkConnectivity( + connectivityCheckInfo, + wrap(branchHeadObjectId /* as direct parent */), + pm); + } + + @Test + public void testSuccessfulNewBranchBasedOnOldWithTip() throws Exception { + createNewCommit(branchHeadCommitObject); + connectivityCheckInfo.setCommands( + Collections.singletonList(createNewBrachCommand())); + + connectivityChecker.setForcedHaves(wrap(openRewiewObjectId)); + + connectivityChecker.checkConnectivity(connectivityCheckInfo, + advertisedHaves, pm); + + verify(connectivityCheckerDelegate).checkConnectivity( + connectivityCheckInfo, + wrap(branchHeadObjectId /* as direct parent */, + openRewiewObjectId), + pm); + } + + @Test + public void testSuccessfulNewBranchMerge() throws Exception { + createNewCommit(branchHeadCommitObject, openReviewCommitObject); + connectivityCheckInfo.setCommands( + Collections.singletonList(createNewBrachCommand())); + + connectivityChecker.checkConnectivity(connectivityCheckInfo, + advertisedHaves, pm); + + verify(connectivityCheckerDelegate).checkConnectivity( + connectivityCheckInfo, + wrap(branchHeadObjectId /* as direct parent */, + openRewiewObjectId), + pm); + } + + @Test + public void testSuccessfulNewBranchBasedOnNewWithTip() throws Exception { + createNewCommit(); + connectivityCheckInfo.setCommands( + Collections.singletonList(createNewBrachCommand())); + + connectivityChecker.setForcedHaves(wrap(openRewiewObjectId)); + + connectivityChecker.checkConnectivity(connectivityCheckInfo, + advertisedHaves, pm); + + verify(connectivityCheckerDelegate).checkConnectivity( + connectivityCheckInfo, wrap(openRewiewObjectId), pm); + } + + @Test + public void testSuccessfulPushOldBranch() throws Exception { + createNewCommit(branchHeadCommitObject); + connectivityCheckInfo.setCommands( + Collections.singletonList(pushOldBranchCommand())); + + connectivityChecker.checkConnectivity(connectivityCheckInfo, + advertisedHaves, pm); + + verify(connectivityCheckerDelegate).checkConnectivity( + connectivityCheckInfo, wrap(branchHeadObjectId /* as direct parent */), + pm); + } + + @Test + public void testSuccessfulPushOldBranchMergeCommit() throws Exception { + createNewCommit(branchHeadCommitObject, openReviewCommitObject); + connectivityCheckInfo.setCommands( + Collections.singletonList(pushOldBranchCommand())); + + connectivityChecker.checkConnectivity(connectivityCheckInfo, + advertisedHaves, pm); + + verify(connectivityCheckerDelegate).checkConnectivity( + connectivityCheckInfo, + wrap(branchHeadObjectId /* as direct parent */, + openRewiewObjectId), + pm); + } + + + @Test + public void testNoChecksIfCantFindSubset() throws Exception { + createNewCommit(); + connectivityCheckInfo.setCommands( + Collections.singletonList(createNewBrachCommand())); + + connectivityChecker.checkConnectivity(connectivityCheckInfo, + advertisedHaves, pm); + + verify(connectivityCheckerDelegate) + .checkConnectivity(connectivityCheckInfo, advertisedHaves, pm); + } + + @Test + public void testReiterateInCaseNotSuccessful() throws Exception { + createNewCommit(branchHeadCommitObject); + connectivityCheckInfo.setCommands( + Collections.singletonList(createNewBrachCommand())); + + doThrow(new MissingObjectException(branchHeadCommitObject, + Constants.OBJ_COMMIT)).when(connectivityCheckerDelegate) + .checkConnectivity(connectivityCheckInfo, + wrap(branchHeadObjectId /* as direct parent */), pm); + + connectivityChecker.checkConnectivity(connectivityCheckInfo, + advertisedHaves, pm); + + verify(connectivityCheckerDelegate) + .checkConnectivity(connectivityCheckInfo, advertisedHaves, pm); + } + + @Test + public void testDependOnGrandparent() throws Exception { + RevCommit grandparent = tr.commit(new RevCommit[] {}); + RevCommit parent = tr.commit(grandparent); + createNewCommit(parent); + + branchHeadCommitObject = tr.commit(grandparent); + branchHeadObjectId = branchHeadCommitObject.getId(); + tr.getRevWalk().dispose(); + + connectivityCheckInfo.setCommands( + Collections.singletonList(createNewBrachCommand())); + + connectivityChecker.checkConnectivity(connectivityCheckInfo, + advertisedHaves, pm); + + verify(connectivityCheckerDelegate) + .checkConnectivity(connectivityCheckInfo, advertisedHaves, pm); + } + + private static Set wrap(ObjectId... objectIds) { + return new HashSet<>(Arrays.asList(objectIds)); + } + + private ReceiveCommand createNewBrachCommand() { + return new ReceiveCommand(ObjectId.zeroId(), newCommitObjectId, + "totally/a/new/branch"); + } + + private ReceiveCommand pushOldBranchCommand() { + return new ReceiveCommand(branchHeadObjectId, newCommitObjectId, + "push/to/an/old/branch"); + } + + private void createNewCommit(RevCommit... parents) throws Exception { + newCommitObject = tr.commit(parents); + newCommitObjectId = newCommitObject.getId(); + } + +} diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/connectivity/IterativeConnectivityChecker.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/connectivity/IterativeConnectivityChecker.java new file mode 100644 index 0000000000..b44c4ae5cb --- /dev/null +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/connectivity/IterativeConnectivityChecker.java @@ -0,0 +1,152 @@ +/* + * Copyright (c) 2019, Google LLC and others + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Distribution License v. 1.0 which is available at + * http://www.eclipse.org/org/documents/edl-v10.php. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +package org.eclipse.jgit.internal.transport.connectivity; + +import static java.util.stream.Collectors.toList; + +import java.io.IOException; +import java.util.ArrayDeque; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Queue; +import java.util.Set; +import java.util.stream.Stream; + +import org.eclipse.jgit.errors.MissingObjectException; +import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.ProgressMonitor; +import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevObject; +import org.eclipse.jgit.revwalk.RevWalk; +import org.eclipse.jgit.transport.ConnectivityChecker; +import org.eclipse.jgit.transport.ReceiveCommand; + +/** + * Implementation of connectivity checker which tries to do check with smaller + * set of references first and if it fails will fall back to check against all + * advertised references. + * + * This is useful for big repos with enormous number of references. + */ +public class IterativeConnectivityChecker implements ConnectivityChecker { + private static final int MAXIMUM_PARENTS_TO_CHECK = 128; + + private final ConnectivityChecker delegate; + + private Set forcedHaves = Collections.emptySet(); + + /** + * @param delegate + * Delegate checker which will be called for actual checks. + */ + public IterativeConnectivityChecker(ConnectivityChecker delegate) { + this.delegate = delegate; + } + + @Override + public void checkConnectivity(ConnectivityCheckInfo connectivityCheckInfo, + Set advertisedHaves, ProgressMonitor pm) + throws MissingObjectException, IOException { + try { + Set newRefs = new HashSet<>(); + Set expectedParents = new HashSet<>(); + + getAllObjectIds(connectivityCheckInfo.getCommands()) + .forEach(oid -> { + if (advertisedHaves.contains(oid)) { + expectedParents.add(oid); + } else { + newRefs.add(oid); + } + }); + if (!newRefs.isEmpty()) { + expectedParents.addAll(extractAdvertisedParentCommits(newRefs, + advertisedHaves, connectivityCheckInfo.getWalk())); + } + + expectedParents.addAll(forcedHaves); + + if (!expectedParents.isEmpty()) { + delegate.checkConnectivity(connectivityCheckInfo, + expectedParents, pm); + return; + } + } catch (MissingObjectException e) { + // This is fine, retry with all haves. + } + delegate.checkConnectivity(connectivityCheckInfo, advertisedHaves, pm); + } + + private static Stream getAllObjectIds( + List commands) { + return commands.stream().flatMap(cmd -> { + if (cmd.getType() == ReceiveCommand.Type.UPDATE || cmd + .getType() == ReceiveCommand.Type.UPDATE_NONFASTFORWARD) { + return Stream.of(cmd.getOldId(), cmd.getNewId()); + } else if (cmd.getType() == ReceiveCommand.Type.CREATE) { + return Stream.of(cmd.getNewId()); + } + return Stream.of(); + }); + } + + /** + * Sets additional haves that client can depend on (e.g. gerrit changes). + * + * @param forcedHaves + * Haves server expects client to depend on. + */ + public void setForcedHaves(Set forcedHaves) { + this.forcedHaves = Collections.unmodifiableSet(forcedHaves); + } + + private static Set extractAdvertisedParentCommits( + Set newRefs, Set advertisedHaves, RevWalk rw) + throws MissingObjectException, IOException { + Set advertisedParents = new HashSet<>(); + for (ObjectId newRef : newRefs) { + RevObject object = rw.parseAny(newRef); + if (object instanceof RevCommit) { + int numberOfParentsToCheck = 0; + Queue parents = new ArrayDeque<>( + MAXIMUM_PARENTS_TO_CHECK); + parents.addAll( + parseParents(((RevCommit) object).getParents(), rw)); + // Looking through a chain of ancestors handles the case where a + // series of commits is sent in a single push for a new branch. + while (!parents.isEmpty()) { + RevCommit parentCommit = parents.poll(); + if (advertisedHaves.contains(parentCommit.getId())) { + advertisedParents.add(parentCommit.getId()); + } else if (numberOfParentsToCheck < MAXIMUM_PARENTS_TO_CHECK) { + RevCommit[] grandParents = parentCommit.getParents(); + numberOfParentsToCheck += grandParents.length; + parents.addAll(parseParents(grandParents, rw)); + } + } + } + } + return advertisedParents; + } + + private static List parseParents(RevCommit[] parents, + RevWalk rw) { + return Arrays.stream(parents).map((commit) -> { + try { + return rw.parseCommit(commit); + } catch (Exception e) { + throw new RuntimeException(e); + } + }).collect(toList()); + } +} -- cgit v1.2.3