From 93f18999c7aae181bb77ea1fbc73422e947519ac Mon Sep 17 00:00:00 2001 From: Mike Schmid Date: Sat, 26 Oct 2024 16:32:53 +0200 Subject: [PATCH] refactor: cleanup code (#52) * refactor: cleanup code * refactor: cleanup code * refactor: cleanup code * feat: allow args --------- Co-authored-by: mike --- src/lib.rs | 68 +++++++++---------- src/main.rs | 15 +++-- src/models/qr_data.rs | 2 + src/pdf_converter.rs | 2 +- src/qr_parser.rs | 151 +++++++++++++++--------------------------- tests/edge_cases.rs | 40 +++++------ tests/six_examples.rs | 12 ++-- 7 files changed, 125 insertions(+), 165 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 9cbd39e..014b380 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,4 +1,4 @@ -//! # Simple program to read swiss invoices QR codes as pdf or png files and the relevant data +//! # Simple program to read Swiss invoice QR codes from PDF or PNG files and extract relevant data //! //! This program reads QR codes from Swiss invoices and outputs the relevant data as JSON. //! @@ -9,43 +9,51 @@ mod pdf_converter; mod qr_parser; use crate::models::qr_data::QRData; -use image; +use image::DynamicImage; use rayon::prelude::*; use rqrr::PreparedImage; +use std::path::Path; use tempfile::tempdir; -pub fn get_qr_bill_data(file_path: String, fail_on_error: bool) -> Vec { - let tmp_dir = tempdir().expect("Error creating temporary directory"); - - let images = match file_path.to_lowercase().as_str() { - input if input.ends_with(".pdf") => { - pdf_converter::convert_to_png(&file_path, &tmp_dir.path()) - } +pub fn get_qr_bill_data(file_path: &str, fail_on_error: bool) -> Vec { + let tmp_dir = tempdir().expect("Failed to create temporary directory"); + + let images = load_images(file_path, tmp_dir.path()); + + let qr_data_results: Vec<_> = images + .into_par_iter() + .flat_map(|img| extract_qr_data(&img)) + .collect(); + + handle_errors(&qr_data_results, fail_on_error); + + qr_data_results.into_iter().filter_map(Result::ok).collect() +} + +fn load_images(file_path: &str, tmp_dir_path: &Path) -> Vec { + match file_path.to_lowercase().as_str() { + input if input.ends_with(".pdf") => pdf_converter::convert_to_png(file_path, tmp_dir_path), input if input.ends_with(".png") || input.ends_with(".jpg") || input.ends_with(".jpeg") => { - vec![image::open(&file_path).expect("Error loading image")] + vec![image::open(file_path).expect("Failed to load image")] } _ => panic!("Unsupported file format"), - }; + } +} - let all_qr_codes: Vec<_> = images +fn extract_qr_data(img: &DynamicImage) -> Vec> { + PreparedImage::prepare(img.to_luma8()) + .detect_grids() .into_par_iter() - .map(|img| { - let mut img = PreparedImage::prepare(img.to_luma8()); - img.detect_grids() - .into_par_iter() - .filter_map(|result| result.decode().ok()) - .map(|(_, content)| qr_parser::get_qr_code_data(&content)) - .collect::>() - }) - .flatten() - .collect(); + .filter_map(|grid| grid.decode().ok()) + .map(|(_, content)| qr_parser::get_qr_code_data(&content)) + .collect() +} - // check if there were any errors - if fail_on_error && all_qr_codes.iter().any(|result| result.is_err()) { +fn handle_errors(results: &[Result], fail_on_error: bool) { + if fail_on_error && results.iter().any(Result::is_err) { eprintln!("Error parsing QR codes"); - // print the errors - for result in all_qr_codes { + for result in results { if let Err(err) = result { eprintln!("{}", err); } @@ -53,12 +61,4 @@ pub fn get_qr_bill_data(file_path: String, fail_on_error: bool) -> Vec { std::process::exit(1); } - - let all_qr_codes: Vec<_> = all_qr_codes - .into_iter() - .filter(|result| result.is_ok()) - .map(|result| result.unwrap()) - .collect(); - - return all_qr_codes; } diff --git a/src/main.rs b/src/main.rs index 624d9b2..f745496 100644 --- a/src/main.rs +++ b/src/main.rs @@ -11,13 +11,14 @@ use swiss_qr_bill_decoder::get_qr_bill_data; fn main() { let args = args::Args::parse(); - let all_qr_codes: Vec<_> = get_qr_bill_data(args.input, args.fail_on_error); + let all_qr_codes: Vec<_> = get_qr_bill_data(args.input.as_ref(), args.fail_on_error); - // send the QR code data to stdout - if args.pretty { - serde_json::to_writer_pretty(std::io::stdout(), &all_qr_codes) + // Serialize QR code data to stdout + let writer = if args.pretty { + serde_json::to_writer_pretty } else { - serde_json::to_writer(std::io::stdout(), &all_qr_codes) - } - .expect("Error writing JSON"); + serde_json::to_writer + }; + + writer(std::io::stdout(), &all_qr_codes).expect("Error writing JSON"); } diff --git a/src/models/qr_data.rs b/src/models/qr_data.rs index be1148a..f502d1c 100644 --- a/src/models/qr_data.rs +++ b/src/models/qr_data.rs @@ -14,6 +14,8 @@ pub struct QRData { } impl QRData { + + #[allow(clippy::too_many_arguments)] pub fn new( iban: String, recipient_address: Address, diff --git a/src/pdf_converter.rs b/src/pdf_converter.rs index 1111291..ed0c4b6 100644 --- a/src/pdf_converter.rs +++ b/src/pdf_converter.rs @@ -21,7 +21,7 @@ where Q: AsRef, { Command::new("gs") - .args(&[ + .args([ "-q", "-dBATCH", "-dSAFER", diff --git a/src/qr_parser.rs b/src/qr_parser.rs index 591116e..6285505 100644 --- a/src/qr_parser.rs +++ b/src/qr_parser.rs @@ -3,98 +3,60 @@ use crate::models::qr_data::QRData; use std::str::Lines; /// Get the QR code data from a String according to the Swiss QR bill standard -pub fn get_qr_code_data(text: &String) -> Result { +pub fn get_qr_code_data(text: &str) -> Result { let mut lines = text.lines(); - if lines.next() != Some("SPC") { - return Err("First line is not 'SPC'".to_string()); - } - - if lines.next() != Some("0200") { - return Err("Only version 0200 is supported".to_string()); - } - - if lines.next() != Some("1") { - return Err("Only coding type 1 (UTF-8) is supported".to_string()); - } + check_line(&mut lines, "SPC", "First line is not 'SPC'")?; + check_line(&mut lines, "0200", "Only version 0200 is supported")?; + check_line(&mut lines, "1", "Only coding type 1 (UTF-8) is supported")?; let iban = match lines.next() { - Some(iban) if iban.is_empty() => return Err("Missing IBAN".to_string()), + Some("") => return Err("Missing IBAN".to_string()), Some(iban) if iban.starts_with("CH") || iban.starts_with("LI") => iban.to_string(), _ => return Err("Only CH and LI IBANs are supported".to_string()), }; - let address_type = match lines.next() { - Some(address_type) if address_type.is_empty() => { - return Err("Recipient address type is empty".to_string()) - } - Some(address_type) => address_type, - _ => return Err("Missing recipient address type".to_string()), - }; - + let address_type = lines.next().ok_or("Missing recipient address type")?; let recipient_address = to_address(&mut lines, address_type)?; skip_lines(&mut lines, 7); - let amount = match lines.next() { - Some(amount) if amount.is_empty() => None, - Some(amount) => Some(amount.trim().to_string()), - _ => return Err("Missing amount".to_string()), - }; - + let amount = lines.next().filter(|s| !s.is_empty()).map(str::to_string); let currency = match lines.next() { - Some(currency) if currency.is_empty() => return Err("Missing currency".to_string()), - Some(currency) if currency.eq("CHF") || currency.eq("EUR") => currency.to_string(), + Some("") => return Err("Missing currency".to_string()), + Some(currency) if currency == "CHF" || currency == "EUR" => currency.to_string(), _ => return Err("Only CHF and EUR currencies are supported".to_string()), }; - let address_type = match lines.next() { - Some(address_type) if address_type.is_empty() => None, - Some(address_type) => Some(address_type), - _ => return Err("Missing address type".to_string()), - }; - - let sender_address = if address_type.is_some() { - Some(to_address(&mut lines, address_type.unwrap())?) + let address_type = lines.next().filter(|s| !s.is_empty()); + let sender_address = if let Some(address_type) = address_type { + Some(to_address(&mut lines, address_type)?) } else { skip_lines(&mut lines, 6); None }; let reference_type = match lines.next() { - Some(reference_type) if reference_type.is_empty() => { - return Err("Missing reference type".to_string()) - } - Some(reference_type) - if reference_type.eq("NON") - || reference_type.eq("QRR") - || reference_type.eq("SCOR") => - { + Some("") => return Err("Missing reference type".to_string()), + Some(reference_type) if ["NON", "QRR", "SCOR"].contains(&reference_type) => { reference_type.to_string() } - _ => return Err("Only reference types NON, QRR and SCOR are supported".to_string()), + _ => return Err("Only reference types NON, QRR, and SCOR are supported".to_string()), }; let reference = match lines.next() { Some(reference) - if reference.is_empty() && (reference_type.eq("QRR") || reference_type.eq("SCOR")) => + if reference.is_empty() && ["QRR", "SCOR"].contains(&reference_type.as_str()) => { - return Err("Reference is empty".to_string()) + return Err("Reference is empty".to_string()); } - Some(reference) if reference.is_empty() => None, + Some("") => None, Some(reference) => Some(reference.trim().to_string()), _ => return Err("Missing reference".to_string()), }; - let message = match lines.next() { - Some(message) if message.is_empty() => None, - Some(message) => Some(message.trim().to_string()), - _ => return Err("Missing message".to_string()), - }; - - if lines.next() != Some("EPD") { - return Err("Missing trailing 'EPD'".to_string()); - } + let message = lines.next().filter(|s| !s.is_empty()).map(str::to_string); + check_line(&mut lines, "EPD", "Missing trailing 'EPD'")?; Ok(QRData::new( iban, @@ -108,60 +70,55 @@ pub fn get_qr_code_data(text: &String) -> Result { )) } -fn skip_lines(lines: &mut Lines, skip_lines: i32) { - for _ in 0..skip_lines { +fn check_line(lines: &mut Lines, expected: &str, error_msg: &str) -> Result<(), String> { + if lines.next() != Some(expected) { + return Err(error_msg.to_string()); + } + Ok(()) +} + +fn skip_lines(lines: &mut Lines, skip_count: i32) { + for _ in 0..skip_count { let _ = lines.next(); } } fn to_address(lines: &mut Lines, address_type: &str) -> Result { let address_type = match address_type { - address_type if address_type.is_empty() => return Err("Address type is empty".to_string()), - address_type if !address_type.eq("K") && !address_type.eq("S") => { + "" => return Err("Address type is empty".to_string()), + address_type if !["K", "S"].contains(&address_type) => { return Err("Only address types K and S are supported".to_string()) } address_type => address_type.to_string(), }; - let name = match lines.next() { - None => return Err("Missing name".to_string()), - Some(name) if name.is_empty() => return Err("Recipient name is empty".to_string()), - Some(name) => name.to_string(), - }; - - let street_or_address_line_1 = match lines.next() { - None => return Err("Missing street or address line 1".to_string()), - Some(street_or_address_line_1) => street_or_address_line_1.to_string(), - }; - - let building_number_or_address_line_2 = match lines.next() { - None => return Err("Missing building number or address line 2".to_string()), - Some(building_number_or_address_line_2) => building_number_or_address_line_2.to_string(), - }; - - let postal_code = match lines.next() { - None => return Err("Missing postal code".to_string()), - Some(postal_code) => postal_code.to_string(), - }; - - let town = match lines.next() { - None => return Err("Missing town".to_string()), - Some(town) => town.to_string(), - }; - - let country = match lines.next() { - None => return Err("Missing country".to_string()), - Some(country) => country.to_string(), - }; - - let address_line_1 = if address_type.eq("K") { - street_or_address_line_1.to_string() + let name = lines.next().ok_or("Missing name".to_string())?.to_string(); + let street_or_address_line_1 = lines + .next() + .ok_or("Missing street or address line 1".to_string())? + .to_string(); + let building_number_or_address_line_2 = lines + .next() + .ok_or("Missing building number or address line 2".to_string())? + .to_string(); + let postal_code = lines + .next() + .ok_or("Missing postal code".to_string())? + .to_string(); + let town = lines.next().ok_or("Missing town".to_string())?.to_string(); + let country = lines + .next() + .ok_or("Missing country".to_string())? + .to_string(); + + let address_line_1 = if address_type == "K" { + street_or_address_line_1.clone() } else { format!("{street_or_address_line_1} {building_number_or_address_line_2}") }; - let address_line_2 = if address_type.eq("K") { - building_number_or_address_line_2.to_string() + let address_line_2 = if address_type == "K" { + building_number_or_address_line_2.clone() } else { format!("{postal_code} {town}") }; diff --git a/tests/edge_cases.rs b/tests/edge_cases.rs index 85924cb..58a8777 100644 --- a/tests/edge_cases.rs +++ b/tests/edge_cases.rs @@ -4,7 +4,7 @@ use swiss_qr_bill_decoder::models::qr_data::QRData; #[test] fn minimal_png() { - let actual = get_qr_bill_data("tests/data/minimal.png".to_string(), true); + let actual = get_qr_bill_data("tests/data/minimal.png", true); assert_eq!(actual.len(), 1); assert_eq!(actual[0], expected_minimal()); @@ -12,7 +12,7 @@ fn minimal_png() { #[test] fn minimal_jpg() { - let actual = get_qr_bill_data("tests/data/minimal.jpg".to_string(), true); + let actual = get_qr_bill_data("tests/data/minimal.jpg", true); assert_eq!(actual.len(), 1); assert_eq!(actual[0], expected_minimal()); @@ -20,7 +20,7 @@ fn minimal_jpg() { #[test] fn minimal_jpeg() { - let actual = get_qr_bill_data("tests/data/minimal.jpeg".to_string(), true); + let actual = get_qr_bill_data("tests/data/minimal.jpeg", true); assert_eq!(actual.len(), 1); assert_eq!(actual[0], expected_minimal()); @@ -29,7 +29,7 @@ fn minimal_jpeg() { #[test] #[ignore] fn minimal_pdf() { - let actual = get_qr_bill_data("tests/data/minimal.pdf".to_string(), true); + let actual = get_qr_bill_data("tests/data/minimal.pdf", true); assert_eq!(actual.len(), 1); assert_eq!(actual[0], expected_minimal()); @@ -37,7 +37,7 @@ fn minimal_pdf() { #[test] fn full_png() { - let actual = get_qr_bill_data("tests/data/full.png".to_string(), true); + let actual = get_qr_bill_data("tests/data/full.png", true); assert_eq!(actual.len(), 1); assert_eq!(actual[0], expected_full()); @@ -45,7 +45,7 @@ fn full_png() { #[test] fn full_jpg() { - let actual = get_qr_bill_data("tests/data/full.jpg".to_string(), true); + let actual = get_qr_bill_data("tests/data/full.jpg", true); assert_eq!(actual.len(), 1); assert_eq!(actual[0], expected_full()); @@ -53,7 +53,7 @@ fn full_jpg() { #[test] fn full_jpeg() { - let actual = get_qr_bill_data("tests/data/full.jpeg".to_string(), true); + let actual = get_qr_bill_data("tests/data/full.jpeg", true); assert_eq!(actual.len(), 1); assert_eq!(actual[0], expected_full()); @@ -62,7 +62,7 @@ fn full_jpeg() { #[test] #[ignore] fn full_pdf() { - let actual = get_qr_bill_data("tests/data/full.pdf".to_string(), true); + let actual = get_qr_bill_data("tests/data/full.pdf", true); assert_eq!(actual.len(), 1); assert_eq!(actual[0], expected_full()); @@ -70,7 +70,7 @@ fn full_pdf() { #[test] fn rotated_png() { - let actual = get_qr_bill_data("tests/data/rotated.png".to_string(), true); + let actual = get_qr_bill_data("tests/data/rotated.png", true); assert_eq!(actual.len(), 1); assert_eq!(actual[0], expected_rotated()); @@ -78,7 +78,7 @@ fn rotated_png() { #[test] fn rotated_jpg() { - let actual = get_qr_bill_data("tests/data/rotated.jpg".to_string(), true); + let actual = get_qr_bill_data("tests/data/rotated.jpg", true); assert_eq!(actual.len(), 1); assert_eq!(actual[0], expected_rotated()); @@ -86,7 +86,7 @@ fn rotated_jpg() { #[test] fn rotated_jpeg() { - let actual = get_qr_bill_data("tests/data/rotated.jpeg".to_string(), true); + let actual = get_qr_bill_data("tests/data/rotated.jpeg", true); assert_eq!(actual.len(), 1); assert_eq!(actual[0], expected_rotated()); @@ -95,7 +95,7 @@ fn rotated_jpeg() { #[test] #[ignore] fn rotated_pdf() { - let actual = get_qr_bill_data("tests/data/rotated.pdf".to_string(), true); + let actual = get_qr_bill_data("tests/data/rotated.pdf", true); assert_eq!(actual.len(), 1); assert_eq!(actual[0], expected_rotated()); @@ -103,7 +103,7 @@ fn rotated_pdf() { #[test] fn double_png() { - let actual = get_qr_bill_data("tests/data/double.png".to_string(), true); + let actual = get_qr_bill_data("tests/data/double.png", true); let expected = vec![expected_double_1(), expected_double_2()]; @@ -114,7 +114,7 @@ fn double_png() { #[test] fn double_jpg() { - let actual = get_qr_bill_data("tests/data/double.jpg".to_string(), true); + let actual = get_qr_bill_data("tests/data/double.jpg", true); let expected = vec![expected_double_1(), expected_double_2()]; @@ -125,7 +125,7 @@ fn double_jpg() { #[test] fn double_jpeg() { - let actual = get_qr_bill_data("tests/data/double.jpeg".to_string(), true); + let actual = get_qr_bill_data("tests/data/double.jpeg", true); let expected = vec![expected_double_1(), expected_double_2()]; @@ -137,7 +137,7 @@ fn double_jpeg() { #[test] #[ignore] fn double_pdf() { - let actual = get_qr_bill_data("tests/data/double.pdf".to_string(), true); + let actual = get_qr_bill_data("tests/data/double.pdf", true); let expected = vec![expected_double_1(), expected_double_2()]; assert_eq!(actual.len(), expected.len()); @@ -147,21 +147,21 @@ fn double_pdf() { #[test] fn none_png() { - let actual = get_qr_bill_data("tests/data/none.png".to_string(), true); + let actual = get_qr_bill_data("tests/data/none.png", true); assert!(actual.is_empty()); } #[test] fn none_jpg() { - let actual = get_qr_bill_data("tests/data/none.jpg".to_string(), true); + let actual = get_qr_bill_data("tests/data/none.jpg", true); assert!(actual.is_empty()); } #[test] fn none_jpeg() { - let actual = get_qr_bill_data("tests/data/none.jpeg".to_string(), true); + let actual = get_qr_bill_data("tests/data/none.jpeg", true); assert!(actual.is_empty()); } @@ -169,7 +169,7 @@ fn none_jpeg() { #[test] #[ignore] fn none_pdf() { - let actual = get_qr_bill_data("tests/data/none.pdf".to_string(), true); + let actual = get_qr_bill_data("tests/data/none.pdf", true); assert!(actual.is_empty()); } diff --git a/tests/six_examples.rs b/tests/six_examples.rs index da6d007..ded0a64 100644 --- a/tests/six_examples.rs +++ b/tests/six_examples.rs @@ -4,7 +4,7 @@ use swiss_qr_bill_decoder::models::qr_data::QRData; #[test] fn six_example_01() { - let actual = get_qr_bill_data("tests/data/six_examples/01.png".to_string(), true); + let actual = get_qr_bill_data("tests/data/six_examples/01.png", true); let expected = vec![QRData::new( "CH6431961000004421557".to_string(), @@ -35,7 +35,7 @@ fn six_example_01() { #[test] fn six_example_02() { - let actual = get_qr_bill_data("tests/data/six_examples/02.png".to_string(), true); + let actual = get_qr_bill_data("tests/data/six_examples/02.png", true); let expected = vec![QRData::new( "CH4431999123000889012".to_string(), @@ -66,7 +66,7 @@ fn six_example_02() { #[test] fn six_example_03() { - let actual = get_qr_bill_data("tests/data/six_examples/03.png".to_string(), true); + let actual = get_qr_bill_data("tests/data/six_examples/03.png", true); let expected = vec![QRData::new( "CH5204835012345671000".to_string(), @@ -91,7 +91,7 @@ fn six_example_03() { #[test] fn six_example_04() { - let actual = get_qr_bill_data("tests/data/six_examples/04.png".to_string(), true); + let actual = get_qr_bill_data("tests/data/six_examples/04.png", true); let expected = vec![QRData::new( "CH5800791123000889012".to_string(), @@ -122,7 +122,7 @@ fn six_example_04() { #[test] fn six_example_05() { - let actual = get_qr_bill_data("tests/data/six_examples/05.png".to_string(), true); + let actual = get_qr_bill_data("tests/data/six_examples/05.png", true); let expected = vec![QRData::new( "CH5800791123000889012".to_string(), @@ -153,7 +153,7 @@ fn six_example_05() { #[test] fn six_example_06() { - let actual = get_qr_bill_data("tests/data/six_examples/06.png".to_string(), true); + let actual = get_qr_bill_data("tests/data/six_examples/06.png", true); let expected = vec![QRData::new( "CH5800791123000889012".to_string(),