-
Notifications
You must be signed in to change notification settings - Fork 0
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
Refactor metadata to a more concise representation (#14) #15
base: main
Are you sure you want to change the base?
Changes from all commits
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 |
---|---|---|
|
@@ -9,3 +9,6 @@ result | |
|
||
# These are backup files generated by rustfmt | ||
**/*.rs.bk | ||
|
||
# Intellij | ||
.idea |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,9 @@ | ||
use chrono::{DateTime, FixedOffset, TimeZone}; | ||
use exif::{Exif, In, Tag, Value}; | ||
use std::fmt; | ||
use std::fmt::Display; | ||
use std::fs; | ||
use std::fs::metadata; | ||
use std::io; | ||
use std::os::unix::prelude::MetadataExt; | ||
use std::{error::Error, path::Path}; | ||
|
@@ -18,19 +20,13 @@ pub struct MetadataError { | |
} | ||
|
||
impl MetadataError { | ||
fn from_str(message: &str) -> Self { | ||
MetadataError { | ||
message: message.to_string(), | ||
} | ||
} | ||
|
||
fn from_string(message: String) -> Self { | ||
MetadataError { message: message } | ||
MetadataError { message } | ||
} | ||
} | ||
|
||
// Display implementation is required for std::error::Error. | ||
impl fmt::Display for MetadataError { | ||
impl Display for MetadataError { | ||
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { | ||
write!(f, "Metadata error: {}", self.message) | ||
} | ||
|
@@ -50,13 +46,52 @@ impl From<io::Error> for MetadataError { | |
} | ||
} | ||
|
||
fn get_date_time_created(path: &Path) -> Result<chrono::DateTime<FixedOffset>, MetadataError> { | ||
Ok(FixedOffset::west(0).timestamp(fs::metadata(path)?.ctime(), 0)) | ||
fn result_to_option<T, E: Error>(res: Result<T, E>, checkpoint: String) -> Option<T> { | ||
match res { | ||
Ok(t) => Some(t), | ||
Err(e) => { | ||
eprint!("Error occurred during {}, {}", checkpoint, e); | ||
None | ||
} | ||
} | ||
} | ||
|
||
fn convert_exif_date_time_to_chrono_date_time_fixed_offset( | ||
fn open_file(path: &Path, file_name: &String) -> Option<fs::File> { | ||
result_to_option(fs::File::open(path), "opening file ".to_owned() + file_name) | ||
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. Apparently a more common way to format strings is format!() |
||
} | ||
|
||
fn get_date_time_created(path: &Path) -> Result<DateTime<FixedOffset>, MetadataError> { | ||
Ok(FixedOffset::west(0).timestamp(metadata(path)?.ctime(), 0)) | ||
} | ||
|
||
fn get_exif_metadata(file: fs::File, file_name: &String) -> Option<Exif> { | ||
let mut bufreader = io::BufReader::new(&file); | ||
let exif_reader = exif::Reader::new(); | ||
result_to_option( | ||
exif_reader.read_from_container(&mut bufreader), | ||
"extracting exif metadata for ".to_owned() + file_name, | ||
) | ||
} | ||
|
||
fn get_exif_date_time_field(exif: Exif, tag: Tag, file_name: &String) -> Option<exif::DateTime> { | ||
exif.get_field(tag, In::PRIMARY) | ||
.map(|field| match field.value { | ||
Value::Ascii(ref v) => result_to_option( | ||
exif::DateTime::from_ascii(&v[0]), | ||
"converting exif field ".to_owned() + &tag.to_string() + " to date time", | ||
), | ||
_ => { | ||
eprintln!("Exif field {} is not ascii in file {}", tag, file_name); | ||
None | ||
} | ||
}) | ||
.flatten() | ||
} | ||
|
||
fn from_exif_to_chrono_date_time( | ||
exif_date_time: exif::DateTime, | ||
) -> Result<chrono::DateTime<FixedOffset>, MetadataError> { | ||
file_name: &String, | ||
) -> Option<DateTime<FixedOffset>> { | ||
let offset = match exif_date_time.offset { | ||
Some(offset_minutes) => FixedOffset::west((offset_minutes * 60).into()), | ||
None => FixedOffset::west(0), | ||
|
@@ -76,49 +111,35 @@ fn convert_exif_date_time_to_chrono_date_time_fixed_offset( | |
); | ||
|
||
match maybe_date { | ||
chrono::LocalResult::Single(date) => Ok(date), | ||
chrono::LocalResult::Ambiguous(_, _) => Err(MetadataError::from_str("Ambiguous date")), | ||
chrono::LocalResult::None => Err(MetadataError { | ||
message: "Invalid date".to_string(), | ||
}), | ||
chrono::LocalResult::Single(date) => Some(date), | ||
chrono::LocalResult::Ambiguous(_, _) => { | ||
eprintln!( | ||
"Date time {} in {} was ambiguous", | ||
file_name, exif_date_time | ||
); | ||
None | ||
} | ||
chrono::LocalResult::None => { | ||
eprintln!("Date time {} in {} was invalid", file_name, exif_date_time); | ||
None | ||
} | ||
} | ||
} | ||
|
||
fn extract_date_time_exif_field( | ||
exif: &Exif, | ||
tag: Tag, | ||
) -> Result<Option<chrono::DateTime<FixedOffset>>, MetadataError> { | ||
match exif.get_field(tag, In::PRIMARY) { | ||
Some(field) => match field.value { | ||
Value::Ascii(ref v) => match convert_exif_date_time_to_chrono_date_time_fixed_offset( | ||
exif::DateTime::from_ascii(&v[0])?, | ||
) { | ||
Ok(date_time) => Ok(Some(date_time)), | ||
Err(err) => Err(err), | ||
}, | ||
_ => Err(MetadataError { | ||
message: "Exif date field is not ascii".to_string(), | ||
}), | ||
}, | ||
None => Ok(None), | ||
} | ||
fn get_date_time_taken(path: &Path) -> Option<DateTime<FixedOffset>> { | ||
let file_name = path.display().to_string(); | ||
open_file(path, &file_name) | ||
.map(|file| get_exif_metadata(file, &file_name)) | ||
.flatten() | ||
Comment on lines
+132
to
+133
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. flat map is called and_then() in Rust. This would have been nice if we remembered from scala 😅 |
||
.map(|exif| get_exif_date_time_field(exif, Tag::DateTimeOriginal, &file_name)) | ||
.flatten() | ||
.map(|exif_date_time| from_exif_to_chrono_date_time(exif_date_time, &file_name)) | ||
.flatten() | ||
} | ||
|
||
pub fn read_metadata(path: &Path) -> Result<Metadata, MetadataError> { | ||
let date_time_created = get_date_time_created(path)?; | ||
|
||
let file = std::fs::File::open(path)?; | ||
let mut bufreader = std::io::BufReader::new(&file); | ||
let exifreader = exif::Reader::new(); | ||
|
||
let date_time_taken = match exifreader.read_from_container(&mut bufreader) { | ||
Ok(exif) => extract_date_time_exif_field(&exif, Tag::DateTimeOriginal)?, | ||
Err(err) => { | ||
eprintln!("Could not read EXIF data: {}", err); | ||
None | ||
} | ||
}; | ||
|
||
let date_time_taken = get_date_time_taken(path); | ||
Ok(Metadata { | ||
date_time_created, | ||
date_time_taken, | ||
|
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.
I think we should use
Result
here as the return value because it already encodes the concept of an error case by design (same way you wouldn't return an Optional in Java instead of throwing an exception). By extension, I'd makeread_metadata()
return aResult
.Result
also allows using?
which a nice way to chain function calls that return errors.