Skip to content

Commit

Permalink
fix regression (#8)
Browse files Browse the repository at this point in the history
  • Loading branch information
yoshuawuyts authored May 8, 2018
1 parent 39630e1 commit ebb5d7f
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 9 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,6 @@ random-access-storage = "0.3.0"
mkdirp = "0.1.0"

[dev-dependencies]
clippy = "0.0"
# clippy = "0.0"
quickcheck = "0.6.2"
tempdir = "0.3.7"
4 changes: 2 additions & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
#![cfg_attr(test, deny(warnings))]
#![feature(external_doc)]
#![doc(include = "../README.md")]
#![cfg_attr(test, feature(plugin))]
#![cfg_attr(test, plugin(clippy))]
// #![cfg_attr(test, feature(plugin))]
// #![cfg_attr(test, plugin(clippy))]

#[macro_use]
extern crate failure;
Expand Down
25 changes: 19 additions & 6 deletions src/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,15 @@ use failure::Error;
use std::fs;
use std::fs::OpenOptions;
use std::io::{Read, Seek, SeekFrom, Write};
use std::ops::Drop;
use std::path;

/// Main constructor.
pub struct Sync {}

impl Sync {
/// Create a new instance.
#[cfg_attr(test, allow(new_ret_no_self))]
// #[cfg_attr(test, allow(new_ret_no_self))]
pub fn new(filename: path::PathBuf) -> random_access::Sync<SyncMethods> {
random_access::Sync::new(SyncMethods {
filename,
Expand All @@ -26,6 +27,7 @@ impl Sync {
/// Methods that have been implemented to provide synchronous access to disk. .
/// These should generally be kept private, but exposed to prevent leaking
/// internals.
#[derive(Debug)]
pub struct SyncMethods {
filename: path::PathBuf,
file: Option<fs::File>,
Expand All @@ -37,12 +39,14 @@ impl random_access::SyncMethods for SyncMethods {
if let Some(dirname) = self.filename.parent() {
mkdirp::mkdirp(&dirname)?;
}

self.file = Some(OpenOptions::new()
.create_new(true)
let file = OpenOptions::new()
.create(true)
.read(true)
.write(true)
.open(&self.filename)?);
.open(&self.filename)?;
file.sync_all()?;

self.file = Some(file);

let metadata = fs::metadata(&self.filename)?;
self.length = metadata.len();
Expand All @@ -53,6 +57,7 @@ impl random_access::SyncMethods for SyncMethods {
let mut file = self.file.as_ref().expect("self.file was None.");
file.seek(SeekFrom::Start(offset as u64))?;
file.write_all(&data)?;
file.sync_all()?;

// We've changed the length of our file.
let new_len = (offset + data.len()) as u64;
Expand All @@ -70,7 +75,7 @@ impl random_access::SyncMethods for SyncMethods {
// whether it's okay to return a fully zero'd out slice. It's a bit weird,
// because we're replacing empty data with actual zeroes - which does not
// reflect the state of the world.
#[cfg_attr(test, allow(unused_io_amount))]
// #[cfg_attr(test, allow(unused_io_amount))]
fn read(&mut self, offset: usize, length: usize) -> Result<Vec<u8>, Error> {
ensure!(
(offset + length) as u64 <= self.length,
Expand All @@ -87,3 +92,11 @@ impl random_access::SyncMethods for SyncMethods {
panic!("Not implemented yet");
}
}

impl Drop for SyncMethods {
fn drop(&mut self) {
if let Some(file) = &self.file {
file.sync_all().unwrap();
}
}
}
13 changes: 13 additions & 0 deletions tests/sync_regression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ extern crate random_access_disk as rad;
extern crate tempdir;

use self::tempdir::TempDir;
use std::env;

#[test]
// postmortem: read_exact wasn't behaving like we hoped, so we had to switch
Expand All @@ -12,3 +13,15 @@ fn regress_1() {
file.write(27, b"").unwrap();
file.read(13, 5).unwrap();
}

#[test]
// postmortem: accessing the same file twice would fail, so we had to switch to
// from `.create_new()` to `.create()`.
//
// NOTE: test needs to be run twice in a row to trigger regression. I'm sorry.
fn regress_2() {
let mut dir = env::temp_dir();
dir.push("regression-2.db");
let mut file = rad::Sync::new(dir);
file.write(27, b"").unwrap();
}

0 comments on commit ebb5d7f

Please sign in to comment.