Repository: SweBarre/sps

Issue:
warn when cache is old
Add the warning when cache is not updated for a long time.
Was accidentally removed when code was refactored

Discussion:

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5b0d703..5c13e99 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,6 +3,7 @@ All notable changes will be documented in this file.
 
 ## [Unreleased]
 ### Added
+- cache age check
 - --sort-table/-S option to product/package output
 - --no-header/-H option to product/package output
 - CHANGELOG.md
diff --git a/src/sps/cache.py b/src/sps/cache.py
index 0b940dd..47aecec 100644
--- a/src/sps/cache.py
+++ b/src/sps/cache.py
@@ -1,6 +1,8 @@
 import json
 import sys
 from pathlib import Path
+from datetime import datetime
+from typing import Dict
 
 
 def save(key, filename, data):
@@ -41,6 +43,11 @@ def save(key, filename, data):
         cache_data = load(filename)
 
     cache_data[key] = data
+    try:
+        cache_data["age"][key] = datetime.now().strftime("%Y-%m-%d")
+    except KeyError:
+        cache_data["age"] = {}
+        cache_data["age"][key] = datetime.now().strftime("%Y-%m-%d")
 
     try:
         with open(filename, "w") as f:
@@ -50,7 +57,7 @@ def save(key, filename, data):
         sys.exit(13)
 
 
-def load(filename):
+def load(filename) -> Dict:
     """Load cache from filename
 
     The cache file is a json file and retruns a dictionary
@@ -72,6 +79,7 @@ def load(filename):
         if unable to decode cache file as JSON
     """
 
+    data = {}
     try:
         with open(filename, "r") as f:
             data = json.load(f)
@@ -81,8 +89,40 @@ def load(filename):
     except json.decoder.JSONDecodeError as err:
         print(f"Error: unable to parse {filename} as JSON, {err}", file=sys.stderr)
         sys.exit(1)
+    except FileNotFoundError:
+        pass
     return data
 
+def age(filename, age) -> Dict:
+    """Check the age of the different elements in the cache
+
+    Parameters
+    ----------
+    filename: str
+        path to cache file
+    age: int
+        number of days that would indicate old cache data
+
+    Returns
+    -------
+    list
+        returns a list of cache elements that is old
+        the key is the cache element as str
+        the value is the date str %Y-%m-%d when the cache was saved
+    """
+
+
+    ret = {}
+    cache_data = load(filename)
+    now = datetime.now()
+    if cache_data:
+        for key, value in cache_data["age"].items():
+            cacheage = now - datetime.strptime(value, "%Y-%m-%d")
+            if cacheage.days > age:
+                ret[key]=value
+    return ret
+
+
 
 def lookup_product(identifier, filename):
     """Lookup a product in the cache
diff --git a/src/sps/cli.py b/src/sps/cli.py
index a5820ee..22d66b4 100644
--- a/src/sps/cli.py
+++ b/src/sps/cli.py
@@ -2,7 +2,7 @@ import sys
 from pathlib import Path
 from argparse import ArgumentParser
 from prettytable import PrettyTable
-from sps import products, packages, completion, __version__
+from sps import cache, products, packages, completion, __version__
 
 
 def create_parser(args=sys.argv[1:]):
@@ -22,7 +22,7 @@ def create_parser(args=sys.argv[1:]):
     """
 
     parser = ArgumentParser()
-    for opt in ["-C", "--cache-file"]:
+    for opt in ["-C", "--cache-file", "-a", "--cache-age"]:
         try:
             i = args.index(opt)
             args.pop(i + 1)
@@ -36,6 +36,13 @@ def create_parser(args=sys.argv[1:]):
         help=f"cache file to use, (default: { str(Path.home()) }/.cache/sps_cache.json",
         default=f"{ str(Path.home()) }/.cache/sps_cache.json",
     )
+    parser.add_argument(
+        "--cache-age",
+        "-a",
+        help="Number of days before cahce entry is flagged as old",
+        type=int,
+        default=60,
+    )
     parser.add_argument(
         "--version", "-v", action="version", version=f"%(prog)s {__version__}"
     )
@@ -180,3 +187,7 @@ def main():
         print(table)
     if args.command == "completion":
         print(completion.get(args.cache_file, args.shell))
+
+    cacheages = cache.age(args.cache_file, args.cache_age)
+    for key, value in cacheages.items():
+        print(f"Warning: The {key} cache is old, last updated {value}", file=sys.stderr)
diff --git a/src/sps/completion.sh b/src/sps/completion.sh
index b2e08f9..4321c60 100644
--- a/src/sps/completion.sh
+++ b/src/sps/completion.sh
@@ -3,9 +3,13 @@ SPS_COMPLETE="\
     product\
     completion\
     --help\
-    --version"
+    --version\
+    --cache-age\
+    --cache-file"
 
 SPS_PRODUCT_COMPLETE="\
+    --cache-age\
+    --cache-file\
     --update-cache\
     --no-cache\
     --help\
@@ -34,6 +38,8 @@ SPS_COMPLETION_COMLPETE="\
     --help"
 
 SPS_PACKAGE_COMPLETE="\
+    --cache-age\
+    --cache-file\
     --sort-table\
     --no-borders\
     --no-header\
@@ -62,6 +68,12 @@ _sps_complete()
                 --sort-table)
                     suggestions="$SPS_PRODUCT_SORT_TABLE_COMPLETE"
                     ;;
+                --cache-file)
+                    suggestions=""
+                    ;;
+                --cache-age)
+                    suggestions=""
+                    ;;
                 *)
                     suggestions="$SPS_PRODUCT_COMPLETE"
                     ;;
@@ -75,6 +87,12 @@ _sps_complete()
                 --sort-table)
                     suggestions="$SPS_PACKAGE_SORT_TABLE_COMPLETE"
                     ;;
+                --cache-file)
+                    suggestions=""
+                    ;;
+                --cache-age)
+                    suggestions=""
+                    ;;
                 *)
                     if [[ "${COMP_WORDS[COMP_CWORD-1]}" = "package" ]];then
                         suggestions=""
@@ -87,11 +105,30 @@ _sps_complete()
             esac
             ;;
         completion)
-            suggestions="$SPS_COMPLETION_COMLPETE"
+            case "${prev}" in
+                --cache-file)
+                    suggestions=""
+                    ;;
+                --cache-age)
+                    suggestions=""
+                    ;;
+                *)
+                    suggestions="$SPS_COMPLETION_COMLPETE"
+                ;;
+            esac
             ;;
         *)
-            suggestions="$SPS_COMPLETE"
-            ;;
+            case "${prev}" in
+                --cache-file)
+                    suggestions=""
+                    ;;
+                 --cache-age)
+                    suggestions=""
+                    ;;
+                *)
+                    suggestions="$SPS_COMPLETE"
+                    ;;
+            esac
     esac
 
     if [[ "${suggestions}x" == "x" ]];then
@@ -110,7 +147,7 @@ _sps_get_firstword() {
     firstword=
     for ((i = 1; i < ${#COMP_WORDS[@]}; ++i)); do
         if [[ ${COMP_WORDS[i]} != -* ]] && \
-           [[ ! "-C --cache-file -S --sort-table" =~ "${COMP_WORDS[i-1]}" ]]; then
+           [[ ! "-C --cache-file -S --sort-table -a --cache-age" =~ "${COMP_WORDS[i-1]}" ]]; then
             firstword=${COMP_WORDS[i]}
             break
         fi


Test patch:
diff --git a/tests/test_cache.py b/tests/test_cache.py
index 300b4c9..93cf2a4 100644
--- a/tests/test_cache.py
+++ b/tests/test_cache.py
@@ -4,11 +4,13 @@ import json
 import os
 import stat
 from sps import cache
+from datetime import datetime
 
 
 @pytest.fixture
 def data():
     return {
+        "age": {"product": "2019-01-01"},
         "product": [
             {
                 "id": 1899,
@@ -28,7 +30,7 @@ def data():
                 "edition": "15 SP2",
                 "architecture": "x86_64",
             },
-        ]
+        ],
     }
 
 
@@ -39,6 +41,7 @@ def test_cache_save_load(data):
     fn = tempfile.NamedTemporaryFile(delete=False)
     os.remove(fn.name)
     assert cache.save("product", fn.name, data["product"]) == None
+    data["age"] = {"product": datetime.now().strftime("%Y-%m-%d")}
     assert cache.load(fn.name) == data
     os.remove(fn.name)
 
@@ -92,11 +95,32 @@ def test_cache_save_key_load_old(data):
     with open(fn.name, "r") as f:
         cachedata = json.load(f)
 
-    new_data = {"testing": "testing", "product": data["product"]}
+    new_data = {
+        "testing": "testing",
+        "age": {"product": datetime.now().strftime("%Y-%m-%d")},
+        "product": data["product"],
+    }
 
     assert new_data == cachedata
 
 
+def test_cache_save_permission_denied(data):
+    """
+    load should exit the cache
+    """
+    fn = tempfile.NamedTemporaryFile(delete=False)
+    os.remove(fn.name)
+    cache.save("product", fn.name, data["product"])
+    os.chmod(fn.name, stat.S_IRUSR)
+    with pytest.raises(SystemExit):
+        cache.save("product", fn.name, data["product"])
+    os.remove(fn.name)
+
+
+def test_cache_load_file_not_found(data):
+    assert cache.load("file-not-found") == {}
+
+
 def test_cache_load_permission_denied(data):
     """
     load should exit the cache
@@ -116,7 +140,7 @@ def test_cache_lookup_product_found(data):
     """
     fn = tempfile.NamedTemporaryFile(delete=False)
     os.remove(fn.name)
-    cache.save("product", fn.name, data["product"]) == None
+    cache.save("product", fn.name, data["product"])
     assert cache.lookup_product("SLED/15.2/x86_64", fn.name) == 1935
     os.remove(fn.name)
 
@@ -127,7 +151,24 @@ def test_cache_lookup_product_notfound(data):
     """
     fn = tempfile.NamedTemporaryFile(delete=False)
     os.remove(fn.name)
-    cache.save("product", fn.name, data["product"]) == None
+    cache.save("product", fn.name, data["product"])
     with pytest.raises(SystemExit):
         cache.lookup_product("Bogus product name", fn.name)
     os.remove(fn.name)
+
+
+def test_cache_age_not_old(data):
+    """
+    """
+    fn = tempfile.NamedTemporaryFile(delete=False)
+    os.remove(fn.name)
+    cache.save("product", fn.name, data["product"])
+    assert cache.age(fn.name, 60) == {}
+    os.remove(fn.name)
+
+
+def test_cache_age_old(data, mocker):
+    mocker.patch("sps.cache.load")
+    cache.load.return_value = data
+    aged = cache.age("fake-file-name", 60)
+    assert aged == data["age"]
diff --git a/tests/test_cli.py b/tests/test_cli.py
index a0eb586..7ee2b3e 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -3,7 +3,7 @@ from argparse import ArgumentParser, Namespace
 from prettytable import PrettyTable
 import prettytable
 from pathlib import Path
-from sps import cli, products, packages, completion, __version__
+from sps import cli, products, packages, completion, cache, __version__
 
 
 @pytest.fixture
@@ -36,6 +36,30 @@ def args():
         no_cache=False,
     )
 
+@pytest.fixture
+def data():
+    return {
+        "product": [
+            {
+                "id": 1899,
+                "name": "SUSE Manager Server",
+                "identifier": "SUSE-Manager-Server/4.0/x86_64",
+                "type": "base",
+                "free": False,
+                "edition": "4.0",
+                "architecture": "x86_64",
+            },
+            {
+                "id": 1935,
+                "name": "SUSE Linux Enterprise Desktop",
+                "identifier": "SLED/15.2/x86_64",
+                "type": "base",
+                "free": False,
+                "edition": "15 SP2",
+                "architecture": "x86_64",
+            },
+        ]
+    }
 
 def test_parser_with_unknown_command(parser):
     """
@@ -111,6 +135,15 @@ def test_parser_cache_file(parser):
     assert args.cache_file == "testing2"
 
 
+def test_parser_cache_age(parser):
+    args = parser.parse_args(["product"])
+    assert args.cache_age == 60
+    args = parser.parse_args(["product", "--cache-age", "30"])
+    assert args.cache_age == 30
+    args = parser.parse_args(["product", "-a", "20"])
+    assert args.cache_age == 20
+
+
 def test_parser_product_command_update_cache(parser_product):
     """
     parser with product command --update-cache, false default
@@ -260,6 +293,7 @@ def test_main_product_output(mocker, capsys):
                 no_borders=False,
                 no_header=False,
                 sort_table="id",
+                cache_age=60,
             )
 
     data = {
@@ -276,6 +310,8 @@ def test_main_product_output(mocker, capsys):
         ]
     }
 
+    mocker.patch("sps.cache.age", autospec=True)
+    cache.age.return_value = {}
     mocker.patch("sps.products.get", autospec=True)
     products.get.return_value = data["data"]
     mocker.patch("sps.cli.create_parser", autospec=True)
@@ -306,6 +342,7 @@ def test_main_product_output_short(mocker, capsys):
                 no_borders=False,
                 no_header=False,
                 sort_table="id",
+                cache_age=60,
             )
 
     data = {
@@ -322,6 +359,8 @@ def test_main_product_output_short(mocker, capsys):
         ]
     }
 
+    mocker.patch("sps.cache.age", autospec=True)
+    cache.age.return_value = {}
     mocker.patch("sps.products.get", autospec=True)
     products.get.return_value = data["data"]
     mocker.patch("sps.cli.create_parser", autospec=True)
@@ -347,6 +386,7 @@ def test_main_package_output(mocker, capsys):
                 no_borders=False,
                 no_header=False,
                 sort_table="Name",
+                cache_age=60,
             )
 
     data = {
@@ -390,6 +430,8 @@ def test_main_package_output(mocker, capsys):
         ]
     }
 
+    mocker.patch("sps.cache.age", autospec=True)
+    cache.age.return_value = {}
     mocker.patch("sps.packages.get", autospec=True)
     packages.get.return_value = data["data"]
     mocker.patch("sps.cli.create_parser", autospec=True)
@@ -421,6 +463,7 @@ def test_main_package_output_exact_match(mocker, capsys):
                 no_borders=False,
                 no_header=False,
                 sort_table="Name",
+                cache_age=60,
             )
 
     data = {
@@ -464,6 +507,8 @@ def test_main_package_output_exact_match(mocker, capsys):
         ]
     }
 
+    mocker.patch("sps.cache.age", autospec=True)
+    cache.age.return_value = {}
     mocker.patch("sps.packages.get", autospec=True)
     packages.get.return_value = data["data"]
     mocker.patch("sps.cli.create_parser", autospec=True)
@@ -489,11 +534,47 @@ def test_main_completion(mocker):
                 short=None,
                 cache_file="fake-file-name",
                 shell="bash",
+                cache_age=60,
             )
 
+    mocker.patch("sps.cache.age", autospec=True)
+    cache.age.return_values={}
     mocker.patch("sps.cli.create_parser", autospec=True)
     cli.create_parser.return_value = parser_proxy()
     mocker.patch("sps.completion.get", autospec=True)
     completion.get.return_value = "\n"
     cli.main()
     completion.get.assert_called_with("fake-file-name", "bash")
+
+
+def test_main_aged_cache_no_return(mocker, capsys, data):
+    class parser_proxy:
+        def parse_args(self):
+            return Namespace(
+                command="product",
+                pattern=None,
+                cache_file="fake-file-name",
+                update_cache=False,
+                no_cache=False,
+                short=False,
+                no_borders=False,
+                no_header=False,
+                sort_table="id",
+                cache_age=60
+            )
+
+
+    mocker.patch("sps.cli.PrettyTable", autospec=True)
+    mocker.patch("sps.cli.create_parser", autospec=True)
+    cli.create_parser.return_value = parser_proxy()
+    mocker.patch("sps.products.get", autospec=True)
+    products.get.return_value = {}
+    mocker.patch("sps.cache.age", autospec=True)
+    cache.age.return_value = {}
+    cli.main()
+    cache.age.assert_called_once_with("fake-file-name", 60)
+    cache.age.return_value = {"testing": "date"}
+    cli.main()
+    captured = capsys.readouterr()
+    assert captured.err.strip() == "Warning: The testing cache is old, last updated date"
+