altair823-org/kebab
0
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 40 Wiki Activity

review(p6-1): 회차 1 지적 반영

Browse Source 
- 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.
This commit is contained in:
altair823
2026-05-02 05:11:40 +00:00
parent d11a810119
commit 194dd34668
6 changed files with 50 additions and 59 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
Cargo.lock generated
View File

@@ -3557,10 +3557,8 @@ dependencies = [
"image",
"kamadak-exif",
"kebab-core",
"serde",
"serde_json",
"tempfile",
"thiserror 2.0.18",
"time",
"tracing",
]

3
crates/kebab-parse-image/Cargo.toml
View File

@@ -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 }

72
crates/kebab-parse-image/src/exif_extract.rs
View File

@@ -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<String, JsonValue> {
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<String> {
let f = exif.get_field(tag, ifd)?;
fn ascii_field(exif: &exif::Exif, tag: Tag) -> Option<String> {
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<String> {
}
}
fn u32_field(exif: &exif::Exif, tag: Tag, ifd: In) -> Option<u32> {
let f = exif.get_field(tag, ifd)?;
fn u32_field(exif: &exif::Exif, tag: Tag) -> Option<u32> {
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<f64> {
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;

4
crates/kebab-parse-image/src/lib.rs
View File

@@ -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(),

8
crates/kebab-parse-image/tests/common/mod.rs
View File

@@ -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)
}

20
crates/kebab-parse-image/tests/extractor.rs
View File

@@ -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",
]