fix(api): accept an existing tag UUID in the watch tag field, and deprecate it (#4361)

`tag` on POST /watch was documented as taking a tag UUID, but the value went to
add_tag(title): a UUID silently created a junk tag *titled* with that UUID and never
applied the tag the caller asked for. `tags` (UUIDs) was the only thing that worked.

- `tag=` now resolves an existing tag UUID to that tag, still falling back to title
  matching/creation for names. A UUID-shaped value matching nothing is skipped with a
  warning rather than becoming a group named after a UUID.
- Blank tokens ("One,,Two,") no longer store False in watch['tags'] - add_tag() returns
  False for an empty title and it was appended unguarded. Consumers tolerate it
  (get_all_tags_for_watch() dictfilt()s over known tags) but it is not valid data.
- add_tag()'s title search is extracted to tag_uuid_for_title(), so existence can be
  tested without creating as a side effect. add_tag()'s contract is unchanged.
- api-spec: `tag` is marked `deprecated: true` (so Redoc renders the badge) and states
  plainly that it takes names, not UUIDs. `tags` now says what it really does - applied
  verbatim, never creates, unknown UUIDs stored as dangling refs. Rendered docs rebuilt.

Every claim in the new field docs is asserted in test_api_tags.py against the real API.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

changedetectionio/store/__init__.py

@@ -49,6 +49,9 @@ dictfilt = lambda x, y: dict([(i, x[i]) for i in x if i in set(y)])
 # Is there an existing library to ensure some data store (JSON etc) is in sync with CRUD methods?
 # Open a github issue if you know something :)
 # https://stackoverflow.com/questions/6190468/how-to-trigger-function-on-value-change
+_TAG_UUID_RE = re.compile(r'^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$', re.IGNORECASE)
+
+
 class ChangeDetectionStore(DatastoreUpdatesMixin, FileSavingDataStore):
     __version_check = True
 
@@ -791,11 +794,31 @@ class ChangeDetectionStore(DatastoreUpdatesMixin, FileSavingDataStore):
             return None
 
         if tag and type(tag) == str:
-            # Then it's probably a string of the actual tag by name, split and add it
-            for t in tag.split(','):
-                # for each stripped tag, add tag as UUID
-                for a_t in t.split(','):
-                    tag_uuid = self.add_tag(a_t)
+            # A comma separated string of tag *titles*, created when they don't exist yet.
+            # An existing tag's UUID is accepted here too: the API documented this field as taking
+            # a UUID for years, and honouring that beats creating a tag *titled* with the UUID.
+            existing_tag_uuids = self.__data['settings']['application'].get('tags', {})
+
+            for tag_name in tag.split(','):
+                tag_name = tag_name.strip()
+                if not tag_name:
+                    continue
+
+                if _TAG_UUID_RE.match(tag_name):
+                    if tag_name in existing_tag_uuids:
+                        apply_extras['tags'].append(tag_name)
+                        continue
+                    # UUID-shaped but no such tag, and no tag literally titled that either -
+                    # a stale or foreign ID. Skip it rather than leave behind a group named
+                    # after a UUID, which is never what the caller wanted.
+                    if not self.tag_uuid_for_title(tag_name):
+                        logger.warning(f"Tag '{tag_name}' looks like a UUID but no such tag exists, skipping")
+                        continue
+
+                tag_uuid = self.add_tag(tag_name)
+                # add_tag() returns False for a title it won't create - never let that into the list,
+                # a falsy entry blows up every lookup of watch['tags']
+                if tag_uuid:
                     apply_extras['tags'].append(tag_uuid)
 
         # Or if UUIDs given directly
@@ -1069,6 +1092,18 @@ class ChangeDetectionStore(DatastoreUpdatesMixin, FileSavingDataStore):
 
         return ret
 
+    def tag_uuid_for_title(self, title):
+        """UUID of the tag with this title (case/space insensitive), or None. Creates nothing."""
+        n = title.strip().lower()
+        if not n:
+            return None
+
+        for uuid, tag in self.__data['settings']['application'].get('tags', {}).items():
+            if n == tag.get('title', '').lower().strip():
+                return uuid
+
+        return None
+
     def add_tag(self, title):
         # If name exists, return that
         n = title.strip().lower()
@@ -1076,10 +1111,10 @@ class ChangeDetectionStore(DatastoreUpdatesMixin, FileSavingDataStore):
         if not n:
             return False
 
-        for uuid, tag in self.__data['settings']['application'].get('tags', {}).items():
-            if n == tag.get('title', '').lower().strip():
-                logger.warning(f"Tag '{title}' already exists, skipping creation.")
-                return uuid
+        existing_uuid = self.tag_uuid_for_title(title)
+        if existing_uuid:
+            logger.warning(f"Tag '{title}' already exists, skipping creation.")
+            return existing_uuid
 
         # Eventually almost everything todo with a watch will apply as a Tag
         # So we use the same model as a Watch

changedetectionio/tests/test_api_tags.py

@@ -321,3 +321,80 @@ def test_roundtrip_API(client, live_server, measure_memory_usage, datastore_path
     date_created = res.json.get('date_created')
     assert date_created != 454444444444, "ReadOnly date_created should not be updateable"
     assert date_created != "454444444444", "ReadOnly date_created should not be updateable"
+
+
+def test_api_watch_tag_field_accepts_names_and_uuids(client, live_server, measure_memory_usage, datastore_path):
+    """The `tag` field on a watch takes tag *names*, `tags` takes UUIDs.
+
+    `tag` was documented as taking a UUID for years while the code fed it to add_tag(title),
+    so a UUID silently created a junk group *titled* with that UUID and never applied the tag
+    the caller asked for. Both spellings now resolve to the same tag.
+    """
+    api_key = live_server.app.config['DATASTORE'].data['settings']['application'].get('api_access_token')
+    datastore = live_server.app.config['DATASTORE']
+    test_url = url_for('test_endpoint', _external=True)
+    hdr = {'x-api-key': api_key, 'content-type': 'application/json'}
+
+    def titles_of(watch_uuid):
+        tags = datastore.data['settings']['application']['tags']
+        return sorted(tags[t].get('title') for t in datastore.data['watching'][watch_uuid].get('tags'))
+
+    # A name creates the group
+    res = client.post(url_for("createwatch"), data=json.dumps({"url": test_url, "tag": "helloworld"}), headers=hdr)
+    assert res.status_code == 201
+    assert titles_of(res.json['uuid']) == ['helloworld']
+
+    # An existing tag's UUID links to that tag rather than making a group named after the UUID
+    res = client.post(url_for("tag"), data=json.dumps({"title": "RealTag"}), headers=hdr)
+    assert res.status_code == 201
+    real_tag_uuid = res.json['uuid']
+    tag_count_before = len(datastore.data['settings']['application']['tags'])
+
+    res = client.post(url_for("createwatch"), data=json.dumps({"url": f"{test_url}?p=2", "tag": real_tag_uuid}), headers=hdr)
+    assert res.status_code == 201
+    assert real_tag_uuid in datastore.data['watching'][res.json['uuid']].get('tags')
+    assert titles_of(res.json['uuid']) == ['RealTag']
+    assert len(datastore.data['settings']['application']['tags']) == tag_count_before, "No junk tag titled with a UUID"
+
+    # `tags` with UUIDs keeps working
+    res = client.post(url_for("createwatch"), data=json.dumps({"url": f"{test_url}?p=3", "tags": [real_tag_uuid]}), headers=hdr)
+    assert res.status_code == 201
+    assert titles_of(res.json['uuid']) == ['RealTag']
+
+    # Names and UUIDs can be mixed, and blank entries from a trailing comma are dropped -
+    # add_tag() returns False for those and a falsy entry breaks every watch['tags'] lookup
+    res = client.post(url_for("createwatch"),
+                      data=json.dumps({"url": f"{test_url}?p=4", "tag": f"Mixed,,{real_tag_uuid},"}), headers=hdr)
+    assert res.status_code == 201
+    assert titles_of(res.json['uuid']) == ['Mixed', 'RealTag']
+    assert all(datastore.data['watching'][res.json['uuid']].get('tags')), "No falsy entries in tags"
+
+    # A UUID that matches no tag is skipped rather than becoming a group named after it
+    unknown_uuid = '0be0272a-19dc-4c97-8aae-5a68df319489'
+    tag_count_before = len(datastore.data['settings']['application']['tags'])
+    res = client.post(url_for("createwatch"),
+                      data=json.dumps({"url": f"{test_url}?p=5", "tag": unknown_uuid}), headers=hdr)
+    assert res.status_code == 201
+    assert datastore.data['watching'][res.json['uuid']].get('tags') == []
+    assert len(datastore.data['settings']['application']['tags']) == tag_count_before
+
+    # Names are matched case-insensitively against existing tags, as the spec claims
+    tag_count_before = len(datastore.data['settings']['application']['tags'])
+    res = client.post(url_for("createwatch"),
+                      data=json.dumps({"url": f"{test_url}?p=6", "tag": "rEaLtAg"}), headers=hdr)
+    assert res.status_code == 201
+    assert datastore.data['watching'][res.json['uuid']].get('tags') == [real_tag_uuid]
+    assert len(datastore.data['settings']['application']['tags']) == tag_count_before, "Casing must not fork a second tag"
+
+    # `tags` is applied verbatim and never creates: an unknown UUID is stored as a dangling
+    # reference that simply resolves to no group. Documented, and harmless because the lookup
+    # is a dictfilt() over known tags - pinned here so changing it has to be deliberate.
+    bogus = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee'
+    tag_count_before = len(datastore.data['settings']['application']['tags'])
+    res = client.post(url_for("createwatch"),
+                      data=json.dumps({"url": f"{test_url}?p=7", "tags": [bogus]}), headers=hdr)
+    assert res.status_code == 201
+    assert datastore.data['watching'][res.json['uuid']].get('tags') == [bogus]
+    assert len(datastore.data['settings']['application']['tags']) == tag_count_before
+    assert datastore.get_all_tags_for_watch(res.json['uuid']) == {}
+    assert client.get(url_for("watchlist.index")).status_code == 200, "A dangling tag ref must not break the list"

docs/api-spec.yaml

@@ -299,13 +299,24 @@ components:
           maxLength: 5000
         tag:
           type: string
-          description: Tag UUID to associate with this web page change monitor (watch)
+          deprecated: true
+          description: |
+            **Deprecated - use `tags` instead.** Kept working for API v1 and may be removed in v2.
+
+            Takes comma-separated tag *names* (not UUIDs, despite what earlier revisions of this
+            document said). Names are matched case-insensitively against existing tags and created
+            when they don't exist yet. An existing tag's UUID is also accepted and resolves to that
+            tag; a UUID matching no tag is ignored.
           maxLength: 5000
+          example: "Production, Price tracking"
         tags:
           type: array
           items:
             type: string
-          description: Array of tag UUIDs
+          description: |
+            Array of tag UUIDs. Unlike `tag`, these are applied exactly as given - no tag is ever
+            created from this field. The UUIDs are not validated: an unknown one is stored but
+            resolves to no group.
         paused:
           type: boolean
           description: Whether the web page change monitor (watch) is paused