From c9f3fdfd933d3dc43d0981a08d5089a94336c480 Mon Sep 17 00:00:00 2001 From: Ryan Brooks Date: Fri, 27 Feb 2026 11:43:28 -0800 Subject: [PATCH 1/8] Upload image metadata json data as part of snapshots job --- src/api/data_types/snapshots.rs | 11 ++++- src/commands/build/snapshots.rs | 85 ++++++++++++++++++++++++++++++++- 2 files changed, 93 insertions(+), 3 deletions(-) diff --git a/src/api/data_types/snapshots.rs b/src/api/data_types/snapshots.rs index 40d73354c67..0cff9a05403 100644 --- a/src/api/data_types/snapshots.rs +++ b/src/api/data_types/snapshots.rs @@ -22,9 +22,18 @@ pub struct SnapshotsManifest { // Keep in sync with https://github.com/getsentry/sentry/blob/master/src/sentry/preprod/snapshots/manifest.py /// Metadata for a single image in a snapshot manifest. -#[derive(Debug, Serialize)] +#[derive(Debug, Deserialize, Serialize)] pub struct ImageMetadata { pub image_file_name: String, pub width: u32, pub height: u32, + #[serde(skip_serializing_if = "Option::is_none")] + pub display_name: Option, +} + +// Keep in sync with https://github.com/getsentry/sentry/blob/master/src/sentry/preprod/snapshots/manifest.py +/// A JSON manifest file that users can drop alongside images to provide additional metadata. +#[derive(Debug, Deserialize)] +pub struct SnapshotManifestFile { + pub images: HashMap, } diff --git a/src/commands/build/snapshots.rs b/src/commands/build/snapshots.rs index 2ded8360733..8ba37cf3edb 100644 --- a/src/commands/build/snapshots.rs +++ b/src/commands/build/snapshots.rs @@ -12,7 +12,9 @@ use secrecy::ExposeSecret as _; use sha2::{Digest as _, Sha256}; use walkdir::WalkDir; -use crate::api::{Api, CreateSnapshotResponse, ImageMetadata, SnapshotsManifest}; +use crate::api::{ + Api, CreateSnapshotResponse, ImageMetadata, SnapshotManifestFile, SnapshotsManifest, +}; use crate::config::{Auth, Config}; use crate::utils::args::ArgExt as _; @@ -95,7 +97,13 @@ pub fn execute(matches: &ArgMatches) -> Result<()> { style(images.len()).yellow(), if images.len() == 1 { "file" } else { "files" } ); - let manifest_entries = upload_images(images, &org, &project)?; + let mut manifest_entries = upload_images(images, &org, &project)?; + + // Parse JSON manifest files and merge metadata into discovered images + let json_manifests = collect_manifests(dir_path); + if !json_manifests.is_empty() { + merge_manifest_metadata(&mut manifest_entries, &json_manifests); + } // Build manifest from discovered images let manifest = SnapshotsManifest { @@ -253,6 +261,7 @@ fn upload_images( image_file_name, width: image.width, height: image.height, + display_name: None, }, ); } @@ -280,3 +289,75 @@ fn upload_images( } } } + +fn collect_manifests(dir: &Path) -> Vec { + WalkDir::new(dir) + .follow_links(true) + .into_iter() + .filter_entry(|e| !is_hidden(dir, e.path())) + .filter_map(|res| match res { + Ok(entry) => Some(entry), + Err(err) => { + warn!("Failed to access file during directory walk: {err}"); + None + } + }) + .filter(|entry| entry.file_type().is_file()) + .filter(|entry| { + entry + .path() + .extension() + .and_then(|ext| ext.to_str()) + .map(|ext| ext.eq_ignore_ascii_case("json")) + .unwrap_or(false) + }) + .filter_map(|entry| { + let path = entry.path(); + debug!("Reading manifest file: {}", path.display()); + let contents = match fs::read_to_string(path) { + Ok(c) => c, + Err(err) => { + warn!("Failed to read manifest file {}: {err}", path.display()); + return None; + } + }; + match serde_json::from_str::(&contents) { + Ok(manifest) => Some(manifest), + Err(err) => { + warn!("Failed to parse manifest file {}: {err}", path.display()); + None + } + } + }) + .collect() +} + +fn merge_manifest_metadata( + manifest_entries: &mut HashMap, + json_manifests: &[SnapshotManifestFile], +) { + for json_manifest in json_manifests { + for json_image in json_manifest.images.values() { + let matched = manifest_entries + .values_mut() + .find(|entry| entry.image_file_name == json_image.image_file_name); + match matched { + Some(entry) => { + if let Some(ref display_name) = json_image.display_name { + debug!( + "Setting display_name for {}: {display_name}", + entry.image_file_name + ); + entry.display_name = Some(display_name.clone()); + } + } + None => { + warn!( + "Manifest entry for '{}' does not match any discovered image", + json_image.image_file_name + ); + } + } + } + } +} From ef67db9806c4d832d9f557b9113b71d6c79beff4 Mon Sep 17 00:00:00 2001 From: Ryan Brooks Date: Fri, 27 Feb 2026 12:34:13 -0800 Subject: [PATCH 2/8] Cleanups --- src/api/data_types/snapshots.rs | 9 +-- src/commands/build/snapshots.rs | 128 ++++++++++++++------------------ 2 files changed, 57 insertions(+), 80 deletions(-) diff --git a/src/api/data_types/snapshots.rs b/src/api/data_types/snapshots.rs index 0cff9a05403..b976f788073 100644 --- a/src/api/data_types/snapshots.rs +++ b/src/api/data_types/snapshots.rs @@ -22,7 +22,7 @@ pub struct SnapshotsManifest { // Keep in sync with https://github.com/getsentry/sentry/blob/master/src/sentry/preprod/snapshots/manifest.py /// Metadata for a single image in a snapshot manifest. -#[derive(Debug, Deserialize, Serialize)] +#[derive(Debug, Serialize)] pub struct ImageMetadata { pub image_file_name: String, pub width: u32, @@ -30,10 +30,3 @@ pub struct ImageMetadata { #[serde(skip_serializing_if = "Option::is_none")] pub display_name: Option, } - -// Keep in sync with https://github.com/getsentry/sentry/blob/master/src/sentry/preprod/snapshots/manifest.py -/// A JSON manifest file that users can drop alongside images to provide additional metadata. -#[derive(Debug, Deserialize)] -pub struct SnapshotManifestFile { - pub images: HashMap, -} diff --git a/src/commands/build/snapshots.rs b/src/commands/build/snapshots.rs index 8ba37cf3edb..1c8ddc0b1c7 100644 --- a/src/commands/build/snapshots.rs +++ b/src/commands/build/snapshots.rs @@ -12,9 +12,9 @@ use secrecy::ExposeSecret as _; use sha2::{Digest as _, Sha256}; use walkdir::WalkDir; -use crate::api::{ - Api, CreateSnapshotResponse, ImageMetadata, SnapshotManifestFile, SnapshotsManifest, -}; +use serde::Deserialize; + +use crate::api::{Api, CreateSnapshotResponse, ImageMetadata, SnapshotsManifest}; use crate::config::{Auth, Config}; use crate::utils::args::ArgExt as _; @@ -97,13 +97,8 @@ pub fn execute(matches: &ArgMatches) -> Result<()> { style(images.len()).yellow(), if images.len() == 1 { "file" } else { "files" } ); - let mut manifest_entries = upload_images(images, &org, &project)?; - - // Parse JSON manifest files and merge metadata into discovered images - let json_manifests = collect_manifests(dir_path); - if !json_manifests.is_empty() { - merge_manifest_metadata(&mut manifest_entries, &json_manifests); - } + let display_names = collect_display_names(dir_path); + let manifest_entries = upload_images(images, &display_names, &org, &project)?; // Build manifest from discovered images let manifest = SnapshotsManifest { @@ -198,6 +193,7 @@ fn is_image_file(path: &Path) -> bool { fn upload_images( images: Vec, + display_names: &HashMap, org: &str, project: &str, ) -> Result> { @@ -255,13 +251,14 @@ fn upload_images( .unwrap_or_default() .to_string_lossy() .into_owned(); + let display_name = display_names.get(&image_file_name).cloned(); manifest_entries.insert( hash, ImageMetadata { image_file_name, width: image.width, height: image.height, - display_name: None, + display_name, }, ); } @@ -290,74 +287,61 @@ fn upload_images( } } -fn collect_manifests(dir: &Path) -> Vec { - WalkDir::new(dir) +/// Input format for user-provided JSON manifest files. +#[derive(Deserialize)] +struct ManifestFile { + images: HashMap, +} + +#[derive(Deserialize)] +struct ManifestFileEntry { + image_file_name: String, + display_name: Option, +} + +/// Collects `image_file_name -> display_name` mappings from JSON manifest files in a directory. +fn collect_display_names(dir: &Path) -> HashMap { + let mut display_names = HashMap::new(); + let entries = WalkDir::new(dir) .follow_links(true) .into_iter() - .filter_entry(|e| !is_hidden(dir, e.path())) - .filter_map(|res| match res { - Ok(entry) => Some(entry), + .filter_entry(|e| !is_hidden(dir, e.path())); + + for entry in entries.flatten() { + if !entry.file_type().is_file() { + continue; + } + let path = entry.path(); + let is_json = path + .extension() + .and_then(|ext| ext.to_str()) + .map(|ext| ext.eq_ignore_ascii_case("json")) + .unwrap_or(false); + if !is_json { + continue; + } + + debug!("Reading manifest file: {}", path.display()); + let contents = match fs::read_to_string(path) { + Ok(c) => c, Err(err) => { - warn!("Failed to access file during directory walk: {err}"); - None + warn!("Failed to read manifest file {}: {err}", path.display()); + continue; } - }) - .filter(|entry| entry.file_type().is_file()) - .filter(|entry| { - entry - .path() - .extension() - .and_then(|ext| ext.to_str()) - .map(|ext| ext.eq_ignore_ascii_case("json")) - .unwrap_or(false) - }) - .filter_map(|entry| { - let path = entry.path(); - debug!("Reading manifest file: {}", path.display()); - let contents = match fs::read_to_string(path) { - Ok(c) => c, - Err(err) => { - warn!("Failed to read manifest file {}: {err}", path.display()); - return None; - } - }; - match serde_json::from_str::(&contents) { - Ok(manifest) => Some(manifest), - Err(err) => { - warn!("Failed to parse manifest file {}: {err}", path.display()); - None - } + }; + let manifest: ManifestFile = match serde_json::from_str(&contents) { + Ok(m) => m, + Err(err) => { + warn!("Failed to parse manifest file {}: {err}", path.display()); + continue; } - }) - .collect() -} + }; -fn merge_manifest_metadata( - manifest_entries: &mut HashMap, - json_manifests: &[SnapshotManifestFile], -) { - for json_manifest in json_manifests { - for json_image in json_manifest.images.values() { - let matched = manifest_entries - .values_mut() - .find(|entry| entry.image_file_name == json_image.image_file_name); - match matched { - Some(entry) => { - if let Some(ref display_name) = json_image.display_name { - debug!( - "Setting display_name for {}: {display_name}", - entry.image_file_name - ); - entry.display_name = Some(display_name.clone()); - } - } - None => { - warn!( - "Manifest entry for '{}' does not match any discovered image", - json_image.image_file_name - ); - } + for entry in manifest.images.into_values() { + if let Some(display_name) = entry.display_name { + display_names.insert(entry.image_file_name, display_name); } } } + display_names } From 0f51cccd39dd08157602bbc8be976cb103751e72 Mon Sep 17 00:00:00 2001 From: Ryan Brooks Date: Mon, 2 Mar 2026 09:33:05 -0800 Subject: [PATCH 3/8] Updates --- src/api/data_types/snapshots.rs | 8 ++- src/commands/build/snapshots.rs | 106 ++++++++++++-------------------- 2 files changed, 46 insertions(+), 68 deletions(-) diff --git a/src/api/data_types/snapshots.rs b/src/api/data_types/snapshots.rs index b976f788073..77e17c8af01 100644 --- a/src/api/data_types/snapshots.rs +++ b/src/api/data_types/snapshots.rs @@ -22,11 +22,15 @@ pub struct SnapshotsManifest { // Keep in sync with https://github.com/getsentry/sentry/blob/master/src/sentry/preprod/snapshots/manifest.py /// Metadata for a single image in a snapshot manifest. +/// +/// The `image_file_name`, `width`, and `height` fields are set by the CLI. +/// Any additional fields from a companion JSON sidecar file are included +/// via `extra` and flattened into the serialized output. #[derive(Debug, Serialize)] pub struct ImageMetadata { pub image_file_name: String, pub width: u32, pub height: u32, - #[serde(skip_serializing_if = "Option::is_none")] - pub display_name: Option, + #[serde(flatten)] + pub extra: HashMap, } diff --git a/src/commands/build/snapshots.rs b/src/commands/build/snapshots.rs index 1c8ddc0b1c7..d4367007451 100644 --- a/src/commands/build/snapshots.rs +++ b/src/commands/build/snapshots.rs @@ -12,8 +12,6 @@ use secrecy::ExposeSecret as _; use sha2::{Digest as _, Sha256}; use walkdir::WalkDir; -use serde::Deserialize; - use crate::api::{Api, CreateSnapshotResponse, ImageMetadata, SnapshotsManifest}; use crate::config::{Auth, Config}; use crate::utils::args::ArgExt as _; @@ -97,8 +95,8 @@ pub fn execute(matches: &ArgMatches) -> Result<()> { style(images.len()).yellow(), if images.len() == 1 { "file" } else { "files" } ); - let display_names = collect_display_names(dir_path); - let manifest_entries = upload_images(images, &display_names, &org, &project)?; + + let manifest_entries = upload_images(images, &org, &project)?; // Build manifest from discovered images let manifest = SnapshotsManifest { @@ -191,9 +189,42 @@ fn is_image_file(path: &Path) -> bool { .unwrap_or(false) } +/// Reads the companion JSON sidecar for an image, if it exists. +/// +/// For an image at `path/to/button.png`, looks for `path/to/button.json`. +/// Returns a map of all key-value pairs from the JSON file. +fn read_sidecar_metadata(image_path: &Path) -> HashMap { + let sidecar_path = image_path.with_extension("json"); + if !sidecar_path.is_file() { + return HashMap::new(); + } + + debug!("Reading sidecar metadata: {}", sidecar_path.display()); + let contents = match fs::read_to_string(&sidecar_path) { + Ok(c) => c, + Err(err) => { + warn!( + "Failed to read sidecar file {}: {err}", + sidecar_path.display() + ); + return HashMap::new(); + } + }; + + match serde_json::from_str(&contents) { + Ok(map) => map, + Err(err) => { + warn!( + "Failed to parse sidecar file {}: {err}", + sidecar_path.display() + ); + HashMap::new() + } + } +} + fn upload_images( images: Vec, - display_names: &HashMap, org: &str, project: &str, ) -> Result> { @@ -251,14 +282,16 @@ fn upload_images( .unwrap_or_default() .to_string_lossy() .into_owned(); - let display_name = display_names.get(&image_file_name).cloned(); + + let extra = read_sidecar_metadata(&image.path); + manifest_entries.insert( hash, ImageMetadata { + extra, image_file_name, width: image.width, height: image.height, - display_name, }, ); } @@ -286,62 +319,3 @@ fn upload_images( } } } - -/// Input format for user-provided JSON manifest files. -#[derive(Deserialize)] -struct ManifestFile { - images: HashMap, -} - -#[derive(Deserialize)] -struct ManifestFileEntry { - image_file_name: String, - display_name: Option, -} - -/// Collects `image_file_name -> display_name` mappings from JSON manifest files in a directory. -fn collect_display_names(dir: &Path) -> HashMap { - let mut display_names = HashMap::new(); - let entries = WalkDir::new(dir) - .follow_links(true) - .into_iter() - .filter_entry(|e| !is_hidden(dir, e.path())); - - for entry in entries.flatten() { - if !entry.file_type().is_file() { - continue; - } - let path = entry.path(); - let is_json = path - .extension() - .and_then(|ext| ext.to_str()) - .map(|ext| ext.eq_ignore_ascii_case("json")) - .unwrap_or(false); - if !is_json { - continue; - } - - debug!("Reading manifest file: {}", path.display()); - let contents = match fs::read_to_string(path) { - Ok(c) => c, - Err(err) => { - warn!("Failed to read manifest file {}: {err}", path.display()); - continue; - } - }; - let manifest: ManifestFile = match serde_json::from_str(&contents) { - Ok(m) => m, - Err(err) => { - warn!("Failed to parse manifest file {}: {err}", path.display()); - continue; - } - }; - - for entry in manifest.images.into_values() { - if let Some(display_name) = entry.display_name { - display_names.insert(entry.image_file_name, display_name); - } - } - } - display_names -} From df4b7197e0affd0549f6e91e5b101d0987fd3fc8 Mon Sep 17 00:00:00 2001 From: Ryan Brooks Date: Mon, 2 Mar 2026 10:23:15 -0800 Subject: [PATCH 4/8] custom serializer --- src/api/data_types/snapshots.rs | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/src/api/data_types/snapshots.rs b/src/api/data_types/snapshots.rs index 77e17c8af01..12d48854bac 100644 --- a/src/api/data_types/snapshots.rs +++ b/src/api/data_types/snapshots.rs @@ -2,6 +2,7 @@ use std::collections::HashMap; +use serde::ser::{SerializeMap, Serializer}; use serde::{Deserialize, Serialize}; /// Response from the create snapshot endpoint. @@ -23,14 +24,32 @@ pub struct SnapshotsManifest { // Keep in sync with https://github.com/getsentry/sentry/blob/master/src/sentry/preprod/snapshots/manifest.py /// Metadata for a single image in a snapshot manifest. /// -/// The `image_file_name`, `width`, and `height` fields are set by the CLI. -/// Any additional fields from a companion JSON sidecar file are included -/// via `extra` and flattened into the serialized output. -#[derive(Debug, Serialize)] +/// Serializes as a flat JSON object. User-provided sidecar fields are included +/// first, then CLI-managed fields (`image_file_name`, `width`, `height`) are +/// written last so they always take precedence — similar to TypeScript's +/// `{ ...extras, image_file_name, width, height }`. +#[derive(Debug)] pub struct ImageMetadata { pub image_file_name: String, pub width: u32, pub height: u32, - #[serde(flatten)] pub extra: HashMap, } + +impl Serialize for ImageMetadata { + fn serialize(&self, serializer: S) -> Result { + let mut map = serializer.serialize_map(Some(self.extra.len() + 3))?; + + // Sidecar fields first (user-provided extras) + for (key, value) in &self.extra { + map.serialize_entry(key, value)?; + } + + // CLI-managed fields last — these always win + map.serialize_entry("image_file_name", &self.image_file_name)?; + map.serialize_entry("width", &self.width)?; + map.serialize_entry("height", &self.height)?; + + map.end() + } +} From 524559fd52b3bd2d127e64a37fd146932c4adda9 Mon Sep 17 00:00:00 2001 From: Ryan Brooks Date: Mon, 2 Mar 2026 10:23:30 -0800 Subject: [PATCH 5/8] Comment --- src/api/data_types/snapshots.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/api/data_types/snapshots.rs b/src/api/data_types/snapshots.rs index 12d48854bac..bf9186b8a68 100644 --- a/src/api/data_types/snapshots.rs +++ b/src/api/data_types/snapshots.rs @@ -26,8 +26,7 @@ pub struct SnapshotsManifest { /// /// Serializes as a flat JSON object. User-provided sidecar fields are included /// first, then CLI-managed fields (`image_file_name`, `width`, `height`) are -/// written last so they always take precedence — similar to TypeScript's -/// `{ ...extras, image_file_name, width, height }`. +/// written last so they always take precedence. #[derive(Debug)] pub struct ImageMetadata { pub image_file_name: String, From 3bbb55ae94a53a7cfd2281bd0b58aa1029e7d2b3 Mon Sep 17 00:00:00 2001 From: Ryan Brooks Date: Mon, 2 Mar 2026 10:39:21 -0800 Subject: [PATCH 6/8] clippy --- src/api/data_types/snapshots.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/api/data_types/snapshots.rs b/src/api/data_types/snapshots.rs index bf9186b8a68..688b205e28a 100644 --- a/src/api/data_types/snapshots.rs +++ b/src/api/data_types/snapshots.rs @@ -2,7 +2,7 @@ use std::collections::HashMap; -use serde::ser::{SerializeMap, Serializer}; +use serde::ser::{SerializeMap as _, Serializer}; use serde::{Deserialize, Serialize}; /// Response from the create snapshot endpoint. From 3a87a254fb12cf3d731d4a1454f60db0694448fd Mon Sep 17 00:00:00 2001 From: Ryan Brooks Date: Mon, 2 Mar 2026 10:54:07 -0800 Subject: [PATCH 7/8] Feedback --- src/api/data_types/snapshots.rs | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/src/api/data_types/snapshots.rs b/src/api/data_types/snapshots.rs index 688b205e28a..615d1337a7e 100644 --- a/src/api/data_types/snapshots.rs +++ b/src/api/data_types/snapshots.rs @@ -35,20 +35,29 @@ pub struct ImageMetadata { pub extra: HashMap, } +const RESERVED_KEYS: &[&str] = &["image_file_name", "width", "height"]; + impl Serialize for ImageMetadata { fn serialize(&self, serializer: S) -> Result { - let mut map = serializer.serialize_map(Some(self.extra.len() + 3))?; - - // Sidecar fields first (user-provided extras) - for (key, value) in &self.extra { - map.serialize_entry(key, value)?; - } - - // CLI-managed fields last — these always win + let extra_count = self + .extra + .keys() + .filter(|k| !RESERVED_KEYS.contains(&k.as_str())) + .count(); + let mut map = serializer.serialize_map(Some(extra_count + 3))?; + + // CLI-managed fields first map.serialize_entry("image_file_name", &self.image_file_name)?; map.serialize_entry("width", &self.width)?; map.serialize_entry("height", &self.height)?; + // User-provided sidecar fields, skipping any that conflict with CLI fields + for (key, value) in &self.extra { + if !RESERVED_KEYS.contains(&key.as_str()) { + map.serialize_entry(key, value)?; + } + } + map.end() } } From ea18d9d6c81852a968289df23c66d67e97fe5dcb Mon Sep 17 00:00:00 2001 From: Daniel Szoke Date: Tue, 3 Mar 2026 18:33:26 +0100 Subject: [PATCH 8/8] Suggestion for PR #3163 --- src/api/data_types/snapshots.rs | 90 +++++++++++++++++++++------------ src/commands/build/snapshots.rs | 7 +-- 2 files changed, 58 insertions(+), 39 deletions(-) diff --git a/src/api/data_types/snapshots.rs b/src/api/data_types/snapshots.rs index 615d1337a7e..d8205df8e26 100644 --- a/src/api/data_types/snapshots.rs +++ b/src/api/data_types/snapshots.rs @@ -2,8 +2,12 @@ use std::collections::HashMap; -use serde::ser::{SerializeMap as _, Serializer}; use serde::{Deserialize, Serialize}; +use serde_json::Value; + +const IMAGE_FILE_NAME_FIELD: &str = "image_file_name"; +const WIDTH_FIELD: &str = "width"; +const HEIGHT_FIELD: &str = "height"; /// Response from the create snapshot endpoint. #[derive(Debug, Deserialize)] @@ -24,40 +28,60 @@ pub struct SnapshotsManifest { // Keep in sync with https://github.com/getsentry/sentry/blob/master/src/sentry/preprod/snapshots/manifest.py /// Metadata for a single image in a snapshot manifest. /// -/// Serializes as a flat JSON object. User-provided sidecar fields are included -/// first, then CLI-managed fields (`image_file_name`, `width`, `height`) are -/// written last so they always take precedence. -#[derive(Debug)] +/// Serializes as a flat JSON object. +/// +/// CLI-managed fields (`image_file_name`, `width`, `height`) override any +/// identically named fields provided by user sidecar metadata. +#[derive(Debug, Serialize)] pub struct ImageMetadata { - pub image_file_name: String, - pub width: u32, - pub height: u32, - pub extra: HashMap, + #[serde(flatten)] + data: HashMap, +} + +impl ImageMetadata { + pub fn new( + image_file_name: String, + width: u32, + height: u32, + mut extra: HashMap, + ) -> Self { + extra.insert( + IMAGE_FILE_NAME_FIELD.to_owned(), + Value::String(image_file_name), + ); + extra.insert(WIDTH_FIELD.to_owned(), Value::from(width)); + extra.insert(HEIGHT_FIELD.to_owned(), Value::from(height)); + + Self { data: extra } + } } -const RESERVED_KEYS: &[&str] = &["image_file_name", "width", "height"]; - -impl Serialize for ImageMetadata { - fn serialize(&self, serializer: S) -> Result { - let extra_count = self - .extra - .keys() - .filter(|k| !RESERVED_KEYS.contains(&k.as_str())) - .count(); - let mut map = serializer.serialize_map(Some(extra_count + 3))?; - - // CLI-managed fields first - map.serialize_entry("image_file_name", &self.image_file_name)?; - map.serialize_entry("width", &self.width)?; - map.serialize_entry("height", &self.height)?; - - // User-provided sidecar fields, skipping any that conflict with CLI fields - for (key, value) in &self.extra { - if !RESERVED_KEYS.contains(&key.as_str()) { - map.serialize_entry(key, value)?; - } - } - - map.end() +#[cfg(test)] +mod tests { + use super::*; + + use serde_json::json; + + #[test] + fn cli_managed_fields_override_sidecar_fields() { + let extra = serde_json::from_value(json!({ + (IMAGE_FILE_NAME_FIELD): "from-sidecar.png", + (WIDTH_FIELD): 1, + (HEIGHT_FIELD): 2, + "custom": "keep-me" + })) + .unwrap(); + + let metadata = ImageMetadata::new("from-cli.png".to_owned(), 100, 200, extra); + let serialized = serde_json::to_value(metadata).unwrap(); + + let expected = json!({ + (IMAGE_FILE_NAME_FIELD): "from-cli.png", + (WIDTH_FIELD): 100, + (HEIGHT_FIELD): 200, + "custom": "keep-me" + }); + + assert_eq!(serialized, expected); } } diff --git a/src/commands/build/snapshots.rs b/src/commands/build/snapshots.rs index d4367007451..261a87636ab 100644 --- a/src/commands/build/snapshots.rs +++ b/src/commands/build/snapshots.rs @@ -287,12 +287,7 @@ fn upload_images( manifest_entries.insert( hash, - ImageMetadata { - extra, - image_file_name, - width: image.width, - height: image.height, - }, + ImageMetadata::new(image_file_name, image.width, image.height, extra), ); }