From 559f09bbff5fd8d1f987d3055d3dd87693bb6112 Mon Sep 17 00:00:00 2001
From: Nico Wagner <n.wagner@dnb.de>
Date: Tue, 11 Feb 2025 21:22:42 +0100
Subject: [PATCH] fix: handling of empty allow-lists (#903)

Signed-off-by: Nico Wagner <n.wagner@dnb.de>
---
 crates/pica-cli/src/utils.rs        | 32 +++++++++++++-----------
 crates/pica-cli/tests/filter/mod.rs | 38 +++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 14 deletions(-)

diff --git a/crates/pica-cli/src/utils.rs b/crates/pica-cli/src/utils.rs
index 129a04630..3419fb592 100644
--- a/crates/pica-cli/src/utils.rs
+++ b/crates/pica-cli/src/utils.rs
@@ -20,8 +20,8 @@ pub(crate) enum FilterSetError {
 }
 
 pub(crate) struct FilterSet {
-    allow: HashSet<BString>,
-    deny: HashSet<BString>,
+    allow: Option<HashSet<BString>>,
+    deny: Option<HashSet<BString>>,
 }
 
 impl FilterSet {
@@ -29,29 +29,33 @@ impl FilterSet {
         allow: Vec<P>,
         deny: Vec<P>,
     ) -> Result<Self, FilterSetError> {
-        Ok(Self {
-            allow: read_filter_set(allow)?,
-            deny: read_filter_set(deny)?,
-        })
+        let allow = read_filter_set(allow)?;
+        let deny = read_filter_set(deny)?;
+
+        Ok(Self { allow, deny })
     }
 
     #[inline]
     pub(crate) fn check(&self, ppn: &BStr) -> bool {
-        if self.allow.is_empty() && self.deny.is_empty() {
-            return true;
+        if let Some(ref deny) = self.deny {
+            if deny.contains(ppn) {
+                return false;
+            }
         }
 
-        if !self.allow.is_empty() && !self.allow.contains(ppn) {
-            false
-        } else {
-            !self.deny.contains(ppn)
+        if let Some(ref allow) = self.allow {
+            if !allow.contains(ppn) || allow.is_empty() {
+                return false;
+            }
         }
+
+        true
     }
 }
 
 fn read_filter_set<P: AsRef<Path>>(
     paths: Vec<P>,
-) -> Result<HashSet<BString>, FilterSetError> {
+) -> Result<Option<HashSet<BString>>, FilterSetError> {
     let mut set = HashSet::new();
 
     for path in paths.iter() {
@@ -73,7 +77,7 @@ fn read_filter_set<P: AsRef<Path>>(
         );
     }
 
-    Ok(set)
+    Ok(if !paths.is_empty() { Some(set) } else { None })
 }
 
 fn read_df<P: AsRef<Path>>(
diff --git a/crates/pica-cli/tests/filter/mod.rs b/crates/pica-cli/tests/filter/mod.rs
index 1a130f070..2cea36f03 100644
--- a/crates/pica-cli/tests/filter/mod.rs
+++ b/crates/pica-cli/tests/filter/mod.rs
@@ -355,6 +355,25 @@ fn filter_allow() -> TestResult {
         .stderr(predicates::str::is_empty());
     temp_dir.close().unwrap();
 
+    // empty allow list
+    let mut cmd = Command::cargo_bin("pica")?;
+    let temp_dir = TempDir::new().unwrap();
+    let allow = temp_dir.child("allow.csv");
+    allow.write_str("idn\n").unwrap();
+
+    let assert = cmd
+        .args(["filter", "-s", "003@?"])
+        .args(["-A", allow.to_str().unwrap()])
+        .arg(data_dir().join("DUMP.dat.gz"))
+        .assert();
+
+    assert
+        .success()
+        .code(0)
+        .stdout(predicates::str::is_empty())
+        .stderr(predicates::str::is_empty());
+    temp_dir.close().unwrap();
+
     Ok(())
 }
 
@@ -430,6 +449,25 @@ fn filter_deny() -> TestResult {
         .stderr(predicates::str::is_empty());
     temp_dir.close().unwrap();
 
+    // empty deny list
+    let mut cmd = Command::cargo_bin("pica")?;
+    let temp_dir = TempDir::new().unwrap();
+    let deny = temp_dir.child("deny.csv");
+    deny.write_str("idn\n").unwrap();
+
+    let assert = cmd
+        .args(["filter", "-s", "003@?"])
+        .args(["-D", deny.to_str().unwrap()])
+        .arg(data_dir().join("ada.dat"))
+        .assert();
+
+    assert
+        .success()
+        .code(0)
+        .stdout(predicates::path::eq_file(data_dir().join("ada.dat")))
+        .stderr(predicates::str::is_empty());
+    temp_dir.close().unwrap();
+
     Ok(())
 }