threads / patch / 18918

patchRefactor EclipseSshSessionFactory to extend SshConfigSessionFactory

Subject: [JGIT PATCH] Refactor EclipseSshSessionFactory to extend SshConfigSessionFactory

## tl;dr

One message between Apr 17, 2009 and Apr 17, 2009. Diffs are folded; open one to read it.

replies: 0people: 1as markdown or json

Shawn O. Pearce· Apr 17, 2009, 15:35 UTC · lore

Keeping the EclipseSshSessionFactory in sync with our OpenSSH compliant DefaultSshSessionFactory was a problem; e.g. a few commits ago we had support for StrictHostKeyChecking added, but it was missed in the fork we use under Eclipse.

Constantine Plotnikov's recent patch pointed out we should clean up the code and make EclipseSshSessionFactory use the same code as the other JGit clients use. This makes it possible by refactoring the way we create the Session and obtain a handle on the JSch.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../spearce/egit/ui/EclipseSshSessionFactory.java  |   86 +++-----------------
 .../jgit/transport/SshConfigSessionFactory.java    |   34 +++++++-
 2 files changed, 42 insertions(+), 78 deletions(-)
Show changes to 2 files +42 −78

org.spearce.egit.ui/src/org/spearce/egit/ui/EclipseSshSessionFactory.java, org.spearce.jgit/src/org/spearce/jgit/transport/SshConfigSessionFactory.java

diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/EclipseSshSessionFactory.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/EclipseSshSessionFactory.java
index 098d234..2cafdfe 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/EclipseSshSessionFactory.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/EclipseSshSessionFactory.java
@@ -8,96 +8,36 @@
  *******************************************************************************/
 package org.spearce.egit.ui;
 
-import java.io.File;
-import java.io.IOException;
-import java.io.OutputStream;
-import java.util.HashSet;
-import java.util.Set;
-
 import org.eclipse.jsch.core.IJSchService;
 import org.eclipse.jsch.ui.UserInfoPrompter;
-import org.spearce.jgit.transport.OpenSshConfig;
-import org.spearce.jgit.transport.SshSessionFactory;
+import org.spearce.jgit.transport.SshConfigSessionFactory;
+import org.spearce.jgit.transport.OpenSshConfig.Host;
 
+import com.jcraft.jsch.JSch;
 import com.jcraft.jsch.JSchException;
 import com.jcraft.jsch.Session;
 
-class EclipseSshSessionFactory extends SshSessionFactory {
+class EclipseSshSessionFactory extends SshConfigSessionFactory {
 	private final IJSchService provider;
 
-	private final Set<String> loadedIdentities = new HashSet<String>();
-
-	private OpenSshConfig config;
-
 	EclipseSshSessionFactory(final IJSchService p) {
 		provider = p;
 	}
 
 	@Override
-	public Session getSession(String user, String pass, String host, int port)
-			throws JSchException {
-		final OpenSshConfig.Host hc = getConfig().lookup(host);
-		host = hc.getHostName();
-		if (port <= 0)
-			port = hc.getPort();
-		if (user == null)
-			user = hc.getUser();
-
-		final Session session = provider.createSession(host, port, user);
-		if (hc.getIdentityFile() != null)
-			addIdentity(hc.getIdentityFile());
-		if (pass != null)
-			session.setPassword(pass);
-		else if (!hc.isBatchMode())
-			new UserInfoPrompter(session);
-
-		final String pauth = hc.getPreferredAuthentications();
-		if (pauth != null)
-			session.setConfig("PreferredAuthentications", pauth);
-		return session;
-	}
-
-	private synchronized OpenSshConfig getConfig() {
-		if (config == null)
-			config = OpenSshConfig.get();
-		return config;
+	protected Session createSession(final String user, final String host,
+			final int port) throws JSchException {
+		return provider.createSession(host, port, user);
 	}
 
-	private void addIdentity(final File identityFile)
-			throws JSchException {
-		final String path = identityFile.getAbsolutePath();
-		if (loadedIdentities.add(path))
-			provider.getJSch().addIdentity(path);
+	@Override
+	protected void configure(final Host hc, final Session session) {
+		if (!hc.isBatchMode())
+			new UserInfoPrompter(session);
 	}
 
 	@Override
-	public OutputStream getErrorStream() {
-		return new OutputStream() {
-
-			StringBuilder all = new StringBuilder();
-
-			StringBuilder sb = new StringBuilder();
-
-			public String toString() {
-				String r = all.toString();
-				while (r.endsWith("\n"))
-					r = r.substring(0, r.length() - 1);
-				return r;
-			}
-
-			@Override
-			public void write(int b) throws IOException {
-				if (b == '\r')
-					return;
-				sb.append((char) b);
-				if (b == '\n') {
-					String s = sb.toString();
-					all.append(s);
-					sb = new StringBuilder();
-					Activator.logError(s, new Throwable());
-				}
-			}
-		};
+	protected JSch getUserJSch() throws JSchException {
+		return provider.getJSch();
 	}
-
 }
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/SshConfigSessionFactory.java b/org.spearce.jgit/src/org/spearce/jgit/transport/SshConfigSessionFactory.java
index ec91494..f929e6b 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/SshConfigSessionFactory.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/SshConfigSessionFactory.java
@@ -66,7 +66,7 @@
  * to supply appropriate {@link UserInfo} to the session.
  */
 public abstract class SshConfigSessionFactory extends SshSessionFactory {
-	private Set<String> loadedIdentities;
+	private final Set<String> loadedIdentities = new HashSet<String>();
 
 	private JSch userJSch;
 
@@ -82,7 +82,7 @@ public synchronized Session getSession(String user, String pass,
 		if (user == null)
 			user = hc.getUser();
 
-		final Session session = getUserJSch().getSession(user, host, port);
+		final Session session = createSession(user, host, port);
 		if (hc.getIdentityFile() != null)
 			addIdentity(hc.getIdentityFile());
 		if (pass != null)
@@ -100,6 +100,24 @@ public synchronized Session getSession(String user, String pass,
 	}
 
 	/**
+	 * Create a new JSch session for the requested address.
+	 *
+	 * @param user
+	 *            login to authenticate as.
+	 * @param host
+	 *            server name to connect to.
+	 * @param port
+	 *            port number of the SSH daemon (typically 22).
+	 * @return new session instance, but otherwise unconfigured.
+	 * @throws JSchException
+	 *             the session could not be created.
+	 */
+	protected Session createSession(String user, String host, int port)
+			throws JSchException {
+		return getUserJSch().getSession(user, host, port);
+	}
+
+	/**
 	 * Provide additional configuration for the session based on the host
 	 * information. This method could be used to supply {@link UserInfo}.
 	 *
@@ -110,9 +128,15 @@ public synchronized Session getSession(String user, String pass,
 	 */
 	protected abstract void configure(OpenSshConfig.Host hc, Session session);
 
-	private JSch getUserJSch() throws JSchException {
+	/**
+	 * Obtain the JSch used to create new sessions.
+	 *
+	 * @return the JSch instance to use.
+	 * @throws JSchException
+	 *             the user configuration could not be created.
+	 */
+	protected JSch getUserJSch() throws JSchException {
 		if (userJSch == null) {
-			loadedIdentities = new HashSet<String>();
 			userJSch = new JSch();
 			knownHosts(userJSch);
 			identities();
@@ -173,7 +197,7 @@ private void identities() {
 	private void addIdentity(final File identityFile) throws JSchException {
 		final String path = identityFile.getAbsolutePath();
 		if (!loadedIdentities.contains(path)) {
-			userJSch.addIdentity(path);
+			getUserJSch().addIdentity(path);
 			loadedIdentities.add(path);
 		}
 	}
-- 
1.6.3.rc0.200.g4086

← back to recent threads