Unify metadata and tag namespaces.

For #266.

src/reader/_storage.py

@@ -67,7 +67,6 @@
 def create_db(db: sqlite3.Connection) -> None:
     create_feeds(db)
     create_entries(db)
-    create_feed_metadata(db)
     create_feed_tags(db)
     create_indexes(db)
 
@@ -145,31 +144,15 @@ def create_entries(db: sqlite3.Connection, name: str = 'entries') -> None:
     )
 
 
-def create_feed_metadata(db: sqlite3.Connection) -> None:
-    db.execute(
-        """
-        CREATE TABLE feed_metadata (
-            feed TEXT NOT NULL,
-            key TEXT NOT NULL,
-            value TEXT NOT NULL,
-
-            PRIMARY KEY (feed, key),
-            FOREIGN KEY (feed) REFERENCES feeds(url)
-                ON UPDATE CASCADE
-                ON DELETE CASCADE
-        );
-        """
-    )
-
-
 def create_feed_tags(db: sqlite3.Connection) -> None:
     db.execute(
         """
         CREATE TABLE feed_tags (
             feed TEXT NOT NULL,
-            tag TEXT NOT NULL,
+            key TEXT NOT NULL,
+            value TEXT NOT NULL,  -- FIXME: should be nullable
 
-            PRIMARY KEY (feed, tag),
+            PRIMARY KEY (feed, key),
             FOREIGN KEY (feed) REFERENCES feeds(url)
                 ON UPDATE CASCADE
                 ON DELETE CASCADE
@@ -325,6 +308,12 @@ def update_from_33_to_34(db: sqlite3.Connection) -> None:  # pragma: no cover
     recreate_search_triggers(db)
 
 
+def update_from_34_to_35(db: sqlite3.Connection) -> None:  # pragma: no cover
+    # for https://github.com/lemon24/reader/issues/266
+    # FIXME
+    raise NotImplementedError
+
+
 MINIMUM_SQLITE_VERSION = (3, 15)
 REQUIRED_SQLITE_COMPILE_OPTIONS = ["ENABLE_JSON1"]
 
@@ -333,7 +322,7 @@ def setup_db(db: sqlite3.Connection, wal_enabled: Optional[bool]) -> None:
     return setup_sqlite_db(
         db,
         create=create_db,
-        version=34,
+        version=35,
         migrations={
             # 1-9 removed before 0.1 (last in e4769d8ba77c61ec1fe2fbe99839e1826c17ace7)
             # 10-16 removed before 1.0 (last in 618f158ebc0034eefb724a55a84937d21c93c1a7)
@@ -343,6 +332,7 @@ def setup_db(db: sqlite3.Connection, wal_enabled: Optional[bool]) -> None:
             31: update_from_31_to_32,
             32: update_from_32_to_33,
             33: update_from_33_to_34,
+            34: update_from_34_to_35,
         },
         id=APPLICATION_ID,
         # Row value support was added in 3.15.
@@ -1083,31 +1073,37 @@ def get_entry_counts(
 
         return EntryCounts(*row[:4], row[4:7])  # type: ignore[call-arg]
 
-    def iter_metadata(
+    def get_tags(
         self,
-        object_id: Tuple[str, ...],
+        object_id: Tuple[Optional[str], ...],
         key: Optional[str] = None,
     ) -> Iterable[Tuple[str, JSONType]]:
         yield from join_paginated_iter(
-            partial(self.iter_metadata_page, object_id, key),
+            partial(self.get_tags_page, object_id, key),
             self.chunk_size,
         )
 
     @wrap_exceptions_iter(StorageError)
-    def iter_metadata_page(
+    def get_tags_page(
         self,
-        object_id: Tuple[str, ...],
+        object_id: Tuple[Optional[str], ...],
         key: Optional[str] = None,
         chunk_size: Optional[int] = None,
         last: Optional[_T] = None,
     ) -> Iterable[Tuple[Tuple[str, JSONType], Optional[_T]]]:
         info = SCHEMA_INFO[len(object_id)]
 
-        query = Query().SELECT("key", "value").FROM(f"{info.table_prefix}metadata")
+        query = Query().SELECT("key").FROM(f"{info.table_prefix}tags")
+        context: Dict[str, Any] = dict()
 
-        for column in info.id_columns:
-            query.WHERE(f"{column} = :{column}")
-        context = dict(zip(info.id_columns, object_id))
+        if not any(p is None for p in object_id):
+            query.SELECT("value")
+            for column in info.id_columns:
+                query.WHERE(f"{column} = :{column}")
+            context.update(zip(info.id_columns, object_id))
+
+        else:
+            query.SELECT_DISTINCT("'null'")
 
         if key is not None:
             query.WHERE("key = :key")
@@ -1124,14 +1120,12 @@ def row_factory(t: Tuple[Any, ...]) -> Tuple[str, JSONType]:
         )
 
     @wrap_exceptions(StorageError)
-    def set_metadata(
-        self, object_id: Tuple[str, ...], key: str, value: JSONType
-    ) -> None:
+    def set_tag(self, object_id: Tuple[str, ...], key: str, value: JSONType) -> None:
         info = SCHEMA_INFO[len(object_id)]
 
         columns = info.id_columns + ('key', 'value')
         query = f"""
-            INSERT OR REPLACE INTO {info.table_prefix}metadata (
+            INSERT OR REPLACE INTO {info.table_prefix}tags (
                 {', '.join(columns)}
             ) VALUES (
                 {', '.join(('?' for _ in columns))}
@@ -1149,12 +1143,12 @@ def set_metadata(
                 raise info.not_found_exc(*object_id) from None
 
     @wrap_exceptions(StorageError)
-    def delete_metadata(self, object_id: Tuple[str, ...], key: str) -> None:
+    def delete_tag(self, object_id: Tuple[str, ...], key: str) -> None:
         info = SCHEMA_INFO[len(object_id)]
 
         columns = info.id_columns + ('key',)
         query = f"""
-            DELETE FROM {info.table_prefix}metadata
+            DELETE FROM {info.table_prefix}tags
             WHERE (
                 {', '.join(columns)}
             ) = (
@@ -1169,86 +1163,6 @@ def delete_metadata(self, object_id: Tuple[str, ...], key: str) -> None:
             cursor, lambda: info.metadata_not_found_exc(*object_id, key=key)
         )
 
-    @wrap_exceptions(StorageError)
-    def add_tag(self, object_id: Tuple[str, ...], tag: str) -> None:
-        info = SCHEMA_INFO[len(object_id)]
-
-        columns = info.id_columns + ('tag',)
-        query = f"""
-            INSERT INTO {info.table_prefix}tags (
-                {', '.join(columns)}
-            ) VALUES (
-                {', '.join(('?' for _ in columns))}
-            )
-        """
-        params = object_id + (tag,)
-
-        with self.db:
-            try:
-                self.db.execute(query, params)
-            except sqlite3.IntegrityError as e:
-                if "foreign key constraint failed" in str(e).lower():
-                    raise info.not_found_exc(*object_id) from None
-                # tag exists is a no-op; it looks like:
-                # "UNIQUE constraint failed: feed_tags.feed, feed_tags.tag"
-
-    @wrap_exceptions(StorageError)
-    def remove_tag(self, object_id: Tuple[str, ...], tag: str) -> None:
-        info = SCHEMA_INFO[len(object_id)]
-
-        columns = info.id_columns + ('tag',)
-        query = f"""
-            DELETE FROM {info.table_prefix}tags
-            WHERE (
-                {', '.join(columns)}
-            ) = (
-                {', '.join(('?' for _ in columns))}
-            )
-        """
-        params = object_id + (tag,)
-
-        with self.db:
-            self.db.execute(query, params)
-
-    # Tuple[Optional[str], ...] seems hacky,
-    # but we can't have Optional[Tuple[str, ...]]
-    # because we depend on the tuple length
-    # to distinguish between feeds and entries.
-
-    def get_tags(self, object_id: Tuple[Optional[str], ...]) -> Iterable[str]:
-        yield from join_paginated_iter(
-            partial(self.get_tags_page, object_id),
-            self.chunk_size,
-        )
-
-    @wrap_exceptions_iter(StorageError)
-    def get_tags_page(
-        self,
-        object_id: Tuple[Optional[str], ...],
-        chunk_size: Optional[int] = None,
-        last: Optional[_T] = None,
-    ) -> Iterable[Tuple[str, Optional[_T]]]:
-        info = SCHEMA_INFO[len(object_id)]
-
-        query = Query().SELECT_DISTINCT("tag").FROM(f"{info.table_prefix}tags")
-        context: Dict[str, Any] = dict()
-
-        if not any(p is None for p in object_id):
-            for column in info.id_columns:
-                query.WHERE(f"{column} = :{column}")
-            context.update(zip(info.id_columns, object_id))
-
-        query.scrolling_window_order_by("tag")
-
-        def row_factory(t: Tuple[Any, ...]) -> str:
-            tag = t[0]
-            assert isinstance(tag, str)
-            return tag
-
-        yield from paginated_query(
-            self.db, query, context, chunk_size, last, row_factory
-        )
-
 
 def make_get_feeds_query(
     filter_options: FeedFilterOptions, sort: FeedSortOrder
@@ -1475,13 +1389,13 @@ def apply_feed_tags_filter_options(
         add(str(tag_query))
 
     if add_tags_cte:
-        query.WITH(("tags", f"SELECT tag FROM feed_tags WHERE feed = {url_column}"))
+        query.WITH(("tags", f"SELECT key FROM feed_tags WHERE feed = {url_column}"))
 
     if add_tags_count_cte:
         query.WITH(
             (
                 "tags_count",
-                f"SELECT count(tag) FROM feed_tags WHERE feed = {url_column}",
+                f"SELECT count(key) FROM feed_tags WHERE feed = {url_column}",
             )
         )
 

src/reader/core.py

@@ -1541,7 +1541,7 @@ def get_feed_metadata(
 
         # get_feed_metadata(feed, *, key=None) -> (key, value), ...
         feed_url = _feed_argument(feed)
-        return self._storage.iter_metadata((feed_url,), key)
+        return self._storage.get_tags((feed_url,), key)
 
     @overload
     def get_feed_metadata_item(
@@ -1606,7 +1606,7 @@ def set_feed_metadata_item(
 
         """
         feed_url = _feed_argument(feed)
-        self._storage.set_metadata((feed_url,), key, value)
+        self._storage.set_tag((feed_url,), key, value)
 
     def delete_feed_metadata_item(self, feed: FeedInput, key: str) -> None:
         """Delete metadata for a feed.
@@ -1624,7 +1624,7 @@ def delete_feed_metadata_item(self, feed: FeedInput, key: str) -> None:
 
         """
         feed_url = _feed_argument(feed)
-        self._storage.delete_metadata((feed_url,), key)
+        self._storage.delete_tag((feed_url,), key)
 
     def enable_search(self) -> None:
         """Enable full-text search.
@@ -1875,7 +1875,13 @@ def add_feed_tag(self, feed: FeedInput, tag: str) -> None:
 
         """
         feed_url = _feed_argument(feed)
-        self._storage.add_tag((feed_url,), tag)
+
+        # FIXME: race condition
+        self.set_feed_metadata_item(
+            feed_url,
+            tag,
+            self.get_feed_metadata_item(feed_url, tag, None),  # type: ignore
+        )
 
     def remove_feed_tag(self, feed: FeedInput, tag: str) -> None:
         """Remove a tag from a feed.
@@ -1893,7 +1899,10 @@ def remove_feed_tag(self, feed: FeedInput, tag: str) -> None:
 
         """
         feed_url = _feed_argument(feed)
-        self._storage.remove_tag((feed_url,), tag)
+        try:
+            self.delete_feed_metadata_item(feed_url, tag)
+        except FeedMetadataNotFoundError:
+            pass
 
     def get_feed_tags(self, feed: Optional[FeedInput] = None) -> Iterable[str]:
         """Get all or some of the feed tags.
@@ -1911,7 +1920,7 @@ def get_feed_tags(self, feed: Optional[FeedInput] = None) -> Iterable[str]:
 
         """
         feed_url = _feed_argument(feed) if feed is not None else None
-        return self._storage.get_tags((feed_url,))
+        return (k for k, _ in self._storage.get_tags((feed_url,)))
 
     def make_reader_reserved_name(self, key: str) -> str:
         """Create a *reader*-reserved tag or metadata name.

tests/test_plugins_global_metadata.py

@@ -7,20 +7,27 @@ def test_plugin(make_reader, db_path):
 
     reader.update_feeds()
 
-    assert dict(reader.get_global_metadata()) == {}
+    assert dict(reader.get_global_metadata()) == {'.reader.hidden': None}
     assert reader.get_global_metadata_item('one', None) is None
 
     reader.set_global_metadata_item('one', [1])
-    assert dict(reader.get_global_metadata()) == {'one': [1]}
+    assert dict(reader.get_global_metadata()) == {'.reader.hidden': None, 'one': [1]}
     assert reader.get_global_metadata_item('one') == [1]
 
     reader.set_global_metadata_item('two', {'ii': 2})
-    assert dict(reader.get_global_metadata()) == {'one': [1], 'two': {'ii': 2}}
+    assert dict(reader.get_global_metadata()) == {
+        '.reader.hidden': None,
+        'one': [1],
+        'two': {'ii': 2},
+    }
     assert reader.get_global_metadata_item('two') == {'ii': 2}
 
     reader.delete_global_metadata_item('one')
     assert reader.get_global_metadata_item('one', None) is None
-    assert dict(reader.get_global_metadata()) == {'two': {'ii': 2}}
+    assert dict(reader.get_global_metadata()) == {
+        '.reader.hidden': None,
+        'two': {'ii': 2},
+    }
 
     assert {f.url for f in reader.get_feeds()} == set()
     assert {f.url for f in type(reader).get_feeds(reader)} == {'reader:global-metadata'}

tests/test_reader.py

@@ -3536,3 +3536,12 @@ def test_delete_entry(reader):
     assert {(e.id, e.added_by) for e in reader.get_entries()} == {
         ('1, 2', 'feed'),
     }
+
+
+def test_tags_and_metadata_share_the_same_namespace(reader):
+    feed = 'http://www.example.com'
+    reader.add_feed(feed)
+    reader.set_feed_metadata_item(feed, 'one', {})
+    reader.add_feed_tag(feed, 'two')
+    assert dict(reader.get_feed_metadata(feed)) == {'one': {}, 'two': None}
+    assert set(reader.get_feed_tags(feed)) == {'one', 'two'}