Test and fix serialization

Add `TryFrom<Meta> for Value` and `TryFrom<&Meta> for String` to handle serialization. Then add tests to ensure that Meta object loaded from v2 meta files deserialize into the same data structure. Done mainly by omitting Optional fields set to `None`, but also by eliminating the `meta-spec.url` default value and a custom `x_` property, which will be restored in a future commit. In the process, add support for badges in the resources object. This was an oversight in e803409. While at it, change a parameter instance of `&Vec<T>` to `&[T]`.
pgxn · Sep 5, 2024 · a20ac19 · a20ac19
1 parent e803409
commit a20ac19
Show file tree

Hide file tree

Showing 6 changed files with 121 additions and 13 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -15,6 +15,7 @@ exclude = [ ".github", ".vscode", ".gitignore", ".ci", ".pre-*.yaml"]
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
+assert-json-diff = "2.0.2"
 boon = "0.6"
 email_address = "0.2.9"
 lexopt = "0.3.0"

diff --git a/corpus/v2/typical-pgrx.json b/corpus/v2/typical-pgrx.json
@@ -19,7 +19,6 @@
       "jsonschema": {
         "abstract": "JSON Schema validation functions for PostgreSQL",
         "doc": "doc/jsonschema.md",
-        "x_comment": "Paths non-deterministic until pgrx ",
         "sql": "target/release/**/jsonschema--0.1.1.sql",
         "control": "target/release/**/jsonschema.control"
       }

diff --git a/src/meta/mod.rs b/src/meta/mod.rs
@@ -8,23 +8,21 @@ use serde_json::Value;
 mod v1;
 mod v2;
 
-fn meta_url() -> String {
-    "https://rfcs.pgxn.org/0003-meta-spec-v2.html".to_string()
-}
-
 /// Represents the `meta-spec` object in [`Meta`].
 #[derive(Serialize, Deserialize, PartialEq, Debug)]
 pub struct Spec {
     version: String,
-    #[serde(default = "meta_url")]
-    url: String,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    url: Option<String>,
 }
 
 /// Maintainer represents an object in the list of `maintainers` in [`Meta`].
 #[derive(Serialize, Deserialize, PartialEq, Debug)]
 pub struct Maintainer {
     name: String,
+    #[serde(skip_serializing_if = "Option::is_none")]
     email: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     url: Option<String>,
 }
 
@@ -33,9 +31,12 @@ pub struct Maintainer {
 pub struct Extension {
     control: RelativePathBuf,
     #[serde(rename = "abstract")]
+    #[serde(skip_serializing_if = "Option::is_none")]
     abs_tract: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     tle: Option<bool>,
     sql: RelativePathBuf,
+    #[serde(skip_serializing_if = "Option::is_none")]
     doc: Option<RelativePathBuf>,
 }
 
@@ -65,45 +66,60 @@ pub struct Module {
     #[serde(rename = "type")]
     kind: ModuleType,
     #[serde(rename = "abstract")]
+    #[serde(skip_serializing_if = "Option::is_none")]
     abs_tract: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     preload: Option<Preload>,
     lib: RelativePathBuf,
+    #[serde(skip_serializing_if = "Option::is_none")]
     doc: Option<RelativePathBuf>,
 }
 
 /// Represents an app under `apps` in [`Contents`].
 #[derive(Serialize, Deserialize, PartialEq, Debug)]
 pub struct App {
+    #[serde(skip_serializing_if = "Option::is_none")]
     lang: Option<String>,
     #[serde(rename = "abstract")]
+    #[serde(skip_serializing_if = "Option::is_none")]
     abs_tract: Option<String>,
     bin: RelativePathBuf,
+    #[serde(skip_serializing_if = "Option::is_none")]
     doc: Option<RelativePathBuf>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     lib: Option<RelativePathBuf>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     man: Option<RelativePathBuf>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     html: Option<RelativePathBuf>,
 }
 
 /// Represents the contents of a distribution, under `contents` in [`Meta`].
 #[derive(Serialize, Deserialize, PartialEq, Debug)]
 pub struct Contents {
+    #[serde(skip_serializing_if = "Option::is_none")]
     extensions: Option<HashMap<String, Extension>>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     modules: Option<HashMap<String, Module>>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     apps: Option<HashMap<String, App>>,
 }
 
 /// Represents the classifications of a distribution, under `classifications`
 /// in [`Meta`].
 #[derive(Serialize, Deserialize, PartialEq, Debug)]
 pub struct Classifications {
+    #[serde(skip_serializing_if = "Option::is_none")]
     tags: Option<Vec<String>>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     categories: Option<Vec<String>>,
 }
 
 /// Represents Postgres requirements under `postgres` in [`Dependencies`].
 #[derive(Serialize, Deserialize, PartialEq, Debug)]
 pub struct Postgres {
     version: String,
+    #[serde(skip_serializing_if = "Option::is_none")]
     with: Option<Vec<String>>,
 }
 
@@ -141,20 +157,29 @@ pub enum VersionRange {
 /// Defines the relationships for a build phase in [`Packages`].
 #[derive(Serialize, Deserialize, PartialEq, Debug)]
 pub struct Phase {
+    #[serde(skip_serializing_if = "Option::is_none")]
     requires: Option<HashMap<String, VersionRange>>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     recommends: Option<HashMap<String, VersionRange>>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     suggests: Option<HashMap<String, VersionRange>>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     conflicts: Option<HashMap<String, VersionRange>>,
 }
 
 /// Defines package dependencies for build phases under `packages` in
 /// [`Dependencies`].
 #[derive(Serialize, Deserialize, PartialEq, Debug)]
 pub struct Packages {
+    #[serde(skip_serializing_if = "Option::is_none")]
     configure: Option<Phase>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     build: Option<Phase>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     test: Option<Phase>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     run: Option<Phase>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     develop: Option<Phase>,
 }
 
@@ -169,21 +194,42 @@ pub struct Variations {
 /// Defines the distribution dependencies under `dependencies` in [`Meta`].
 #[derive(Serialize, Deserialize, PartialEq, Debug)]
 pub struct Dependencies {
+    #[serde(skip_serializing_if = "Option::is_none")]
     platforms: Option<Vec<String>>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     postgres: Option<Postgres>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     pipeline: Option<Pipeline>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     packages: Option<Packages>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     variations: Option<Vec<Variations>>,
 }
 
+/// Defines the badges under `badges` in [`Resources`].
+#[derive(Serialize, Deserialize, PartialEq, Debug)]
+pub struct Badge {
+    src: String,
+    alt: String,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    url: Option<String>,
+}
+
 /// Defines the resources under `resources` in [`Meta`].
 #[derive(Serialize, Deserialize, PartialEq, Debug)]
 pub struct Resources {
+    #[serde(skip_serializing_if = "Option::is_none")]
     homepage: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     issues: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     repository: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     docs: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     support: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    badges: Option<Vec<Badge>>,
 }
 
 /// Defines the artifacts in the array under `artifacts` in [`Meta`].
@@ -192,8 +238,11 @@ pub struct Artifact {
     url: String,
     #[serde(rename = "type")]
     kind: String,
+    #[serde(skip_serializing_if = "Option::is_none")]
     platform: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     sha256: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     sha512: Option<String>,
 }
 
@@ -204,17 +253,24 @@ pub struct Meta {
     version: Version,
     #[serde(rename = "abstract")]
     abs_tract: String,
+    #[serde(skip_serializing_if = "Option::is_none")]
     description: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     producer: Option<String>,
     license: String, // use spdx::Expression.
     #[serde(rename = "meta-spec")]
     spec: Spec,
     maintainers: Vec<Maintainer>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     classifications: Option<Classifications>,
     contents: Contents,
+    #[serde(skip_serializing_if = "Option::is_none")]
     ignore: Option<Vec<String>>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     dependencies: Option<Dependencies>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     resources: Option<Resources>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     artifacts: Option<Vec<Artifact>>,
 }
 
@@ -241,6 +297,14 @@ impl TryFrom<Value> for Meta {
     }
 }
 
+impl TryFrom<Meta> for Value {
+    type Error = Box<dyn Error>;
+    fn try_from(meta: Meta) -> Result<Self, Self::Error> {
+        let val = serde_json::to_value(meta)?;
+        Ok(val)
+    }
+}
+
 impl TryFrom<&PathBuf> for Meta {
     type Error = Box<dyn Error>;
     fn try_from(file: &PathBuf) -> Result<Self, Self::Error> {
@@ -257,5 +321,13 @@ impl TryFrom<&String> for Meta {
     }
 }
 
+impl TryFrom<Meta> for String {
+    type Error = Box<dyn Error>;
+    fn try_from(meta: Meta) -> Result<Self, Self::Error> {
+        let val = serde_json::to_string(&meta)?;
+        Ok(val)
+    }
+}
+
 #[cfg(test)]
 mod tests;
diff --git a/src/meta/tests.rs b/src/meta/tests.rs
@@ -13,6 +13,7 @@ fn test_corpus() -> Result<(), Box<dyn Error>> {
 
         for path in glob.walk(dir) {
             let path = path?.into_path();
+            let contents: Value = serde_json::from_reader(File::open(&path)?)?;
 
             // Test try_from path.
             if let Err(e) = Meta::try_from(&path) {
@@ -21,14 +22,38 @@ fn test_corpus() -> Result<(), Box<dyn Error>> {
 
             // Test try_from str.
             let str: String = fs::read_to_string(&path)?;
-            if let Err(e) = Meta::try_from(&str) {
-                panic!("{v_dir}/{:?} failed: {e}", path.file_name().unwrap());
+            match Meta::try_from(&str) {
+                Err(e) => panic!("{v_dir}/{:?} failed: {e}", path.file_name().unwrap()),
+                Ok(m) => {
+                    if v_dir == "v2" {
+                        // Make sure round-trip produces the same JSON.
+                        let output: Result<Value, Box<dyn Error>> = m.try_into();
+                        match output {
+                            Err(e) => panic!("{v_dir}/{:?} failed: {e}", path.file_name().unwrap()),
+                            Ok(val) => {
+                                assert_json_diff::assert_json_eq!(&contents, &val);
+                            }
+                        };
+                    }
+                }
             }
 
             // Test try_from value.
-            let meta: Value = serde_json::from_reader(File::open(&path)?)?;
-            if let Err(e) = Meta::try_from(meta) {
-                panic!("{v_dir}/{:?} failed: {e}", path.file_name().unwrap());
+            match Meta::try_from(contents.clone()) {
+                Err(e) => panic!("{v_dir}/{:?} failed: {e}", path.file_name().unwrap()),
+                Ok(m) => {
+                    if v_dir == "v2" {
+                        // Make sure value round-trip produces the same JSON.
+                        let output: Result<String, Box<dyn Error>> = m.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);
+                            }
+                        };
+                    }
+                }
             }
 
             println!("Example {v_dir}/{:?} ok", path.file_name().unwrap());

diff --git a/src/meta/v1/mod.rs b/src/meta/v1/mod.rs
@@ -97,7 +97,7 @@ fn v1_to_v2_maintainers(v1: &Value) -> Result<Value, Box<dyn Error>> {
 /// Otherwise the string will be saved as the maintainer `name` and the `url`
 /// set to either the `homepage` in the `resources` object in `v1`, or else
 /// `https://pgxn.org`.
-fn parse_v1_maintainers(v1: &Value, list: &Vec<Value>) -> Result<Value, Box<dyn Error>> {
+fn parse_v1_maintainers(v1: &Value, list: &[Value]) -> Result<Value, Box<dyn Error>> {
     let mut new_list: Vec<Value> = Vec::with_capacity(list.len());
     for v in list {
         if let Some(str) = v.as_str() {