Merge pull request #69 from jedie/less-requests

Make lest requests on save

README.creole

@@ -158,6 +158,7 @@ Files are separated into: "/src/" and "/development/"
 
 * [[https://github.com/jedie/PyInventory/compare/v0.10.0...master|compare v0.10.0...master]] **dev**
 ** Update to Django 3.1.x
+** Don't make requests to the a name for a Link, if we already have one or if last request was not long ago.
 ** tbc
 * [[https://github.com/jedie/PyInventory/compare/v0.9.4...v0.10.0|v0.10.0 - 29.09.2021]]
 ** Group item: default "automatic" mode and can be disabled by filter action

README.rst

@@ -222,6 +222,8 @@ history
 
     * Update to Django 3.1.x
 
+    * Don't make requests to the a name for a Link, if we already have one or if last request was not long ago.
+
     * tbc
 
 * `v0.10.0 - 29.09.2021 <https://github.com/jedie/PyInventory/compare/v0.9.4...v0.10.0>`_ 
@@ -407,4 +409,4 @@ donation
 
 ------------
 
-``Note: this file is generated from README.creole 2021-10-09 16:15:45 with "python-creole"``
+``Note: this file is generated from README.creole 2021-10-09 16:58:57 with "python-creole"``

poetry.lock

@@ -1193,6 +1193,22 @@ urllib3 = ">=1.21.1,<1.27"
 socks = ["PySocks (>=1.5.6,!=1.5.7)", "win-inet-pton"]
 use_chardet_on_py3 = ["chardet (>=3.0.2,<5)"]
 
+[[package]]
+name = "requests-mock"
+version = "1.9.3"
+description = "Mock out responses from the requests package"
+category = "dev"
+optional = false
+python-versions = "*"
+
+[package.dependencies]
+requests = ">=2.3,<3"
+six = "*"
+
+[package.extras]
+fixture = ["fixtures"]
+test = ["fixtures", "mock", "purl", "pytest", "sphinx", "testrepository (>=0.0.18)", "testtools"]
+
 [[package]]
 name = "requests-toolbelt"
 version = "0.9.1"
@@ -1487,7 +1503,7 @@ psycopg2-source = ["psycopg2"]
 [metadata]
 lock-version = "1.1"
 python-versions = ">=3.7,<4.0.0"
-content-hash = "fff6d60788fe4aaeee86f67dfa3220afabb6ab0e6fdec391030fbefe2c8987a0"
+content-hash = "5eaa826b474da770088d7ca448725afc339023a9ff5493e8496a4f878c844148"
 
 [metadata.files]
 asgiref = [
@@ -2223,6 +2239,10 @@ requests = [
     {file = "requests-2.26.0-py2.py3-none-any.whl", hash = "sha256:6c1246513ecd5ecd4528a0906f910e8f0f9c6b8ec72030dc9fd154dc1a6efd24"},
     {file = "requests-2.26.0.tar.gz", hash = "sha256:b8aa58f8cf793ffd8782d3d8cb19e66ef36f7aba4353eec859e74678b01b07a7"},
 ]
+requests-mock = [
+    {file = "requests-mock-1.9.3.tar.gz", hash = "sha256:8d72abe54546c1fc9696fa1516672f1031d72a55a1d66c85184f972a24ba0eba"},
+    {file = "requests_mock-1.9.3-py2.py3-none-any.whl", hash = "sha256:0a2d38a117c08bb78939ec163522976ad59a6b7fdd82b709e23bb98004a44970"},
+]
 requests-toolbelt = [
     {file = "requests-toolbelt-0.9.1.tar.gz", hash = "sha256:968089d4584ad4ad7c171454f0a5c6dac23971e9472521ea3b6d49d610aa6fc0"},
     {file = "requests_toolbelt-0.9.1-py2.py3-none-any.whl", hash = "sha256:380606e1d10dc85c3bd47bf5a6095f815ec007be7a8b69c878507068df059e6f"},

pyproject.toml

@@ -88,6 +88,7 @@ pyupgrade = "*"
 model_bakery = "*"  # https://github.com/model-bakers/model_bakery
 beautifulsoup4 = "*"
 lxml = "*"
+requests-mock = "*"
 
 [tool.poetry.scripts]
 devshell = 'inventory_project.dev_shell:devshell_cmdloop'

src/inventory/migrations/0008_last_check_datetime.py

@@ -0,0 +1,18 @@
+# Generated by Django 3.1.13 on 2021-10-09 14:29
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('inventory', '0007_add_file_attachment'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='itemlinkmodel',
+            name='last_check',
+            field=models.DateTimeField(blank=True, editable=False, help_text='Link.url.help_text', null=True, verbose_name='Link.url.verbose_name'),
+        ),
+    ]

src/inventory/models/links.py

@@ -1,3 +1,4 @@
+import datetime
 import logging
 import re
 
@@ -23,7 +24,7 @@ class BaseLink(BaseModel):
         verbose_name=_('Link.url.verbose_name'),
         help_text=_('Link.url.help_text')
     )
-    last_check = models.DateField(
+    last_check = models.DateTimeField(
         blank=True, null=True, editable=False,
         verbose_name=_('Link.url.verbose_name'),
         help_text=_('Link.url.help_text')
@@ -46,6 +47,17 @@ class BaseLink(BaseModel):
     )
 
     def update_response_info(self):
+        if self.name:
+            logger.debug('Skip link request: because we have a name: %r', self.name)
+            return
+
+        if self.last_check:
+            delta = timezone.now() - self.last_check
+            logger.debug('Last check is %s ago.', delta)
+            if delta < datetime.timedelta(minutes=1):
+                logger.info('Skip request for: %r', self.url)
+                return
+
         try:
             r = requests.get(url=self.url, allow_redirects=True, timeout=10)
         except Exception as err:
@@ -62,7 +74,7 @@ class BaseLink(BaseModel):
         if r.status_code == 200:
             titles = re.findall(r'<title>(.+?)</title>', r.text)
             if not titles:
-                logger.warning(f'No title found in {self.url!r}')
+                logger.warning('No title found in %r', self.url)
             else:
                 title = titles[0]
                 logger.info('Found title: %r', title)

src/inventory/tests/test_link_model.py

@@ -0,0 +1,82 @@
+import datetime
+import logging
+from unittest.mock import patch
+
+import requests_mock
+from bx_django_utils.test_utils.datetime import MockDatetimeGenerator
+from bx_py_utils.test_utils.datetime import parse_dt
+from django.test import TestCase
+from django.utils import timezone
+from model_bakery import baker
+
+from inventory.models import ItemLinkModel, ItemModel
+
+
+class ItemLinkModelTestCase(TestCase):
+    def test_set_name_by_request(self):
+        with self.assertLogs('django_tools'):
+            item = baker.make(ItemModel)
+
+            link = ItemLinkModel(
+                item=item,
+                url='http://test.tld/foo/bar'
+            )
+
+            offset = datetime.timedelta(seconds=30)
+            with patch.object(timezone, 'now', MockDatetimeGenerator(offset)):
+                with requests_mock.Mocker() as m:
+                    m.get('http://test.tld/foo/bar', text='No title')
+
+                    assert link.last_check is None
+                    with self.assertLogs('inventory.models.links', level=logging.WARNING) as logs:
+                        link.full_clean()
+                        assert link.page_title is None
+                        assert link.name is None
+                        assert link.last_check == parse_dt('2000-01-01T00:00:30+0000')
+
+                    logs = logs.output
+                    assert logs == [
+                        "WARNING:inventory.models.links:No title found in 'http://test.tld/foo/bar'"
+                    ]
+
+                    # We should not create request on every admin save call
+
+                    with self.assertLogs('inventory.models.links', level=logging.DEBUG) as logs:
+                        link.full_clean()
+                        assert link.page_title is None
+                        assert link.name is None
+
+                    logs = logs.output
+                    assert logs == [
+                        'DEBUG:inventory.models.links:Last check is 0:00:30 ago.',
+                        "INFO:inventory.models.links:Skip request for: 'http://test.tld/foo/bar'"
+                    ]
+
+                    # Next try after 1 Min
+
+                    m.get('http://test.tld/foo/bar', text='<title>A <boom>Title</boom>!</title>')
+                    with self.assertLogs('inventory.models.links', level=logging.INFO) as logs:
+                        link.full_clean()
+                        assert link.page_title == 'A Title!'
+                        assert link.name == 'A Title!'
+
+                    logs = logs.output
+                    assert logs == [
+                        "INFO:inventory.models.links:Found title: 'A <boom>Title</boom>!'"
+                    ]
+
+                # Don't make requests, if we have a link name!
+
+                with requests_mock.Mocker():
+                    with self.assertLogs('inventory.models.links', level=logging.DEBUG) as logs:
+                        link.full_clean()
+                        assert link.page_title == 'A Title!'
+                        assert link.name == 'A Title!'
+
+                    logs = logs.output
+                    assert logs == [
+                        (
+                            "DEBUG:inventory.models.links:Skip link request:"
+                            " because we have a name: 'A Title!'"
+                        )
+                    ]