From 3a5316cedc7ec1f06f8ff3346a0323367f18777f Mon Sep 17 00:00:00 2001
From: Jonathan van der Steege <jonathan@jonakeys.nl>
Date: Sat, 16 Jul 2022 23:21:21 +0200
Subject: [PATCH] Check for duplicate keys in config file (#1511)

* Check for duplicate keys in config file
* Corrected BDD tests
* Unneeded check removed
* Make use of ruamel DuplicateKeyError exception
* Remove unneeded import and function
* slightly reword warning message
* fix merge conflicts
* update tests

Co-authored-by: Jonathan Wren <jonathan@nowandwren.com>
---
 jrnl/config.py                         | 24 +++++++++++++++++++----
 jrnl/messages/MsgText.py               |  7 +++++++
 tests/bdd/features/config_file.feature | 17 ++++++++++++++++
 tests/data/configs/duplicate_keys.yaml | 27 ++++++++++++++++++++++++++
 4 files changed, 71 insertions(+), 4 deletions(-)
 create mode 100644 tests/data/configs/duplicate_keys.yaml

diff --git a/jrnl/config.py b/jrnl/config.py
index 045958c1..cea3720e 100644
--- a/jrnl/config.py
+++ b/jrnl/config.py
@@ -7,6 +7,7 @@ import os
 import colorama
 import xdg.BaseDirectory
 from ruamel.yaml import YAML
+from ruamel.yaml import constructor
 
 from jrnl import __version__
 from jrnl.exception import JrnlException
@@ -159,10 +160,25 @@ def verify_config_colors(config):
 
 def load_config(config_path):
     """Tries to load a config file from YAML."""
-    with open(config_path, encoding=YAML_FILE_ENCODING) as f:
-        yaml = YAML(typ="safe")
-        yaml.allow_duplicate_keys = True
-        return yaml.load(f)
+    try:
+        with open(config_path, encoding=YAML_FILE_ENCODING) as f:
+            yaml = YAML(typ="safe")
+            yaml.allow_duplicate_keys = False
+            return yaml.load(f)
+    except constructor.DuplicateKeyError as e:
+        print_msg(
+            Message(
+                MsgText.ConfigDoubleKeys,
+                MsgStyle.WARNING,
+                {
+                    "error_message": e,
+                },
+            )
+        )
+        with open(config_path, encoding=YAML_FILE_ENCODING) as f:
+            yaml = YAML(typ="safe")
+            yaml.allow_duplicate_keys = True
+            return yaml.load(f)
 
 
 def is_config_json(config_path):
diff --git a/jrnl/messages/MsgText.py b/jrnl/messages/MsgText.py
index d26f1dfd..b8250970 100644
--- a/jrnl/messages/MsgText.py
+++ b/jrnl/messages/MsgText.py
@@ -198,6 +198,13 @@ class MsgText(Enum):
         Configuration updated to newest version at {config_path}
         """
 
+    ConfigDoubleKeys = """
+        There is at least one duplicate key in your configuration file.
+
+        Details:
+        {error_message}
+        """
+
     # --- Password --- #
     Password = "Password:"
     PasswordFirstEntry = "Enter password for journal '{journal_name}': "
diff --git a/tests/bdd/features/config_file.feature b/tests/bdd/features/config_file.feature
index 76e93059..d6b6121f 100644
--- a/tests/bdd/features/config_file.feature
+++ b/tests/bdd/features/config_file.feature
@@ -106,3 +106,20 @@ Feature: Multiple journals
         And we use the config "basic_onefile.yaml"
         When we run "jrnl --cf empty_file.yaml"
         Then the error output should contain "Unable to parse config file"
+
+    Scenario: Show a warning message when the config file contains duplicate keys at the same level
+        Given the config "duplicate_keys.yaml" exists
+        And we use the config "duplicate_keys.yaml"
+        When we run "jrnl -1"
+        Then the output should contain "There is at least one duplicate key in your configuration file"
+    
+    Scenario: Show a warning message when using --config-file with duplicate keys
+        Given the config "duplicate_keys.yaml" exists
+        And we use the config "multiple.yaml"
+        When we run "jrnl --cf duplicate_keys.yaml -1"
+        Then the output should contain "There is at least one duplicate key in your configuration file"
+
+    Scenario: Don't show a duplicate keys warning message when using --config-override on an existing value
+        Given we use the config "multiple.yaml"
+        When we run "jrnl --config-override highlight false"
+        Then the output should not contain "There is at least one duplicate key in your configuration file"
\ No newline at end of file
diff --git a/tests/data/configs/duplicate_keys.yaml b/tests/data/configs/duplicate_keys.yaml
new file mode 100644
index 00000000..ca74c738
--- /dev/null
+++ b/tests/data/configs/duplicate_keys.yaml
@@ -0,0 +1,27 @@
+default_hour: 9
+default_minute: 0
+editor: ''
+encrypt: false
+highlight: true
+template: false
+template: false
+journals:
+  default: 
+    encrypt: false
+    journal: features/journals/simple.journal
+    journal: features/journals/simple.journal
+  ideas:
+    encrypt: false
+    journal: features/journals/does-not-exist.journal
+  simple: 
+    encrypt: false
+    journal: features/journals/simple.journal
+    encrypt: false
+  work:
+    encrypt: false
+    journal: features/journals/work.journal
+linewrap: 80
+tagsymbols: '@'
+editor: nano
+timeformat: '%Y-%m-%d %H:%M'
+indent_character: "|"