-
Notifications
You must be signed in to change notification settings - Fork 602
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SCP : limit the used bandwidth #208
Changes from 2 commits
782ff9b
e81fdb8
dec00ef
9c424f9
84990ad
d18e9d9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
package net.schmizz.sshj.xfer.scp; | ||
|
||
abstract class AbstractSCPClient { | ||
|
||
protected final SCPEngine engine; | ||
protected int bandwidthLimit; | ||
|
||
AbstractSCPClient(SCPEngine engine) { | ||
this.engine = engine; | ||
} | ||
|
||
AbstractSCPClient(SCPEngine engine, int bandwidthLimit) { | ||
this.engine = engine; | ||
this.bandwidthLimit = bandwidthLimit; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,18 +28,23 @@ public class SCPFileTransfer | |
extends AbstractFileTransfer | ||
implements FileTransfer { | ||
|
||
/** Default bandwidth limit for SCP transfert in Kbit/s (-1 means unlimited) */ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. typo in comment (transfert). Also is it kilobit, or kilobyte? The difference is a factor 8. |
||
private static final int DEFAULT_BANDWIDTH_LIMIT = -1; | ||
|
||
private final SessionFactory sessionFactory; | ||
private int bandwidthLimit; | ||
|
||
public SCPFileTransfer(SessionFactory sessionFactory) { | ||
this.sessionFactory = sessionFactory; | ||
this.bandwidthLimit = DEFAULT_BANDWIDTH_LIMIT; | ||
} | ||
|
||
public SCPDownloadClient newSCPDownloadClient() { | ||
return new SCPDownloadClient(newSCPEngine()); | ||
return new SCPDownloadClient(newSCPEngine(), bandwidthLimit); | ||
} | ||
|
||
public SCPUploadClient newSCPUploadClient() { | ||
return new SCPUploadClient(newSCPEngine()); | ||
return new SCPUploadClient(newSCPEngine(), bandwidthLimit); | ||
} | ||
|
||
private SCPEngine newSCPEngine() { | ||
|
@@ -70,4 +75,10 @@ public void upload(LocalSourceFile localFile, String remotePath) | |
newSCPUploadClient().copy(localFile, remotePath); | ||
} | ||
|
||
public SCPFileTransfer bandwidthLimit(int limit) { | ||
if (limit > 0) { | ||
this.bandwidthLimit = limit; | ||
} | ||
return this; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,82 @@ | ||
package net.schmizz.sshj.xfer.scp; | ||
|
||
import com.hierynomus.sshj.test.SshFixture; | ||
import com.hierynomus.sshj.test.util.FileUtil; | ||
import net.schmizz.sshj.SSHClient; | ||
import org.junit.After; | ||
import org.junit.Before; | ||
import org.junit.Rule; | ||
import org.junit.Test; | ||
import org.junit.rules.TemporaryFolder; | ||
|
||
import java.io.File; | ||
import java.io.IOException; | ||
|
||
import static junit.framework.Assert.assertFalse; | ||
import static junit.framework.Assert.assertTrue; | ||
|
||
public class SCPFileTransferTest { | ||
|
||
public static final String DEFAULT_FILE_NAME = "my_file.txt"; | ||
File targetDir; | ||
File sourceFile; | ||
File targetFile; | ||
SSHClient sshClient; | ||
|
||
@Rule | ||
public SshFixture fixture = new SshFixture(); | ||
|
||
@Rule | ||
public TemporaryFolder tempFolder = new TemporaryFolder(); | ||
|
||
@Before | ||
public void init() throws IOException { | ||
sourceFile = tempFolder.newFile(DEFAULT_FILE_NAME); | ||
FileUtil.writeToFile(sourceFile, "This is my file"); | ||
targetDir = tempFolder.newFolder(); | ||
targetFile = new File(targetDir + File.separator + DEFAULT_FILE_NAME); | ||
sshClient = fixture.setupConnectedDefaultClient(); | ||
sshClient.authPassword("test", "test"); | ||
} | ||
|
||
@After | ||
public void cleanup() { | ||
if (targetFile.exists()) { | ||
targetFile.delete(); | ||
} | ||
} | ||
|
||
@Test | ||
public void should_SCP_Upload_File() throws IOException { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like the tests, but could you name them without the underscores and with just camelcasing? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
SCPFileTransfer scpFileTransfer = sshClient.newSCPFileTransfer(); | ||
assertFalse(targetFile.exists()); | ||
scpFileTransfer.upload(sourceFile.getAbsolutePath(), targetDir.getAbsolutePath()); | ||
assertTrue(targetFile.exists()); | ||
} | ||
|
||
@Test | ||
public void should_SCP_Upload_File_With_Bandwidth_Limit() throws IOException { | ||
// Limit upload transfert at 2Mo/s | ||
SCPFileTransfer scpFileTransfer = sshClient.newSCPFileTransfer().bandwidthLimit(16000); | ||
assertFalse(targetFile.exists()); | ||
scpFileTransfer.upload(sourceFile.getAbsolutePath(), targetDir.getAbsolutePath()); | ||
assertTrue(targetFile.exists()); | ||
} | ||
|
||
@Test | ||
public void should_SCP_Download_File() throws IOException { | ||
SCPFileTransfer scpFileTransfer = sshClient.newSCPFileTransfer(); | ||
assertFalse(targetFile.exists()); | ||
scpFileTransfer.download(sourceFile.getAbsolutePath(), targetDir.getAbsolutePath()); | ||
assertTrue(targetFile.exists()); | ||
} | ||
|
||
@Test | ||
public void should_SCP_Download_File_With_Bandwidth_Limit() throws IOException { | ||
// Limit download transfert at 128Ko/s | ||
SCPFileTransfer scpFileTransfer = sshClient.newSCPFileTransfer().bandwidthLimit(1024); | ||
assertFalse(targetFile.exists()); | ||
scpFileTransfer.download(sourceFile.getAbsolutePath(), targetDir.getAbsolutePath()); | ||
assertTrue(targetFile.exists()); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the extra 'v' here? It is not used currently. The
LIMIT
seems to use it, but as it is an enum it is always the empty string.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some scp options needs a key/value structure. The value is initialized with an empty string but can be set through the helper class SCPArguments. This the way I have used to set the value of the LIMIT arg.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh but this is even more evil, as this is an enum, setting the value for 1 process means that you're setting it (inadvertently) for all processes. Because there is only 1 instance of the same enum key in a JVM. So please refactor that and remove the
setValue(..)