Merge pull request #166 from jazzband/various-improvements

Various fixes and improvements

.github/workflows/test.yml

@@ -11,10 +11,31 @@ jobs:
       max-parallel: 5
       matrix:
         python-version: ['3.8', '3.9', '3.10', '3.11', '3.12-dev']
-        django-version: ['3.2', '4.0', 'main']
+        django-version: ['3.2', '4.0', '4.1', '4.2', 'main']
         include:
           - python-version: '3.7'
             django-version: '3.2'
+        exclude:
+          - python-version: '3.11'
+            django-version: '3.2'
+          - python-version: '3.12-dev'
+            django-version: '3.2'
+
+          - python-version: '3.11'
+            django-version: '4.0'
+          - python-version: '3.12-dev'
+            django-version: '4.0'
+
+          - python-version: '3.12-dev'
+            django-version: '4.1'
+
+          - python-version: '3.8'
+            django-version: 'main'
+          - python-version: '3.9'
+            django-version: 'main'
+          - python-version: '3.10'
+            django-version: 'main'
+
     steps:
     - uses: actions/checkout@v3
 
@@ -31,6 +52,7 @@ jobs:
         python -m pip install --upgrade tox tox-gh-actions
 
     - name: Tox tests
+      continue-on-error: ${{ endsWith(matrix.python-version, '-dev') || matrix.django-version == 'main' }}
       run: |
         tox -v
       env:

.gitignore

@@ -1,6 +1,8 @@
 /example/database.sqlite3
 /example/GeoLiteCity.dat
 /django_user_sessions.egg-info/
+/tests/test_city.mmdb
+/tests/test_country.mmdb
 
 /htmlcov/
 
@@ -10,8 +12,8 @@ coverage.xml
 
 /docs/_build/
 /GeoLite2-City.mmdb
-
-__pycache__
-/venv/
-/.eggs/
-/build/
+
+__pycache__
+/venv/
+/.eggs/
+/build/

Makefile

@@ -13,7 +13,11 @@ check:
 	DJANGO_SETTINGS_MODULE=example.settings PYTHONPATH=. \
 		python -Wd example/manage.py check
 
-test:
+generate-mmdb-fixtures:
+	docker --context=default buildx build -f tests/Dockerfile --tag test-mmdb-maker tests
+	docker run --rm --volume $$(pwd)/tests:/data test-mmdb-maker
+
+test: generate-mmdb-fixtures
 	DJANGO_SETTINGS_MODULE=tests.settings PYTHONPATH=. \
 		django-admin.py test ${TARGET}
 

docs/reference.rst

@@ -22,7 +22,3 @@ Views
 -----
 .. autoclass:: user_sessions.views.SessionListView
 .. autoclass:: user_sessions.views.SessionDeleteView
-
-Unit tests
-----------
-.. autoclass:: user_sessions.utils.tests.Client

tests/Dockerfile

@@ -0,0 +1,9 @@
+FROM perl:latest
+
+RUN cpanm MaxMind::DB::Writer
+
+COPY generate_mmdb.pl /
+
+VOLUME ["/data"]
+
+CMD ["perl", "/generate_mmdb.pl"]

tests/generate_mmdb.pl

@@ -0,0 +1,89 @@
+#!perl
+
+use MaxMind::DB::Writer::Tree;
+
+my %city_types = (
+    city                 => 'map',
+    code                 => 'utf8_string',
+    continent            => 'map',
+    country              => 'map',
+    en                   => 'utf8_string',
+    is_in_european_union => 'boolean',
+    iso_code             => 'utf8_string',
+    latitude             => 'double',
+    location             => 'map',
+    longitude            => 'double',
+    metro_code           => 'utf8_string',
+    names                => 'map',
+    postal               => 'map',
+    subdivisions         => ['array', 'map'],
+    region               => 'utf8_string',
+    time_zone            => 'utf8_string',
+);
+
+my $city_tree = MaxMind::DB::Writer::Tree->new(
+    ip_version            => 6,
+    record_size           => 24,
+    database_type         => 'GeoLite2-City',
+    languages             => ['en'],
+    description           => { en => 'Test database of IP city data' },
+    map_key_type_callback => sub { $city_types{ $_[0] } },
+);
+
+$city_tree->insert_network(
+    '44.55.66.77/32',
+    {
+        city                 => { names => {en => 'San Diego'} },
+        continent            => { code => 'NA', names => {en => 'North America'} },
+        country              => { iso_code => 'US', names => {en => 'United States'} },
+        is_in_european_union => false,
+        location             => {
+            latitude => 37.751,
+            longitude => -97.822,
+            metro_code => 'custom metro code',
+            time_zone => 'America/Los Angeles',
+        },
+        postal               => { code => 'custom postal code' },
+        subdivisions         => [
+            { iso_code => 'ABC', names => {en => 'Absolute Basic Class'} },
+        ],
+    },
+);
+
+my $outfile = ($ENV{'DATA_DIR'} || '/data/') . ($ENV{'CITY_FILENAME'} || 'test_city.mmdb');
+open my $fh, '>:raw', $outfile;
+$city_tree->write_tree($fh);
+
+
+
+my %country_types = (
+    country  => 'map',
+    iso_code => 'utf8_string',
+    names    => 'map',
+    en       => 'utf8_string',
+);
+
+my $country_tree = MaxMind::DB::Writer::Tree->new(
+    ip_version            => 6,
+    record_size           => 24,
+    database_type         => 'GeoLite2-Country',
+    languages             => ['en'],
+    description           => { en => 'Test database of IP country data' },
+    map_key_type_callback => sub { $country_types{ $_[0] } },
+);
+
+$country_tree->insert_network(
+    '8.8.8.8/32',
+    {
+        country => {
+            iso_code => 'US',
+            names    => {
+                en => 'United States',
+            },
+        },
+    },
+);
+
+my $outfile = ($ENV{'DATA_DIR'} || '/data/') . ($ENV{'COUNTRY_FILENAME'} || 'test_country.mmdb');
+open my $fh, '>:raw', $outfile;
+$country_tree->write_tree($fh);

tests/settings.py

@@ -1,6 +1,7 @@
-import os
+from pathlib import Path
 
-BASE_DIR = os.path.dirname(__file__)
+# Build paths inside the project like this: BASE_DIR / 'subdir'.
+BASE_DIR = Path(__file__).resolve().parent.parent
 
 SECRET_KEY = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890'
 
@@ -38,7 +39,7 @@
     {
         'BACKEND': 'django.template.backends.django.DjangoTemplates',
         'DIRS': [
-            os.path.join(BASE_DIR, 'templates'),
+            BASE_DIR / 'templates',
         ],
         'APP_DIRS': True,
         'OPTIONS': {
@@ -47,6 +48,7 @@
                 'django.template.context_processors.debug',
                 'django.template.context_processors.i18n',
                 'django.template.context_processors.media',
+                'django.template.context_processors.request',
                 'django.template.context_processors.static',
                 'django.template.context_processors.tz',
                 'django.contrib.messages.context_processors.messages',
@@ -55,7 +57,10 @@
     },
 ]
 
-GEOIP_PATH = os.path.join(os.path.dirname(BASE_DIR), 'GeoLite2-City.mmdb')
+GEOIP_PATH = BASE_DIR / 'tests'
+GEOIP_CITY = 'test_city.mmdb'
+GEOIP_COUNTRY = 'test_country.mmdb'
+
 SESSION_ENGINE = 'user_sessions.backends.db'
 
 LOGIN_URL = '/admin/'

tests/tests.py

@@ -1,4 +1,4 @@
-from datetime import datetime, timedelta
+from datetime import timedelta
 from unittest import skipUnless
 from unittest.mock import patch
 from urllib.parse import urlencode
@@ -16,15 +16,16 @@
 from user_sessions.backends.db import SessionStore
 from user_sessions.models import Session
 from user_sessions.templatetags.user_sessions import (
-    browser, device, location, platform,
+    browser, city, country, device, location, platform,
 )
-from user_sessions.utils.tests import Client
+
+from .utils import Client
 
 try:
     from django.contrib.gis.geoip2 import GeoIP2
     geoip = GeoIP2()
     geoip_msg = None
-except Exception as error_geoip2:
+except Exception as error_geoip2:  # pragma: no cover
     try:
         from django.contrib.gis.geoip import GeoIP
         geoip = GeoIP()
@@ -82,9 +83,10 @@ def setUp(self):
 
     def test_list(self):
         self.user.session_set.create(session_key='ABC123', ip='127.0.0.1',
-                                     expire_date=datetime.now() + timedelta(days=1),
+                                     expire_date=now() + timedelta(days=1),
                                      user_agent='Firefox')
-        response = self.client.get(reverse('user_sessions:session_list'))
+        with self.assertWarnsRegex(UserWarning, r"The address 127\.0\.0\.1 is not in the database"):
+            response = self.client.get(reverse('user_sessions:session_list'))
         self.assertContains(response, 'Active Sessions')
         self.assertContains(response, 'Firefox')
         self.assertNotContains(response, 'ABC123')
@@ -96,19 +98,21 @@ def test_delete(self):
         self.assertRedirects(response, '/')
 
     def test_delete_all_other(self):
-        self.user.session_set.create(ip='127.0.0.1', expire_date=datetime.now() + timedelta(days=1))
+        self.user.session_set.create(ip='127.0.0.1', expire_date=now() + timedelta(days=1))
         self.assertEqual(self.user.session_set.count(), 2)
         response = self.client.post(reverse('user_sessions:session_delete_other'))
-        self.assertRedirects(response, reverse('user_sessions:session_list'))
+        with self.assertWarnsRegex(UserWarning, r"The address 127\.0\.0\.1 is not in the database"):
+            self.assertRedirects(response, reverse('user_sessions:session_list'))
         self.assertEqual(self.user.session_set.count(), 1)
 
     def test_delete_some_other(self):
         other = self.user.session_set.create(session_key='OTHER', ip='127.0.0.1',
-                                             expire_date=datetime.now() + timedelta(days=1))
+                                             expire_date=now() + timedelta(days=1))
         self.assertEqual(self.user.session_set.count(), 2)
         response = self.client.post(reverse('user_sessions:session_delete',
                                             args=[other.session_key]))
-        self.assertRedirects(response, reverse('user_sessions:session_list'))
+        with self.assertWarnsRegex(UserWarning, r"The address 127\.0\.0\.1 is not in the database"):
+            self.assertRedirects(response, reverse('user_sessions:session_list'))
         self.assertEqual(self.user.session_set.count(), 1)
 
 
@@ -128,33 +132,38 @@ def setUp(self):
         self.admin_url = reverse('admin:user_sessions_session_changelist')
 
     def test_list(self):
-        response = self.client.get(self.admin_url)
+        with self.assertWarnsRegex(UserWarning, r"The address 1\.1\.1\.1 is not in the database"):
+            response = self.client.get(self.admin_url)
         self.assertContains(response, 'Select session to change')
         self.assertContains(response, '127.0.0.1')
         self.assertContains(response, '20.13.1.1')
         self.assertContains(response, '1.1.1.1')
 
     def test_search(self):
-        response = self.client.get(self.admin_url, {'q': 'bouke'})
+        with self.assertWarnsRegex(UserWarning, r"The address 127\.0\.0\.1 is not in the database"):
+            response = self.client.get(self.admin_url, {'q': 'bouke'})
         self.assertContains(response, '127.0.0.1')
         self.assertNotContains(response, '20.13.1.1')
         self.assertNotContains(response, '1.1.1.1')
 
     def test_mine(self):
-        my_sessions = '{}?{}'.format(self.admin_url, urlencode({'owner': 'my'}))
-        response = self.client.get(my_sessions)
+        my_sessions = f"{self.admin_url}?{urlencode({'owner': 'my'})}"
+        with self.assertWarnsRegex(UserWarning, r"The address 127\.0\.0\.1 is not in the database"):
+            response = self.client.get(my_sessions)
         self.assertContains(response, '127.0.0.1')
         self.assertNotContains(response, '1.1.1.1')
 
     def test_expired(self):
-        expired = '{}?{}'.format(self.admin_url, urlencode({'active': '0'}))
-        response = self.client.get(expired)
+        expired = f"{self.admin_url}?{urlencode({'active': '0'})}"
+        with self.assertWarnsRegex(UserWarning, r"The address 20\.13\.1\.1 is not in the database"):
+            response = self.client.get(expired)
         self.assertContains(response, '20.13.1.1')
         self.assertNotContains(response, '1.1.1.1')
 
     def test_unexpired(self):
-        unexpired = '{}?{}'.format(self.admin_url, urlencode({'active': '1'}))
-        response = self.client.get(unexpired)
+        unexpired = f"{self.admin_url}?{urlencode({'active': '1'})}"
+        with self.assertWarnsRegex(UserWarning, r"The address 1\.1\.1\.1 is not in the database"):
+            response = self.client.get(unexpired)
         self.assertContains(response, '1.1.1.1')
         self.assertNotContains(response, '20.13.1.1')
 
@@ -316,7 +325,20 @@ def test_no_session(self):
 class LocationTemplateFilterTest(TestCase):
     @override_settings(GEOIP_PATH=None)
     def test_no_location(self):
-        self.assertEqual(location('127.0.0.1'), None)
+        with self.assertWarnsRegex(
+            UserWarning,
+            r"The address 127\.0\.0\.1 is not in the database",
+        ):
+            loc = location('127.0.0.1')
+        self.assertEqual(loc, None)
+
+    @skipUnless(geoip, geoip_msg)
+    def test_city(self):
+        self.assertEqual('San Diego', city('44.55.66.77'))
+
+    @skipUnless(geoip, geoip_msg)
+    def test_country(self):
+        self.assertEqual('United States', country('8.8.8.8'))
 
     @skipUnless(geoip, geoip_msg)
     def test_locations(self):
@@ -751,7 +773,7 @@ def test_windows_only(self):
 
 class ClearsessionsCommandTest(TestCase):
     def test_can_call(self):
-        Session.objects.create(expire_date=datetime.now() - timedelta(days=1),
+        Session.objects.create(expire_date=now() - timedelta(days=1),
                                ip='127.0.0.1')
         call_command('clearsessions')
         self.assertEqual(Session.objects.count(), 0)

user_sessions/utils/tests.py → tests/utils.py

@@ -3,7 +3,7 @@
 from django.http import HttpRequest
 from django.test import Client as BaseClient
 
-from ..backends.db import SessionStore
+from user_sessions.backends.db import SessionStore
 
 
 class Client(BaseClient):