Skip to content

Commit

Permalink
Use libcurl's support for download resumption directly.
Browse files Browse the repository at this point in the history
Allows the removal of http test server, and simplifies test cases as we
can just read/write files on disk, like for existing dist tests
  • Loading branch information
jelford committed Mar 29, 2017
1 parent 7aa8b57 commit 1286686
Show file tree
Hide file tree
Showing 7 changed files with 73 additions and 244 deletions.
37 changes: 0 additions & 37 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion src/download/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ lazy_static = { version = "0.2", optional = true }
native-tls = { version = "0.1", optional = true }

[dev-dependencies]
rustup-mock = { path = "../rustup-mock", version = "1.0.0" }
tempdir = "0.3.4"

[dependencies.hyper]
Expand Down
15 changes: 12 additions & 3 deletions src/download/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,13 @@ pub fn download_to_path_with_backend(
0
};

let mut possible_partial = try!(OpenOptions::new().write(true).create(true).open(&path).chain_err(|| "error opening file for download"));
let mut possible_partial =
try!(OpenOptions::new()
.write(true)
.create(true)
.open(&path)
.chain_err(|| "error opening file for download"));

try!(possible_partial.seek(SeekFrom::End(0)));

(possible_partial, downloaded_so_far)
Expand Down Expand Up @@ -164,9 +170,12 @@ pub mod curl {
try!(handle.follow_location(true).chain_err(|| "failed to set follow redirects"));

if resume_from > 0 {
try!(handle.range(&(resume_from.to_string() + "-")).chain_err(|| "setting the range-header for download resumption"));
try!(handle.resume_from(resume_from)
.chain_err(|| "setting the range header for download resumption"));
} else {
try!(handle.range("").chain_err(|| "clearing range header"));
// an error here indicates that the range header isn't supported by underlying curl,
// so there's nothing to "clear" - safe to ignore this error.
let _ = handle.resume_from(0);
}

// Take at most 30s to connect
Expand Down
111 changes: 61 additions & 50 deletions src/download/tests/download-curl-resume.rs
Original file line number Diff line number Diff line change
@@ -1,25 +1,21 @@
#![cfg(feature = "curl-backend")]

extern crate download;
extern crate rustup_mock;
extern crate tempdir;
extern crate url;

use std::sync::{Arc, Mutex};
use std::fs::{self, File};
use std::io::{Read};
use std::io::{self, Read};
use std::path::Path;

use tempdir::TempDir;

use rustup_mock::http_server;
use url::Url;

use download::*;

fn setup(test: &Fn(TempDir, http_server::Server) -> ()) {
let tmp = TempDir::new("rustup-download-test-").expect("creating tempdir for test");
let served_dir = &tmp.path().join("test-files");
fs::DirBuilder::new().create(served_dir).expect("setting up a folder to server files from");
let server = http_server::Server::serve_from(served_dir).expect("setting up http server for test");
test(tmp, server);
fn tmp_dir() -> TempDir {
TempDir::new("rustup-download-test-").expect("creating tempdir for test")
}

fn file_contents(path: &Path) -> String {
Expand All @@ -28,60 +24,75 @@ fn file_contents(path: &Path) -> String {
result
}

#[test]
fn when_download_is_interrupted_partial_file_is_left_on_disk() {
setup(&|tmpdir, mut server| {
let target_path = tmpdir.path().join("downloaded");

server.put_file_from_bytes("test-file", b"12345");
pub fn write_file(path: &Path, contents: &str) {
let mut file = fs::OpenOptions::new()
.write(true)
.truncate(true)
.create(true)
.open(path)
.expect("writing test data");

server.stop_after_bytes(3);
download_to_path_with_backend(
Backend::Curl, &server.address().join("test-file").unwrap(), &target_path, true, None)
.expect("Test download failed");
io::Write::write_all(&mut file, contents.as_bytes()).expect("writing test data");

assert_eq!(file_contents(&target_path), "123");
});
file.sync_data().expect("writing test data");
}

#[test]
fn download_interrupted_and_resumed() {
setup(&|tmpdir, mut server| {
let target_path = tmpdir.path().join("downloaded");

server.put_file_from_bytes("test-file", b"12345");

server.stop_after_bytes(3);
download_to_path_with_backend(
Backend::Curl, &server.address().join("test-file").unwrap(), &target_path, true, None)
.expect("Test download failed");

server.stop_after_bytes(2);
download_to_path_with_backend(
Backend::Curl, &server.address().join("test-file").unwrap(), &target_path, true, None)
fn partially_downloaded_file_gets_resumed_from_byte_offset() {
let tmpdir = tmp_dir();
let from_path = tmpdir.path().join("download-source");
write_file(&from_path, "xxx45");

let target_path = tmpdir.path().join("downloaded");
write_file(&target_path, "123");

let from_url = Url::from_file_path(&from_path).unwrap();
download_to_path_with_backend(
Backend::Curl,
&from_url,
&target_path,
true,
None)
.expect("Test download failed");

assert_eq!(file_contents(&target_path), "12345");
});
assert_eq!(file_contents(&target_path), "12345");
}

#[test]
fn resuming_download_with_callback_that_needs_to_read_contents() {
setup(&|tmpdir, mut server| {
let target_path = tmpdir.path().join("downloaded");
fn callback_gets_all_data_as_if_the_download_happened_all_at_once() {
let tmpdir = tmp_dir();

server.put_file_from_bytes("test-file", b"12345");
let from_path = tmpdir.path().join("download-source");
write_file(&from_path, "xxx45");

let target_path = tmpdir.path().join("downloaded");
write_file(&target_path, "123");

let from_url = Url::from_file_path(&from_path).unwrap();

let received_in_callback = Arc::new(Mutex::new(Vec::new()));

download_to_path_with_backend(Backend::Curl,
&from_url,
&target_path,
true,
Some(&|msg| {
match msg {
Event::DownloadDataReceived(data) => {
for b in data.iter() {
received_in_callback.lock().unwrap().push(b.clone());
}
}
_ => {}
}

server.stop_after_bytes(3);
download_to_path_with_backend(
Backend::Curl, &server.address().join("test-file").unwrap(), &target_path, true, Some(&|_| {Ok(())}))
.expect("Test download failed");

server.stop_after_bytes(2);
download_to_path_with_backend(
Backend::Curl, &server.address().join("test-file").unwrap(), &target_path, true, Some(&|_| {Ok(())}))
Ok(())
}))
.expect("Test download failed");

assert_eq!(file_contents(&target_path), "12345");
});
let ref observed_bytes = *received_in_callback.lock().unwrap();
assert_eq!(observed_bytes, &vec![b'1', b'2', b'3', b'4', b'5']);
assert_eq!(file_contents(&target_path), "12345");
}
1 change: 0 additions & 1 deletion src/rustup-mock/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ toml = "0.1.27"
rustup-utils = { path = "../rustup-utils", version = "1.0.0" }
sha2 = "0.1.2"
wait-timeout = "0.1.3"
hyper = "0.10.5"

[target."cfg(windows)".dependencies]
winapi = "0.2.8"
Expand Down
Loading

0 comments on commit 1286686

Please sign in to comment.