From db116971ce5182084c4eebc25b1088b1fad58e2e Mon Sep 17 00:00:00 2001
From: Adam Warner <me@adamwarner.co.uk>
Date: Thu, 14 Apr 2022 22:53:38 +0100
Subject: [PATCH] I tried to do too many things in one function, vastly
 overcomplicating what should have been _this_ all along

Signed-off-by: Adam Warner <me@adamwarner.co.uk>
---
 advanced/Scripts/utils.sh | 42 ++++++++++++++++++++++-----------------
 pihole                    |  4 ++--
 test/test_any_utils.py    |  4 ++--
 3 files changed, 28 insertions(+), 22 deletions(-)

diff --git a/advanced/Scripts/utils.sh b/advanced/Scripts/utils.sh
index f457427f..f0a7cc37 100755
--- a/advanced/Scripts/utils.sh
+++ b/advanced/Scripts/utils.sh
@@ -17,43 +17,49 @@
 #  - New functions must have a test added for them in test/test_any_utils.py
 
 #######################
-# Takes either
-#   - Three arguments: file, key, and value.
-#   - Two arguments: file, and key.
+# Takes Three arguments: file, key, and value.
 #
 # Checks the target file for the existence of the key
 #   - If it exists, it changes the value
 #   - If it does not exist, it adds the value
 #
 # Example usage:
-# addOrEditKeyValuePair "/etc/pihole/setupVars.conf" "BLOCKING_ENABLED" "true"
+# addOrEditKeyValPair "/etc/pihole/setupVars.conf" "BLOCKING_ENABLED" "true"
 #######################
 addOrEditKeyValPair() {
   local file="${1}"
   local key="${2}"
   local value="${3}"
 
-  if [ "${value}" != "" ]; then
-    # value has a value, so it is a key-value pair
-    if grep -q "^${key}=" "${file}"; then
+  if grep -q "^${key}=" "${file}"; then
       # Key already exists in file, modify the value
       sed -i "/^${key}=/c\\${key}=${value}" "${file}"
-    else
-      # Key does not already exist, add it and it's value
-      echo "${key}=${value}" >> "${file}"
-    fi
   else
-    # value has no value, so it is just a key. Add it if it does not already exist
-    if ! grep -q "^${key}" "${file}"; then
-      # Key does not exist, add it.
-      echo "${key}" >> "${file}"
-    fi
+    # Key does not already exist, add it and it's value
+    echo "${key}=${value}" >> "${file}"
   fi
 }
 
 #######################
-# Takes two arguments file, and key.
-# Deletes a key from target file
+# Takes two arguments: file, and key.
+# Adds a key to target file
+#
+# Example usage:
+# addKey "/etc/dnsmasq.d/01-pihole.conf" "log-queries"
+#######################
+addKey(){
+  local file="${1}"
+  local key="${2}"
+
+  if ! grep -q "^${key}" "${file}"; then
+      # Key does not exist, add it.
+      echo "${key}" >> "${file}"
+  fi
+}
+
+#######################
+# Takes two arguments: file, and key.
+# Deletes a key or key/value pair from target file
 #
 # Example usage:
 # removeKey "/etc/pihole/setupVars.conf" "PIHOLE_DNS_1"
diff --git a/pihole b/pihole
index 6823b3b6..f51fd956 100755
--- a/pihole
+++ b/pihole
@@ -260,7 +260,7 @@ Options:
     exit 0
   elif [[ "${1}" == "off" ]]; then
     # Disable logging
-    addOrEditKeyValPair /etc/dnsmasq.d/01-pihole.conf "log-queries"
+    removeKey /etc/dnsmasq.d/01-pihole.conf "log-queries"
     addOrEditKeyValPair "${setupVars}" "QUERY_LOGGING" "false"
     if [[ "${2}" != "noflush" ]]; then
       # Flush logs
@@ -270,7 +270,7 @@ Options:
     local str="Logging has been disabled!"
   elif [[ "${1}" == "on" ]]; then
     # Enable logging
-    removeKey /etc/dnsmasq.d/01-pihole.conf "log-queries"
+    addKey /etc/dnsmasq.d/01-pihole.conf "log-queries"
     addOrEditKeyValPair "${setupVars}" "QUERY_LOGGING" "true"
     echo -e "  ${INFO} Enabling logging..."
     local str="Logging has been enabled!"
diff --git a/test/test_any_utils.py b/test/test_any_utils.py
index 998c1c84..07feaf0f 100644
--- a/test/test_any_utils.py
+++ b/test/test_any_utils.py
@@ -6,8 +6,8 @@ def test_key_val_replacement_works(host):
     addOrEditKeyValPair "./testoutput" "KEY_TWO" "value2"
     addOrEditKeyValPair "./testoutput" "KEY_ONE" "value3"
     addOrEditKeyValPair "./testoutput" "KEY_FOUR" "value4"
-    addOrEditKeyValPair "./testoutput" "KEY_FIVE_NO_VALUE"
-    addOrEditKeyValPair "./testoutput" "KEY_FIVE_NO_VALUE"
+    addKey "./testoutput" "KEY_FIVE_NO_VALUE"
+    addKey "./testoutput" "KEY_FIVE_NO_VALUE"
     ''')
     output = host.run('''
     cat ./testoutput