Skip to content

Commit

Permalink
Replace remaining errors
Browse files Browse the repository at this point in the history
All the APIs now return errors from the error module in this crate. The
only bits left that return boxed errors are:

*   The custom JSON Schema format validation functions in
    `src/valid/compiler/mod.rs`, which are required to be boxed errors
    by the boon crate.
*   `build.rs` and `main.rs`, where it seems harmless, since those
    errors are always printed ton the terminal.
  • Loading branch information
theory committed Nov 13, 2024
1 parent fb02ea2 commit bf6db96
Show file tree
Hide file tree
Showing 9 changed files with 112 additions and 131 deletions.
6 changes: 4 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,14 @@ All notable changes to this project will be documented in this file. It uses the

* Added the [error module], which defines all the errors returned by
pgxn_meta.
* Changed the errors returned from the [valid module] from boxed
[boon] errors with lifetimes to [error module] errors with no lifetimes.
* Changed the errors returned by all the APIs from boxed errors [error
module] errors.

### 📔 Notes

* Removed the `valid::ValidationError` enum.
* Changed the errors returned from the [valid module] from boxed [boon]
errors with lifetimes to [error module] errors with no lifetimes.

[v0.5.0]: https://github.com/pgxn/meta/compare/v0.4.0...v0.5.0
[error module]: https://docs.rs/pgxn_meta/0.5.0/pgxn_meta/error/
Expand Down
41 changes: 16 additions & 25 deletions src/release/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,11 @@ It supports both the [v1] and [v2] specs.
*/

use crate::dist::*;
use crate::util;

use crate::{dist::*, error::Error, util};
use chrono::{DateTime, Utc};
use serde::{de, Deserialize, Deserializer, Serialize};
use serde_json::Value;
use std::{borrow::Borrow, collections::HashMap, error::Error, fs::File, path::Path};
use std::{borrow::Borrow, collections::HashMap, fs::File, path::Path};

mod v1;
mod v2;
Expand Down Expand Up @@ -160,11 +158,11 @@ impl Release {

/// Deserializes `meta`, which contains PGXN distribution release
/// metadata, into a [`Release`].
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),
}

// XXX: Add signature validation.
Expand All @@ -173,7 +171,7 @@ impl Release {
/// Loads the release `META.json` data from `file` then converts into a
/// [`Release`]. 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 @@ -266,14 +264,13 @@ impl Release {
}

impl TryFrom<Value> for Release {
type Error = Box<dyn Error>;
type Error = Error;
/// Converts the PGXN release `META.json` data from `meta` into a
/// [`Release`]. Returns an error if `meta` is invalid.
///
/// # Example
///
/// ``` rust
/// # use std::error::Error;
/// use serde_json::json;
/// use pgxn_meta::release::*;
///
Expand Down Expand Up @@ -309,16 +306,13 @@ impl TryFrom<Value> for Release {
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_release(&meta) {
Err(e) => return Err(Box::from(e.to_string())),
Ok(v) => v,
};
let version = validator.validate_release(&meta)?;
Release::from_version(version, meta)
}
}

impl TryFrom<&[&Value]> for Release {
type Error = Box<dyn Error>;
type Error = Error;
/// Merge multiple PGXN release `META.json` data from `meta` into a
/// [`Release`]. Returns an error if `meta` is invalid.
///
Expand All @@ -329,7 +323,6 @@ impl TryFrom<&[&Value]> for Release {
/// # Example
///
/// ``` rust
/// # use std::error::Error;
/// use serde_json::json;
/// use pgxn_meta::release::*;
///
Expand Down Expand Up @@ -369,12 +362,11 @@ impl TryFrom<&[&Value]> for Release {
/// [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_else(|| Error::UnknownSpec)?;

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

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

impl TryFrom<Release> for Value {
type Error = Box<dyn Error>;
type Error = Error;
/// Converts PGXN release `meta` into a [serde_json::Value].
///
/// # Example
///
/// ``` rust
/// # use std::error::Error;
/// use serde_json::{json, Value};
/// use pgxn_meta::release::*;
/// use pgxn_meta::{error::Error, release::*};
///
/// let meta_json = json!({
/// "name": "pair",
Expand Down Expand Up @@ -434,7 +425,7 @@ impl TryFrom<Release> for Value {
///
/// let meta = Release::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: Release) -> Result<Self, Self::Error> {
Expand All @@ -444,7 +435,7 @@ impl TryFrom<Release> for Value {
}

impl TryFrom<&String> for Release {
type Error = Box<dyn Error>;
type Error = Error;
/// Converts `str` into JSON and then into a [`Release`]. 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 @@ -454,7 +445,7 @@ impl TryFrom<&String> for Release {
}

impl TryFrom<Release> for String {
type Error = Box<dyn Error>;
type Error = Error;
/// Converts `meta` into a JSON String.
fn try_from(meta: Release) -> Result<Self, Self::Error> {
let val = serde_json::to_string(&meta)?;
Expand Down
31 changes: 16 additions & 15 deletions src/release/tests.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use super::*;
use crate::error::Error;
use chrono::prelude::*;
use serde_json::{json, Value};
use std::{error::Error, fs::File, io::Write, path::PathBuf};
use std::{fs::File, io::Write, path::PathBuf};
use tempfile::NamedTempFile;
use wax::Glob;

Expand Down Expand Up @@ -34,7 +35,7 @@ fn release_date() -> DateTime<Utc> {
}

#[test]
fn test_corpus() -> Result<(), Box<dyn Error>> {
fn test_corpus() -> Result<(), Error> {
let certs = certs();
let payload = get_payload(&certs);
for (version, patch) in [
Expand Down Expand Up @@ -82,7 +83,7 @@ fn test_corpus() -> Result<(), Box<dyn Error>> {
);

// Make sure round-trip produces the same JSON.
let output: Result<Value, Box<dyn Error>> = release.try_into();
let output: Result<Value, Error> = release.try_into();
match output {
Err(e) => panic!("{v_dir}/{bn} failed: {e}"),
Ok(val) => {
Expand All @@ -100,7 +101,7 @@ fn test_corpus() -> Result<(), Box<dyn Error>> {
Ok(dist) => {
if version == 2 {
// Make sure value round-trip produces the same JSON.
let output: Result<String, Box<dyn Error>> = dist.try_into();
let output: Result<String, Error> = dist.try_into();
match output {
Err(e) => panic!("{v_dir}/{bn} failed: {e}"),
Ok(val) => {
Expand Down Expand Up @@ -146,7 +147,7 @@ fn get_payload(meta: &Value) -> ReleasePayload {
}

#[test]
fn test_bad_corpus() -> Result<(), Box<dyn Error>> {
fn test_bad_corpus() -> Result<(), Error> {
// Load valid distribution metadata.
let file: PathBuf = [env!("CARGO_MANIFEST_DIR"), "corpus", "invalid.json"]
.iter()
Expand All @@ -170,7 +171,7 @@ fn test_bad_corpus() -> Result<(), Box<dyn Error>> {
// Make sure we fail on invalid version.
match Release::from_version(99, meta.clone()) {
Ok(_) => panic!("Unexpected success with invalid version"),
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 Down Expand Up @@ -199,7 +200,7 @@ fn test_bad_corpus() -> 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 @@ -240,7 +241,7 @@ fn test_try_merge_v2() -> 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 @@ -294,7 +295,7 @@ fn run_merge_case(
orig: &Value,
patches: &[Value],
expect: &Value,
) -> Result<(), Box<dyn Error>> {
) -> Result<(), Error> {
let patch = certs();
let mut meta = vec![orig, &patch];
for p in patches {
Expand All @@ -304,7 +305,7 @@ fn run_merge_case(
Err(e) => panic!("patching {name} failed: {e}"),
Ok(dist) => {
// Convert the Release 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 @@ -321,7 +322,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 @@ -335,12 +336,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", vec![&invalid], "missing properties 'version'"),
] {
Expand Down Expand Up @@ -422,7 +423,7 @@ fn release_payload() {
}

#[test]
fn release() -> Result<(), Box<dyn Error>> {
fn release() -> Result<(), Error> {
let dir: PathBuf = [env!("CARGO_MANIFEST_DIR"), "corpus", "v2"]
.iter()
.collect();
Expand Down Expand Up @@ -544,7 +545,7 @@ fn release() -> Result<(), Box<dyn Error>> {
}

#[test]
fn release_deserialize_err() -> Result<(), Box<dyn Error>> {
fn release_deserialize_err() -> 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
13 changes: 6 additions & 7 deletions src/release/v1/mod.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
use super::Release;
use crate::dist::v1 as dist;
use crate::{dist::v1 as dist, error::Error};
use serde_json::{json, Value};
use std::error::Error;

/// to_v2 parses v1, which contains PGXN v1 release metadata, into a JSON
/// object containing valid PGXN v2 release certification.
pub fn to_v2(v1: &Value) -> Result<Value, Box<dyn Error>> {
pub fn to_v2(v1: &Value) -> Result<Value, Error> {
let mut v2_val = dist::to_v2(v1)?;
let v2 = v2_val
.as_object_mut()
.ok_or("Date returned from v1::to_v2 is not an object")?;
.ok_or_else(|| Error::Param("data returned from v1::to_v2 is not an object"))?;

// Convert release.
v2.insert("certs".to_string(), v1_to_v2_release(v1)?);
Expand All @@ -19,14 +18,14 @@ pub fn to_v2(v1: &Value) -> Result<Value, Box<dyn Error>> {

/// from_value parses v1, which contains PGXN v1 metadata, into a
/// [`Release`] object containing valid PGXN v2 metadata.
pub fn from_value(v1: Value) -> Result<Release, Box<dyn Error>> {
pub fn from_value(v1: Value) -> Result<Release, Error> {
to_v2(&v1)?.try_into()
}

/// v1_to_v2_release clones release metadata from v1 to the v2 format. The
/// included signature information will be random and un-verifiable, but
/// adequate for v2 JSON Schema validation.
fn v1_to_v2_release(v1: &Value) -> Result<Value, Box<dyn Error>> {
fn v1_to_v2_release(v1: &Value) -> Result<Value, Error> {
use base64::{engine::general_purpose::URL_SAFE_NO_PAD, Engine as _};
use rand::distributions::{Alphanumeric, DistString};

Expand Down Expand Up @@ -66,7 +65,7 @@ fn v1_to_v2_release(v1: &Value) -> Result<Value, Box<dyn Error>> {
}
}
}
Err(Box::from(format!("missing release property {:?}", field)))
Err(Error::Missing(field))
}

#[cfg(test)]
Expand Down
2 changes: 1 addition & 1 deletion src/release/v1/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ fn test_v1_v2_release_err() {
match v1_to_v2_release(&input) {
Ok(_) => panic!("{name} unexpectedly succeeded"),
Err(e) => assert_eq!(
format!("missing release property \"{name}\""),
format!("{name} property missing"),
e.to_string(),
"{name}: {e}"
),
Expand Down
9 changes: 3 additions & 6 deletions src/release/v2/mod.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
use super::Release;
use crate::error::Error;
use serde_json::Value;
use std::error::Error;

pub fn from_value(meta: Value) -> Result<Release, Box<dyn Error>> {
match serde_json::from_value(meta) {
Ok(m) => Ok(m),
Err(e) => Err(Box::from(e)),
}
pub fn from_value(meta: Value) -> Result<Release, Error> {
Ok(serde_json::from_value(meta)?)
}
Loading

0 comments on commit bf6db96

Please sign in to comment.