tests and fixes for Writer and Syncer

* separate these out into a new file, writer.rs, as dir.rs was getting
  unwieldy.
* extract traits for the parts of SampleFileDir and std::fs::File they needed;
  set up mock implementations.
* move clock.rs to a new base crate to be accessible from the db crate.
* add tests that exercise all the retry paths.
* bugfix: account for the new recording's bytes when calculating how much to
  delete.
* bugfix: when retrying an unlink failure in collect_garbage, we shouldn't
  warn about all the recordings no longer existing. Do this by retrying each
  step rather than the whole procedure again.
* avoid double-panic scenarios, which I hit while tweaking the mocks. These
  are quite annoying to debug as Rust doesn't print information about either
  panic. I ended up using lldb to get a backtrace. Better to be cautious about
  what we're doing when already panicking.
* give more context on raw::insert_recording errors, which I hit as well while
  tweaking the new tests.

src/clock.rs

@@ -1,134 +0,0 @@
-// This file is part of Moonfire NVR, a security camera digital video recorder.
-// Copyright (C) 2016 Scott Lamb <slamb@slamb.org>
-//
-// This program is free software: you can redistribute it and/or modify
-// it under the terms of the GNU General Public License as published by
-// the Free Software Foundation, either version 3 of the License, or
-// (at your option) any later version.
-//
-// In addition, as a special exception, the copyright holders give
-// permission to link the code of portions of this program with the
-// OpenSSL library under certain conditions as described in each
-// individual source file, and distribute linked combinations including
-// the two.
-//
-// You must obey the GNU General Public License in all respects for all
-// of the code used other than OpenSSL. If you modify file(s) with this
-// exception, you may extend this exception to your version of the
-// file(s), but you are not obligated to do so. If you do not wish to do
-// so, delete this exception statement from your version. If you delete
-// this exception statement from all source files in the program, then
-// also delete it here.
-//
-// This program is distributed in the hope that it will be useful,
-// but WITHOUT ANY WARRANTY; without even the implied warranty of
-// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-// GNU General Public License for more details.
-//
-// You should have received a copy of the GNU General Public License
-// along with this program.  If not, see <http://www.gnu.org/licenses/>.
-
-//! Clock interface and implementations for testability.
-
-use libc;
-#[cfg(test)] use parking_lot::Mutex;
-use std::mem;
-use std::thread;
-use time::{Duration, Timespec};
-
-/// Abstract interface to the system clocks. This is for testability.
-pub trait Clocks : Sync {
-    /// Gets the current time from `CLOCK_REALTIME`.
-    fn realtime(&self) -> Timespec;
-
-    /// Gets the current time from `CLOCK_MONOTONIC`.
-    fn monotonic(&self) -> Timespec;
-
-    /// Causes the current thread to sleep for the specified time.
-    fn sleep(&self, how_long: Duration);
-}
-
-/// Singleton "real" clocks.
-pub static REAL: RealClocks = RealClocks {};
-
-/// Real clocks; see static `REAL` instance.
-pub struct RealClocks {}
-
-impl RealClocks {
-    fn get(&self, clock: libc::clockid_t) -> Timespec {
-        unsafe {
-            let mut ts = mem::uninitialized();
-            assert_eq!(0, libc::clock_gettime(clock, &mut ts));
-            Timespec::new(ts.tv_sec as i64, ts.tv_nsec as i32)
-        }
-    }
-}
-
-impl Clocks for RealClocks {
-    fn realtime(&self) -> Timespec { self.get(libc::CLOCK_REALTIME) }
-    fn monotonic(&self) -> Timespec { self.get(libc::CLOCK_MONOTONIC) }
-
-    fn sleep(&self, how_long: Duration) {
-        match how_long.to_std() {
-            Ok(d) => thread::sleep(d),
-            Err(e) => warn!("Invalid duration {:?}: {}", how_long, e),
-        };
-    }
-}
-
-/// Logs a warning if the TimerGuard lives "too long", using the label created by a supplied
-/// function.
-pub struct TimerGuard<'a, C: Clocks + 'a, S: AsRef<str>, F: FnOnce() -> S + 'a> {
-    clocks: &'a C,
-    label_f: Option<F>,
-    start: Timespec,
-}
-
-impl<'a, C: Clocks + 'a, S: AsRef<str>, F: FnOnce() -> S + 'a> TimerGuard<'a, C, S, F> {
-    pub fn new(clocks: &'a C, label_f: F) -> Self {
-        TimerGuard {
-            clocks,
-            label_f: Some(label_f),
-            start: clocks.monotonic(),
-        }
-    }
-}
-
-impl<'a, C: Clocks + 'a, S: AsRef<str>, F: FnOnce() -> S + 'a> Drop for TimerGuard<'a, C, S, F> {
-    fn drop(&mut self) {
-        let elapsed = self.clocks.monotonic() - self.start;
-        if elapsed.num_seconds() >= 1 {
-            let label_f = self.label_f.take().unwrap();
-            warn!("{} took {}!", label_f().as_ref(), elapsed);
-        }
-    }
-}
-
-/// Simulated clock for testing.
-#[cfg(test)]
-pub struct SimulatedClocks {
-    boot: Timespec,
-    uptime: Mutex<Duration>,
-}
-
-#[cfg(test)]
-impl SimulatedClocks {
-    pub fn new(boot: Timespec) -> SimulatedClocks {
-        SimulatedClocks {
-            boot: boot,
-            uptime: Mutex::new(Duration::seconds(0)),
-        }
-    }
-}
-
-#[cfg(test)]
-impl Clocks for SimulatedClocks {
-    fn realtime(&self) -> Timespec { self.boot + *self.uptime.lock() }
-    fn monotonic(&self) -> Timespec { Timespec::new(0, 0) + *self.uptime.lock() }
-
-    /// Advances the clock by the specified amount without actually sleeping.
-    fn sleep(&self, how_long: Duration) {
-        let mut l = self.uptime.lock();
-        *l = *l + how_long;
-    }
-}

src/cmds/config/cameras.rs

@@ -33,7 +33,7 @@ extern crate cursive;
 use self::cursive::Cursive;
 use self::cursive::traits::{Boxable, Identifiable, Finder};
 use self::cursive::views;
-use db::{self, dir};
+use db::{self, writer};
 use failure::Error;
 use std::collections::BTreeMap;
 use std::str::FromStr;
@@ -188,7 +188,7 @@ fn confirm_deletion(siv: &mut Cursive, db: &Arc<db::Database>, id: i32, to_delet
                         None => continue,
                     };
                     let l = zero_limits.entry(dir_id).or_insert_with(|| Vec::with_capacity(2));
-                    l.push(dir::NewLimit {
+                    l.push(writer::NewLimit {
                         stream_id,
                         limit: 0,
                     });
@@ -209,12 +209,12 @@ fn confirm_deletion(siv: &mut Cursive, db: &Arc<db::Database>, id: i32, to_delet
     }
 }
 
-fn lower_retention(db: &Arc<db::Database>, zero_limits: BTreeMap<i32, Vec<dir::NewLimit>>)
+fn lower_retention(db: &Arc<db::Database>, zero_limits: BTreeMap<i32, Vec<writer::NewLimit>>)
                    -> Result<(), Error> {
     let dirs_to_open: Vec<_> = zero_limits.keys().map(|id| *id).collect();
     db.lock().open_sample_file_dirs(&dirs_to_open[..])?;
     for (&dir_id, l) in &zero_limits {
-        dir::lower_retention(db.clone(), dir_id, &l)?;
+        writer::lower_retention(db.clone(), dir_id, &l)?;
     }
     Ok(())
 }

src/cmds/config/dirs.rs

@@ -33,7 +33,7 @@ extern crate cursive;
 use self::cursive::Cursive;
 use self::cursive::traits::{Boxable, Identifiable};
 use self::cursive::views;
-use db::{self, dir};
+use db::{self, writer};
 use failure::Error;
 use std::cell::RefCell;
 use std::collections::BTreeMap;
@@ -142,7 +142,7 @@ fn actually_delete(model: &RefCell<Model>, siv: &mut Cursive) {
     let model = &*model.borrow();
     let new_limits: Vec<_> =
         model.streams.iter()
-             .map(|(&id, s)| dir::NewLimit {stream_id: id, limit: s.retain.unwrap()})
+             .map(|(&id, s)| writer::NewLimit {stream_id: id, limit: s.retain.unwrap()})
              .collect();
     siv.pop_layer();  // deletion confirmation
     siv.pop_layer();  // retention dialog
@@ -150,7 +150,7 @@ fn actually_delete(model: &RefCell<Model>, siv: &mut Cursive) {
         let mut l = model.db.lock();
         l.open_sample_file_dirs(&[model.dir_id]).unwrap();  // TODO: don't unwrap.
     }
-    if let Err(e) = dir::lower_retention(model.db.clone(), model.dir_id, &new_limits[..]) {
+    if let Err(e) = writer::lower_retention(model.db.clone(), model.dir_id, &new_limits[..]) {
         siv.add_layer(views::Dialog::text(format!("Unable to delete excess video: {}", e))
                       .title("Error")
                       .dismiss_button("Abort"));

src/cmds/run.rs

@@ -29,7 +29,7 @@
 // along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 use clock;
-use db::{self, dir};
+use db::{self, dir, writer};
 use failure::Error;
 use fnv::FnvHashMap;
 use futures::{Future, Stream};
@@ -90,7 +90,7 @@ fn resolve_zone() -> String {
 
 struct Syncer {
     dir: Arc<dir::SampleFileDir>,
-    channel: dir::SyncerChannel,
+    channel: writer::SyncerChannel<::std::fs::File>,
     join: thread::JoinHandle<()>,
 }
 
@@ -122,7 +122,7 @@ pub fn run() -> Result<(), Error> {
         let streams = l.streams_by_id().len();
         let env = streamer::Environment {
             db: &db,
-            clocks: &clock::REAL,
+            clocks: &clock::RealClocks{},
             opener: &*stream::FFMPEG,
             shutdown: &shutdown_streamers,
         };
@@ -142,7 +142,7 @@ pub fn run() -> Result<(), Error> {
         drop(l);
         let mut syncers = FnvHashMap::with_capacity_and_hasher(dirs.len(), Default::default());
         for (id, dir) in dirs.drain() {
-            let (channel, join) = dir::start_syncer(db.clone(), id)?;
+            let (channel, join) = writer::start_syncer(db.clone(), id)?;
             syncers.insert(id, Syncer {
                 dir,
                 channel,

src/main.rs

@@ -46,6 +46,7 @@ extern crate reffers;
 extern crate rusqlite;
 extern crate memmap;
 extern crate mime;
+extern crate moonfire_base as base;
 extern crate moonfire_db as db;
 extern crate moonfire_ffmpeg;
 extern crate mylog;
@@ -62,7 +63,8 @@ extern crate tokio_signal;
 extern crate url;
 extern crate uuid;
 
-mod clock;
+use base::clock as clock;
+
 mod cmds;
 mod h264;
 mod json;

src/mp4.rs

@@ -1448,7 +1448,7 @@ impl FileInner {
         let f = self.dirs_by_stream_id
                     .get(&s.s.id.stream())
                     .ok_or_else(|| format_err!("{}: stream not found", s.s.id))?
-                    .open_sample_file(s.s.id)?;
+                    .open_file(s.s.id)?;
         let start = s.s.sample_file_range().start + r.start;
         let mmap = Box::new(unsafe {
             memmap::MmapOptions::new()
@@ -1520,6 +1520,7 @@ mod tests {
     use byteorder::{BigEndian, ByteOrder};
     use db::recording::{self, TIME_UNITS_PER_SEC};
     use db::testutil::{self, TestDb, TEST_STREAM_ID};
+    use db::writer;
     use futures::Future;
     use futures::Stream as FuturesStream;
     use hyper::header;
@@ -1755,8 +1756,9 @@ mod tests {
             extra_data.width, extra_data.height, extra_data.sample_entry,
             extra_data.rfc6381_codec).unwrap();
         let dir = db.dirs_by_stream_id.get(&TEST_STREAM_ID).unwrap();
-        let mut output = dir::Writer::new(dir, &db.db, &db.syncer_channel, TEST_STREAM_ID,
-                                          video_sample_entry_id);
+        let mut output = writer::Writer::new(&::base::clock::RealClocks{}, dir, &db.db,
+                                             &db.syncer_channel, TEST_STREAM_ID,
+                                             video_sample_entry_id);
 
         // end_pts is the pts of the end of the most recent frame (start + duration).
         // It's needed because dir::Writer calculates a packet's duration from its pts and the

src/streamer.rs

@@ -29,7 +29,7 @@
 // along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 use clock::{Clocks, TimerGuard};
-use db::{Camera, Database, Stream, dir, recording};
+use db::{Camera, Database, Stream, dir, recording, writer};
 use failure::Error;
 use h264;
 use std::result::Result;
@@ -48,7 +48,7 @@ pub struct Environment<'a, 'b, C, S> where C: 'a + Clocks, S: 'a + stream::Strea
     pub shutdown: &'b Arc<AtomicBool>,
 }
 
-pub struct Streamer<'a, C, S> where C: 'a + Clocks, S: 'a + stream::Stream {
+pub struct Streamer<'a, C, S> where C: Clocks, S: 'a + stream::Stream {
     shutdown: Arc<AtomicBool>,
 
     // State below is only used by the thread in Run.
@@ -56,7 +56,7 @@ pub struct Streamer<'a, C, S> where C: 'a + Clocks, S: 'a + stream::Stream {
     rotate_interval_sec: i64,
     db: Arc<Database>,
     dir: Arc<dir::SampleFileDir>,
-    syncer_channel: dir::SyncerChannel,
+    syncer_channel: writer::SyncerChannel<::std::fs::File>,
     clocks: &'a C,
     opener: &'a stream::Opener<S>,
     stream_id: i32,
@@ -67,7 +67,7 @@ pub struct Streamer<'a, C, S> where C: 'a + Clocks, S: 'a + stream::Stream {
 
 impl<'a, C, S> Streamer<'a, C, S> where C: 'a + Clocks, S: 'a + stream::Stream {
     pub fn new<'b>(env: &Environment<'a, 'b, C, S>, dir: Arc<dir::SampleFileDir>,
-                   syncer_channel: dir::SyncerChannel,
+                   syncer_channel: writer::SyncerChannel<::std::fs::File>,
                    stream_id: i32, c: &Camera, s: &Stream, rotate_offset_sec: i64,
                    rotate_interval_sec: i64) -> Self {
         Streamer {
@@ -121,8 +121,8 @@ impl<'a, C, S> Streamer<'a, C, S> where C: 'a + Clocks, S: 'a + stream::Stream {
         // Seconds since epoch at which to next rotate.
         let mut rotate: Option<i64> = None;
         let mut transformed = Vec::new();
-        let mut w = dir::Writer::new(&self.dir, &self.db, &self.syncer_channel, self.stream_id,
-                                     video_sample_entry_id);
+        let mut w = writer::Writer::new(self.clocks, &self.dir, &self.db, &self.syncer_channel,
+                                        self.stream_id, video_sample_entry_id);
         while !self.shutdown.load(Ordering::SeqCst) {
             let pkt = {
                 let _t = TimerGuard::new(self.clocks, || "getting next packet");