Simplify CatalogPart::load by removing manual JSON parsing fallbacks and introducing test coverage for sample catalogs.

This commit is contained in:
Till Wegmueller 2025-12-08 22:39:28 +01:00
parent 84b2c50ed6
commit 55decc16ff
No known key found for this signature in database
2 changed files with 59 additions and 133 deletions

View file

@ -65,29 +65,27 @@ fn test_catalog_methods() {
// Create a simple base catalog part
let base_content = r#"{
"packages": {
"test": {
"example/package": [
{
"version": "1.0",
"actions": [
"set name=pkg.fmri value=pkg://test/example/package@1.0",
"set name=pkg.summary value=\"Example package\"",
"set name=pkg.description value=\"An example package for testing\""
]
}
],
"example/obsolete": [
{
"version": "1.0",
"actions": [
"set name=pkg.fmri value=pkg://test/example/obsolete@1.0",
"set name=pkg.summary value=\"Obsolete package\"",
"set name=pkg.obsolete value=true"
]
}
]
}
"test": {
"example/package": [
{
"version": "1.0",
"actions": [
"set name=pkg.fmri value=pkg://test/example/package@1.0",
"set name=pkg.summary value=\"Example package\"",
"set name=pkg.description value=\"An example package for testing\""
]
}
],
"example/obsolete": [
{
"version": "1.0",
"actions": [
"set name=pkg.fmri value=pkg://test/example/obsolete@1.0",
"set name=pkg.summary value=\"Obsolete package\"",
"set name=pkg.obsolete value=true"
]
}
]
}
}"#;
println!("Writing base catalog part to {:?}", publisher_dir.join("base"));

View file

@ -211,6 +211,7 @@ pub struct CatalogPart {
pub signature: Option<HashMap<String, String>>,
/// Packages by publisher and stem
#[serde(flatten)]
pub packages: HashMap<String, HashMap<String, Vec<PackageVersionEntry>>>,
}
@ -274,118 +275,12 @@ impl CatalogPart {
/// Load catalog part from a file
pub fn load<P: AsRef<Path>>(path: P) -> Result<Self> {
let path_ref = path.as_ref();
let json = fs::read_to_string(path_ref)?;
let json = fs::File::open(path_ref)?;
// Print the first 100 characters of the JSON file for debugging
let preview = if json.len() > 100 {
&json[0..100]
} else {
&json
};
println!("Loading catalog part from {:?}, preview: {}", path_ref, preview);
// Try to parse the JSON directly first
match serde_json::from_str::<CatalogPart>(&json) {
Ok(part) => return Ok(part),
// Try to parse the JSON directly
match serde_json::from_reader(json) {
Ok(part) => Ok(part),
Err(e) => {
println!("Failed to parse catalog part directly: {}", e);
// If the error is about a missing 'packages' field, try to directly construct a CatalogPart
if e.to_string().contains("missing field `packages`") {
println!("Trying to directly construct a CatalogPart");
// Parse the JSON as a generic Value
match serde_json::from_str::<serde_json::Value>(&json) {
Ok(value) => {
// Try to manually construct a CatalogPart
if let serde_json::Value::Object(map) = value {
let mut catalog_part = CatalogPart::new();
// Process each publisher
for (publisher, publisher_value) in map {
if let serde_json::Value::Object(publisher_map) = publisher_value {
let mut publisher_packages = HashMap::new();
// Process each package stem
for (stem, stem_value) in publisher_map {
if let serde_json::Value::Array(versions) = stem_value {
let mut package_versions = Vec::new();
// Process each version
for version_value in versions {
if let serde_json::Value::Object(version_map) = version_value {
// Extract version
let version = match version_map.get("version") {
Some(serde_json::Value::String(v)) => v.clone(),
_ => {
// If version field is missing, use an empty string
// This allows us to handle catalog files that don't have a version field
println!("Missing version field, using empty string");
String::new()
}
};
// Extract signature-sha-1 if present
let signature_sha1 = match version_map.get("signature-sha-1") {
Some(serde_json::Value::String(s)) => Some(s.clone()),
_ => None,
};
// Extract actions if present
let actions = match version_map.get("actions") {
Some(serde_json::Value::Array(a)) => {
let mut action_strings = Vec::new();
for action in a {
if let serde_json::Value::String(s) = action {
action_strings.push(s.clone());
}
}
if action_strings.is_empty() {
None
} else {
Some(action_strings)
}
},
Some(serde_json::Value::String(s)) => {
// Handle the case where actions is a string
Some(vec![s.clone()])
},
_ => None,
};
// Create a PackageVersionEntry
let entry = PackageVersionEntry {
version,
signature_sha1,
actions,
};
package_versions.push(entry);
}
}
publisher_packages.insert(stem, package_versions);
}
}
catalog_part.packages.insert(publisher, publisher_packages);
}
}
return Ok(catalog_part);
}
return Err(CatalogError::JsonSerializationError(e));
},
Err(e) => {
println!("Failed to parse JSON as generic Value: {}", e);
return Err(CatalogError::JsonSerializationError(e));
}
}
}
// If we get here, the error wasn't about a missing packages field or we couldn't fix it
println!("Failed to parse catalog part: {}", e);
Err(CatalogError::JsonSerializationError(e))
}
}
@ -692,3 +587,36 @@ impl CatalogManager {
}
}
}
#[cfg(test)]
mod tests {
use super::*;
use std::path::PathBuf;
#[test]
fn test_load_sample_catalog() {
// Path is relative to the crate root (libips)
let path = PathBuf::from("../sample_data/sample-repo/publisher/openindiana.org/catalog/catalog.base.C");
// Only run this test if the sample data exists
if path.exists() {
println!("Testing with sample catalog at {:?}", path);
match CatalogPart::load(&path) {
Ok(part) => {
println!("Successfully loaded catalog part");
// Verify we loaded the correct data structure
// The sample file has "openindiana.org" as a key
assert!(part.packages.contains_key("openindiana.org"), "Catalog should contain openindiana.org publisher");
let packages = part.packages.get("openindiana.org").unwrap();
println!("Found {} packages for openindiana.org", packages.len());
assert!(packages.len() > 0, "Should have loaded packages");
},
Err(e) => panic!("Failed to load catalog part: {}", e),
}
} else {
println!("Sample data not found at {:?}, skipping test. This is expected in some CI environments.", path);
}
}
}