Refactor watch datatype

.github/workflows/test-stack-reusable-workflow.yml

@@ -71,6 +71,7 @@ jobs:
           docker run test-changedetectionio  bash -c 'python3 -m unittest changedetectionio.tests.unit.test_watch_model'
           docker run test-changedetectionio  bash -c 'python3 -m unittest changedetectionio.tests.unit.test_jinja2_security'
           docker run test-changedetectionio  bash -c 'python3 -m unittest changedetectionio.tests.unit.test_semver'
+          docker run test-changedetectionio  bash -c 'python3 -m unittest changedetectionio.tests.unit.test_update_watch_deep_merge'
 
       - name: Test built container with Pytest (generally as requests/plaintext fetching)
         run: |

changedetectionio/blueprint/ui/__init__.py

@@ -303,7 +303,7 @@ def form_share_put_watch(uuid):
         watch['ignore_text'] += datastore.data['settings']['application']['global_ignore_text']
         watch['subtractive_selectors'] += datastore.data['settings']['application']['global_subtractive_selectors']
 
-        watch_json = json.dumps(watch)
+        watch_json = json.dumps(dict(watch))
 
         try:
             r = requests.request(method="POST",

changedetectionio/model/Watch.py

@@ -43,14 +43,14 @@ def __init__(self, *arg, **kw):
         self.__datastore_path = kw.get('datastore_path')
         if kw.get('datastore_path'):
             del kw['datastore_path']
-
+
+        # Save default before passing to parent, since parent will delete it
+        default_values = kw.get('default')
+
         super(model, self).__init__(*arg, **kw)
-        if kw.get('default'):
-            self.update(kw['default'])
-            del kw['default']
-
-        if self.get('default'):
-            del self['default']
+
+        if default_values:
+            self.update(default_values)
 
         # Be sure the cached timestamp is ready
         bump = self.history
@@ -227,8 +227,8 @@ def history(self):
 
     @property
     def has_history(self):
-        fname = os.path.join(self.watch_data_dir, "history.txt")
-        return os.path.isfile(fname)
+        fname = self._get_data_file_path("history.txt")
+        return fname and os.path.isfile(fname)
 
     @property
     def has_browser_steps(self):
@@ -405,16 +405,16 @@ def lines_contain_something_unique_compared_to_history(self, lines: list, ignore
         return not local_lines.issubset(existing_history)
 
     def get_screenshot(self):
-        fname = os.path.join(self.watch_data_dir, "last-screenshot.png")
-        if os.path.isfile(fname):
+        fname = self._get_data_file_path("last-screenshot.png")
+        if fname and os.path.isfile(fname):
             return fname
 
         # False is not an option for AppRise, must be type None
         return None
 
     def __get_file_ctime(self, filename):
-        fname = os.path.join(self.watch_data_dir, filename)
-        if os.path.isfile(fname):
+        fname = self._get_data_file_path(filename)
+        if fname and os.path.isfile(fname):
             return int(os.path.getmtime(fname))
         return False
 
@@ -441,20 +441,28 @@ def snapshot_error_screenshot_ctime(self):
     @property
     def watch_data_dir(self):
         # The base dir of the watch data
-        return os.path.join(self.__datastore_path, self['uuid']) if self.__datastore_path else None
+        if self.__datastore_path and self.get('uuid'):
+            return os.path.join(self.__datastore_path, self['uuid'])
+        return None
+
+    def _get_data_file_path(self, filename):
+        """Safely get the full path to a data file, returns None if watch_data_dir is None"""
+        if self.watch_data_dir:
+            return os.path.join(self.watch_data_dir, filename)
+        return None
 
     def get_error_text(self):
         """Return the text saved from a previous request that resulted in a non-200 error"""
-        fname = os.path.join(self.watch_data_dir, "last-error.txt")
-        if os.path.isfile(fname):
+        fname = self._get_data_file_path("last-error.txt")
+        if fname and os.path.isfile(fname):
             with open(fname, 'r') as f:
                 return f.read()
         return False
 
     def get_error_snapshot(self):
         """Return path to the screenshot that resulted in a non-200 error"""
-        fname = os.path.join(self.watch_data_dir, "last-error-screenshot.png")
-        if os.path.isfile(fname):
+        fname = self._get_data_file_path("last-error-screenshot.png")
+        if fname and os.path.isfile(fname):
             return fname
         return False
 

changedetectionio/model/__init__.py

@@ -1,13 +1,14 @@
 import os
 import uuid
+import json
 
 from changedetectionio import strtobool
 default_notification_format_for_watch = 'System default'
 
-class watch_base(dict):
+class watch_base:
 
     def __init__(self, *arg, **kw):
-        self.update({
+        self.__data = {
             # Custom notification content
             # Re #110, so then if this is set to None, we know to use the default value instead
             # Requires setting to None on submit if it's the same as the default
@@ -128,9 +129,78 @@ def __init__(self, *arg, **kw):
             'uuid': str(uuid.uuid4()),
             'webdriver_delay': None,
             'webdriver_js_execute_code': None,  # Run before change-detection
-        })
+        }
+
+        if len(arg) == 1 and (isinstance(arg[0], dict) or hasattr(arg[0], 'keys')):
+            self.__data.update(arg[0])
+        if kw:
+            self.__data.update(kw)
 
-        super(watch_base, self).__init__(*arg, **kw)
+        if self.__data.get('default'):
+            del self.__data['default']
 
-        if self.get('default'):
-            del self['default']
+    def __getitem__(self, key):
+        return self.__data[key]
+
+    def __setitem__(self, key, value):
+        self.__data[key] = value
+
+    def __delitem__(self, key):
+        del self.__data[key]
+
+    def __iter__(self):
+        return iter(self.__data)
+
+    def __len__(self):
+        return len(self.__data)
+
+    def __contains__(self, key):
+        return key in self.__data
+
+    def __repr__(self):
+        return repr(self.__data)
+
+    def __str__(self):
+        return str(self.__data)
+
+    def keys(self):
+        return self.__data.keys()
+
+    def values(self):
+        return self.__data.values()
+
+    def items(self):
+        return self.__data.items()
+
+    def get(self, key, default=None):
+        return self.__data.get(key, default)
+
+    def pop(self, key, *args):
+        return self.__data.pop(key, *args)
+
+    def popitem(self):
+        return self.__data.popitem()
+
+    def clear(self):
+        self.__data.clear()
+
+    def update(self, *args, **kwargs):
+        self.__data.update(*args, **kwargs)
+
+    def setdefault(self, key, default=None):
+        return self.__data.setdefault(key, default)
+
+    def copy(self):
+        return self.__data.copy()
+
+    def __deepcopy__(self, memo):
+        from copy import deepcopy
+        new_instance = self.__class__()
+        new_instance.__data = deepcopy(self.__data, memo)
+        return new_instance
+
+    def __reduce__(self):
+        return (self.__class__, (self.__data,))
+
+    def to_dict(self):
+        return dict(self.__data)

changedetectionio/processors/restock_diff/__init__.py

@@ -42,10 +42,10 @@ def __init__(self, *args, **kwargs):
 
         # Update with any provided positional arguments (dictionaries)
         if args:
-            if len(args) == 1 and isinstance(args[0], dict):
+            if len(args) == 1 and (isinstance(args[0], dict) or hasattr(args[0], 'keys')):
                 self.update(args[0])
             else:
-                raise ValueError("Only one positional argument of type 'dict' is allowed")
+                raise ValueError("Only one positional argument of type 'dict' or dict-like is allowed")
 
     def __setitem__(self, key, value):
         # Custom logic to handle setting price and original_price

changedetectionio/store.py

@@ -23,6 +23,13 @@
 from .processors import get_custom_watch_obj_for_processor
 from .processors.restock_diff import Restock
 
+class WatchEncoder(json.JSONEncoder):
+    def default(self, obj):
+        from .model import watch_base
+        if isinstance(obj, watch_base):
+            return dict(obj)
+        return super().default(obj)
+
 # Because the server will run as a daemon and wont know the URL for notification links when firing off a notification
 BASE_URL_NOT_SET_TEXT = '("Base URL" not set - see settings - notifications)'
 
@@ -51,9 +58,6 @@ def __init__(self, datastore_path="/datastore", include_default_watches=True, ve
         self.needs_write = False
         self.start_time = time.time()
         self.stop_thread = False
-        # Base definition for all watchers
-        # deepcopy part of #569 - not sure why its needed exactly
-        self.generic_definition = deepcopy(Watch.model(datastore_path = datastore_path, default={}))
 
         if path.isfile('changedetectionio/source.txt'):
             with open('changedetectionio/source.txt') as f:
@@ -174,22 +178,23 @@ def remove_password(self):
         self.__data['settings']['application']['password'] = False
         self.needs_write = True
 
+    def _deep_merge(self, target, source):
+        """Recursively merge source dict into target dict"""
+        for key, value in source.items():
+            if key in target and isinstance(target[key], dict) and isinstance(value, dict):
+                self._deep_merge(target[key], value)
+            else:
+                target[key] = value
+
     def update_watch(self, uuid, update_obj):
 
         # It's possible that the watch could be deleted before update
         if not self.__data['watching'].get(uuid):
             return
 
         with self.lock:
-
-            # In python 3.9 we have the |= dict operator, but that still will lose data on nested structures...
-            for dict_key, d in self.generic_definition.items():
-                if isinstance(d, dict):
-                    if update_obj is not None and dict_key in update_obj:
-                        self.__data['watching'][uuid][dict_key].update(update_obj[dict_key])
-                        del (update_obj[dict_key])
-
-            self.__data['watching'][uuid].update(update_obj)
+            # Use recursive merge to handle nested dictionaries properly
+            self._deep_merge(self.__data['watching'][uuid], update_obj)
         self.needs_write = True
 
     @property
@@ -393,6 +398,51 @@ def visualselector_data_is_ready(self, watch_uuid):
 
         return False
 
+    import json
+    import os
+    import tempfile
+    from pathlib import Path  # just for nicer paths
+
+    JSON_INDENT = 2  # or None in production
+    ENCODER = WatchEncoder  # your custom encoder
+
+    def save_json_atomic(self, save_path: str | os.PathLike, data) -> None:
+        """
+        Atomically (re)write *path* with *data* encoded as JSON.
+        The original file is left untouched if anything fails.
+        """
+        import tempfile
+        from pathlib import Path  # just for nicer paths
+
+        JSON_INDENT = 2  # or None in production
+        ENCODER = WatchEncoder  # your custom encoder
+
+        datapath = Path(save_path)
+        directory = datapath.parent
+
+        # 1. create a unique temp file in the same directory
+        fd, tmp_name = tempfile.mkstemp(
+            dir=directory,
+            prefix=f"{datapath.name}.",
+            suffix=".tmp",
+        )
+        try:
+            with os.fdopen(fd, "w", encoding="utf-8") as tmp:
+                json.dump(data, tmp, indent=JSON_INDENT, cls=ENCODER)
+                if os.getenv('JSON_SAVE_FORCE_FLUSH'):
+                    tmp.flush()  # push Python buffers
+                    os.fsync(tmp.fileno())  # force kernel to write to disk
+            os.replace(tmp_name, datapath)
+
+        except Exception as e:
+            logger.critical(f"Failed to write JSON to {datapath} - {str(e)}")
+            # if anything above blew up, ensure we don't leave junk lying around
+            try:
+                os.unlink(tmp_name)
+            finally:
+                raise
+
+
     def sync_to_json(self):
         logger.info("Saving JSON..")
         try:
@@ -404,18 +454,7 @@ def sync_to_json(self):
             self.sync_to_json()
             return
         else:
-
-            try:
-                # Re #286  - First write to a temp file, then confirm it looks OK and rename it
-                # This is a fairly basic strategy to deal with the case that the file is corrupted,
-                # system was out of memory, out of RAM etc
-                with open(self.json_store_path+".tmp", 'w') as json_file:
-                    # Use compact JSON in production for better performance
-                    json.dump(data, json_file, indent=2)
-                    os.replace(self.json_store_path+".tmp", self.json_store_path)
-            except Exception as e:
-                logger.error(f"Error writing JSON!! (Main JSON file save was skipped) : {str(e)}")
-
+            self.save_json_atomic(save_path = self.json_store_path, data =data)
             self.needs_write = False
             self.needs_write_urgent = False