Repository: alcarney/llyfrau

Issue:
Record number of times a link has been opened
Would make for some interesting stats

Discussion:

Patch:
diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml
index 0234c68..bcb9b49 100644
--- a/.github/workflows/release.yml
+++ b/.github/workflows/release.yml
@@ -73,7 +73,7 @@ jobs:
       env:
         GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
       with:
-        tag_name: V${{ steps.info.outputs.VERSION }}
+        tag_name: v${{ steps.info.outputs.VERSION }}
         release_name: V${{ steps.info.outputs.VERSION}} - ${{ steps.info.outputs.RELEASE_DATE }}
         draft: false
         prerelease: ${{ github.ref == 'refs/heads/develop' }}
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 8c9add5..34c4f12 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -6,6 +6,8 @@ Features
 
 - :code:`llyfr open` now make use of a TUI powered by :code:`prompt_toolkit` to search
   for, select and open links in the database.
+- The database now records how many times a link has been opened and uses that to
+  surface the most frequently used links when searching.
 
 v0.2.0
 ======
diff --git a/llyfrau/cli/tui.py b/llyfrau/cli/tui.py
index c905c59..dc28e2f 100644
--- a/llyfrau/cli/tui.py
+++ b/llyfrau/cli/tui.py
@@ -157,7 +157,7 @@ class LinkTable:
         if inpt is not None and len(inpt.text) > 0:
             name = inpt.text
 
-        links = Link.search(self.db, name=name, top=10)
+        links = Link.search(self.db, name=name, top=10, sort='visits')
 
         for idx, link in enumerate(links):
 
diff --git a/llyfrau/data.py b/llyfrau/data.py
index 2e707bf..4218c69 100644
--- a/llyfrau/data.py
+++ b/llyfrau/data.py
@@ -1,13 +1,58 @@
 import pathlib
 import webbrowser
 
-from sqlalchemy import create_engine, Column, ForeignKey, Integer, Text
+from sqlalchemy import Column, ForeignKey, Integer, Text, create_engine, desc
 from sqlalchemy.ext.declarative import declarative_base
-from sqlalchemy.orm import sessionmaker, relationship
+from sqlalchemy.orm import relationship, sessionmaker
 
 Base = declarative_base()
 
 
+class Database:
+    """Manages connections to the database."""
+
+    def __init__(self, filepath, create=False, verbose=False):
+        """Parameters
+
+        :param filepath: The path to the database
+        :param create: Optional. If :code:`True` the database will be created if it
+                       doesn't already exist.
+        :param verbose: Optional. If :code:`True` enable sqlaclhemy's logging of SQL
+                        commands
+        """
+
+        self.filepath = pathlib.Path(filepath)
+        self.engine = create_engine("sqlite:///" + filepath, echo=verbose)
+        self.new_session = sessionmaker(bind=self.engine)
+        self._session = None
+
+        if create:
+            Source.__table__.create(bind=self.engine, checkfirst=True)
+            Link.__table__.create(bind=self.engine, checkfirst=True)
+
+    def commit(self):
+
+        if self._session:
+            self._session.commit()
+            return
+
+        raise RuntimeError("There is no session to commit!")
+
+    @property
+    def exists(self):
+        """Determine if the database exists on disk."""
+        return self.filepath.exists()
+
+    @property
+    def session(self):
+        """Return the current session object."""
+
+        if self._session is None:
+            self._session = self.new_session()
+
+        return self._session
+
+
 class DbOps:
     """Mixin class for database operations with ORM models."""
 
@@ -37,28 +82,8 @@ class DbOps:
 
         session = db.session
         item = session.query(cls).filter(cls.id == id).first()
-        db.commit()
-
         return item
 
-    @classmethod
-    def search(cls, db, name=None, top=10):
-        """Search the given database for links."""
-
-        session = db.session
-        filters = []
-
-        if name is not None:
-            filters.append(cls.name.ilike(f"%{name}%"))
-
-        if len(filters) > 0:
-            items = session.query(cls).filter(*filters)
-        else:
-            items = session.query(cls)
-
-        db.commit()
-        return items[:top]
-
     @classmethod
     def remove(cls, db, id):
         """Remove the given link id from the database."""
@@ -101,6 +126,24 @@ class Source(Base, DbOps):
     def __repr__(self):
         return f"Source<{self.name}, {self.uri}>"
 
+    @classmethod
+    def search(cls, db, name=None, top=10):
+        """Search the given database for sources."""
+
+        session = db.session
+        filters = []
+
+        if name is not None:
+            filters.append(cls.name.ilike(f"%{name}%"))
+
+        if len(filters) > 0:
+            items = session.query(cls).filter(*filters)
+        else:
+            items = session.query(cls)
+
+        db.commit()
+        return items[:top]
+
 
 class Link(Base, DbOps):
     """Represents an individual link."""
@@ -116,6 +159,9 @@ class Link(Base, DbOps):
     url = Column(Text, nullable=False)
     """The url of the link."""
 
+    visits = Column(Integer, default=0)
+    """The number of times a link has been visited."""
+
     source_id = Column(Integer, ForeignKey("sources.id"), nullable=True)
     """The id of the source the link was added with, if applicable"""
 
@@ -125,7 +171,11 @@ class Link(Base, DbOps):
             return False
 
         return all(
-            [self.id == other.id, self.name == other.name, self.url == other.url]
+            [
+                self.id == other.id,
+                self.url == other.url,
+                self.source_id == other.source_id,
+            ]
         )
 
     def __repr__(self):
@@ -148,47 +198,41 @@ class Link(Base, DbOps):
         url = link.url_expanded
         webbrowser.open(url)
 
+        # Update the stats
+        link.visits += 1
+        db.commit()
 
-class Database:
-    """Manages connections to the database."""
+    @classmethod
+    def search(cls, db: Database, name: str = None, top: int = 10, sort: str = None):
+        """Search the given database for links.
 
-    def __init__(self, filepath, create=False, verbose=False):
-        """Parameters
+        The :code:`sort` parameter can be used to control the order in which the
+        results are sorted by. The following options are valid:
 
-        :param filepath: The path to the database
-        :param create: Optional. If :code:`True` the database will be created if it
-                       doesn't already exist.
-        :param verbose: Optional. If :code:`True` enable sqlaclhemy's logging of SQL
-                        commands
-        """
-
-        self.filepath = pathlib.Path(filepath)
-        self.engine = create_engine("sqlite:///" + filepath, echo=verbose)
-        self.new_session = sessionmaker(bind=self.engine)
-        self._session = None
+        - :code:`None` (default), results are returned in the default sort order from
+          the database
+        - :code:`"visits"`, results are returned with the most visited links first.
 
-        if create:
-            Source.__table__.create(bind=self.engine, checkfirst=True)
-            Link.__table__.create(bind=self.engine, checkfirst=True)
+        Invalid options will be ignored
 
-    def commit(self):
+        :param db: The database object to search
+        :param name: Only return links whose name contains the given string.
+        :param top: Only return the top :code:`N` results. (Default :code:`10`)
+        :param sort: The criteria to sort the results by. (Default :code:`None`)
+        """
 
-        if self._session:
-            self._session.commit()
-            return
+        session = db.session
+        filters = []
 
-        raise RuntimeError("There is no session to commit!")
+        if name is not None:
+            filters.append(cls.name.ilike(f"%{name}%"))
 
-    @property
-    def exists(self):
-        """Determine if the database exists on disk."""
-        return self.filepath.exists()
+        query = session.query(cls)
 
-    @property
-    def session(self):
-        """Return the current session object."""
+        if len(filters) > 0:
+            query = query.filter(*filters)
 
-        if self._session is None:
-            self._session = self.new_session()
+        if sort == "visits":
+            query = query.order_by(desc(cls.visits))
 
-        return self._session
+        return query[:top]


Test patch:
diff --git a/tests/test_data.py b/tests/test_data.py
index b4c6e23..97f417a 100644
--- a/tests/test_data.py
+++ b/tests/test_data.py
@@ -160,6 +160,16 @@ def test_link_add_many_instances():
     assert Link.search(db) == links
 
 
+def test_link_add_default_visits():
+    """Ensure that the visits field on a link is initialised to a sane value."""
+
+    db = Database(":memory:", create=True, verbose=True)
+    Link.add(db, name="Github", url="https://github.com/")
+
+    link = Link.get(db, 1)
+    assert link.visits == 0
+
+
 def test_link_requires_name():
     """Ensure that a link requires a name before it can be added to the database"""
 
@@ -207,10 +217,10 @@ def test_link_source_reference():
     assert link.source == source
 
 
-def test_open_link():
+def test_link_open():
     """Ensure that we can open a link in the database"""
 
-    db = Database(":memory:", create=True)
+    db = Database(":memory:", create=True, verbose=True)
     Link.add(db, name="Github", url="https://www.github.com")
 
     with mock.patch("llyfrau.data.webbrowser") as m_webbrowser:
@@ -219,10 +229,10 @@ def test_open_link():
     m_webbrowser.open.assert_called_with("https://www.github.com")
 
 
-def test_open_link_with_prefix():
+def test_link_open_with_prefix():
     """Ensure that we can open a link that has a prefix on its source"""
 
-    db = Database(":memory:", create=True)
+    db = Database(":memory:", create=True, verbose=True)
 
     Source.add(
         db,
@@ -236,3 +246,113 @@ def test_open_link_with_prefix():
         Link.open(db, 1)
 
     m_webbrowser.open.assert_called_with("https://docs.scipy.org/doc/numpy/reference/")
+
+
+def test_link_open_updates_stats():
+    """Ensure that when we visit a link, its stats are updated."""
+
+    db = Database(":memory:", create=True, verbose=True)
+    Link.add(db, name="Github", url="https://www.github.com", visits=1)
+
+    with mock.patch("llyfrau.data.webbrowser") as m_webbrowser:
+        Link.open(db, 1)
+
+    m_webbrowser.open.assert_called_with("https://www.github.com")
+
+    link = Link.get(db, 1)
+    assert link.visits == 2
+
+
+def test_link_search_basic():
+    """Ensure that the simplest search just returns records in the db."""
+
+    db = Database(":memory:", create=True, verbose=True)
+    links = [Link(name=f"Link {i}", url=f"https://{i}") for i in range(20)]
+    Link.add(db, items=links)
+
+    results = Link.search(db)
+    assert len(results) == 10
+
+
+def test_link_search_top():
+    """Ensure that the number of search results can be set."""
+
+    db = Database(":memory:", create=True, verbose=True)
+    links = [Link(name=f"Link {i}", url=f"https://{i}") for i in range(20)]
+    Link.add(db, items=links)
+
+    results = Link.search(db, top=15)
+    assert len(results) == 15
+
+
+def test_link_search_large_top():
+    """Ensure that we can safely handle a page size larger than the number of
+    results."""
+
+    db = Database(":memory:", create=True, verbose=True)
+    links = [Link(name=f"Link {i}", url=f"https://{i}") for i in range(20)]
+    Link.add(db, items=links)
+
+    results = Link.search(db, top=25)
+    assert len(results) == 20
+
+
+def test_link_search_by_name():
+    """Ensure that we can filter search results by name and the search is case
+    insensitive."""
+
+    db = Database(":memory:", create=True, verbose=True)
+
+    links = [
+        Link(name="link 1", url="https://1"),
+        Link(name="LiNk 2", url="https://2"),
+        Link(name="BLINK3", url="https://3"),
+        Link(name="item 4", url="https://4"),
+        Link(name="9linkz", url="https://5")
+    ]
+
+    Link.add(db, items=links)
+    results = Link.search(db, name='link')
+
+    assert len(results) == 4
+    assert all(['link' in l.name.lower() for l in results])
+
+
+def test_link_search_by_name_returns_nothing():
+    """Ensure that if the name matches nothing then nothing is returned."""
+
+    db = Database(":memory:", create=True, verbose=True)
+
+    links = [
+        Link(name="link 1", url="https://1"),
+        Link(name="LiNk 2", url="https://2"),
+        Link(name="LINK3", url="https://3"),
+        Link(name="item 4", url="https://4")
+    ]
+
+    Link.add(db, items=links)
+    results = Link.search(db, name='kiln')
+
+    assert len(results) == 0
+
+
+def test_link_search_sort_by_visits():
+    """Ensure that we can sort results by the number of times they have been visited."""
+
+    db = Database(":memory:", create=True, verbose=True)
+
+    links = [
+        Link(name="link 1", url="https://1", visits=1),
+        Link(name="LiNk 2", url="https://2", visits=0),
+        Link(name="LINK3", url="https://3", visits=1),
+        Link(name="item 4", url="https://4", visits=2)
+    ]
+
+    Link.add(db, items=links)
+    results = Link.search(db, sort='visits')
+
+    assert results[0].url == "https://4"
+    assert results[1].url == "https://1"
+    assert results[2].url == "https://3"
+    assert results[3].url == "https://2"
+