From 194dd3466849f43bd48b522a48fab8060d2bdf79 Mon Sep 17 00:00:00 2001 From: altair823 Date: Sat, 2 May 2026 05:11:40 +0000 Subject: [PATCH] =?UTF-8?q?review(p6-1):=20=ED=9A=8C=EC=B0=A8=201=20?= =?UTF-8?q?=EC=A7=80=EC=A0=81=20=EB=B0=98=EC=98=81?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Cargo.toml: 미사용 deps 제거 (`serde`, `thiserror`) + dev-deps 의 `serde_json` 중복 선언 제거. - src/lib.rs: 변수명 `decode_warning` → `dim_warning` (16k cap 초과 분기까지 포괄하므로 더 정확). - src/exif_extract.rs: `ascii_field` / `u32_field` 의 dead-flexibility `In` 인자 제거 (모든 호출이 `In::PRIMARY` 였음). 두 단 `if let` 을 Rust 2024 let-chain 으로 정리. EXIF 화이트리스트 출력 키를 workspace wire-schema 컨벤션에 맞춰 snake_case 로 통일 (`Make` → `make`, `DateTimeOriginal` → `date_time_original` 등). - tests/common/mod.rs: 호출되지 않는 `fake_path` 헬퍼 + `Path` import 제거. - tests/extractor.rs: snake_case 키로 assertion 갱신. cargo test -p kebab-parse-image — 14건 모두 pass. cargo clippy -p kebab-parse-image --all-targets -- -D warnings — pass. --- Cargo.lock | 2 - crates/kebab-parse-image/Cargo.toml | 3 - crates/kebab-parse-image/src/exif_extract.rs | 72 ++++++++++---------- crates/kebab-parse-image/src/lib.rs | 4 +- crates/kebab-parse-image/tests/common/mod.rs | 8 +-- crates/kebab-parse-image/tests/extractor.rs | 20 +++--- 6 files changed, 50 insertions(+), 59 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9a560fe..96e76ea 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3557,10 +3557,8 @@ dependencies = [ "image", "kamadak-exif", "kebab-core", - "serde", "serde_json", "tempfile", - "thiserror 2.0.18", "time", "tracing", ] diff --git a/crates/kebab-parse-image/Cargo.toml b/crates/kebab-parse-image/Cargo.toml index 1445307..5767807 100644 --- a/crates/kebab-parse-image/Cargo.toml +++ b/crates/kebab-parse-image/Cargo.toml @@ -10,11 +10,9 @@ description = "Image extractor — produces a single-block CanonicalDocument w [dependencies] kebab-core = { path = "../kebab-core" } anyhow = { workspace = true } -serde = { workspace = true } serde_json = { workspace = true } time = { workspace = true } tracing = { workspace = true } -thiserror = { workspace = true } # `image` ships a wide format menagerie under default features (BMP, DDS, # Farbfeld, …). We only need PNG / JPEG / WebP / GIF / TIFF for v1 (per # task spec out-of-scope HEIC/RAW). Trim defaults to keep the dep @@ -27,4 +25,3 @@ kamadak-exif = "0.6" [dev-dependencies] tempfile = { workspace = true } blake3 = { workspace = true } -serde_json = { workspace = true } diff --git a/crates/kebab-parse-image/src/exif_extract.rs b/crates/kebab-parse-image/src/exif_extract.rs index 1939f91..77a8eaf 100644 --- a/crates/kebab-parse-image/src/exif_extract.rs +++ b/crates/kebab-parse-image/src/exif_extract.rs @@ -5,17 +5,19 @@ //! camera state) is dropped on the floor so the on-disk wire form keeps a //! tight PII surface. //! -//! Whitelisted tags: +//! Whitelisted tags (output uses snake_case to match the rest of the +//! workspace's wire-schema convention; the EXIF tag identity is preserved +//! in the column name where reasonable): //! -//! | tag | output JSON shape | -//! |--------------------|----------------------------| -//! | DateTimeOriginal | `"YYYY-MM-DDTHH:MM:SS"` | -//! | GPSLatitude / Ref | merged into `gps_lat: f64` | -//! | GPSLongitude / Ref | merged into `gps_lon: f64` | -//! | Make | `String` | -//! | Model | `String` | -//! | Orientation | `u32` (1..=8) | -//! | Software | `String` | +//! | EXIF tag | output key | output JSON shape | +//! |-------------------------|-----------------------|-------------------------| +//! | DateTimeOriginal | `date_time_original` | `"YYYY-MM-DDTHH:MM:SS"` | +//! | GPSLatitude / Ref | `gps_lat` | `f64` (signed degrees) | +//! | GPSLongitude / Ref | `gps_lon` | `f64` (signed degrees) | +//! | Make | `make` | `String` | +//! | Model | `model` | `String` | +//! | Orientation | `orientation` | `u32` (1..=8) | +//! | Software | `software` | `String` | //! //! Any tag whose source value cannot be parsed into the documented shape //! is silently dropped — extractor failure must never fail the whole @@ -36,41 +38,41 @@ pub(crate) fn extract_whitelisted(bytes: &[u8]) -> Map { Err(_) => return out, }; - if let Some(s) = ascii_field(&exif, Tag::DateTimeOriginal, In::PRIMARY) { - if let Some(iso) = exif_datetime_to_iso(&s) { - out.insert("DateTimeOriginal".into(), JsonValue::String(iso)); - } + if let Some(s) = ascii_field(&exif, Tag::DateTimeOriginal) + && let Some(iso) = exif_datetime_to_iso(&s) + { + out.insert("date_time_original".into(), JsonValue::String(iso)); } - if let Some(lat) = gps_decimal(&exif, Tag::GPSLatitude, Tag::GPSLatitudeRef) { - if let Some(num) = serde_json::Number::from_f64(lat) { - out.insert("gps_lat".into(), JsonValue::Number(num)); - } + if let Some(lat) = gps_decimal(&exif, Tag::GPSLatitude, Tag::GPSLatitudeRef) + && let Some(num) = serde_json::Number::from_f64(lat) + { + out.insert("gps_lat".into(), JsonValue::Number(num)); } - if let Some(lon) = gps_decimal(&exif, Tag::GPSLongitude, Tag::GPSLongitudeRef) { - if let Some(num) = serde_json::Number::from_f64(lon) { - out.insert("gps_lon".into(), JsonValue::Number(num)); - } + if let Some(lon) = gps_decimal(&exif, Tag::GPSLongitude, Tag::GPSLongitudeRef) + && let Some(num) = serde_json::Number::from_f64(lon) + { + out.insert("gps_lon".into(), JsonValue::Number(num)); } - if let Some(s) = ascii_field(&exif, Tag::Make, In::PRIMARY) { - out.insert("Make".into(), JsonValue::String(s)); + if let Some(s) = ascii_field(&exif, Tag::Make) { + out.insert("make".into(), JsonValue::String(s)); } - if let Some(s) = ascii_field(&exif, Tag::Model, In::PRIMARY) { - out.insert("Model".into(), JsonValue::String(s)); + if let Some(s) = ascii_field(&exif, Tag::Model) { + out.insert("model".into(), JsonValue::String(s)); } - if let Some(o) = u32_field(&exif, Tag::Orientation, In::PRIMARY) { - out.insert("Orientation".into(), JsonValue::Number(o.into())); + if let Some(o) = u32_field(&exif, Tag::Orientation) { + out.insert("orientation".into(), JsonValue::Number(o.into())); } - if let Some(s) = ascii_field(&exif, Tag::Software, In::PRIMARY) { - out.insert("Software".into(), JsonValue::String(s)); + if let Some(s) = ascii_field(&exif, Tag::Software) { + out.insert("software".into(), JsonValue::String(s)); } out } -fn ascii_field(exif: &exif::Exif, tag: Tag, ifd: In) -> Option { - let f = exif.get_field(tag, ifd)?; +fn ascii_field(exif: &exif::Exif, tag: Tag) -> Option { + let f = exif.get_field(tag, In::PRIMARY)?; match &f.value { Value::Ascii(parts) => { // The EXIF 2.x ASCII type is one or more null-terminated C @@ -92,8 +94,8 @@ fn ascii_field(exif: &exif::Exif, tag: Tag, ifd: In) -> Option { } } -fn u32_field(exif: &exif::Exif, tag: Tag, ifd: In) -> Option { - let f = exif.get_field(tag, ifd)?; +fn u32_field(exif: &exif::Exif, tag: Tag) -> Option { + let f = exif.get_field(tag, In::PRIMARY)?; match &f.value { Value::Short(v) => v.first().map(|x| *x as u32), Value::Long(v) => v.first().copied(), @@ -140,7 +142,7 @@ fn gps_decimal(exif: &exif::Exif, value_tag: Tag, ref_tag: Tag) -> Option { let min = rational_to_f64(&dms[1])?; let sec = rational_to_f64(&dms[2])?; let mut decimal = deg + min / 60.0 + sec / 3600.0; - if let Some(reference) = ascii_field(exif, ref_tag, In::PRIMARY) { + if let Some(reference) = ascii_field(exif, ref_tag) { let r = reference.to_ascii_uppercase(); if r.starts_with('S') || r.starts_with('W') { decimal = -decimal; diff --git a/crates/kebab-parse-image/src/lib.rs b/crates/kebab-parse-image/src/lib.rs index 70cd0fc..35780fc 100644 --- a/crates/kebab-parse-image/src/lib.rs +++ b/crates/kebab-parse-image/src/lib.rs @@ -80,7 +80,7 @@ impl Extractor for ImageExtractor { // versa), so the two probes are independent. let exif_map = exif_extract::extract_whitelisted(bytes); - let (span, dims_value, decode_warning) = match &dim_outcome { + let (span, dims_value, dim_warning) = match &dim_outcome { dims::DimOutcome::Ok { width, height, format } => { let mut dims = Map::new(); dims.insert("w".into(), Value::Number((*width).into())); @@ -142,7 +142,7 @@ impl Extractor for ImageExtractor { kind: ProvenanceKind::Parsed, note: Some(format!("parser_version={}", parser_version.0)), }); - if let Some(reason) = decode_warning { + if let Some(reason) = dim_warning { events.push(ProvenanceEvent { at: now, agent: "kb-parse-image".to_string(), diff --git a/crates/kebab-parse-image/tests/common/mod.rs b/crates/kebab-parse-image/tests/common/mod.rs index 3d05418..9202338 100644 --- a/crates/kebab-parse-image/tests/common/mod.rs +++ b/crates/kebab-parse-image/tests/common/mod.rs @@ -21,7 +21,7 @@ use kebab_core::{ AssetStorage, Checksum, ExtractConfig, ExtractContext, ImageType, MediaType, RawAsset, SourceUri, WorkspacePath, }; -use std::path::{Path, PathBuf}; +use std::path::PathBuf; use time::OffsetDateTime; /// 100×50 solid-red PNG, no EXIF. @@ -233,9 +233,3 @@ pub fn strip_dynamic_at(json: &mut serde_json::Value) { } } -/// Stable ASCII path constant — avoids depending on `Path::new` or the -/// host's path separator in the call sites. -#[allow(dead_code)] -pub fn fake_path(p: &str) -> &Path { - Path::new(p) -} diff --git a/crates/kebab-parse-image/tests/extractor.rs b/crates/kebab-parse-image/tests/extractor.rs index 637b8ce9..451d08d 100644 --- a/crates/kebab-parse-image/tests/extractor.rs +++ b/crates/kebab-parse-image/tests/extractor.rs @@ -68,17 +68,17 @@ fn jpeg_with_exif_gps_captures_whitelisted_tags() { .get("exif") .and_then(|v| v.as_object()) .expect("exif object present"); - assert_eq!(exif.get("Make"), Some(&Value::String("KebabCam".into()))); - assert_eq!(exif.get("Model"), Some(&Value::String("X1".into()))); + assert_eq!(exif.get("make"), Some(&Value::String("KebabCam".into()))); + assert_eq!(exif.get("model"), Some(&Value::String("X1".into()))); assert_eq!( - exif.get("Software"), + exif.get("software"), Some(&Value::String("kebab-test".into())) ); assert_eq!( - exif.get("DateTimeOriginal"), + exif.get("date_time_original"), Some(&Value::String("2024-08-15T12:34:56".into())) ); - assert_eq!(exif.get("Orientation"), Some(&Value::Number(1.into()))); + assert_eq!(exif.get("orientation"), Some(&Value::Number(1.into()))); let lat = exif.get("gps_lat").and_then(|v| v.as_f64()).expect("gps_lat"); let lon = exif.get("gps_lon").and_then(|v| v.as_f64()).expect("gps_lon"); assert!((lat - 37.5).abs() < 1e-6, "lat={lat}"); @@ -86,11 +86,11 @@ fn jpeg_with_exif_gps_captures_whitelisted_tags() { // Maker notes / thumbnails / unrelated tags must NOT have leaked in. let allowed: std::collections::HashSet<&str> = [ - "Make", - "Model", - "Software", - "DateTimeOriginal", - "Orientation", + "make", + "model", + "software", + "date_time_original", + "orientation", "gps_lat", "gps_lon", ]