diff options
Diffstat (limited to 'crates/shirabe')
34 files changed, 188 insertions, 164 deletions
diff --git a/crates/shirabe/src/command/show_command.rs b/crates/shirabe/src/command/show_command.rs index c4519924..bc36869c 100644 --- a/crates/shirabe/src/command/show_command.rs +++ b/crates/shirabe/src/command/show_command.rs @@ -768,7 +768,7 @@ impl Command for ShowCommand { self.print_package_info_as_json( package.clone(), &versions_map, - &*installed_repo.borrow(), + &mut *installed_repo.borrow_mut(), latest_package, )?; } else { @@ -1696,7 +1696,7 @@ impl ShowCommand { } // select an exact match if it is in the installed repo and no specific version was required - if version.is_null() && installed_repo.has_package(p.clone()) { + if version.is_null() && installed_repo.has_package(p.clone())? { matched_package = Some(p.clone()); } @@ -1766,7 +1766,7 @@ impl ShowCommand { latest_package: Option<PackageInterfaceHandle>, ) -> anyhow::Result<()> { let is_installed_package = !PlatformRepository::is_platform_package(&package.get_name()) - && installed_repo.has_package(package.clone().into()); + && installed_repo.has_package(package.clone().into())?; self.get_io().write(&format!( "<info>name</info> : {}", @@ -2022,7 +2022,7 @@ impl ShowCommand { &self, package: CompletePackageInterfaceHandle, versions: &IndexMap<String, String>, - installed_repo: &dyn RepositoryInterface, + installed_repo: &mut dyn RepositoryInterface, latest_package: Option<PackageInterfaceHandle>, ) -> anyhow::Result<()> { let mut json: IndexMap<String, PhpMixed> = IndexMap::new(); @@ -2113,7 +2113,7 @@ impl ShowCommand { } if !PlatformRepository::is_platform_package(&package.get_name()) - && installed_repo.has_package(package.clone().into()) + && installed_repo.has_package(package.clone().into())? { let composer = self.require_composer(None, None)?; let installation_manager = composer.borrow_partial().get_installation_manager(); diff --git a/crates/shirabe/src/installer/installation_manager.rs b/crates/shirabe/src/installer/installation_manager.rs index 9517eada..3a04a8e9 100644 --- a/crates/shirabe/src/installer/installation_manager.rs +++ b/crates/shirabe/src/installer/installation_manager.rs @@ -198,22 +198,21 @@ impl InstallationManager { /// Checks whether provided package is installed in one of the registered installers. pub fn is_package_installed( &self, - repo: &dyn InstalledRepositoryInterface, + repo: &mut dyn InstalledRepositoryInterface, package: PackageInterfaceHandle, ) -> anyhow::Result<bool> { // For testing only (ref InstallationManagerMock::isPackageInstalled). if self.mock.is_some() { - return Ok(repo.has_package(package)); + return repo.has_package(package); } if let Some(alias) = package.as_alias() { let alias_of: PackageInterfaceHandle = alias.get_alias_of().into(); - return Ok(repo.has_package(package) && self.is_package_installed(repo, alias_of)?); + return Ok(repo.has_package(package)? && self.is_package_installed(repo, alias_of)?); } - Ok(self - .get_installer(&package.get_type())? - .is_installed(repo, package)) + self.get_installer(&package.get_type())? + .is_installed(repo, package) } /// Install binary for the given package. @@ -264,7 +263,7 @@ impl InstallationManager { mock.updated.push((initial.clone(), target.clone())); mock.trace.push(trace); repo.remove_package(initial); - if !repo.has_package(target.clone()) { + if !repo.has_package(target.clone())? { repo.add_package(PackageInterfaceHandle::dup(&target)); } } @@ -278,7 +277,7 @@ impl InstallationManager { let package: PackageInterfaceHandle = op.get_package().into(); mock.installed.push(package.clone()); mock.trace.push(trace); - if !repo.has_package(package.clone()) { + if !repo.has_package(package.clone())? { repo.add_package(PackageInterfaceHandle::dup(&package)); } } @@ -578,7 +577,7 @@ impl InstallationManager { } match &operation { AnyOperation::MarkAliasInstalled(op) => { - self.mark_alias_installed(&mut **repo.borrow_mut(), op); + self.mark_alias_installed(&mut **repo.borrow_mut(), op)?; } AnyOperation::MarkAliasUninstalled(op) => { self.mark_alias_uninstalled(&mut **repo.borrow_mut(), op); @@ -760,12 +759,14 @@ impl InstallationManager { &self, repo: &mut dyn InstalledRepositoryInterface, operation: &MarkAliasInstalledOperation, - ) { + ) -> anyhow::Result<()> { let package = operation.get_package(); - if !repo.has_package(package.clone().into()) { + if !repo.has_package(package.clone().into())? { repo.add_package(crate::package::PackageInterfaceHandle::dup(&package.into())); } + + Ok(()) } /// Executes markAlias operation. @@ -1016,7 +1017,7 @@ pub trait InstallationManagerInterface: std::fmt::Debug { fn disable_plugins(&mut self); fn is_package_installed( &mut self, - repo: &dyn InstalledRepositoryInterface, + repo: &mut dyn InstalledRepositoryInterface, package: PackageInterfaceHandle, ) -> anyhow::Result<bool>; fn ensure_binaries_presence(&mut self, package: PackageInterfaceHandle); @@ -1052,7 +1053,7 @@ impl InstallationManagerInterface for InstallationManager { fn is_package_installed( &mut self, - repo: &dyn InstalledRepositoryInterface, + repo: &mut dyn InstalledRepositoryInterface, package: PackageInterfaceHandle, ) -> anyhow::Result<bool> { InstallationManager::is_package_installed(self, repo, package) diff --git a/crates/shirabe/src/installer/installer_interface.rs b/crates/shirabe/src/installer/installer_interface.rs index 6edaddb7..f2c0cd7c 100644 --- a/crates/shirabe/src/installer/installer_interface.rs +++ b/crates/shirabe/src/installer/installer_interface.rs @@ -12,9 +12,9 @@ pub trait InstallerInterface: std::fmt::Debug { fn is_installed( &self, - repo: &dyn InstalledRepositoryInterface, + repo: &mut dyn InstalledRepositoryInterface, package: PackageInterfaceHandle, - ) -> bool; + ) -> anyhow::Result<bool>; async fn download( &self, diff --git a/crates/shirabe/src/installer/library_installer.rs b/crates/shirabe/src/installer/library_installer.rs index 8efd985a..976c5a1f 100644 --- a/crates/shirabe/src/installer/library_installer.rs +++ b/crates/shirabe/src/installer/library_installer.rs @@ -241,32 +241,32 @@ impl InstallerInterface for LibraryInstaller { fn is_installed( &self, - repo: &dyn InstalledRepositoryInterface, + repo: &mut dyn InstalledRepositoryInterface, package: PackageInterfaceHandle, - ) -> bool { - if !repo.has_package(package.clone()) { - return false; + ) -> anyhow::Result<bool> { + if !repo.has_package(package.clone())? { + return Ok(false); } let install_path = self.get_install_path(package).unwrap(); if Filesystem::is_readable(&install_path) { - return true; + return Ok(true); } if Platform::is_windows() && self.filesystem.borrow_mut().is_junction(&install_path) { - return true; + return Ok(true); } if is_link(&install_path) { if realpath(&install_path).is_none() { - return false; + return Ok(false); } - return true; + return Ok(true); } - false + Ok(false) } async fn download( @@ -322,7 +322,9 @@ impl InstallerInterface for LibraryInstaller { let download_path = self.get_install_path(package.clone()).unwrap(); // remove the binaries if it appears the package files are missing - if !Filesystem::is_readable(&download_path) && repo.borrow().has_package(package.clone()) { + if !Filesystem::is_readable(&download_path) + && repo.borrow_mut().has_package(package.clone())? + { self.binary_installer .borrow_mut() .remove_binaries(package.clone()); @@ -335,7 +337,7 @@ impl InstallerInterface for LibraryInstaller { .borrow_mut() .install_binaries(package.clone(), &install_path, true); let mut repo = repo.borrow_mut(); - if !repo.has_package(package.clone()) { + if !repo.has_package(package.clone())? { repo.add_package(PackageInterfaceHandle::dup(&package)); } @@ -348,7 +350,7 @@ impl InstallerInterface for LibraryInstaller { initial: PackageInterfaceHandle, target: PackageInterfaceHandle, ) -> anyhow::Result<Option<PhpMixed>> { - if !repo.borrow().has_package(initial.clone()) { + if !repo.borrow_mut().has_package(initial.clone())? { return Err(InvalidArgumentException { message: format!("Package is not installed: {}", initial), code: 0, @@ -369,7 +371,7 @@ impl InstallerInterface for LibraryInstaller { .install_binaries(target.clone(), &install_path, true); let mut repo = repo.borrow_mut(); repo.remove_package(initial.clone()); - if !repo.has_package(target.clone()) { + if !repo.has_package(target.clone())? { repo.add_package(PackageInterfaceHandle::dup(&target)); } @@ -381,7 +383,7 @@ impl InstallerInterface for LibraryInstaller { repo: &std::cell::RefCell<&mut dyn InstalledRepositoryInterface>, package: PackageInterfaceHandle, ) -> anyhow::Result<Option<PhpMixed>> { - if !repo.borrow().has_package(package.clone()) { + if !repo.borrow_mut().has_package(package.clone())? { return Err(InvalidArgumentException { message: format!("Package is not installed: {}", package), code: 0, diff --git a/crates/shirabe/src/installer/metapackage_installer.rs b/crates/shirabe/src/installer/metapackage_installer.rs index cb6d13c3..26032a6d 100644 --- a/crates/shirabe/src/installer/metapackage_installer.rs +++ b/crates/shirabe/src/installer/metapackage_installer.rs @@ -30,9 +30,9 @@ impl InstallerInterface for MetapackageInstaller { fn is_installed( &self, - repo: &dyn InstalledRepositoryInterface, + repo: &mut dyn InstalledRepositoryInterface, package: PackageInterfaceHandle, - ) -> bool { + ) -> anyhow::Result<bool> { repo.has_package(package) } @@ -85,7 +85,7 @@ impl InstallerInterface for MetapackageInstaller { initial: PackageInterfaceHandle, target: PackageInterfaceHandle, ) -> anyhow::Result<Option<PhpMixed>> { - if !repo.borrow().has_package(initial.clone()) { + if !repo.borrow_mut().has_package(initial.clone())? { return Err(InvalidArgumentException { message: format!("Package is not installed: {}", initial), code: 0, @@ -114,7 +114,7 @@ impl InstallerInterface for MetapackageInstaller { repo: &std::cell::RefCell<&mut dyn InstalledRepositoryInterface>, package: PackageInterfaceHandle, ) -> anyhow::Result<Option<PhpMixed>> { - if !repo.borrow().has_package(package.clone()) { + if !repo.borrow_mut().has_package(package.clone())? { return Err(InvalidArgumentException { message: format!("Package is not installed: {}", package), code: 0, diff --git a/crates/shirabe/src/installer/noop_installer.rs b/crates/shirabe/src/installer/noop_installer.rs index c83bf674..d76ad4ad 100644 --- a/crates/shirabe/src/installer/noop_installer.rs +++ b/crates/shirabe/src/installer/noop_installer.rs @@ -16,9 +16,9 @@ impl InstallerInterface for NoopInstaller { fn is_installed( &self, - repo: &dyn InstalledRepositoryInterface, + repo: &mut dyn InstalledRepositoryInterface, package: PackageInterfaceHandle, - ) -> bool { + ) -> anyhow::Result<bool> { repo.has_package(package) } @@ -54,7 +54,7 @@ impl InstallerInterface for NoopInstaller { package: PackageInterfaceHandle, ) -> anyhow::Result<Option<PhpMixed>> { let mut repo = repo.borrow_mut(); - if !repo.has_package(package.clone()) { + if !repo.has_package(package.clone())? { repo.add_package(PackageInterfaceHandle::dup(&package)); } @@ -68,7 +68,7 @@ impl InstallerInterface for NoopInstaller { target: PackageInterfaceHandle, ) -> anyhow::Result<Option<PhpMixed>> { let mut repo = repo.borrow_mut(); - if !repo.has_package(initial.clone()) { + if !repo.has_package(initial.clone())? { return Err(InvalidArgumentException { message: format!("Package is not installed: {}", initial), code: 0, @@ -77,7 +77,7 @@ impl InstallerInterface for NoopInstaller { } repo.remove_package(initial); - if !repo.has_package(target.clone()) { + if !repo.has_package(target.clone())? { repo.add_package(PackageInterfaceHandle::dup(&target)); } @@ -90,7 +90,7 @@ impl InstallerInterface for NoopInstaller { package: PackageInterfaceHandle, ) -> anyhow::Result<Option<PhpMixed>> { let mut repo = repo.borrow_mut(); - if !repo.has_package(package.clone()) { + if !repo.has_package(package.clone())? { return Err(InvalidArgumentException { message: format!("Package is not installed: {}", package), code: 0, diff --git a/crates/shirabe/src/installer/plugin_installer.rs b/crates/shirabe/src/installer/plugin_installer.rs index 86c579b2..c0473fd0 100644 --- a/crates/shirabe/src/installer/plugin_installer.rs +++ b/crates/shirabe/src/installer/plugin_installer.rs @@ -79,9 +79,9 @@ impl InstallerInterface for PluginInstaller { fn is_installed( &self, - repo: &dyn InstalledRepositoryInterface, + repo: &mut dyn InstalledRepositoryInterface, package: PackageInterfaceHandle, - ) -> bool { + ) -> anyhow::Result<bool> { self.inner.is_installed(repo, package) } diff --git a/crates/shirabe/src/installer/project_installer.rs b/crates/shirabe/src/installer/project_installer.rs index 0f2c3ba6..628e4252 100644 --- a/crates/shirabe/src/installer/project_installer.rs +++ b/crates/shirabe/src/installer/project_installer.rs @@ -37,10 +37,10 @@ impl InstallerInterface for ProjectInstaller { fn is_installed( &self, - _repo: &dyn InstalledRepositoryInterface, + _repo: &mut dyn InstalledRepositoryInterface, _package: PackageInterfaceHandle, - ) -> bool { - false + ) -> anyhow::Result<bool> { + Ok(false) } async fn download( diff --git a/crates/shirabe/src/repository/array_repository.rs b/crates/shirabe/src/repository/array_repository.rs index 09c37766..30598b62 100644 --- a/crates/shirabe/src/repository/array_repository.rs +++ b/crates/shirabe/src/repository/array_repository.rs @@ -205,6 +205,16 @@ impl ArrayRepository { *self.packages.borrow_mut() = Some(vec![]); } + /// Shared body of `RepositoryInterface::count`, kept on `&self` for `get_repo_name` (PHP's + /// `getRepoName` also triggers lazy initialization through `count()`). + pub(crate) fn base_count(&self) -> usize { + if self.packages.borrow().is_none() { + self.initialize(); + } + + self.packages.borrow().as_ref().unwrap().len() + } + /// Resets the packages cache so the next access re-runs `initialize`. pub(crate) fn reset_packages(&self) { *self.packages.borrow_mut() = None; @@ -217,16 +227,12 @@ impl ArrayRepository { impl RepositoryInterface for ArrayRepository { /// Returns the number of packages in this repository - fn count(&self) -> anyhow::Result<usize> { - if self.packages.borrow().is_none() { - self.initialize(); - } - - Ok(self.packages.borrow().as_ref().unwrap().len()) + fn count(&mut self) -> anyhow::Result<usize> { + Ok(self.base_count()) } fn get_repo_name(&self) -> String { - let count = self.count().expect("ArrayRepository::count is infallible"); + let count = self.base_count(); format!( "array repo (defining {} package{})", count, @@ -409,7 +415,7 @@ impl RepositoryInterface for ArrayRepository { Ok(matches.into_values().collect()) } - fn has_package(&self, package: PackageInterfaceHandle) -> bool { + fn has_package(&mut self, package: PackageInterfaceHandle) -> anyhow::Result<bool> { if self.package_map.borrow().is_none() { let mut map: IndexMap<String, BasePackageHandle> = IndexMap::new(); for repo_package in self.get_packages_internal() { @@ -418,11 +424,12 @@ impl RepositoryInterface for ArrayRepository { *self.package_map.borrow_mut() = Some(map); } - self.package_map + Ok(self + .package_map .borrow() .as_ref() .unwrap() - .contains_key(&package.get_unique_name()) + .contains_key(&package.get_unique_name())) } fn get_providers( diff --git a/crates/shirabe/src/repository/artifact_repository.rs b/crates/shirabe/src/repository/artifact_repository.rs index 7d9c02b6..f18f7f04 100644 --- a/crates/shirabe/src/repository/artifact_repository.rs +++ b/crates/shirabe/src/repository/artifact_repository.rs @@ -247,16 +247,13 @@ impl RepositoryInterface for ArtifactRepository { // The structural methods are inherited from ArrayRepository in PHP, where the lazy directory // scan is driven by the overridden initialize(). Here each one first ensures that scan has // happened (see ensure_initialized), then delegates to the inner ArrayRepository. - fn count(&self) -> anyhow::Result<usize> { + fn count(&mut self) -> anyhow::Result<usize> { self.ensure_initialized()?; self.inner.count() } - fn has_package(&self, package: PackageInterfaceHandle) -> bool { - // TODO(phase-d): hasPackage returns bool and cannot surface an initialization error; a - // failed scan leaves the inner repository with whatever packages were added before the - // failure. - let _ = self.ensure_initialized(); + fn has_package(&mut self, package: PackageInterfaceHandle) -> anyhow::Result<bool> { + self.ensure_initialized()?; self.inner.has_package(package) } diff --git a/crates/shirabe/src/repository/composer_repository.rs b/crates/shirabe/src/repository/composer_repository.rs index 558d7e00..ba68d22c 100644 --- a/crates/shirabe/src/repository/composer_repository.rs +++ b/crates/shirabe/src/repository/composer_repository.rs @@ -79,15 +79,6 @@ pub struct ProviderListingEntry { #[derive(Debug)] pub struct ComposerRepository { - // TODO(phase-c): PHP's ArrayRepository methods that aren't overridden here (findPackage, - // findPackages, count, hasPackage) call $this->getPackages(), which virtual-dispatches back - // to ComposerRepository::getPackages() (see its "embedded inheritance does not dispatch back - // to the wrapper" comment below, and the identical guard in load_packages()). Composition - // doesn't get that dispatch for free: self.inner.find_package()/find_packages()/count()/ - // has_package() below call ArrayRepository::initialize() (a no-op stub) instead, and will - // silently see an empty package list if called before get_packages()/load_packages() has - // run once on this instance. Not yet known to be hit by any test; audit call sites and add - // the same `if !self.inner.is_initialized() { self.initialize()?; }` guard where needed. inner: ArrayRepository, /// Weak reference to the outermost repository handle wrapping this `ComposerRepository`, /// injected via `set_self_handle`. Used to wire package -> repository back-references. @@ -3409,11 +3400,20 @@ fn clone_root_data(rd: &RootData) -> RootData { } impl RepositoryInterface for ComposerRepository { - fn count(&self) -> anyhow::Result<usize> { + // PHP's ArrayRepository::count()/hasPackage() call $this->initialize(), which + // virtual-dispatches to ComposerRepository::initialize(); the guard restores that + // (same guard as in get_packages()). + fn count(&mut self) -> anyhow::Result<usize> { + if !self.inner.is_initialized() { + self.initialize()?; + } self.inner.count() } - fn has_package(&self, package: PackageInterfaceHandle) -> bool { + fn has_package(&mut self, package: PackageInterfaceHandle) -> anyhow::Result<bool> { + if !self.inner.is_initialized() { + self.initialize()?; + } self.inner.has_package(package) } diff --git a/crates/shirabe/src/repository/composite_repository.rs b/crates/shirabe/src/repository/composite_repository.rs index 9e5c9757..a9c55a0f 100644 --- a/crates/shirabe/src/repository/composite_repository.rs +++ b/crates/shirabe/src/repository/composite_repository.rs @@ -60,7 +60,7 @@ impl CompositeRepository { } impl RepositoryInterface for CompositeRepository { - fn count(&self) -> anyhow::Result<usize> { + fn count(&mut self) -> anyhow::Result<usize> { let mut total = 0; for repository in &self.repositories { total += repository.count()?; @@ -78,13 +78,13 @@ impl RepositoryInterface for CompositeRepository { format!("composite repo ({})", names.join(", ")) } - fn has_package(&self, package: PackageInterfaceHandle) -> bool { + fn has_package(&mut self, package: PackageInterfaceHandle) -> anyhow::Result<bool> { for repository in &self.repositories { - if repository.has_package(package.clone()) { - return true; + if repository.has_package(package.clone())? { + return Ok(true); } } - false + Ok(false) } fn find_package( diff --git a/crates/shirabe/src/repository/filesystem_repository.rs b/crates/shirabe/src/repository/filesystem_repository.rs index d4b9b72e..9186a836 100644 --- a/crates/shirabe/src/repository/filesystem_repository.rs +++ b/crates/shirabe/src/repository/filesystem_repository.rs @@ -716,11 +716,15 @@ impl FilesystemRepository { } impl RepositoryInterface for FilesystemRepository { - fn count(&self) -> anyhow::Result<usize> { + // PHP's ArrayRepository::count()/hasPackage() call $this->initialize(), which + // virtual-dispatches to FilesystemRepository::initialize(); the guard restores that. + fn count(&mut self) -> anyhow::Result<usize> { + self.ensure_initialized()?; self.inner.count() } - fn has_package(&self, package: PackageInterfaceHandle) -> bool { + fn has_package(&mut self, package: PackageInterfaceHandle) -> anyhow::Result<bool> { + self.ensure_initialized()?; self.inner.has_package(package) } diff --git a/crates/shirabe/src/repository/filter_repository.rs b/crates/shirabe/src/repository/filter_repository.rs index bdf17c54..32a32ba1 100644 --- a/crates/shirabe/src/repository/filter_repository.rs +++ b/crates/shirabe/src/repository/filter_repository.rs @@ -148,7 +148,7 @@ impl FilterRepository { } impl RepositoryInterface for FilterRepository { - fn count(&self) -> anyhow::Result<usize> { + fn count(&mut self) -> anyhow::Result<usize> { if self.repo.count()? > 0 { Ok(self .repo @@ -161,7 +161,7 @@ impl RepositoryInterface for FilterRepository { } } - fn has_package(&self, package: PackageInterfaceHandle) -> bool { + fn has_package(&mut self, package: PackageInterfaceHandle) -> anyhow::Result<bool> { self.repo.has_package(package) } diff --git a/crates/shirabe/src/repository/handle.rs b/crates/shirabe/src/repository/handle.rs index f093f66b..497e2b91 100644 --- a/crates/shirabe/src/repository/handle.rs +++ b/crates/shirabe/src/repository/handle.rs @@ -84,7 +84,7 @@ impl RepositoryInterfaceHandle { } pub fn count(&self) -> anyhow::Result<usize> { - self.0.borrow().count() + self.0.borrow_mut().count() } pub fn get_repo_name(&self) -> String { @@ -95,8 +95,8 @@ impl RepositoryInterfaceHandle { self.0.borrow_mut().get_packages() } - pub fn has_package(&self, package: PackageInterfaceHandle) -> bool { - self.0.borrow().has_package(package) + pub fn has_package(&self, package: PackageInterfaceHandle) -> anyhow::Result<bool> { + self.0.borrow_mut().has_package(package) } pub fn find_package( diff --git a/crates/shirabe/src/repository/installed_array_repository.rs b/crates/shirabe/src/repository/installed_array_repository.rs index dd0f7d38..b458ff61 100644 --- a/crates/shirabe/src/repository/installed_array_repository.rs +++ b/crates/shirabe/src/repository/installed_array_repository.rs @@ -39,10 +39,7 @@ impl InstalledRepositoryInterface for InstalledArrayRepository { } fn is_fresh(&self) -> bool { - self.inner - .count() - .expect("WritableArrayRepository::count is infallible") - == 0 + self.inner.base_count() == 0 } } @@ -82,11 +79,11 @@ impl WritableRepositoryInterface for InstalledArrayRepository { } impl RepositoryInterface for InstalledArrayRepository { - fn count(&self) -> anyhow::Result<usize> { + fn count(&mut self) -> anyhow::Result<usize> { self.inner.count() } - fn has_package(&self, package: PackageInterfaceHandle) -> bool { + fn has_package(&mut self, package: PackageInterfaceHandle) -> anyhow::Result<bool> { self.inner.has_package(package) } fn find_package( diff --git a/crates/shirabe/src/repository/installed_filesystem_repository.rs b/crates/shirabe/src/repository/installed_filesystem_repository.rs index 7fab762a..85fb514b 100644 --- a/crates/shirabe/src/repository/installed_filesystem_repository.rs +++ b/crates/shirabe/src/repository/installed_filesystem_repository.rs @@ -113,11 +113,11 @@ impl WritableRepositoryInterface for InstalledFilesystemRepository { } impl RepositoryInterface for InstalledFilesystemRepository { - fn count(&self) -> anyhow::Result<usize> { + fn count(&mut self) -> anyhow::Result<usize> { self.inner.count() } - fn has_package(&self, package: PackageInterfaceHandle) -> bool { + fn has_package(&mut self, package: PackageInterfaceHandle) -> anyhow::Result<bool> { self.inner.has_package(package) } fn find_package( diff --git a/crates/shirabe/src/repository/installed_repository.rs b/crates/shirabe/src/repository/installed_repository.rs index 388670d4..08b1f36b 100644 --- a/crates/shirabe/src/repository/installed_repository.rs +++ b/crates/shirabe/src/repository/installed_repository.rs @@ -376,7 +376,7 @@ impl InstalledRepository { } impl RepositoryInterface for InstalledRepository { - fn count(&self) -> anyhow::Result<usize> { + fn count(&mut self) -> anyhow::Result<usize> { self.inner.count() } @@ -390,7 +390,7 @@ impl RepositoryInterface for InstalledRepository { format!("installed repo ({})", names.join(", ")) } - fn has_package(&self, package: PackageInterfaceHandle) -> bool { + fn has_package(&mut self, package: PackageInterfaceHandle) -> anyhow::Result<bool> { self.inner.has_package(package) } diff --git a/crates/shirabe/src/repository/lock_array_repository.rs b/crates/shirabe/src/repository/lock_array_repository.rs index 390fae38..8b909fbc 100644 --- a/crates/shirabe/src/repository/lock_array_repository.rs +++ b/crates/shirabe/src/repository/lock_array_repository.rs @@ -31,11 +31,11 @@ impl LockArrayRepository { } impl RepositoryInterface for LockArrayRepository { - fn count(&self) -> anyhow::Result<usize> { + fn count(&mut self) -> anyhow::Result<usize> { self.inner.count() } - fn has_package(&self, package: PackageInterfaceHandle) -> bool { + fn has_package(&mut self, package: PackageInterfaceHandle) -> anyhow::Result<bool> { self.inner.has_package(package) } diff --git a/crates/shirabe/src/repository/package_repository.rs b/crates/shirabe/src/repository/package_repository.rs index 3397f071..06b28451 100644 --- a/crates/shirabe/src/repository/package_repository.rs +++ b/crates/shirabe/src/repository/package_repository.rs @@ -108,16 +108,13 @@ impl RepositoryInterface for PackageRepository { // The structural methods are inherited from ArrayRepository in PHP, where the lazy package load // is driven by the overridden initialize(). Here each one first ensures that load has happened // (see ensure_initialized), then delegates to the inner ArrayRepository. - fn count(&self) -> anyhow::Result<usize> { + fn count(&mut self) -> anyhow::Result<usize> { self.ensure_initialized()?; self.inner.count() } - fn has_package(&self, package: PackageInterfaceHandle) -> bool { - // TODO(phase-d): hasPackage returns bool and cannot surface an initialization error; a - // failed load leaves the inner repository with whatever packages were added before the - // failure. - let _ = self.ensure_initialized(); + fn has_package(&mut self, package: PackageInterfaceHandle) -> anyhow::Result<bool> { + self.ensure_initialized()?; self.inner.has_package(package) } diff --git a/crates/shirabe/src/repository/path_repository.rs b/crates/shirabe/src/repository/path_repository.rs index d7f24fa7..879b769c 100644 --- a/crates/shirabe/src/repository/path_repository.rs +++ b/crates/shirabe/src/repository/path_repository.rs @@ -141,7 +141,7 @@ impl PathRepository { ) -> anyhow::Result<bool> { self.initialize()?; use crate::repository::RepositoryInterface; - Ok(self.inner.has_package(package)) + self.inner.has_package(package) } // In PHP the inherited ArrayRepository methods lazily call the overridden initialize() to glob @@ -397,16 +397,13 @@ impl RepositoryInterface for PathRepository { // The structural methods are inherited from ArrayRepository in PHP, where the lazy package load // is driven by the overridden initialize(). Here each one first ensures that load has happened // (see ensure_initialized), then delegates to the inner ArrayRepository. - fn count(&self) -> anyhow::Result<usize> { + fn count(&mut self) -> anyhow::Result<usize> { self.ensure_initialized()?; self.inner.count() } - fn has_package(&self, package: PackageInterfaceHandle) -> bool { - // TODO(phase-d): hasPackage returns bool and cannot surface an initialization error; a - // failed load leaves the inner repository with whatever packages were added before the - // failure. - let _ = self.ensure_initialized(); + fn has_package(&mut self, package: PackageInterfaceHandle) -> anyhow::Result<bool> { + self.ensure_initialized()?; self.inner.has_package(package) } diff --git a/crates/shirabe/src/repository/platform_repository.rs b/crates/shirabe/src/repository/platform_repository.rs index e4d93d3b..30164237 100644 --- a/crates/shirabe/src/repository/platform_repository.rs +++ b/crates/shirabe/src/repository/platform_repository.rs @@ -1866,11 +1866,15 @@ impl PlatformRepository { } impl crate::repository::RepositoryInterface for PlatformRepository { - fn count(&self) -> anyhow::Result<usize> { + // PHP's ArrayRepository::count()/hasPackage() call $this->initialize(), which + // virtual-dispatches to PlatformRepository::initialize(); the guard restores that. + fn count(&mut self) -> anyhow::Result<usize> { + self.ensure_initialized()?; self.inner.count() } - fn has_package(&self, package: PackageInterfaceHandle) -> bool { + fn has_package(&mut self, package: PackageInterfaceHandle) -> anyhow::Result<bool> { + self.ensure_initialized()?; self.inner.has_package(package) } diff --git a/crates/shirabe/src/repository/repository_interface.rs b/crates/shirabe/src/repository/repository_interface.rs index 6dbc01da..ab25242c 100644 --- a/crates/shirabe/src/repository/repository_interface.rs +++ b/crates/shirabe/src/repository/repository_interface.rs @@ -52,9 +52,13 @@ pub const SEARCH_NAME: i64 = 1; pub const SEARCH_VENDOR: i64 = 2; pub trait RepositoryInterface: std::fmt::Debug { - fn count(&self) -> anyhow::Result<usize>; + // count/has_package take &mut self (and has_package returns Result) because PHP's + // ArrayRepository::count()/hasPackage() late-bind $this->initialize() to the concrete + // repository class, which lazily loads packages and can throw; lazy repositories need the + // same guard here (see FilesystemRepository/PlatformRepository/ComposerRepository). + fn count(&mut self) -> anyhow::Result<usize>; - fn has_package(&self, package: PackageInterfaceHandle) -> bool; + fn has_package(&mut self, package: PackageInterfaceHandle) -> anyhow::Result<bool>; fn find_package( &mut self, diff --git a/crates/shirabe/src/repository/root_package_repository.rs b/crates/shirabe/src/repository/root_package_repository.rs index 4357ed69..7630f6c5 100644 --- a/crates/shirabe/src/repository/root_package_repository.rs +++ b/crates/shirabe/src/repository/root_package_repository.rs @@ -26,11 +26,11 @@ impl RootPackageRepository { } impl RepositoryInterface for RootPackageRepository { - fn count(&self) -> anyhow::Result<usize> { + fn count(&mut self) -> anyhow::Result<usize> { self.inner.count() } - fn has_package(&self, package: PackageInterfaceHandle) -> bool { + fn has_package(&mut self, package: PackageInterfaceHandle) -> anyhow::Result<bool> { self.inner.has_package(package) } diff --git a/crates/shirabe/src/repository/vcs_repository.rs b/crates/shirabe/src/repository/vcs_repository.rs index c04b1b67..b7ce1a6c 100644 --- a/crates/shirabe/src/repository/vcs_repository.rs +++ b/crates/shirabe/src/repository/vcs_repository.rs @@ -1053,16 +1053,13 @@ impl RepositoryInterface for VcsRepository { // The structural methods are inherited from ArrayRepository in PHP, where the lazy package load // is driven by the overridden initialize(). Here each one first ensures that load has happened // (see ensure_initialized), then delegates to the inner ArrayRepository. - fn count(&self) -> anyhow::Result<usize> { + fn count(&mut self) -> anyhow::Result<usize> { self.ensure_initialized()?; self.inner.count() } - fn has_package(&self, package: PackageInterfaceHandle) -> bool { - // TODO(phase-d): hasPackage returns bool and cannot surface an initialization error; a - // failed load leaves the inner repository with whatever packages were added before the - // failure. - let _ = self.ensure_initialized(); + fn has_package(&mut self, package: PackageInterfaceHandle) -> anyhow::Result<bool> { + self.ensure_initialized()?; self.inner.has_package(package) } diff --git a/crates/shirabe/src/repository/writable_array_repository.rs b/crates/shirabe/src/repository/writable_array_repository.rs index e415a1e1..f5db2bd9 100644 --- a/crates/shirabe/src/repository/writable_array_repository.rs +++ b/crates/shirabe/src/repository/writable_array_repository.rs @@ -31,6 +31,11 @@ impl WritableArrayRepository { self.dev_mode } + /// See `ArrayRepository::base_count`; kept on `&self` for `is_fresh` callers. + pub(crate) fn base_count(&self) -> usize { + self.inner.base_count() + } + pub fn set_dev_package_names(&mut self, dev_package_names: Vec<String>) { self.dev_package_names = dev_package_names; } @@ -124,11 +129,11 @@ impl WritableArrayRepository { } impl RepositoryInterface for WritableArrayRepository { - fn count(&self) -> anyhow::Result<usize> { + fn count(&mut self) -> anyhow::Result<usize> { self.inner.count() } - fn has_package(&self, package: PackageInterfaceHandle) -> bool { + fn has_package(&mut self, package: PackageInterfaceHandle) -> anyhow::Result<bool> { self.inner.has_package(package) } diff --git a/crates/shirabe/tests/advisory/auditor_test.rs b/crates/shirabe/tests/advisory/auditor_test.rs index adc90a4d..65e848dd 100644 --- a/crates/shirabe/tests/advisory/auditor_test.rs +++ b/crates/shirabe/tests/advisory/auditor_test.rs @@ -144,10 +144,10 @@ impl AdvisoryProviderInterface for MockAdvisoryRepository { } impl RepositoryInterface for MockAdvisoryRepository { - fn count(&self) -> anyhow::Result<usize> { + fn count(&mut self) -> anyhow::Result<usize> { unimplemented!("not used by Auditor") } - fn has_package(&self, _package: PackageInterfaceHandle) -> bool { + fn has_package(&mut self, _package: PackageInterfaceHandle) -> anyhow::Result<bool> { unimplemented!("not used by Auditor") } fn find_package( diff --git a/crates/shirabe/tests/autoload/autoload_generator_test.rs b/crates/shirabe/tests/autoload/autoload_generator_test.rs index 09ff2101..5f0629dd 100644 --- a/crates/shirabe/tests/autoload/autoload_generator_test.rs +++ b/crates/shirabe/tests/autoload/autoload_generator_test.rs @@ -39,10 +39,10 @@ impl InstallerInterface for InstallPathStubInstaller { fn is_installed( &self, - _repo: &dyn InstalledRepositoryInterface, + _repo: &mut dyn InstalledRepositoryInterface, _package: PackageInterfaceHandle, - ) -> bool { - true + ) -> anyhow::Result<bool> { + Ok(true) } async fn download( diff --git a/crates/shirabe/tests/installer/installation_manager_test.rs b/crates/shirabe/tests/installer/installation_manager_test.rs index 3f18e925..6cc231f5 100644 --- a/crates/shirabe/tests/installer/installation_manager_test.rs +++ b/crates/shirabe/tests/installer/installation_manager_test.rs @@ -75,10 +75,10 @@ impl InstallerInterface for MockInstaller { fn is_installed( &self, - _repo: &dyn InstalledRepositoryInterface, + _repo: &mut dyn InstalledRepositoryInterface, _package: PackageInterfaceHandle, - ) -> bool { - false + ) -> anyhow::Result<bool> { + Ok(false) } async fn download( @@ -175,10 +175,10 @@ impl InstallerInterface for BinaryInstaller { fn is_installed( &self, - _repo: &dyn InstalledRepositoryInterface, + _repo: &mut dyn InstalledRepositoryInterface, _package: PackageInterfaceHandle, - ) -> bool { - false + ) -> anyhow::Result<bool> { + Ok(false) } async fn download( diff --git a/crates/shirabe/tests/installer/library_installer_test.rs b/crates/shirabe/tests/installer/library_installer_test.rs index 8a063941..baa4f13a 100644 --- a/crates/shirabe/tests/installer/library_installer_test.rs +++ b/crates/shirabe/tests/installer/library_installer_test.rs @@ -200,19 +200,31 @@ fn test_is_installed() { let package = get_package("test/pkg", "1.0.0"); let mut repository = InstalledArrayRepository::new().unwrap(); - assert!(!library.is_installed(&repository, package.clone())); + assert!( + !library + .is_installed(&mut repository, package.clone()) + .unwrap() + ); // package being in repo is not enough to be installed repository.add_package(package.clone()).unwrap(); - assert!(!library.is_installed(&repository, package.clone())); + assert!( + !library + .is_installed(&mut repository, package.clone()) + .unwrap() + ); // package being in repo and vendor/pkg/foo dir present means it is seen as installed let pkg_dir = format!("{}/{}", setup.vendor_dir, package.get_pretty_name()); fs::create_dir_all(&pkg_dir).unwrap(); - assert!(library.is_installed(&repository, package.clone())); + assert!( + library + .is_installed(&mut repository, package.clone()) + .unwrap() + ); repository.remove_package(package.clone()).unwrap(); - assert!(!library.is_installed(&repository, package)); + assert!(!library.is_installed(&mut repository, package).unwrap()); tear_down(&mut setup); } @@ -249,7 +261,7 @@ fn test_install() { .unwrap(); // PHP asserts repository->addPackage was called once with $package. - assert!(repository.has_package(package)); + assert!(repository.has_package(package).unwrap()); assert!( std::path::Path::new(&setup.vendor_dir).exists(), @@ -317,8 +329,8 @@ fn test_update() { ); assert!(!std::path::Path::new(&old_target_dir).exists()); - assert!(!repository.has_package(initial.clone())); - assert!(repository.has_package(target.clone())); + assert!(!repository.has_package(initial.clone()).unwrap()); + assert!(repository.has_package(target.clone()).unwrap()); assert!( std::path::Path::new(&setup.vendor_dir).exists(), @@ -379,7 +391,7 @@ fn test_uninstall() { )) .unwrap(); - assert!(!repository.has_package(package.clone())); + assert!(!repository.has_package(package.clone()).unwrap()); // Uninstalling again, with the package no longer installed, fails. assert!( diff --git a/crates/shirabe/tests/installer/metapackage_installer_test.rs b/crates/shirabe/tests/installer/metapackage_installer_test.rs index 86013871..b87fdebd 100644 --- a/crates/shirabe/tests/installer/metapackage_installer_test.rs +++ b/crates/shirabe/tests/installer/metapackage_installer_test.rs @@ -30,7 +30,7 @@ fn test_install() { )) .unwrap(); - assert!(repository.has_package(package)); + assert!(repository.has_package(package).unwrap()); } #[test] @@ -50,8 +50,8 @@ fn test_update() { )) .unwrap(); - assert!(!repository.has_package(initial.clone())); - assert!(repository.has_package(target.clone())); + assert!(!repository.has_package(initial.clone()).unwrap()); + assert!(repository.has_package(target.clone()).unwrap()); // Updating again, with the initial package no longer installed, fails. assert!( @@ -81,7 +81,7 @@ fn test_uninstall() { )) .unwrap(); - assert!(!repository.has_package(package.clone())); + assert!(!repository.has_package(package.clone()).unwrap()); // Uninstalling again, with the package no longer installed, fails. assert!( diff --git a/crates/shirabe/tests/repository/array_repository_test.rs b/crates/shirabe/tests/repository/array_repository_test.rs index 069e9113..e3fbf93c 100644 --- a/crates/shirabe/tests/repository/array_repository_test.rs +++ b/crates/shirabe/tests/repository/array_repository_test.rs @@ -48,7 +48,7 @@ fn reprs(results: &[SearchResult]) -> Vec<(String, Option<String>, Abandoned)> { #[test] fn test_add_package() { - let repo = ArrayRepository::new(vec![]).unwrap(); + let mut repo = ArrayRepository::new(vec![]).unwrap(); repo.add_package(get_package("foo", "1")).unwrap(); assert_eq!(1, repo.count().unwrap()); @@ -74,12 +74,12 @@ fn test_remove_package() { #[test] fn test_has_package() { - let repo = ArrayRepository::new(vec![]).unwrap(); + let mut repo = ArrayRepository::new(vec![]).unwrap(); repo.add_package(get_package("foo", "1")).unwrap(); repo.add_package(get_package("bar", "2")).unwrap(); - assert!(repo.has_package(get_package("foo", "1"))); - assert!(!repo.has_package(get_package("bar", "1"))); + assert!(repo.has_package(get_package("foo", "1")).unwrap()); + assert!(!repo.has_package(get_package("bar", "1")).unwrap()); } #[test] @@ -100,7 +100,7 @@ fn test_find_packages() { #[test] fn test_automatically_add_aliased_package_but_not_remove() { - let repo = ArrayRepository::new(vec![]).unwrap(); + let mut repo = ArrayRepository::new(vec![]).unwrap(); let package = get_package("foo", "1"); let alias = get_alias_package(&package, "2"); @@ -108,8 +108,8 @@ fn test_automatically_add_aliased_package_but_not_remove() { repo.add_package(alias.clone()).unwrap(); assert_eq!(2, repo.count().unwrap()); - assert!(repo.has_package(get_package("foo", "1"))); - assert!(repo.has_package(get_package("foo", "2"))); + assert!(repo.has_package(get_package("foo", "1")).unwrap()); + assert!(repo.has_package(get_package("foo", "2")).unwrap()); repo.remove_package(alias); diff --git a/crates/shirabe/tests/repository/composite_repository_test.rs b/crates/shirabe/tests/repository/composite_repository_test.rs index 5fa0fb8d..60408b5d 100644 --- a/crates/shirabe/tests/repository/composite_repository_test.rs +++ b/crates/shirabe/tests/repository/composite_repository_test.rs @@ -13,16 +13,16 @@ fn array_repo(packages: Vec<PackageInterfaceHandle>) -> RepositoryInterfaceHandl #[test] fn test_has_package() { - let repo = CompositeRepository::new(vec![ + let mut repo = CompositeRepository::new(vec![ array_repo(vec![get_package("foo", "1")]), array_repo(vec![get_package("bar", "1")]), ]); - assert!(repo.has_package(get_package("foo", "1"))); - assert!(repo.has_package(get_package("bar", "1"))); + assert!(repo.has_package(get_package("foo", "1")).unwrap()); + assert!(repo.has_package(get_package("bar", "1")).unwrap()); - assert!(!repo.has_package(get_package("foo", "2"))); - assert!(!repo.has_package(get_package("bar", "2"))); + assert!(!repo.has_package(get_package("foo", "2")).unwrap()); + assert!(!repo.has_package(get_package("bar", "2")).unwrap()); } #[test] @@ -111,7 +111,7 @@ fn test_add_repository() { #[test] fn test_count() { - let repo = CompositeRepository::new(vec![ + let mut repo = CompositeRepository::new(vec![ array_repo(vec![get_package("foo", "1")]), array_repo(vec![get_package("bar", "1")]), ]); diff --git a/crates/shirabe/tests/repository/filesystem_repository_test.rs b/crates/shirabe/tests/repository/filesystem_repository_test.rs index 054a6d7c..c7b8f9a6 100644 --- a/crates/shirabe/tests/repository/filesystem_repository_test.rs +++ b/crates/shirabe/tests/repository/filesystem_repository_test.rs @@ -99,7 +99,7 @@ mockall::mock! { fn disable_plugins(&mut self); fn is_package_installed( &mut self, - repo: &dyn InstalledRepositoryInterface, + repo: &mut dyn InstalledRepositoryInterface, package: PackageInterfaceHandle, ) -> anyhow::Result<bool>; fn ensure_binaries_presence(&mut self, package: PackageInterfaceHandle); |