Skip to content

Commit

Permalink
Replace dist errors
Browse files Browse the repository at this point in the history 
Add property validation and parameter errors to the error module and
replace all `Box<Error>`s in the dist module with `error:Error`s.

Also, make the first word of all error messages consistently lowercase.
  • Loading branch information
@theory
theory committed Nov 13, 2024
1 parent 3100dbd commit 6766aed
Show file tree
Hide file tree
Showing 9 changed files with 126 additions and 89 deletions.
37 changes: 16 additions & 21 deletions src/dist/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ distribution `META.json` files. It supports both the [v1] and [v2] specs.
[v2]: https://github.com/pgxn/rfcs/pull/3
*/
use std::{borrow::Borrow, collections::HashMap, error::Error, fs::File, path::Path};
use std::{borrow::Borrow, collections::HashMap, fs::File, path::Path};

use crate::util;
use crate::{error::Error, util};
use relative_path::{RelativePath, RelativePathBuf};
use semver::Version;
use serde::{Deserialize, Deserializer, Serialize};
Expand Down Expand Up @@ -828,18 +828,18 @@ where
impl Distribution {
/// Deserializes `meta`, which contains PGXN `version` metadata, into a
/// [`Distribution`].
fn from_version(version: u8, meta: Value) -> Result<Self, Box<dyn Error>> {
fn from_version(version: u8, meta: Value) -> Result<Self, Error> {
match version {
1 => v1::from_value(meta),
2 => v2::from_value(meta),
_ => Err(Box::from(format!("Unknown meta version {version}"))),
_ => Err(Error::UnknownSpec),
}
}

/// Loads the release `META.json` data from `file` then converts into a
/// [`Distribution`]. Returns an error on file error or if the content of
/// `file` is not valid PGXN `META.json` data.
pub fn load<P: AsRef<Path>>(file: P) -> Result<Self, Box<dyn Error>> {
pub fn load<P: AsRef<Path>>(file: P) -> Result<Self, Error> {
let meta: Value = serde_json::from_reader(File::open(file)?)?;
meta.try_into()
}
Expand Down Expand Up @@ -922,7 +922,7 @@ impl Distribution {
}

impl TryFrom<Value> for Distribution {
type Error = Box<dyn Error>;
type Error = Error;
/// Converts the PGXN `META.json` data from `meta` into a
/// [`Distribution`]. Returns an error if `meta` is invalid.
///
Expand Down Expand Up @@ -959,16 +959,13 @@ impl TryFrom<Value> for Distribution {
fn try_from(meta: Value) -> Result<Self, Self::Error> {
// Make sure it's valid.
let mut validator = crate::valid::Validator::new();
let version = match validator.validate(&meta) {
Err(e) => return Err(Box::from(e.to_string())),
Ok(v) => v,
};
let version = validator.validate(&meta)?;
Distribution::from_version(version, meta)
}
}

impl TryFrom<&[&Value]> for Distribution {
type Error = Box<dyn Error>;
type Error = Error;
/// Merge multiple PGXN `META.json` data from `meta` into a
/// [`Distribution`]. Returns an error if `meta` is invalid.
///
Expand Down Expand Up @@ -1013,12 +1010,11 @@ impl TryFrom<&[&Value]> for Distribution {
/// [RFC 7396]: https:///www.rfc-editor.org/rfc/rfc7396.html
fn try_from(meta: &[&Value]) -> Result<Self, Self::Error> {
if meta.is_empty() {
return Err(Box::from("meta contains no values"));
return Err(Error::Param("meta contains no values"));
}

// Find the version of the first doc.
let version =
util::get_version(meta[0]).ok_or("No spec version found in first meta value")?;
let version = util::get_version(meta[0]).ok_or(Error::UnknownSpec)?;

// Convert the first doc to v2 if necessary.
let mut v2 = match version {
Expand All @@ -1034,21 +1030,20 @@ impl TryFrom<&[&Value]> for Distribution {

// Validate the patched doc and return.
let mut validator = crate::valid::Validator::new();
validator.validate(&v2).map_err(|e| e.to_string())?;
validator.validate(&v2)?;
Distribution::from_version(2, v2)
}
}

impl TryFrom<Distribution> for Value {
type Error = Box<dyn Error>;
type Error = Error;
/// Converts `meta` into a [serde_json::Value].
///
/// # Example
///
/// ``` rust
/// # use std::error::Error;
/// use serde_json::{json, Value};
/// use pgxn_meta::dist::*;
/// use pgxn_meta::{error::Error, dist::*};
///
/// let meta_json = json!({
/// "name": "pair",
Expand All @@ -1072,7 +1067,7 @@ impl TryFrom<Distribution> for Value {
///
/// let meta = Distribution::try_from(meta_json);
/// assert!(meta.is_ok());
/// let val: Result<Value, Box<dyn Error>> = meta.unwrap().try_into();
/// let val: Result<Value, Error> = meta.unwrap().try_into();
/// assert!(val.is_ok());
/// ```
fn try_from(meta: Distribution) -> Result<Self, Self::Error> {
Expand All @@ -1082,7 +1077,7 @@ impl TryFrom<Distribution> for Value {
}

impl TryFrom<&String> for Distribution {
type Error = Box<dyn Error>;
type Error = Error;
/// Converts `str` into JSON and then into a [`Distribution`]. Returns an
/// error if the content of `str` is not valid PGXN `META.json` data.
fn try_from(str: &String) -> Result<Self, Self::Error> {
Expand All @@ -1092,7 +1087,7 @@ impl TryFrom<&String> for Distribution {
}

impl TryFrom<Distribution> for String {
type Error = Box<dyn Error>;
type Error = Error;
/// Converts `meta` into a JSON String.
fn try_from(meta: Distribution) -> Result<Self, Self::Error> {
let val = serde_json::to_string(&meta)?;
Expand Down
34 changes: 16 additions & 18 deletions src/dist/tests.rs
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
use super::*;
use serde_json::{json, Value};
use std::{
error::Error,
fs::{self, File},
path::PathBuf,
};
use wax::Glob;

#[test]
fn test_corpus() -> Result<(), Box<dyn Error>> {
fn test_corpus() -> Result<(), Error> {
for v_dir in ["v1", "v2"] {
let dir: PathBuf = [env!("CARGO_MANIFEST_DIR"), "corpus", v_dir]
.iter()
Expand All @@ -32,7 +31,7 @@ fn test_corpus() -> Result<(), Box<dyn Error>> {
if v_dir == "v2" {
assert_eq!(contents.get("license").unwrap(), dist.license());
// Make sure round-trip produces the same JSON.
let output: Result<Value, Box<dyn Error>> = dist.try_into();
let output: Result<Value, Error> = dist.try_into();
match output {
Err(e) => panic!("{v_dir}/{:?} failed: {e}", path.file_name().unwrap()),
Ok(val) => {
Expand All @@ -49,11 +48,10 @@ fn test_corpus() -> Result<(), Box<dyn Error>> {
Ok(dist) => {
if v_dir == "v2" {
// Make sure value round-trip produces the same JSON.
let output: Result<String, Box<dyn Error>> = dist.try_into();
let output: Result<Value, Error> = dist.try_into();
match output {
Err(e) => panic!("{v_dir}/{:?} failed: {e}", path.file_name().unwrap()),
Ok(val) => {
let val: Value = serde_json::from_str(&val)?;
assert_json_diff::assert_json_eq!(&contents, &val);
}
};
Expand All @@ -68,7 +66,7 @@ fn test_corpus() -> Result<(), Box<dyn Error>> {
}

#[test]
fn test_bad_corpus() -> Result<(), Box<dyn Error>> {
fn test_bad_corpus() -> Result<(), Error> {
let file: PathBuf = [env!("CARGO_MANIFEST_DIR"), "corpus", "invalid.json"]
.iter()
.collect();
Expand All @@ -89,7 +87,7 @@ fn test_bad_corpus() -> Result<(), Box<dyn Error>> {
"Should have failed on {:?} but did not",
file.file_name().unwrap()
),
Err(e) => assert_eq!("Unknown meta version 99", e.to_string()),
Err(e) => assert_eq!("cannot determine meta-spec version", e.to_string()),
}

// Should fail when no meta-spec.
Expand All @@ -99,7 +97,7 @@ fn test_bad_corpus() -> Result<(), Box<dyn Error>> {
"Should have failed on {:?} but did not",
file.file_name().unwrap()
),
Err(e) => assert_eq!("Cannot determine meta-spec version", e.to_string()),
Err(e) => assert_eq!("cannot determine meta-spec version", e.to_string()),
}

// Make sure we catch a failure parsing into a Distribution struct.
Expand All @@ -112,7 +110,7 @@ fn test_bad_corpus() -> Result<(), Box<dyn Error>> {
}

#[test]
fn test_try_merge_v1() -> Result<(), Box<dyn Error>> {
fn test_try_merge_v1() -> Result<(), Error> {
// Load a v1 META file.
let dir: PathBuf = [env!("CARGO_MANIFEST_DIR"), "corpus"].iter().collect();
let widget_file = dir.join("v1").join("widget.json");
Expand Down Expand Up @@ -169,7 +167,7 @@ fn test_try_merge_v1() -> Result<(), Box<dyn Error>> {
}

#[test]
fn test_try_merge_v2() -> Result<(), Box<dyn Error>> {
fn test_try_merge_v2() -> Result<(), Error> {
// Load a v2 META file.
let dir: PathBuf = [env!("CARGO_MANIFEST_DIR"), "corpus"].iter().collect();
let widget_file = dir.join("v2").join("minimal.json");
Expand Down Expand Up @@ -214,7 +212,7 @@ fn run_merge_case(
orig: &Value,
patches: &[Value],
expect: &Value,
) -> Result<(), Box<dyn Error>> {
) -> Result<(), Error> {
let mut meta = vec![orig];
for p in patches {
meta.push(p);
Expand All @@ -223,7 +221,7 @@ fn run_merge_case(
Err(e) => panic!("patching {name} failed: {e}"),
Ok(dist) => {
// Convert the Distribution object to JSON.
let output: Result<Value, Box<dyn Error>> = dist.try_into();
let output: Result<Value, Error> = dist.try_into();
match output {
Err(e) => panic!("{name} serialization failed: {e}"),
Ok(val) => {
Expand All @@ -240,7 +238,7 @@ fn run_merge_case(
}

#[test]
fn test_try_merge_err() -> Result<(), Box<dyn Error>> {
fn test_try_merge_err() -> Result<(), Error> {
// Load invalid meta.
let dir: PathBuf = [env!("CARGO_MANIFEST_DIR"), "corpus"].iter().collect();
let widget_file = dir.join("invalid.json");
Expand All @@ -254,12 +252,12 @@ fn test_try_merge_err() -> Result<(), Box<dyn Error>> {
(
"no version",
vec![&empty],
"No spec version found in first meta value",
"cannot determine meta-spec version",
),
(
"bad version",
vec![&bad_version],
"No spec version found in first meta value",
"cannot determine meta-spec version",
),
(
"invalid",
Expand All @@ -277,7 +275,7 @@ fn test_try_merge_err() -> Result<(), Box<dyn Error>> {
}

#[test]
fn test_try_merge_partman() -> Result<(), Box<dyn Error>> {
fn test_try_merge_partman() -> Result<(), Error> {
// Test the real-world pg_partman JSON with a patch to build the expected
// v2 JSON. First, load the original metadata.
let original_meta = json!({
Expand Down Expand Up @@ -410,7 +408,7 @@ fn test_try_merge_partman() -> Result<(), Box<dyn Error>> {
Err(e) => panic!("patching part man failed: {e}"),
Ok(dist) => {
// Convert the Distributions object to JSON.
let output: Result<Value, Box<dyn Error>> = dist.try_into();
let output: Result<Value, Error> = dist.try_into();
match output {
Err(e) => panic!("partman serialization failed: {e}"),
Ok(val) => {
Expand Down Expand Up @@ -1336,7 +1334,7 @@ fn test_artifact() {
}

#[test]
fn test_distribution() -> Result<(), Box<dyn Error>> {
fn test_distribution() -> Result<(), Error> {
let dir: PathBuf = [env!("CARGO_MANIFEST_DIR"), "corpus", "v2"]
.iter()
.collect();
Expand Down
Loading

0 comments on commit 6766aed

Please sign in to comment.