Skip to content

Commit b907b24

Browse files
brianheinemanbrianheineman
authored
Merge pull request #222 from gazure/ga/refactor-error-from-impls
refactor: reduce map_err by adding some From<Error> implementations
2 parents e439ac6 + 36c556a commit b907b24

File tree

10 files changed

+107
-87
lines changed

10 files changed

+107
-87
lines changed

‎postgresql_archive/src/configuration/zonky/extractor.rs‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,12 +43,10 @@ pub fn extract(bytes: &Vec<u8>, extract_directories: &ExtractDirectories) -> Res
4343
debug!("Extracting archive to {}", extract_dir.to_string_lossy());
4444

4545
let reader = Cursor::new(bytes);
46-
let mut archive = ZipArchive::new(reader).map_err(|error| Unexpected(error.to_string()))?;
46+
let mut archive = ZipArchive::new(reader)?;
4747
let mut archive_bytes = Vec::new();
4848
for i in 0..archive.len() {
49-
let mut file = archive
50-
.by_index(i)
51-
.map_err(|error| Unexpected(error.to_string()))?;
49+
let mut file = archive.by_index(i)?;
5250
let file_name = file.name().to_string();
5351
if file_name.ends_with(".txz") {
5452
debug!("Found archive file: {file_name}");

‎postgresql_archive/src/error.rs‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
use std::sync::PoisonError;
2+
13
/// PostgreSQL archive result type
24
pub type Result<T, E = Error> = core::result::Result<T, E>;
35

@@ -111,6 +113,29 @@ impl From<url::ParseError> for Error {
111113
}
112114
}
113115

116+
#[cfg(feature = "maven")]
117+
/// Converts a [`quick_xml::DeError`] into a [`ParseError`](Error::ParseError)
118+
impl From<quick_xml::DeError> for Error {
119+
fn from(error: quick_xml::DeError) -> Self {
120+
Error::ParseError(error.to_string())
121+
}
122+
}
123+
124+
#[cfg(feature = "zip")]
125+
/// Converts a [`zip::result::ZipError`] into a [`ParseError`](Error::Unexpected)
126+
impl From<zip::result::ZipError> for Error {
127+
fn from(error: zip::result::ZipError) -> Self {
128+
Error::Unexpected(error.to_string())
129+
}
130+
}
131+
132+
/// Converts a [`std::sync::PoisonError<T>`] into a [`ParseError`](Error::PoisonedLock)
133+
impl<T> From<PoisonError<T>> for Error {
134+
fn from(value: PoisonError<T>) -> Self {
135+
Error::PoisonedLock(value.to_string())
136+
}
137+
}
138+
114139
/// These are relatively low value tests; they are here to reduce the coverage gap and
115140
/// ensure that the error conversions are working as expected.
116141
#[cfg(test)]
@@ -199,4 +224,33 @@ mod test {
199224
let error = Error::from(parse_error);
200225
assert_eq!(error.to_string(), "empty host");
201226
}
227+
228+
#[cfg(feature = "maven")]
229+
#[test]
230+
fn test_from_quick_xml_error() {
231+
let xml = "<invalid>";
232+
let quick_xml_error = quick_xml::de::from_str::<String>(xml).expect_err("quick_xml error");
233+
let error = Error::from(quick_xml_error);
234+
assert!(matches!(error, Error::ParseError(_)));
235+
}
236+
237+
#[cfg(feature = "zip")]
238+
#[test]
239+
fn test_from_zip_error() {
240+
let zip_error = zip::result::ZipError::FileNotFound;
241+
let error = Error::from(zip_error);
242+
assert!(matches!(error, Error::Unexpected(_)));
243+
assert!(
244+
error
245+
.to_string()
246+
.contains("specified file not found in archive")
247+
);
248+
}
249+
250+
#[test]
251+
fn test_from_poisoned_lock() {
252+
let error = Error::from(std::sync::PoisonError::new(()));
253+
assert!(matches!(error, Error::PoisonedLock(_)));
254+
assert!(error.to_string().contains("poisoned lock"));
255+
}
202256
}

‎postgresql_archive/src/extractor/registry.rs‎

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use crate::Error::{PoisonedLock, UnsupportedExtractor};
1+
use crate::Error::UnsupportedExtractor;
22
use crate::Result;
33
#[cfg(feature = "theseus")]
44
use crate::configuration::theseus;
@@ -45,13 +45,10 @@ impl RepositoryRegistry {
4545
/// * If the URL is not supported.
4646
fn get(&self, url: &str) -> Result<ExtractFn> {
4747
for (supports_fn, extractor_fn) in &self.extractors {
48-
let supports_function = supports_fn
49-
.read()
50-
.map_err(|error| PoisonedLock(error.to_string()))?;
48+
let supports_function = supports_fn.read()?;
49+
5150
if supports_function(url)? {
52-
let extractor_function = extractor_fn
53-
.read()
54-
.map_err(|error| PoisonedLock(error.to_string()))?;
51+
let extractor_function = extractor_fn.read()?;
5552
return Ok(*extractor_function);
5653
}
5754
}
@@ -77,10 +74,7 @@ impl Default for RepositoryRegistry {
7774
/// # Errors
7875
/// * If the registry is poisoned.
7976
pub fn register(supports_fn: SupportsFn, extractor_fn: ExtractFn) -> Result<()> {
80-
let mut registry = REGISTRY
81-
.lock()
82-
.map_err(|error| PoisonedLock(error.to_string()))?;
83-
registry.register(supports_fn, extractor_fn);
77+
REGISTRY.lock()?.register(supports_fn, extractor_fn);
8478
Ok(())
8579
}
8680

@@ -89,10 +83,7 @@ pub fn register(supports_fn: SupportsFn, extractor_fn: ExtractFn) -> Result<()>
8983
/// # Errors
9084
/// * If the URL is not supported.
9185
pub fn get(url: &str) -> Result<ExtractFn> {
92-
let registry = REGISTRY
93-
.lock()
94-
.map_err(|error| PoisonedLock(error.to_string()))?;
95-
registry.get(url)
86+
REGISTRY.lock()?.get(url)
9687
}
9788

9889
#[cfg(test)]

‎postgresql_archive/src/extractor/zip_extractor.rs‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,11 @@ use zip::ZipArchive;
1616
pub fn extract(bytes: &Vec<u8>, extract_directories: &ExtractDirectories) -> Result<Vec<PathBuf>> {
1717
let mut files = Vec::new();
1818
let reader = Cursor::new(bytes);
19-
let mut archive = ZipArchive::new(reader).map_err(|_| io::Error::other("Zip error"))?;
19+
let mut archive = ZipArchive::new(reader)?;
2020
let mut extracted_bytes = 0;
2121

2222
for i in 0..archive.len() {
23-
let mut file = archive
24-
.by_index(i)
25-
.map_err(|_| io::Error::other("Zip error"))?;
23+
let mut file = archive.by_index(i)?;
2624
let file_path = PathBuf::from(file.name());
2725
let file_path = PathBuf::from(file_path.file_name().unwrap_or_default());
2826
let file_name = file_path.to_string_lossy();

‎postgresql_archive/src/hasher/registry.rs‎

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use crate::Error::{PoisonedLock, UnsupportedHasher};
1+
use crate::Error::UnsupportedHasher;
22
use crate::Result;
33
#[cfg(feature = "theseus")]
44
use crate::configuration::theseus;
@@ -54,13 +54,9 @@ impl HasherRegistry {
5454
let url = url.as_ref();
5555
let extension = extension.as_ref();
5656
for (supports_fn, hasher_fn) in &self.hashers {
57-
let supports_function = supports_fn
58-
.read()
59-
.map_err(|error| PoisonedLock(error.to_string()))?;
57+
let supports_function = supports_fn.read()?;
6058
if supports_function(url, extension)? {
61-
let hasher_function = hasher_fn
62-
.read()
63-
.map_err(|error| PoisonedLock(error.to_string()))?;
59+
let hasher_function = hasher_fn.read()?;
6460
return Ok(*hasher_function);
6561
}
6662
}
@@ -109,10 +105,7 @@ impl Default for HasherRegistry {
109105
/// # Errors
110106
/// * If the registry is poisoned.
111107
pub fn register(supports_fn: SupportsFn, hasher_fn: HasherFn) -> Result<()> {
112-
let mut registry = REGISTRY
113-
.lock()
114-
.map_err(|error| PoisonedLock(error.to_string()))?;
115-
registry.register(supports_fn, hasher_fn);
108+
REGISTRY.lock()?.register(supports_fn, hasher_fn);
116109
Ok(())
117110
}
118111

@@ -121,10 +114,7 @@ pub fn register(supports_fn: SupportsFn, hasher_fn: HasherFn) -> Result<()> {
121114
/// # Errors
122115
/// * If the registry is poisoned.
123116
pub fn get<S: AsRef<str>>(url: S, extension: S) -> Result<HasherFn> {
124-
let registry = REGISTRY
125-
.lock()
126-
.map_err(|error| PoisonedLock(error.to_string()))?;
127-
registry.get(url, extension)
117+
REGISTRY.lock()?.get(url, extension)
128118
}
129119

130120
#[cfg(test)]

‎postgresql_archive/src/matcher/registry.rs‎

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use crate::Error::{PoisonedLock, UnsupportedMatcher};
1+
use crate::Error::UnsupportedMatcher;
22
use crate::Result;
33
#[cfg(feature = "theseus")]
44
use crate::configuration::theseus;
@@ -46,13 +46,9 @@ impl MatchersRegistry {
4646
fn get<S: AsRef<str>>(&self, url: S) -> Result<MatcherFn> {
4747
let url = url.as_ref();
4848
for (supports_fn, matcher_fn) in &self.matchers {
49-
let supports_function = supports_fn
50-
.read()
51-
.map_err(|error| PoisonedLock(error.to_string()))?;
49+
let supports_function = supports_fn.read()?;
5250
if supports_function(url)? {
53-
let matcher_function = matcher_fn
54-
.read()
55-
.map_err(|error| PoisonedLock(error.to_string()))?;
51+
let matcher_function = matcher_fn.read()?;
5652
return Ok(*matcher_function);
5753
}
5854
}
@@ -79,10 +75,7 @@ impl Default for MatchersRegistry {
7975
/// # Errors
8076
/// * If the registry is poisoned.
8177
pub fn register(supports_fn: SupportsFn, matcher_fn: MatcherFn) -> Result<()> {
82-
let mut registry = REGISTRY
83-
.lock()
84-
.map_err(|error| PoisonedLock(error.to_string()))?;
85-
registry.register(supports_fn, matcher_fn);
78+
REGISTRY.lock()?.register(supports_fn, matcher_fn);
8679
Ok(())
8780
}
8881

@@ -91,10 +84,7 @@ pub fn register(supports_fn: SupportsFn, matcher_fn: MatcherFn) -> Result<()> {
9184
/// # Errors
9285
/// * If the registry is poisoned.
9386
pub fn get<S: AsRef<str>>(url: S) -> Result<MatcherFn> {
94-
let registry = REGISTRY
95-
.lock()
96-
.map_err(|error| PoisonedLock(error.to_string()))?;
97-
registry.get(url)
87+
REGISTRY.lock()?.get(url)
9888
}
9989

10090
#[cfg(test)]

‎postgresql_archive/src/repository/maven/repository.rs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use crate::Error::{ArchiveHashMismatch, ParseError, RepositoryFailure, VersionNotFound};
1+
use crate::Error::{ArchiveHashMismatch, RepositoryFailure, VersionNotFound};
22
use crate::repository::Archive;
33
use crate::repository::maven::models::Metadata;
44
use crate::repository::model::Repository;
@@ -60,8 +60,7 @@ impl Maven {
6060
let request = client.get(&url).headers(Self::headers());
6161
let response = request.send().await?.error_for_status()?;
6262
let text = response.text().await?;
63-
let metadata: Metadata =
64-
quick_xml::de::from_str(&text).map_err(|error| ParseError(error.to_string()))?;
63+
let metadata: Metadata = quick_xml::de::from_str(&text)?;
6564
let artifact = metadata.artifact_id;
6665
let mut result = None;
6766
for version in &metadata.versioning.versions.version {

‎postgresql_archive/src/repository/registry.rs‎

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use crate::Error::{PoisonedLock, UnsupportedRepository};
1+
use crate::Error::UnsupportedRepository;
22
use crate::Result;
33
#[cfg(feature = "theseus")]
44
use crate::configuration::theseus;
@@ -46,13 +46,9 @@ impl RepositoryRegistry {
4646
/// * If the URL is not supported.
4747
fn get(&self, url: &str) -> Result<Box<dyn Repository>> {
4848
for (supports_fn, new_fn) in &self.repositories {
49-
let supports_function = supports_fn
50-
.read()
51-
.map_err(|error| PoisonedLock(error.to_string()))?;
49+
let supports_function = supports_fn.read()?;
5250
if supports_function(url)? {
53-
let new_function = new_fn
54-
.read()
55-
.map_err(|error| PoisonedLock(error.to_string()))?;
51+
let new_function = new_fn.read()?;
5652
return new_function(url);
5753
}
5854
}
@@ -84,10 +80,7 @@ impl Default for RepositoryRegistry {
8480
/// # Errors
8581
/// * If the registry is poisoned.
8682
pub fn register(supports_fn: SupportsFn, new_fn: Box<NewFn>) -> Result<()> {
87-
let mut registry = REGISTRY
88-
.lock()
89-
.map_err(|error| PoisonedLock(error.to_string()))?;
90-
registry.register(supports_fn, new_fn);
83+
REGISTRY.lock()?.register(supports_fn, new_fn);
9184
Ok(())
9285
}
9386

@@ -96,10 +89,7 @@ pub fn register(supports_fn: SupportsFn, new_fn: Box<NewFn>) -> Result<()> {
9689
/// # Errors
9790
/// * If the URL is not supported.
9891
pub fn get(url: &str) -> Result<Box<dyn Repository>> {
99-
let registry = REGISTRY
100-
.lock()
101-
.map_err(|error| PoisonedLock(error.to_string()))?;
102-
registry.get(url)
92+
REGISTRY.lock()?.get(url)
10393
}
10494

10595
#[cfg(test)]

‎postgresql_extensions/src/error.rs‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
use std::sync::PoisonError;
2+
13
/// PostgreSQL extensions result type
24
pub type Result<T, E = Error> = core::result::Result<T, E>;
35

@@ -29,3 +31,22 @@ pub enum Error {
2931
#[error("unsupported namespace '{0}'")]
3032
UnsupportedNamespace(String),
3133
}
34+
35+
/// Converts a [`std::sync::PoisonError<T>`] into a [`ParseError`](Error::PoisonedLock)
36+
impl<T> From<PoisonError<T>> for Error {
37+
fn from(value: PoisonError<T>) -> Self {
38+
Error::PoisonedLock(value.to_string())
39+
}
40+
}
41+
42+
#[cfg(test)]
43+
mod tests {
44+
use super::*;
45+
46+
#[test]
47+
fn test_from_poison_error() {
48+
let error = Error::from(std::sync::PoisonError::new(()));
49+
assert!(matches!(error, Error::PoisonedLock(_)));
50+
assert!(error.to_string().contains("poisoned lock"));
51+
}
52+
}

‎postgresql_extensions/src/repository/registry.rs‎

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use crate::Error::{PoisonedLock, UnsupportedNamespace};
1+
use crate::Error::UnsupportedNamespace;
22
use crate::Result;
33
use crate::repository::model::Repository;
44
#[cfg(feature = "portal-corp")]
@@ -44,9 +44,7 @@ impl RepositoryRegistry {
4444
let Some(new_fn) = self.repositories.get(&namespace) else {
4545
return Err(UnsupportedNamespace(namespace.to_string()));
4646
};
47-
let new_function = new_fn
48-
.read()
49-
.map_err(|error| PoisonedLock(error.to_string()))?;
47+
let new_function = new_fn.read()?;
5048
new_function()
5149
}
5250
}
@@ -79,10 +77,7 @@ impl Default for RepositoryRegistry {
7977
/// # Errors
8078
/// * If the registry is poisoned.
8179
pub fn register(namespace: &str, new_fn: Box<NewFn>) -> Result<()> {
82-
let mut registry = REGISTRY
83-
.lock()
84-
.map_err(|error| PoisonedLock(error.to_string()))?;
85-
registry.register(namespace, new_fn);
80+
REGISTRY.lock()?.register(namespace, new_fn);
8681
Ok(())
8782
}
8883

@@ -91,21 +86,15 @@ pub fn register(namespace: &str, new_fn: Box<NewFn>) -> Result<()> {
9186
/// # Errors
9287
/// * If the namespace is not supported.
9388
pub fn get(namespace: &str) -> Result<Box<dyn Repository>> {
94-
let registry = REGISTRY
95-
.lock()
96-
.map_err(|error| PoisonedLock(error.to_string()))?;
97-
registry.get(namespace)
89+
REGISTRY.lock()?.get(namespace)
9890
}
9991

10092
/// Gets the namespaces of the registered repositories.
10193
///
10294
/// # Errors
10395
/// * If the registry is poisoned.
10496
pub fn get_namespaces() -> Result<Vec<String>> {
105-
let registry = REGISTRY
106-
.lock()
107-
.map_err(|error| PoisonedLock(error.to_string()))?;
108-
Ok(registry.repositories.keys().cloned().collect())
97+
Ok(REGISTRY.lock()?.repositories.keys().cloned().collect())
10998
}
11099

111100
/// Gets all the registered repositories.

0 commit comments

Comments
 (0)