diff --git a/poetry.lock b/poetry.lock index d9ecca8..9eda96e 100644 --- a/poetry.lock +++ b/poetry.lock @@ -282,7 +282,7 @@ description = "A Python utility / library to sort Python imports." name = "isort" optional = false python-versions = ">=3.6,<4.0" -version = "5.0.4" +version = "5.0.5" [package.extras] pipfile_deprecated_finder = ["pipreqs", "requirementslib", "tomlkit (>=0.5.3)"] @@ -822,8 +822,8 @@ importlib-metadata = [ {file = "importlib_metadata-1.7.0.tar.gz", hash = "sha256:90bb658cdbbf6d1735b6341ce708fc7024a3e14e99ffdc5783edea9f9b077f83"}, ] isort = [ - {file = "isort-5.0.4-py3-none-any.whl", hash = "sha256:78661ad751751cb3c181d37302e175a0c644b3714877c073df058c596281d7fd"}, - {file = "isort-5.0.4.tar.gz", hash = "sha256:6ae9cf5414e416954e3421f861cbbfc099b3ace63cb270cc76c6670efd960a0a"}, + {file = "isort-5.0.5-py3-none-any.whl", hash = "sha256:e7b11f4317c0668bd2a25f924c14338ca39b92723f24bdaef37452af0dd5a87b"}, + {file = "isort-5.0.5.tar.gz", hash = "sha256:cb76a2fac5ac08eae0020f7e56bab895cf3d8982034aa16f3cd67984edf26223"}, ] lazy-object-proxy = [ {file = "lazy-object-proxy-1.4.3.tar.gz", hash = "sha256:f3900e8a5de27447acbf900b4750b0ddfd7ec1ea7fbaf11dfa911141bc522af0"}, diff --git a/pyproject.toml b/pyproject.toml index 421bed5..8c4d757 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "django-user-visit" -version = "0.3.3" +version = "0.4" description = "Django app used to track user visits." license = "MIT" authors = ["YunoJuno "] diff --git a/tests/test_middleware.py b/tests/test_middleware.py index 29da342..3643c42 100644 --- a/tests/test_middleware.py +++ b/tests/test_middleware.py @@ -4,44 +4,29 @@ import freezegun import pytest from django.contrib.auth.models import User -from django.contrib.sessions.backends.base import SessionBase from django.core.exceptions import MiddlewareNotUsed from django.http import HttpResponse from django.test import Client +from django.utils import timezone -from user_visit.middleware import ( - SESSION_KEY, - UserVisitMiddleware, - cache_visit, - visit_is_cached, -) +from user_visit.middleware import UserVisitMiddleware, save_user_visit from user_visit.models import UserVisit, UserVisitManager -class TestMiddlewareFunctions: - @pytest.mark.parametrize( - "hash_value,cached_value,result", - ( - ("", "", False), - ("", "foo", False), - ("foo", "", False), - ("foo", "bar", False), - ("bar", "bar", True), - ), +@pytest.mark.django_db +def test_save_user_visit(): + """Test standalone save method handles db.IntegrityError.""" + user = User.objects.create(username="Yoda") + timestamp = timezone.now() + uv = UserVisit.objects.create( + user=user, + session_key="test", + ua_string="Chrome", + remote_addr="127.0.0.1", + timestamp=timestamp, ) - def test_visit_is_cached(self, hash_value, cached_value, result): - session = {SESSION_KEY: cached_value} - visit = UserVisit(hash=hash_value) - assert visit_is_cached(visit, session) == result - - @pytest.mark.parametrize("hash_value,cached", (("", False), ("bar", True),)) - def test_cache_visit(self, hash_value, cached): - """Check that hash is not stored if empty.""" - session = {} - visit = UserVisit(hash=hash_value) - assert SESSION_KEY not in session - cache_visit(visit, session) - assert (SESSION_KEY in session) == cached + uv.id = None + save_user_visit(uv) @pytest.mark.django_db @@ -86,24 +71,13 @@ def test_middleware__new_day(self): client.get("/") assert UserVisit.objects.count() == 2 - def test_middleware__duplicate(self): - """Check that middleware handles duplicate uuids.""" - user = User.objects.create_user("Fred") - client = Client() - client.force_login(user) - client.get("/") - with mock.patch("user_visit.middleware.visit_is_cached", return_value=False): - client.get("/") - def test_middleware__db_integrity_error(self): - """Check that middleware stores hash when db raises duplicate hash error.""" + """Check that a failing save doesn't kill middleware.""" user = User.objects.create_user("Fred") client = Client() client.force_login(user) - assert not client.session.get(SESSION_KEY) with mock.patch.object(UserVisit, "save", side_effect=django.db.IntegrityError): client.get("/") - assert client.session[SESSION_KEY] @mock.patch("user_visit.middleware.RECORDING_DISABLED", True) def test_middleware__disabled(self): diff --git a/user_visit/middleware.py b/user_visit/middleware.py index d0853e5..94d72ad 100644 --- a/user_visit/middleware.py +++ b/user_visit/middleware.py @@ -2,7 +2,6 @@ import typing import django.db -from django.contrib.sessions.backends.base import SessionBase from django.core.exceptions import MiddlewareNotUsed from django.http import HttpRequest, HttpResponse from django.utils import timezone @@ -13,32 +12,17 @@ logger = logging.getLogger(__name__) -# used to store unique hash of the visit -SESSION_KEY = "user_visit.hash" - -def visit_is_cached(user_visit: UserVisit, session: SessionBase) -> bool: - """Return True if the visit is already in the request session.""" - if not user_visit.hash: - return False - return session.get(SESSION_KEY) == user_visit.hash - - -def cache_visit(user_visit: UserVisit, session: SessionBase) -> None: - """Cache UserVisit in session.""" - if not user_visit.hash: - return - session[SESSION_KEY] = user_visit.hash +def save_user_visit(user_visit: UserVisit) -> None: + """Save the user visit and handle db.IntegrityError.""" + try: + user_visit.save() + except django.db.IntegrityError: + logger.warning("Error saving user visit (hash='%s')", user_visit.hash) class UserVisitMiddleware: - """ - Middleware to record user visits. - - This middleware caches visits on a daily basis. Same user, device, IP - and session - no new visit. - - """ + """Middleware to record user visits.""" def __init__(self, get_response: typing.Callable) -> None: if RECORDING_DISABLED: @@ -46,24 +30,11 @@ def __init__(self, get_response: typing.Callable) -> None: self.get_response = get_response def __call__(self, request: HttpRequest) -> typing.Optional[HttpResponse]: - # this method will fail hard if session or auth middleware are not configured. if request.user.is_anonymous: return self.get_response(request) uv = UserVisit.objects.build(request, timezone.now()) - if visit_is_cached(uv, request.session): - return self.get_response(request) + if not UserVisit.objects.filter(hash=uv.hash).exists(): + save_user_visit(uv) - try: - uv.save() - except django.db.IntegrityError as ex: - logger.warning("Error saving user visit (hash='%s'): %s", uv.hash, ex) - logger.debug("Session hash='%s'", request.session.get(SESSION_KEY, "")) - logger.debug("UserVisit.session_key='%s'", uv.session_key) - logger.debug("UserVisit.remote_addr='%s'", uv.remote_addr) - logger.debug("UserVisit.ua_string='%s'", uv.ua_string) - logger.debug("UserVisit.user_id='%s'", uv.user_id) - # if the database has raised an IntegrityError it means the hash is already - # stored, but is not in the session for some reason. - cache_visit(uv, request.session) return self.get_response(request)