Skip to content

Commit

Permalink
Fix issue with hash not being stored in session (#5)
Browse files Browse the repository at this point in the history
There is an issue with the current version whereby if the UserVisit is saved,
but the request hash is not stored in the session, every subsequent request
will raise an IntegrityError - as it will try and save another UserVisit.
I haven't got to the bottom of why some hashes are not being store properly,
but it causes a cascade of warnings. This should fix the cascade, by ensuring
that the hash is stored even in the event of an IntegrityError.
  • Loading branch information
hugorodgerbrown authored Jul 8, 2020
1 parent 9d9a6cb commit 7152d84
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 48 deletions.
12 changes: 6 additions & 6 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
repos:
# python import sorting - will amend files
- repo: https://github.com/pre-commit/mirrors-isort
rev: v4.3.21
rev: v5.0.4
hooks:
- id: isort

Expand All @@ -13,23 +13,23 @@ repos:

# PEP8 linting, with added Django goodness, and custom YJ plugin
- repo: https://github.com/pre-commit/mirrors-pylint
rev: v2.4.4
rev: v2.5.3
hooks:
- id: pylint
args:
- --rcfile=.pylintrc

# Flake8 includes pyflakes, pycodestyle, mccabe, pydocstyle, bandit
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v2.4.0
- repo: https://gitlab.com/pycqa/flake8
rev: 3.8.3
hooks:
- id: flake8
additional_dependencies: ["flake8-docstrings", "flake8-bandit"]
exclude: tests

# python static type checking
- repo: https://github.com/pre-commit/mirrors-mypy
rev: v0.761
rev: v0.782
hooks:
- id: mypy
args:
Expand All @@ -43,7 +43,7 @@ repos:

# JS, JSON and SCSS formatting
- repo: https://github.com/prettier/prettier
rev: 1.19.1
rev: 2.0.5
hooks:
- id: prettier
args:
Expand Down
81 changes: 42 additions & 39 deletions poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[tool.poetry]
name = "django-user-visit"
version = "0.3.2"
version = "0.3.3"
description = "Django app used to track user visits."
license = "MIT"
authors = ["YunoJuno <[email protected]>"]
Expand Down
11 changes: 11 additions & 0 deletions tests/test_middleware.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from unittest import mock

import django.db
import freezegun
import pytest
from django.contrib.auth.models import User
Expand Down Expand Up @@ -94,6 +95,16 @@ def test_middleware__duplicate(self):
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."""
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):
"""Test update_cache and check_cache functions."""
Expand Down
5 changes: 3 additions & 2 deletions user_visit/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ def __call__(self, request: HttpRequest) -> typing.Optional[HttpResponse]:
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)
else:
cache_visit(uv, request.session)
# 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)

0 comments on commit 7152d84

Please sign in to comment.