From b9461026a6f2716b5685202c0cabccc5456771ac Mon Sep 17 00:00:00 2001
From: Kevin Veen-Birkenbach <kevin@veen.world>
Date: Mon, 18 Aug 2025 22:36:31 +0200
Subject: [PATCH] refactor: improve get_service_name suffix handling and
 handler usage

- Updated filter_plugins/get_service_name.py:
  * Default suffix handling: auto-select .service (no '@') or .timer (with '@')
  * Explicit False disables suffix entirely
  * Explicit string suffix still supported
- Updated sys-systemctl handler to use new filter instead of SYS_SERVICE_SUFFIX
- Extended unit tests to cover new suffix behavior

Ref: https://chat.openai.com/share/8c2de9e6-daa0-44dd-ae13-d7a7d8d8b6d9
---
 filter_plugins/get_service_name.py            | 31 +++++++++++++------
 roles/sys-systemctl/handlers/main.yml         |  2 +-
 .../filter_plugins/test_get_service_name.py   | 30 +++++++++---------
 3 files changed, 39 insertions(+), 24 deletions(-)

diff --git a/filter_plugins/get_service_name.py b/filter_plugins/get_service_name.py
index 5ec565c4..6b891b96 100644
--- a/filter_plugins/get_service_name.py
+++ b/filter_plugins/get_service_name.py
@@ -1,23 +1,36 @@
-# filter_plugins/get_service_name.py
 """
 Custom Ansible filter to build a systemctl unit name (always lowercase).
 
 Rules:
 - If `systemctl_id` ends with '@': drop the '@' and return
-  "{systemctl_id_without_at}.{software_name}@.{suffix}".
-- Else: return "{systemctl_id}{software_name}.{suffix}".
+  "{systemctl_id_without_at}.{software_name}@{suffix_handling}".
+- Else: return "{systemctl_id}.{software_name}{suffix_handling}".
+
+Suffix handling:
+- Default "" → automatically pick:
+    - ".service" if no '@' in systemctl_id
+    - ".timer"  if '@' in systemctl_id
+- Explicit False → no suffix at all
+- Any string → ".{suffix}" (lowercased)
 """
 
-def get_service_name(systemctl_id, software_name, suffix="service"):
+def get_service_name(systemctl_id, software_name, suffix=""):
     sid = str(systemctl_id).strip().lower()
     sw  = str(software_name).strip().lower()
-    sfx = str(suffix).strip().lower()
 
-    if sid.endswith('@'):
-        base = sid[:-1]  # drop the trailing '@'
-        return f"{base}.{sw}@.{sfx}"
+    # Determine suffix
+    if suffix is False:
+        sfx = ""  # no suffix at all
+    elif suffix == "" or suffix is None:
+        sfx = ".timer" if sid.endswith("@") else ".service"
     else:
-        return f"{sid}.{sw}.{sfx}"
+        sfx = "." + str(suffix).strip().lower()
+
+    if sid.endswith("@"):
+        base = sid[:-1]  # drop the trailing '@'
+        return f"{base}.{sw}@{sfx}"
+    else:
+        return f"{sid}.{sw}{sfx}"
 
 
 class FilterModule(object):
diff --git a/roles/sys-systemctl/handlers/main.yml b/roles/sys-systemctl/handlers/main.yml
index ae8c327d..b5d0aeb3 100644
--- a/roles/sys-systemctl/handlers/main.yml
+++ b/roles/sys-systemctl/handlers/main.yml
@@ -1,6 +1,6 @@
 - name: "refresh systemctl service"
   systemd:
-    name: "{{ systemctl_id }}{{ SYS_SERVICE_SUFFIX }}"
+    name: "{{ systemctl_id | get_service_name(SOFTWARE_NAME) }}"
     daemon_reload: yes
     enabled: yes
     state: "{{ systemctl_state }}"
diff --git a/tests/unit/filter_plugins/test_get_service_name.py b/tests/unit/filter_plugins/test_get_service_name.py
index df89aa4b..fdd42940 100644
--- a/tests/unit/filter_plugins/test_get_service_name.py
+++ b/tests/unit/filter_plugins/test_get_service_name.py
@@ -2,30 +2,32 @@ import unittest
 from filter_plugins import get_service_name
 
 class TestGetServiceName(unittest.TestCase):
-    def test_normal_service(self):
-        # Expect a dot between id and software name
+    def test_default_suffix_service(self):
         self.assertEqual(
             get_service_name.get_service_name("sys-ctl-cln-backups", "nginx"),
             "sys-ctl-cln-backups.nginx.service"
         )
 
-    def test_normal_service_custom_suffix(self):
-        self.assertEqual(
-            get_service_name.get_service_name("sys-ctl-cln-backups", "nginx", "timer"),
-            "sys-ctl-cln-backups.nginx.timer"
-        )
-
-    def test_with_at_suffix(self):
-        # If systemctl_id ends with '@', @ is moved behind software name
+    def test_default_suffix_timer(self):
         self.assertEqual(
             get_service_name.get_service_name("sys-ctl-bkp@", "postgres"),
-            "sys-ctl-bkp.postgres@.service"
+            "sys-ctl-bkp.postgres@.timer"
         )
 
-    def test_with_at_and_custom_suffix(self):
+    def test_explicit_custom_suffix(self):
         self.assertEqual(
-            get_service_name.get_service_name("sys-ctl-bkp@", "postgres", "timer"),
-            "sys-ctl-bkp.postgres@.timer"
+            get_service_name.get_service_name("sys-ctl-bkp@", "postgres", "special"),
+            "sys-ctl-bkp.postgres@.special"
+        )
+
+    def test_explicit_false_suffix(self):
+        self.assertEqual(
+            get_service_name.get_service_name("sys-ctl-bkp@", "postgres", False),
+            "sys-ctl-bkp.postgres@"
+        )
+        self.assertEqual(
+            get_service_name.get_service_name("sys-ctl-cln-backups", "nginx", False),
+            "sys-ctl-cln-backups.nginx"
         )
 
     def test_case_is_lowered(self):