Fix review comments

VolumeGraphics · Oct 5, 2023 · b57849c · b57849c
1 parent 1fc2c77
commit b57849c
Show file tree

Hide file tree

Showing 5 changed files with 165 additions and 20 deletions.
diff --git a/config_scheme.json b/config_scheme.json
@@ -151,12 +151,12 @@
     "JsonConfig": {
       "description": "configuration for the json compare module",
       "type": "object",
-      "required": [
-        "ignore_keys"
-      ],
       "properties": {
         "ignore_keys": {
-          "type": "array",
+          "type": [
+            "array",
+            "null"
+          ],
           "items": {
             "type": "string"
           }

diff --git a/src/json.rs b/src/json.rs
@@ -1,6 +1,7 @@
 use crate::report::{DiffDetail, Difference};
 use crate::Error;
 use itertools::Itertools;
+use regex::Regex;
 use schemars_derive::JsonSchema;
 use serde::{Deserialize, Serialize};
 use std::path::Path;
@@ -9,7 +10,20 @@ use tracing::error;
 #[derive(Debug, Deserialize, Serialize, JsonSchema, Clone)]
 /// configuration for the json compare module
 pub struct JsonConfig {
-    ignore_keys: Vec<String>,
+    ignore_keys: Option<Vec<String>>,
+}
+impl JsonConfig {
+    pub(crate) fn get_ignore_list(&self) -> Result<Vec<Regex>, regex::Error> {
+        let exclusion_list: Option<Result<Vec<_>, regex::Error>> = self
+            .ignore_keys
+            .as_ref()
+            .map(|v| v.iter().map(|v| Regex::new(v)).collect());
+        return if let Some(result) = exclusion_list {
+            result
+        } else {
+            Ok(Vec::new())
+        };
+    }
 }
 
 pub(crate) fn compare_files<P: AsRef<Path>>(
@@ -22,6 +36,7 @@ pub(crate) fn compare_files<P: AsRef<Path>>(
 
     let nominal = vg_errortools::fat_io_wrap_std(&nominal, &std::fs::read_to_string)?;
     let actual = vg_errortools::fat_io_wrap_std(&actual, &std::fs::read_to_string)?;
+    let ignores = config.get_ignore_list()?;
 
     let json_diff = json_diff::process::compare_jsons(&nominal, &actual);
     let json_diff = match json_diff {
@@ -38,24 +53,128 @@ pub(crate) fn compare_files<P: AsRef<Path>>(
     let filtered_diff: Vec<_> = json_diff
         .all_diffs()
         .into_iter()
-        .filter(|(_d, v)| !config.ignore_keys.contains(v))
+        .filter(|(_d, v)| !ignores.iter().any(|excl| excl.is_match(v.get_key())))
         .collect();
 
     if !filtered_diff.is_empty() {
-        let all_diffs_log = filtered_diff
+        for (d_type, key) in filtered_diff.iter() {
+            error!("{d_type}: {key}");
+        }
+        let left = filtered_diff
+            .iter()
+            .filter_map(|(k, v)| {
+                if matches!(k, json_diff::enums::DiffType::LeftExtra) {
+                    Some(v.to_string())
+                } else {
+                    None
+                }
+            })
+            .join("\n");
+        let right = filtered_diff
             .iter()
-            .map(|(d, v)| format!("{d}: {v}"))
+            .filter_map(|(k, v)| {
+                if matches!(k, json_diff::enums::DiffType::RightExtra) {
+                    Some(v.to_string())
+                } else {
+                    None
+                }
+            })
             .join("\n");
+        let differences = filtered_diff
+            .iter()
+            .filter_map(|(k, v)| {
+                if matches!(k, json_diff::enums::DiffType::Mismatch) {
+                    Some(v.to_string())
+                } else {
+                    None
+                }
+            })
+            .join("\n");
+        let root_mismatch = filtered_diff
+            .iter()
+            .find(|(k, _v)| matches!(k, json_diff::enums::DiffType::RootMismatch))
+            .map(|(_, v)| v.to_string());
+
         diff.push_detail(DiffDetail::Json {
-            differences: all_diffs_log,
+            differences,
+            left,
+            right,
+            root_mismatch,
         });
 
         diff.error();
     }
 
-    for (d_type, key) in filtered_diff {
-        error!("{d_type}: {key}");
+    Ok(diff)
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    fn trim_split(list: &str) -> Vec<&str> {
+        list.split("\n").map(|e| e.trim()).collect()
+    }
+
+    #[test]
+    fn no_filter() {
+        let cfg = JsonConfig {
+            ignore_keys: Some(vec![]),
+        };
+        let result = compare_files(
+            "tests/integ/data/json/expected/guy.json",
+            "tests/integ/data/json/actual/guy.json",
+            &cfg,
+        )
+        .unwrap();
+        if let DiffDetail::Json {
+            differences,
+            left,
+            right,
+            root_mismatch,
+        } = result.detail.first().unwrap()
+        {
+            let differences = trim_split(differences);
+            assert!(differences.contains(&"car -> [ \"RX7\" :: \"Panda Trueno\" ]"));
+            assert!(differences.contains(&"age -> [ 21 :: 18 ]"));
+            assert!(differences.contains(&"name -> [ \"Keisuke\" :: \"Takumi\" ]"));
+            assert_eq!(differences.len(), 3);
+
+            assert_eq!(left.as_str(), " brothers");
+            assert!(right.is_empty());
+            assert!(root_mismatch.is_none());
+        } else {
+            panic!("wrong diffdetail");
+        }
     }
 
-    Ok(diff)
+    #[test]
+    fn filter_works() {
+        let cfg = JsonConfig {
+            ignore_keys: Some(vec!["name".to_string(), "brother(s?)".to_string()]),
+        };
+        let result = compare_files(
+            "tests/integ/data/json/expected/guy.json",
+            "tests/integ/data/json/actual/guy.json",
+            &cfg,
+        )
+        .unwrap();
+        if let DiffDetail::Json {
+            differences,
+            left,
+            right,
+            root_mismatch,
+        } = result.detail.first().unwrap()
+        {
+            let differences = trim_split(differences);
+            assert!(differences.contains(&"car -> [ \"RX7\" :: \"Panda Trueno\" ]"));
+            assert!(differences.contains(&"age -> [ 21 :: 18 ]"));
+            assert_eq!(differences.len(), 2);
+            assert!(right.is_empty());
+            assert!(left.is_empty());
+            assert!(root_mismatch.is_none());
+        } else {
+            panic!("wrong diffdetail");
+        }
+    }
 }
diff --git a/src/report/mod.rs b/src/report/mod.rs
@@ -148,6 +148,9 @@ pub enum DiffDetail {
     },
     Json {
         differences: String,
+        right: String,
+        left: String,
+        root_mismatch: Option<String>,
     },
     Properties(MetaDataPropertyDiff),
     Error(String),
@@ -506,7 +509,10 @@ pub fn write_external_detail(
 pub fn write_json_detail(
     nominal: impl AsRef<Path>,
     actual: impl AsRef<Path>,
+    left: &str,
+    right: &str,
     differences: &str,
+    root_mismatch: &Option<String>,
     report_dir: impl AsRef<Path>,
 ) -> Result<Option<DetailPath>, Error> {
     let detail_path = create_detail_folder(report_dir.as_ref())?;
@@ -522,6 +528,9 @@ pub fn write_json_detail(
     ctx.insert("actual", &actual.as_ref().to_string_lossy());
     ctx.insert("nominal", &nominal.as_ref().to_string_lossy());
     ctx.insert("differences", differences);
+    ctx.insert("left", left);
+    ctx.insert("right", right);
+    ctx.insert("root_mismatch", root_mismatch);
 
     let file = fat_io_wrap_std(&detail_file, &File::create)?;
     debug!("detail html {:?} created", &detail_file);
@@ -757,19 +766,27 @@ pub(crate) fn create_html(
                         }
                     }
                     ComparisonMode::Json(_) => {
-                        if let Some(differences) = file
+                        if let Some((differences, left, right, root_mismatch)) = file
                             .detail
                             .iter()
                             .filter_map(|r| match r {
-                                DiffDetail::Json { differences } => Some(differences),
+                                DiffDetail::Json {
+                                    left,
+                                    differences,
+                                    right,
+                                    root_mismatch,
+                                } => Some((differences, left, right, root_mismatch)),
                                 _ => None,
                             })
                             .next()
                         {
                             write_json_detail(
                                 &file.nominal_file,
                                 &file.actual_file,
+                                left,
+                                right,
                                 differences,
+                                root_mismatch,
                                 &sub_folder,
                             )
                             .unwrap_or_else(|e| log_detail_html_creation_error(&e))

diff --git a/src/report/template.rs b/src/report/template.rs
@@ -714,9 +714,12 @@ pub const PLAIN_JSON_DETAIL_TEMPLATE: &str = r###"
 			color: #0d6efdf0;
 		}
 
-		.has_error {
+		.has_right {
 			color:red;
 		}
+		.has_left {
+			color:green;
+		}
 
 		#compare th {
 			text-align:left;
@@ -741,18 +744,26 @@ pub const PLAIN_JSON_DETAIL_TEMPLATE: &str = r###"
 <body>
 
 <h3>Compare Result of {{ actual }} and {{ nominal }}</h3>
-
+<div>{{ root_mismatch }} </div>
 <table id="compare">
 	<thead>
 		<tr>
+			<th>Left extra</th>
 			<th>Differences</th>
+			<th>Right extra</th>
 		</tr>
 	</thead>
 	<tbody>
 		<tr>
-			<td class="has_error">
+			<td class="has_left">
+{{ left }}
+			</td>
+			<td >
 {{ differences }}
 			</td>
+			<td class="has_right">
+{{ right }}
+			</td>
 		</tr>
 	</tbody>
 </table>

diff --git a/tests/integ/json.yml b/tests/integ/json.yml
@@ -2,6 +2,4 @@ rules:
   - name: "Compare JSON files"
     pattern_include:
       - "**/*.json"
-    Json:
-      ignore_keys:
-        - ""
+    Json: {}