Refactor repository operations to use immutable references

- Modify `rebuild` and `refresh` methods to take `&self` instead of `&mut self`.
- Replace mutable repository instances in `pkg6dev`, `pkg6repo`, and core library with immutable ones.
- Introduce `debug` logging for enhanced traceability during metadata rebuild and refresh.
- Extract and simplify catalog rebuild logic into a dedicated `rebuild_catalog` method.
- General cleanup: remove redundant methods, reorganize logic, and refine comments across affected files.
This commit is contained in:
Till Wegmueller 2025-07-27 13:43:56 +02:00
parent 7f4ecf6346
commit f4d1ce73b6
No known key found for this signature in database
5 changed files with 326 additions and 267 deletions

View file

@ -1227,14 +1227,18 @@ impl WritableRepository for FileBackend {
} }
/// Rebuild repository metadata /// Rebuild repository metadata
fn rebuild(&mut self, publisher: Option<&str>, no_catalog: bool, no_index: bool) -> Result<()> { fn rebuild(&self, publisher: Option<&str>, no_catalog: bool, no_index: bool) -> Result<()> {
debug!("rebuild called with publisher: {:?}, no_catalog: {}, no_index: {}", publisher, no_catalog, no_index);
// Filter publishers if specified // Filter publishers if specified
let publishers = if let Some(pub_name) = publisher { let publishers = if let Some(pub_name) = publisher {
if !self.config.publishers.contains(&pub_name.to_string()) { if !self.config.publishers.contains(&pub_name.to_string()) {
return Err(RepositoryError::PublisherNotFound(pub_name.to_string())); return Err(RepositoryError::PublisherNotFound(pub_name.to_string()));
} }
debug!("rebuild: using specified publisher: {}", pub_name);
vec![pub_name.to_string()] vec![pub_name.to_string()]
} else { } else {
debug!("rebuild: using all publishers: {:?}", self.config.publishers);
self.config.publishers.clone() self.config.publishers.clone()
}; };
@ -1244,7 +1248,7 @@ impl WritableRepository for FileBackend {
if !no_catalog { if !no_catalog {
info!("Rebuilding catalog..."); info!("Rebuilding catalog...");
self.generate_catalog_parts(&pub_name, true)?; self.rebuild_catalog(&pub_name, true)?;
} }
if !no_index { if !no_index {
@ -1257,7 +1261,7 @@ impl WritableRepository for FileBackend {
} }
/// Refresh repository metadata /// Refresh repository metadata
fn refresh(&mut self, publisher: Option<&str>, no_catalog: bool, no_index: bool) -> Result<()> { fn refresh(&self, publisher: Option<&str>, no_catalog: bool, no_index: bool) -> Result<()> {
// Filter publishers if specified // Filter publishers if specified
let publishers = if let Some(pub_name) = publisher { let publishers = if let Some(pub_name) = publisher {
if !self.config.publishers.contains(&pub_name.to_string()) { if !self.config.publishers.contains(&pub_name.to_string()) {
@ -1274,7 +1278,7 @@ impl WritableRepository for FileBackend {
if !no_catalog { if !no_catalog {
info!("Refreshing catalog..."); info!("Refreshing catalog...");
self.generate_catalog_parts(&pub_name, true)?; self.rebuild_catalog(&pub_name, true)?;
} }
if !no_index { if !no_index {
@ -1316,62 +1320,21 @@ impl FileBackend {
Ok(()) Ok(())
} }
/// Generate the file path for a given hash using the new directory structure /// Rebuild catalog for a publisher
/// The path will be $REPO/file/XX/YY/XXYY... where XX and YY are the first four letters of the hash
fn generate_file_path(&self, hash: &str) -> PathBuf {
if hash.len() < 4 {
// Fallback for very short hashes (shouldn't happen with SHA256)
return self.path.join("file").join(hash);
}
// Extract the first two and next two characters from the hash
let first_two = &hash[0..2];
let next_two = &hash[2..4];
// Create the path: $REPO/file/XX/YY/XXYY...
self.path.join("file").join(first_two).join(next_two).join(hash)
}
/// Get or initialize the catalog manager
/// ///
/// This method returns a mutable reference to the catalog manager. /// This method generates catalog files for a publisher and stores them in the publisher's
/// It uses interior mutability with RefCell to allow mutation through an immutable reference. /// subdirectory within the catalog directory.
pub fn get_catalog_manager( pub fn rebuild_catalog(&self, publisher: &str, create_update_log: bool) -> Result<()> {
&mut self, info!("Rebuilding catalog for publisher: {}", publisher);
) -> Result<std::cell::RefMut<crate::repository::catalog::CatalogManager>> { debug!("Catalog directory path: {}", self.path.join("catalog").display());
if self.catalog_manager.is_none() {
let catalog_dir = self.path.join("catalog");
let manager = crate::repository::catalog::CatalogManager::new(&catalog_dir)?;
let refcell = std::cell::RefCell::new(manager);
self.catalog_manager = Some(refcell);
}
// This is safe because we just checked that catalog_manager is Some // Create the catalog directory for the publisher if it doesn't exist
Ok(self.catalog_manager.as_ref().unwrap().borrow_mut()) let catalog_dir = self.path.join("catalog").join(publisher);
} debug!("Publisher catalog directory: {}", catalog_dir.display());
fs::create_dir_all(&catalog_dir)?;
debug!("Created publisher catalog directory");
/// URL encode a string for use in a filename // Collect package data
fn url_encode(s: &str) -> String {
let mut result = String::new();
for c in s.chars() {
match c {
'a'..='z' | 'A'..='Z' | '0'..='9' | '-' | '_' | '.' | '~' => result.push(c),
' ' => result.push('+'),
_ => {
result.push('%');
result.push_str(&format!("{:02X}", c as u8));
}
}
}
result
}
/// Generate catalog parts for a publisher
fn generate_catalog_parts(&mut self, publisher: &str, create_update_log: bool) -> Result<()> {
info!("Generating catalog parts for publisher: {}", publisher);
// Collect package data first
let repo_path = self.path.clone();
let packages = self.list_packages(Some(publisher), None)?; let packages = self.list_packages(Some(publisher), None)?;
// Prepare data structures for catalog parts // Prepare data structures for catalog parts
@ -1395,7 +1358,7 @@ impl FileBackend {
} }
// Get the package manifest // Get the package manifest
let pkg_dir = repo_path.join("pkg").join(publisher).join(stem); let pkg_dir = self.path.join("pkg").join(publisher).join(stem);
if !pkg_dir.exists() { if !pkg_dir.exists() {
continue; continue;
} }
@ -1494,40 +1457,103 @@ impl FileBackend {
package_version_count += 1; package_version_count += 1;
} }
// Now get the catalog manager and create the catalog parts // Create and save catalog parts
let mut catalog_manager = self.get_catalog_manager()?;
// Create and populate the base part // Create a catalog.attrs file
let base_part_name = "catalog.base.C".to_string(); let now = SystemTime::now();
let base_part = catalog_manager.create_part(&base_part_name); let timestamp = format_iso8601_timestamp(&now);
// Get the CatalogAttrs struct definition to see what fields it has
let mut attrs = crate::repository::catalog::CatalogAttrs {
created: timestamp.clone(),
last_modified: timestamp.clone(),
package_count,
package_version_count,
parts: HashMap::new(),
version: 1, // CatalogVersion::V1 is 1
signature: None,
updates: HashMap::new(),
};
// Add part information
let base_part_name = "catalog.base.C";
attrs.parts.insert(
base_part_name.to_string(),
crate::repository::catalog::CatalogPartInfo {
last_modified: timestamp.clone(),
signature_sha1: None,
},
);
let dependency_part_name = "catalog.dependency.C";
attrs.parts.insert(
dependency_part_name.to_string(),
crate::repository::catalog::CatalogPartInfo {
last_modified: timestamp.clone(),
signature_sha1: None,
},
);
let summary_part_name = "catalog.summary.C";
attrs.parts.insert(
summary_part_name.to_string(),
crate::repository::catalog::CatalogPartInfo {
last_modified: timestamp.clone(),
signature_sha1: None,
},
);
// Save the catalog.attrs file
let attrs_path = catalog_dir.join("catalog.attrs");
debug!("Writing catalog.attrs to: {}", attrs_path.display());
let attrs_json = serde_json::to_string_pretty(&attrs)?;
fs::write(&attrs_path, attrs_json)?;
debug!("Wrote catalog.attrs file");
// Create and save catalog parts
// Base part
let base_part_path = catalog_dir.join(base_part_name);
debug!("Writing base part to: {}", base_part_path.display());
let mut base_part = crate::repository::catalog::CatalogPart::new();
for (fmri, actions, signature) in base_entries { for (fmri, actions, signature) in base_entries {
base_part.add_package(publisher, &fmri, actions, Some(signature)); base_part.add_package(publisher, &fmri, actions, Some(signature));
} }
catalog_manager.save_part(&base_part_name)?; let base_part_json = serde_json::to_string_pretty(&base_part)?;
fs::write(&base_part_path, base_part_json)?;
debug!("Wrote base part file");
// Create and populate dependency part // Dependency part
let dependency_part_name = "catalog.dependency.C".to_string(); let dependency_part_path = catalog_dir.join(dependency_part_name);
let dependency_part = catalog_manager.create_part(&dependency_part_name); debug!("Writing dependency part to: {}", dependency_part_path.display());
let mut dependency_part = crate::repository::catalog::CatalogPart::new();
for (fmri, actions, signature) in dependency_entries { for (fmri, actions, signature) in dependency_entries {
dependency_part.add_package(publisher, &fmri, actions, Some(signature)); dependency_part.add_package(publisher, &fmri, actions, Some(signature));
} }
catalog_manager.save_part(&dependency_part_name)?; let dependency_part_json = serde_json::to_string_pretty(&dependency_part)?;
fs::write(&dependency_part_path, dependency_part_json)?;
debug!("Wrote dependency part file");
// Create and populate summary part // Summary part
let summary_part_name = "catalog.summary.C".to_string(); let summary_part_path = catalog_dir.join(summary_part_name);
let summary_part = catalog_manager.create_part(&summary_part_name); debug!("Writing summary part to: {}", summary_part_path.display());
let mut summary_part = crate::repository::catalog::CatalogPart::new();
for (fmri, actions, signature) in summary_entries { for (fmri, actions, signature) in summary_entries {
summary_part.add_package(publisher, &fmri, actions, Some(signature)); summary_part.add_package(publisher, &fmri, actions, Some(signature));
} }
catalog_manager.save_part(&summary_part_name)?; let summary_part_json = serde_json::to_string_pretty(&summary_part)?;
fs::write(&summary_part_path, summary_part_json)?;
debug!("Wrote summary part file");
// Create and populate the update log if needed // Create and save the update log if needed
if create_update_log { if create_update_log {
let now = SystemTime::now(); debug!("Creating update log");
let timestamp = format_iso8601_timestamp(&now);
let update_log_name = format!("update.{}Z.C", timestamp.split('.').next().unwrap()); let update_log_name = format!("update.{}Z.C", timestamp.split('.').next().unwrap());
let update_log_path = catalog_dir.join(&update_log_name);
debug!("Update log path: {}", update_log_path.display());
let update_log = catalog_manager.create_update_log(&update_log_name); let mut update_log = crate::repository::catalog::UpdateLog::new();
debug!("Adding {} updates to the log", update_entries.len());
for (fmri, catalog_parts, signature) in update_entries { for (fmri, catalog_parts, signature) in update_entries {
update_log.add_update( update_log.add_update(
publisher, publisher,
@ -1537,49 +1563,82 @@ impl FileBackend {
Some(signature), Some(signature),
); );
} }
catalog_manager.save_update_log(&update_log_name)?;
let update_log_json = serde_json::to_string_pretty(&update_log)?;
fs::write(&update_log_path, update_log_json)?;
debug!("Wrote update log file");
// Add an update log to catalog.attrs
debug!("Adding update log to catalog.attrs");
attrs.updates.insert(
update_log_name.clone(),
crate::repository::catalog::UpdateLogInfo {
last_modified: timestamp.clone(),
signature_sha1: None,
},
);
// Update the catalog.attrs file with the new update log
debug!("Updating catalog.attrs file with new update log");
let attrs_json = serde_json::to_string_pretty(&attrs)?;
fs::write(catalog_dir.join("catalog.attrs"), attrs_json)?;
debug!("Updated catalog.attrs file");
} }
// Update catalog attributes info!("Catalog rebuilt for publisher: {}", publisher);
let now = SystemTime::now();
let timestamp = format_iso8601_timestamp(&now);
let attrs = catalog_manager.attrs_mut();
attrs.last_modified = timestamp.clone();
attrs.package_count = package_count;
attrs.package_version_count = package_version_count;
// Add part information
attrs.parts.insert(
base_part_name.clone(),
crate::repository::catalog::CatalogPartInfo {
last_modified: timestamp.clone(),
signature_sha1: None,
},
);
attrs.parts.insert(
dependency_part_name.clone(),
crate::repository::catalog::CatalogPartInfo {
last_modified: timestamp.clone(),
signature_sha1: None,
},
);
attrs.parts.insert(
summary_part_name.clone(),
crate::repository::catalog::CatalogPartInfo {
last_modified: timestamp.clone(),
signature_sha1: None,
},
);
// Save catalog attributes
catalog_manager.save_attrs()?;
Ok(()) Ok(())
} }
/// Generate the file path for a given hash using the new directory structure
/// The path will be $REPO/file/XX/YY/XXYY... where XX and YY are the first four letters of the hash
fn generate_file_path(&self, hash: &str) -> PathBuf {
if hash.len() < 4 {
// Fallback for very short hashes (shouldn't happen with SHA256)
return self.path.join("file").join(hash);
}
// Extract the first two and next two characters from the hash
let first_two = &hash[0..2];
let next_two = &hash[2..4];
// Create the path: $REPO/file/XX/YY/XXYY...
self.path.join("file").join(first_two).join(next_two).join(hash)
}
/// Get or initialize the catalog manager
///
/// This method returns a mutable reference to the catalog manager.
/// It uses interior mutability with RefCell to allow mutation through an immutable reference.
pub fn get_catalog_manager(
&mut self,
) -> Result<std::cell::RefMut<crate::repository::catalog::CatalogManager>> {
if self.catalog_manager.is_none() {
let catalog_dir = self.path.join("catalog");
let manager = crate::repository::catalog::CatalogManager::new(&catalog_dir)?;
let refcell = std::cell::RefCell::new(manager);
self.catalog_manager = Some(refcell);
}
// This is safe because we just checked that catalog_manager is Some
Ok(self.catalog_manager.as_ref().unwrap().borrow_mut())
}
/// URL encode a string for use in a filename
fn url_encode(s: &str) -> String {
let mut result = String::new();
for c in s.chars() {
match c {
'a'..='z' | 'A'..='Z' | '0'..='9' | '-' | '_' | '.' | '~' => result.push(c),
' ' => result.push('+'),
_ => {
result.push('%');
result.push_str(&format!("{:02X}", c as u8));
}
}
}
result
}
/// Build a search index for a publisher /// Build a search index for a publisher
fn build_search_index(&self, publisher: &str) -> Result<()> { fn build_search_index(&self, publisher: &str) -> Result<()> {
info!("Building search index for publisher: {}", publisher); info!("Building search index for publisher: {}", publisher);

View file

@ -330,10 +330,10 @@ pub trait WritableRepository {
) -> Result<()>; ) -> Result<()>;
/// Rebuild repository metadata /// Rebuild repository metadata
fn rebuild(&mut self, publisher: Option<&str>, no_catalog: bool, no_index: bool) -> Result<()>; fn rebuild(&self, publisher: Option<&str>, no_catalog: bool, no_index: bool) -> Result<()>;
/// Refresh repository metadata /// Refresh repository metadata
fn refresh(&mut self, publisher: Option<&str>, no_catalog: bool, no_index: bool) -> Result<()>; fn refresh(&self, publisher: Option<&str>, no_catalog: bool, no_index: bool) -> Result<()>;
/// Set the default publisher for the repository /// Set the default publisher for the repository
fn set_default_publisher(&mut self, publisher: &str) -> Result<()>; fn set_default_publisher(&mut self, publisher: &str) -> Result<()>;

View file

@ -88,10 +88,50 @@ impl WritableRepository for RestBackend {
Ok(()) Ok(())
} }
/// Rebuild repository metadata /// Set a repository property
fn rebuild(&mut self, publisher: Option<&str>, no_catalog: bool, no_index: bool) -> Result<()> { fn set_property(&mut self, property: &str, value: &str) -> Result<()> {
// This is a stub implementation // This is a stub implementation
// In a real implementation, we would make a REST API call to rebuild metadata // In a real implementation, we would make a REST API call to set the property
self.config
.properties
.insert(property.to_string(), value.to_string());
self.save_config()?;
Ok(())
}
/// Set a publisher property
fn set_publisher_property(
&mut self,
publisher: &str,
property: &str,
value: &str,
) -> Result<()> {
// This is a stub implementation
// In a real implementation, we would make a REST API call to set the publisher property
// Check if the publisher exists
if !self.config.publishers.contains(&publisher.to_string()) {
return Err(RepositoryError::PublisherNotFound(publisher.to_string()));
}
// Create the property key in the format "publisher/property"
let key = format!("{}/{}", publisher, property);
// Set the property
self.config.properties.insert(key, value.to_string());
// Save the updated configuration
self.save_config()?;
Ok(())
}
/// Rebuild repository metadata
fn rebuild(&self, publisher: Option<&str>, no_catalog: bool, no_index: bool) -> Result<()> {
// This is a stub implementation
// In a real implementation; we would make a REST API call to rebuild metadata
// Filter publishers if specified // Filter publishers if specified
let publishers = if let Some(pub_name) = publisher { let publishers = if let Some(pub_name) = publisher {
@ -122,7 +162,7 @@ impl WritableRepository for RestBackend {
} }
/// Refresh repository metadata /// Refresh repository metadata
fn refresh(&mut self, publisher: Option<&str>, no_catalog: bool, no_index: bool) -> Result<()> { fn refresh(&self, publisher: Option<&str>, no_catalog: bool, no_index: bool) -> Result<()> {
// This is a stub implementation // This is a stub implementation
// In a real implementation, we would make a REST API call to refresh metadata // In a real implementation, we would make a REST API call to refresh metadata
@ -172,46 +212,6 @@ impl WritableRepository for RestBackend {
Ok(()) Ok(())
} }
/// Set a repository property
fn set_property(&mut self, property: &str, value: &str) -> Result<()> {
// This is a stub implementation
// In a real implementation, we would make a REST API call to set the property
self.config
.properties
.insert(property.to_string(), value.to_string());
self.save_config()?;
Ok(())
}
/// Set a publisher property
fn set_publisher_property(
&mut self,
publisher: &str,
property: &str,
value: &str,
) -> Result<()> {
// This is a stub implementation
// In a real implementation, we would make a REST API call to set the publisher property
// Check if the publisher exists
if !self.config.publishers.contains(&publisher.to_string()) {
return Err(RepositoryError::PublisherNotFound(publisher.to_string()));
}
// Create the property key in the format "publisher/property"
let key = format!("{}/{}", publisher, property);
// Set the property
self.config.properties.insert(key, value.to_string());
// Save the updated configuration
self.save_config()?;
Ok(())
}
} }
impl ReadableRepository for RestBackend { impl ReadableRepository for RestBackend {

View file

@ -426,7 +426,7 @@ fn publish_package(
// Open the repository // Open the repository
info!("Opening repository at: {}", repo_path.display()); info!("Opening repository at: {}", repo_path.display());
let mut repo = match FileBackend::open(repo_path) { let repo = match FileBackend::open(repo_path) {
Ok(repo) => repo, Ok(repo) => repo,
Err(_) => { Err(_) => {
info!("Repository does not exist, creating a new one..."); info!("Repository does not exist, creating a new one...");

View file

@ -868,7 +868,7 @@ fn main() -> Result<()> {
// Open the repository // Open the repository
// In a real implementation with RestBackend, the key and cert parameters would be used for SSL authentication // In a real implementation with RestBackend, the key and cert parameters would be used for SSL authentication
// For now, we're using FileBackend, which doesn't use these parameters // For now, we're using FileBackend, which doesn't use these parameters
let mut repo = FileBackend::open(repo_uri_or_path)?; let repo = FileBackend::open(repo_uri_or_path)?;
// Get the publisher if specified // Get the publisher if specified
let pub_option = if let Some(publishers) = publisher { let pub_option = if let Some(publishers) = publisher {
@ -899,7 +899,7 @@ fn main() -> Result<()> {
// Open the repository // Open the repository
// In a real implementation with RestBackend, the key and cert parameters would be used for SSL authentication // In a real implementation with RestBackend, the key and cert parameters would be used for SSL authentication
// For now, we're using FileBackend, which doesn't use these parameters // For now, we're using FileBackend, which doesn't use these parameters
let mut repo = FileBackend::open(repo_uri_or_path)?; let repo = FileBackend::open(repo_uri_or_path)?;
// Get the publisher if specified // Get the publisher if specified
let pub_option = if let Some(publishers) = publisher { let pub_option = if let Some(publishers) = publisher {