From 5a7882f6ea81ccb3dcb85ffccde5ad00103da713 Mon Sep 17 00:00:00 2001
From: blissful <blissful@sunsetglow.net>
Date: Tue, 31 Oct 2023 20:44:21 -0400
Subject: [PATCH] fix some rules engine bugs

---
 rose/cli.py              |  2 +-
 rose/rule_parser.py      | 31 +++++++++++++++++++++++++++++--
 rose/rule_parser_test.py |  8 ++++----
 rose/rules.py            | 14 +++++++-------
 4 files changed, 41 insertions(+), 14 deletions(-)

diff --git a/rose/cli.py b/rose/cli.py
index 30a0e26..950343a 100644
--- a/rose/cli.py
+++ b/rose/cli.py
@@ -403,7 +403,7 @@ def metadata() -> None:
 @click.pass_obj
 def run_stored_rules(ctx: Context, yes: bool) -> None:
     """Run the metadata rules stored in the config"""
-    execute_stored_metadata_rules(ctx.config, confirm_yes=yes)
+    execute_stored_metadata_rules(ctx.config, confirm_yes=not yes)
 
 
 @cli.command()
diff --git a/rose/rule_parser.py b/rose/rule_parser.py
index 77b6718..8e26718 100644
--- a/rose/rule_parser.py
+++ b/rose/rule_parser.py
@@ -45,6 +45,22 @@ class InvalidRuleSpecError(RoseError):
 ]
 
 
+SINGLE_VALUE_TAGS: list[Tag] = [
+    "tracktitle",
+    "year",
+    "tracknumber",
+    "discnumber",
+    "albumtitle",
+    "releasetype",
+]
+
+MULTI_VALUE_TAGS: list[Tag] = [
+    "genre",
+    "label",
+    "artist",
+]
+
+
 @dataclass
 class ReplaceAction:
     """
@@ -142,8 +158,6 @@ def parse_dict(cls, data: dict[str, Any]) -> MetadataRule:
                 raise InvalidRuleSpecError(
                     f"Key `tags`'s values must be one of *, {', '.join(ALL_TAGS)}: got {t}"
                 )
-        if any(t == "*" for t in tags):
-            tags = ALL_TAGS
 
         try:
             matcher = data["matcher"]
@@ -228,6 +242,19 @@ def parse_dict(cls, data: dict[str, Any]) -> MetadataRule:
                 f"got {action_kind}"
             )
 
+        # Validate that the action kind and tags are acceptable. Mainly that we are not calling
+        # `replaceall` and `splitall` on single-valued tags.
+        multi_value_action = action_kind == "replaceall" or action_kind == "spliton"
+        if any(t == "*" for t in tags):
+            tags = MULTI_VALUE_TAGS if multi_value_action else ALL_TAGS
+        if multi_value_action:
+            single_valued_tags = [t for t in tags if t in SINGLE_VALUE_TAGS]
+            if single_valued_tags:
+                raise InvalidRuleSpecError(
+                    f"Single valued tags {', '.join(single_valued_tags)} cannot be modified by "
+                    f"multi-value action {action_kind}"
+                )
+
         return cls(
             tags=tags,
             matcher=matcher,
diff --git a/rose/rule_parser_test.py b/rose/rule_parser_test.py
index 0461bcf..2aaec35 100644
--- a/rose/rule_parser_test.py
+++ b/rose/rule_parser_test.py
@@ -70,12 +70,12 @@ def test_rule_parser() -> None:
     # Test replaceall
     assert MetadataRule.parse_dict(
         {
-            "tags": "tracktitle",
+            "tags": "genre",
             "matcher": "lala",
             "action": {"kind": "replaceall", "replacement": ["hihi"]},
         }
     ) == MetadataRule(
-        tags=["tracktitle"],
+        tags=["genre"],
         matcher="lala",
         action=ReplaceAllAction(replacement=["hihi"]),
     )
@@ -96,12 +96,12 @@ def test_rule_parser() -> None:
     # Test spliton
     assert MetadataRule.parse_dict(
         {
-            "tags": "tracktitle",
+            "tags": "genre",
             "matcher": "lala",
             "action": {"kind": "spliton", "delimiter": "."},
         }
     ) == MetadataRule(
-        tags=["tracktitle"],
+        tags=["genre"],
         matcher="lala",
         action=SplitAction(delimiter="."),
     )
diff --git a/rose/rules.py b/rose/rules.py
index efc55d1..89599c8 100644
--- a/rose/rules.py
+++ b/rose/rules.py
@@ -99,7 +99,7 @@ def matches_rule(x: str) -> bool:
         LEFT JOIN releases_labels rl ON rg.release_id = r.id
         LEFT JOIN releases_artists ra ON ra.release_id = r.id
         LEFT JOIN tracks_artists ta ON ta.track_id = t.id
-        WHERE 1=1
+        WHERE false
     """
     args: list[str] = []
     for field in rule.tags:
@@ -123,19 +123,17 @@ def matches_rule(x: str) -> bool:
         if field == "releasetype":
             query += r" OR r.release_type LIKE ? ESCAPE '\'"
             args.append(matchsql)
-        # For genres, labels, and artists, because SQLite lacks arrays, we create a string like
-        # `\\ val1 \\ val2 \\` and match on `\\ {matcher} \\`.
         if field == "genre":
             query += r" OR rg.genre LIKE ? ESCAPE '\'"
-            args.append(rf" \\ {matchsql} \\ ")
+            args.append(matchsql)
         if field == "label":
             query += r" OR rl.label LIKE ? ESCAPE '\'"
-            args.append(rf" \\ {matchsql} \\ ")
+            args.append(matchsql)
         if field == "artist":
             query += r" OR ra.artist LIKE ? ESCAPE '\'"
-            args.append(rf" \\ {matchsql} \\ ")
+            args.append(matchsql)
             query += r" OR ta.artist LIKE ? ESCAPE '\'"
-            args.append(rf" \\ {matchsql} \\ ")
+            args.append(matchsql)
     query += " ORDER BY t.source_path"
     logger.debug(f"Constructed matching query {query} with args {args}")
     # And then execute the SQL query. Note that we don't pull the tag values here. This query is
@@ -144,6 +142,8 @@ def matches_rule(x: str) -> bool:
     with connect(c) as conn:
         track_paths = [Path(row["source_path"]).resolve() for row in conn.execute(query, args)]
     logger.debug(f"Matched {len(track_paths)} tracks from the read cache")
+    if not track_paths:
+        return
 
     # Factor out the logic for executing an action on a single-value tag and a multi-value tag.
     def execute_single_action(value: str | None) -> str | None: