From 0a729184e47fa1c4e3322e0a9918b3b5310b3278 Mon Sep 17 00:00:00 2001 From: Antoine Gersant Date: Sat, 3 Dec 2016 13:42:06 -0800 Subject: [PATCH] Proper error handling in file indexing No longer unwrap like there's no tomorrow --- src/errors.rs | 2 + src/index.rs | 275 +++++++++++++++++++++++++------------------------- 2 files changed, 141 insertions(+), 136 deletions(-) diff --git a/src/errors.rs b/src/errors.rs index 1f32a57..ff99ff4 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -8,6 +8,7 @@ use iron::status::Status; use lewton; use metaflac; use regex; +use sqlite; use std; error_chain! { @@ -20,6 +21,7 @@ error_chain! { Image(image::ImageError); Io(std::io::Error); Regex(regex::Error); + SQLite(sqlite::Error); Vorbis(lewton::VorbisError); } diff --git a/src/index.rs b/src/index.rs index 0ad81bf..d28279b 100644 --- a/src/index.rs +++ b/src/index.rs @@ -95,22 +95,20 @@ struct IndexBuilder<'db> { } impl<'db> IndexBuilder<'db> { - fn new(db: &Connection) -> IndexBuilder { + fn new(db: &Connection) -> Result { let mut queue = Vec::new(); queue.reserve_exact(INDEX_BUILDING_INSERT_BUFFER_SIZE); - IndexBuilder { + Ok(IndexBuilder { queue: queue, db: db, insert_directory: db.prepare("INSERT OR REPLACE INTO directories (path, parent, artwork, year, \ - artist, album) VALUES (?, ?, ?, ?, ?, ?)") - .unwrap(), + artist, album) VALUES (?, ?, ?, ?, ?, ?)")?, insert_song: db.prepare("INSERT OR REPLACE INTO songs (path, parent, disc_number, track_number, \ title, year, album_artist, artist, album, artwork) VALUES (?, ?, ?, ?, \ - ?, ?, ?, ?, ?, ?)") - .unwrap(), - } + ?, ?, ?, ?, ?, ?)")?, + }) } fn get_parent(path: &str) -> Option { @@ -123,63 +121,56 @@ impl<'db> IndexBuilder<'db> { None } - fn flush(&mut self) { - self.db.execute("BEGIN TRANSACTION").ok(); + fn flush(&mut self) -> Result<()> { + self.db.execute("BEGIN TRANSACTION")?; while let Some(file) = self.queue.pop() { match file { // Insert directory CollectionFile::Directory(directory) => { let parent = IndexBuilder::get_parent(directory.path.as_str()); - self.insert_directory.reset().ok(); - self.insert_directory.bind(1, &Value::String(directory.path)).unwrap(); - self.insert_directory.bind(2, &string_option_to_value(parent)).unwrap(); + self.insert_directory.reset()?; + self.insert_directory.bind(1, &Value::String(directory.path))?; + self.insert_directory.bind(2, &string_option_to_value(parent))?; self.insert_directory - .bind(3, &string_option_to_value(directory.artwork)) - .unwrap(); - self.insert_directory.bind(4, &i32_option_to_value(directory.year)).unwrap(); + .bind(3, &string_option_to_value(directory.artwork))?; + self.insert_directory.bind(4, &i32_option_to_value(directory.year))?; self.insert_directory - .bind(5, &string_option_to_value(directory.artist)) - .unwrap(); + .bind(5, &string_option_to_value(directory.artist))?; self.insert_directory - .bind(6, &string_option_to_value(directory.album)) - .unwrap(); - self.insert_directory.next().ok(); + .bind(6, &string_option_to_value(directory.album))?; + self.insert_directory.next()?; } // Insert song CollectionFile::Song(song) => { let parent = IndexBuilder::get_parent(song.path.as_str()); - self.insert_song.reset().ok(); - self.insert_song.bind(1, &Value::String(song.path)).unwrap(); - self.insert_song.bind(2, &string_option_to_value(parent)).unwrap(); - self.insert_song.bind(3, &u32_option_to_value(song.disc_number)).unwrap(); - self.insert_song.bind(4, &u32_option_to_value(song.track_number)).unwrap(); - self.insert_song.bind(5, &string_option_to_value(song.title)).unwrap(); - self.insert_song.bind(6, &i32_option_to_value(song.year)).unwrap(); - self.insert_song.bind(7, &string_option_to_value(song.album_artist)).unwrap(); - self.insert_song.bind(8, &string_option_to_value(song.artist)).unwrap(); - self.insert_song.bind(9, &string_option_to_value(song.album)).unwrap(); - self.insert_song.bind(10, &string_option_to_value(song.artwork)).unwrap(); - self.insert_song.next().ok(); + self.insert_song.reset()?; + self.insert_song.bind(1, &Value::String(song.path))?; + self.insert_song.bind(2, &string_option_to_value(parent))?; + self.insert_song.bind(3, &u32_option_to_value(song.disc_number))?; + self.insert_song.bind(4, &u32_option_to_value(song.track_number))?; + self.insert_song.bind(5, &string_option_to_value(song.title))?; + self.insert_song.bind(6, &i32_option_to_value(song.year))?; + self.insert_song.bind(7, &string_option_to_value(song.album_artist))?; + self.insert_song.bind(8, &string_option_to_value(song.artist))?; + self.insert_song.bind(9, &string_option_to_value(song.album))?; + self.insert_song.bind(10, &string_option_to_value(song.artwork))?; + self.insert_song.next()?; } } } - self.db.execute("END TRANSACTION").ok(); + self.db.execute("END TRANSACTION")?; + Ok(()) } - fn push(&mut self, file: CollectionFile) { + fn push(&mut self, file: CollectionFile) -> Result<()> { if self.queue.len() == self.queue.capacity() { - self.flush(); + self.flush()?; } self.queue.push(file); - } -} - -impl<'db> Drop for IndexBuilder<'db> { - fn drop(&mut self) { - self.flush(); + Ok(()) } } @@ -200,18 +191,18 @@ impl Index { if path.exists() { // Migration } else { - index.init(); + index.init()?; } Ok(index) } - fn init(&self) { + fn init(&self) -> Result<()> { println!("Initializing index database"); - let db = self.connect(); - db.execute("PRAGMA synchronous = NORMAL").unwrap(); + let db = self.connect()?; + db.execute("PRAGMA synchronous = NORMAL")?; db.execute(" CREATE TABLE version @@ -254,62 +245,68 @@ impl Index { , UNIQUE(path) ); - ") - .unwrap(); + ")?; + + Ok(()) } - fn connect(&self) -> Connection { - let mut db = sqlite::open(self.path.clone()).unwrap(); - db.set_busy_timeout(INDEX_LOCK_TIMEOUT).ok(); - db + fn connect(&self) -> Result { + let mut db = sqlite::open(self.path.clone())?; + db.set_busy_timeout(INDEX_LOCK_TIMEOUT)?; + Ok(db) } - fn update_index(&self, db: &Connection) { + fn update_index(&self, db: &Connection) -> Result<()> { let start = time::Instant::now(); println!("Beginning library index update"); - self.clean(db); - self.populate(db); + self.clean(db)?; + self.populate(db)?; println!("Library index update took {} seconds", start.elapsed().as_secs()); + Ok(()) } - fn clean(&self, db: &Connection) { + fn clean(&self, db: &Connection) -> Result<()> { { - let mut select = db.prepare("SELECT path FROM songs").unwrap(); - let mut delete = db.prepare("DELETE FROM songs WHERE path = ?").unwrap(); - while let State::Row = select.next().unwrap() { - let path_string: String = select.read(0).unwrap(); + let mut select = db.prepare("SELECT path FROM songs")?; + let mut delete = db.prepare("DELETE FROM songs WHERE path = ?")?; + while let Ok(State::Row) = select.next() { + let path_string: String = select.read(0)?; let path = Path::new(path_string.as_str()); if !path.exists() || self.vfs.real_to_virtual(path).is_err() { - delete.reset().ok(); - delete.bind(1, &Value::String(path_string.to_owned())).ok(); - delete.next().ok(); + delete.reset()?; + delete.bind(1, &Value::String(path_string.to_owned()))?; + delete.next()?; } } } { - let mut select = db.prepare("SELECT path FROM directories").unwrap(); - let mut delete = db.prepare("DELETE FROM directories WHERE path = ?").unwrap(); - while let State::Row = select.next().unwrap() { - let path_string: String = select.read(0).unwrap(); + let mut select = db.prepare("SELECT path FROM directories")?; + let mut delete = db.prepare("DELETE FROM directories WHERE path = ?")?; + while let Ok(State::Row) = select.next() { + let path_string: String = select.read(0)?; let path = Path::new(path_string.as_str()); if !path.exists() || self.vfs.real_to_virtual(path).is_err() { - delete.reset().ok(); - delete.bind(1, &Value::String(path_string.to_owned())).ok(); - delete.next().ok(); + delete.reset()?; + delete.bind(1, &Value::String(path_string.to_owned()))?; + delete.next()?; } } } + + Ok(()) } - fn populate(&self, db: &Connection) { + fn populate(&self, db: &Connection) -> Result<()> { let vfs = self.vfs.deref(); let mount_points = vfs.get_mount_points(); - let mut builder = IndexBuilder::new(db); + let mut builder = IndexBuilder::new(db)?; for (_, target) in mount_points { - self.populate_directory(&mut builder, target.as_path()); + self.populate_directory(&mut builder, target.as_path())?; } + builder.flush()?; + Ok(()) } fn get_artwork(&self, dir: &Path) -> Option { @@ -333,15 +330,12 @@ impl Index { None } - fn populate_directory(&self, builder: &mut IndexBuilder, path: &Path) { + fn populate_directory(&self, builder: &mut IndexBuilder, path: &Path) -> Result<()> { // Find artwork let artwork = self.get_artwork(path); - let path_string = match path.to_str() { - Some(p) => p, - _ => return, - }; + let path_string = path.to_str().ok_or("Invalid directory path")?; let mut directory_album = None; let mut directory_year = None; @@ -359,7 +353,7 @@ impl Index { }; if file_path.is_dir() { - self.populate_directory(builder, file_path.as_path()); + self.populate_directory(builder, file_path.as_path())?; } else { if let Some(file_path_string) = file_path.to_str() { if let Ok(tags) = metadata::read(file_path.as_path()) { @@ -372,19 +366,18 @@ impl Index { if tags.album.is_some() { inconsistent_directory_album |= directory_album.is_some() && directory_album != tags.album; - directory_album = Some(tags.album.as_ref().unwrap().clone()); + directory_album = tags.album.as_ref().map(|a| a.clone()); } if tags.album_artist.is_some() { inconsistent_directory_artist |= directory_artist.is_some() && directory_artist != tags.album_artist; - directory_artist = - Some(tags.album_artist.as_ref().unwrap().clone()); + directory_artist = tags.album_artist.as_ref().map(|a| a.clone()); } else if tags.artist.is_some() { inconsistent_directory_artist |= directory_artist.is_some() && directory_artist != tags.artist; - directory_artist = Some(tags.artist.as_ref().unwrap().clone()); + directory_artist = tags.artist.as_ref().map(|a| a.clone()); } let song = Song { @@ -399,7 +392,7 @@ impl Index { artwork: artwork.as_ref().map(|s| s.to_owned()), }; - builder.push(CollectionFile::Song(song)); + builder.push(CollectionFile::Song(song))?; } } } @@ -424,47 +417,55 @@ impl Index { artist: directory_artist, year: directory_year, }; - builder.push(CollectionFile::Directory(directory)); + builder.push(CollectionFile::Directory(directory))?; + + Ok(()) } pub fn run(&self) { loop { - { - let db = self.connect(); - self.update_index(&db); + if let Ok(db) = self.connect() { + if let Err(e) = self.update_index(&db) { + println!("Error while updating index: {}", e); + } } thread::sleep(time::Duration::from_secs(self.sleep_duration)); } } - fn select_songs(&self, select: &mut Statement) -> Vec { + fn select_songs(&self, select: &mut Statement) -> Result> { let mut output = Vec::new(); - while let State::Row = select.next().unwrap() { + while let State::Row = select.next()? { - let song_path: String = select.read(0).unwrap(); - let disc_number: Value = select.read(1).unwrap(); - let track_number: Value = select.read(2).unwrap(); - let title: Value = select.read(3).unwrap(); - let year: Value = select.read(4).unwrap(); - let album_artist: Value = select.read(5).unwrap(); - let artist: Value = select.read(6).unwrap(); - let album: Value = select.read(7).unwrap(); - let artwork: Value = select.read(8).unwrap(); + let song_path: String = select.read(0)?; + let disc_number: Value = select.read(1)?; + let track_number: Value = select.read(2)?; + let title: Value = select.read(3)?; + let year: Value = select.read(4)?; + let album_artist: Value = select.read(5)?; + let artist: Value = select.read(6)?; + let album: Value = select.read(7)?; + let artwork: Value = select.read(8)?; let song_path = Path::new(song_path.as_str()); let song_path = match self.vfs.real_to_virtual(song_path) { - Ok(p) => p, + Ok(p) => p.to_str().ok_or("Invalid song path")?.to_owned(), _ => continue, }; - let artwork = artwork.as_string() - .map(|p| Path::new(p)) - .and_then(|p| self.vfs.real_to_virtual(p).ok()); + let artwork_path = artwork.as_string().map(|p| Path::new(p)); + let mut artwork = None; + if let Some(artwork_path) = artwork_path { + artwork = match self.vfs.real_to_virtual(artwork_path) { + Ok(p) => Some(p.to_str().ok_or("Invalid song artwork path")?.to_owned()), + _ => None, + }; + } let song = Song { - path: song_path.to_str().unwrap().to_owned(), + path: song_path, disc_number: disc_number.as_integer().map(|n| n as u32), track_number: track_number.as_integer().map(|n| n as u32), title: title.as_string().map(|s| s.to_owned()), @@ -472,47 +473,51 @@ impl Index { album_artist: album_artist.as_string().map(|s| s.to_owned()), artist: artist.as_string().map(|s| s.to_owned()), album: album.as_string().map(|s| s.to_owned()), - artwork: artwork.map(|p| p.to_str().unwrap().to_owned()), + artwork: artwork, }; output.push(song); } - output + Ok(output) } // List sub-directories within a directory - fn browse_directories(&self, real_path: &Path) -> Vec { - let db = self.connect(); + fn browse_directories(&self, real_path: &Path) -> Result> { + let db = self.connect()?; let mut output = Vec::new(); let path_string = real_path.to_string_lossy(); let mut select = db.prepare("SELECT path, artwork, year, artist, album FROM directories WHERE \ - parent = ? ORDER BY path COLLATE NOCASE ASC") - .unwrap(); - select.bind(1, &Value::String(path_string.deref().to_owned())).unwrap(); + parent = ? ORDER BY path COLLATE NOCASE ASC")?; + select.bind(1, &Value::String(path_string.deref().to_owned()))?; - while let State::Row = select.next().unwrap() { + while let State::Row = select.next()? { - let directory_value: String = select.read(0).unwrap(); - let artwork_path: Value = select.read(1).unwrap(); - let year: Value = select.read(2).unwrap(); - let artist: Value = select.read(3).unwrap(); - let album: Value = select.read(4).unwrap(); + let directory_value: String = select.read(0)?; + let artwork_path: Value = select.read(1)?; + let year: Value = select.read(2)?; + let artist: Value = select.read(3)?; + let album: Value = select.read(4)?; let directory_path = Path::new(directory_value.as_str()); let directory_path = match self.vfs.real_to_virtual(directory_path) { - Ok(p) => p, + Ok(p) => p.to_str().ok_or("Invalid directory path")?.to_owned(), _ => continue, }; - let artwork_path = artwork_path.as_string() - .map(|p| Path::new(p)) - .and_then(|p| self.vfs.real_to_virtual(p).ok()); + let artwork_path = artwork_path.as_string().map(|p| Path::new(p)); + let mut artwork = None; + if let Some(artwork_path) = artwork_path { + artwork = match self.vfs.real_to_virtual(artwork_path) { + Ok(p) => Some(p.to_str().ok_or("Invalid directory artwork path")?.to_owned()), + _ => None, + }; + } let directory = Directory { - path: directory_path.to_str().unwrap().to_owned(), - artwork: artwork_path.map(|p| p.to_str().unwrap().to_owned()), + path: directory_path, + artwork: artwork, year: year.as_integer().map(|n| n as i32), artist: artist.as_string().map(|s| s.to_owned()), album: album.as_string().map(|s| s.to_owned()), @@ -520,20 +525,19 @@ impl Index { output.push(CollectionFile::Directory(directory)); } - output + Ok(output) } // List songs within a directory - fn browse_songs(&self, real_path: &Path) -> Vec { - let db = self.connect(); + fn browse_songs(&self, real_path: &Path) -> Result> { + let db = self.connect()?; let path_string = real_path.to_string_lossy(); let mut select = db.prepare("SELECT path, disc_number, track_number, title, year, album_artist, \ artist, album, artwork FROM songs WHERE parent = ? ORDER BY path \ - COLLATE NOCASE ASC") - .unwrap(); - select.bind(1, &Value::String(path_string.deref().to_owned())).unwrap(); - self.select_songs(&mut select).into_iter().map(|s| CollectionFile::Song(s)).collect() + COLLATE NOCASE ASC")?; + select.bind(1, &Value::String(path_string.deref().to_owned()))?; + Ok(self.select_songs(&mut select)?.into_iter().map(|s| CollectionFile::Song(s)).collect()) } pub fn browse(&self, virtual_path: &Path) -> Result> { @@ -556,8 +560,8 @@ impl Index { // Browse sub-directory } else { let real_path = self.vfs.virtual_to_real(virtual_path)?; - let directories = self.browse_directories(real_path.as_path()); - let songs = self.browse_songs(real_path.as_path()); + let directories = self.browse_directories(real_path.as_path())?; + let songs = self.browse_songs(real_path.as_path())?; output.extend(directories); output.extend(songs); } @@ -566,15 +570,14 @@ impl Index { } pub fn flatten(&self, virtual_path: &Path) -> Result> { - let db = self.connect(); + let db = self.connect()?; let real_path = self.vfs.virtual_to_real(virtual_path)?; let path_string = real_path.to_string_lossy().into_owned() + "%"; let mut select = db.prepare("SELECT path, disc_number, track_number, title, year, album_artist, \ artist, album, artwork FROM songs WHERE path LIKE ? ORDER BY path \ - COLLATE NOCASE ASC") - .unwrap(); - select.bind(1, &Value::String(path_string.deref().to_owned())).unwrap(); - Ok(self.select_songs(&mut select)) + COLLATE NOCASE ASC")?; + select.bind(1, &Value::String(path_string.deref().to_owned()))?; + self.select_songs(&mut select) } }