From dd61e68a6e492c602e1b436614d3fb9e2f1e67b2 Mon Sep 17 00:00:00 2001 From: Dalai Felinto Date: Sat, 18 May 2024 17:41:42 +0200 Subject: [PATCH 01/12] API: Upload new version of an extension URL: /api/v1/version-upload/ Method: POST Parameters: * extension_id * version_file * release_notes --- extensions/tests/test_api.py | 89 +++++++++++++++++++++++++++++++++++ extensions/urls.py | 5 ++ extensions/views/api.py | 90 +++++++++++++++++++++++++++++++++++- files/forms.py | 10 ++++ 4 files changed, 192 insertions(+), 2 deletions(-) create mode 100644 extensions/tests/test_api.py diff --git a/extensions/tests/test_api.py b/extensions/tests/test_api.py new file mode 100644 index 00000000..61eb8564 --- /dev/null +++ b/extensions/tests/test_api.py @@ -0,0 +1,89 @@ +from pathlib import Path + +from django.urls import reverse +from rest_framework import status +from rest_framework.test import APITestCase, APIClient + +from common.tests.factories.users import UserFactory +from common.tests.factories.extensions import create_approved_version + +from extensions.models import Version +from tokens.models import UserToken + + +TEST_FILES_DIR = Path(__file__).resolve().parent / 'files' + + +class VersionUploadAPITest(APITestCase): + def setUp(self): + self.user = UserFactory() + self.token = UserToken.objects.create(user=self.user) + + self.client = APIClient() + self.version = create_approved_version( + extension__extension_id="amaranth", + version="1.0.7", + file__user=self.user, + ) + self.extension = self.version.extension + self.upload_url = reverse('extensions:upload-extension-version') + self.file_path = TEST_FILES_DIR / "amaranth-1.0.8.zip" + + def test_version_upload_extension_not_maintained_by_user(self): + other_user = UserFactory() + other_extension = create_approved_version( + extension__extension_id='other_extension', file__user=other_user + ).extension + + with open(self.file_path, 'rb') as version_file: + response = self.client.post( + self.upload_url, + { + 'version_file': version_file, + 'extension_id': other_extension.extension_id, + 'release_notes': 'These are the release notes', + }, + format='multipart', + HTTP_AUTHORIZATION=f'Bearer {self.token.token}', + ) + + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual( + response.data['message'], + f'Extension "{other_extension.extension_id}" not maintained by user "{self.user.full_name}"', + ) + + def test_version_upload_extension_does_not_exist(self): + extension_name = 'extension_do_not_exist' + with open(self.file_path, 'rb') as version_file: + response = self.client.post( + self.upload_url, + { + 'version_file': version_file, + 'extension_id': extension_name, + 'release_notes': 'These are the release notes', + }, + format='multipart', + HTTP_AUTHORIZATION=f'Bearer {self.token.token}', + ) + + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + self.assertEqual(response.data['message'], f'Extension "{extension_name}" not found') + + def test_version_upload_success(self): + self.assertEqual(Version.objects.filter(extension=self.extension).count(), 1) + with open(self.file_path, 'rb') as version_file: + response = self.client.post( + self.upload_url, + { + 'version_file': version_file, + 'extension_id': self.extension.extension_id, + 'release_notes': 'These are the release notes', + }, + format='multipart', + HTTP_AUTHORIZATION=f'Bearer {self.token.token}', + ) + + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + self.assertIn('version_id', response.data) + self.assertEqual(Version.objects.filter(extension=self.extension).count(), 2) diff --git a/extensions/urls.py b/extensions/urls.py index f247b371..cd28a25a 100644 --- a/extensions/urls.py +++ b/extensions/urls.py @@ -16,6 +16,11 @@ urlpatterns = [ ), # API path('api/v1/extensions/', api.ExtensionsAPIView.as_view(), name='api'), + path( + 'api/v1/version-upload/', + api.UploadExtensionVersionView.as_view(), + name='upload-extension-version', + ), # Public pages path('', public.HomeView.as_view(), name='home'), path('search/', public.SearchView.as_view(), name='search'), diff --git a/extensions/views/api.py b/extensions/views/api.py index 8f60fc02..9a561bd8 100644 --- a/extensions/views/api.py +++ b/extensions/views/api.py @@ -2,14 +2,17 @@ import logging from rest_framework.permissions import AllowAny from rest_framework.response import Response -from rest_framework import serializers +from rest_framework import serializers, status from rest_framework.views import APIView +from rest_framework.permissions import IsAuthenticated from drf_spectacular.utils import OpenApiParameter, extend_schema from django.core.exceptions import ValidationError from common.compare import is_in_version_range, version -from extensions.models import Extension, Platform +from extensions.models import Extension, Platform, Version from extensions.utils import clean_json_dictionary_from_optional_fields +from extensions.views.manage import NewVersionView +from files.forms import FileFormSkipAgreed from constants.base import ( @@ -151,3 +154,86 @@ class ExtensionsAPIView(APIView): 'version': 'v1', } ) + + +class ExtensionVersionSerializer(serializers.Serializer): + extension_id = serializers.CharField(max_length=255) + version_file = serializers.FileField() + release_notes = serializers.CharField(max_length=1024, required=False) + + +class UploadExtensionVersionView(APIView): + permission_classes = [IsAuthenticated] + + @extend_schema( + request=ExtensionVersionSerializer, + responses={201: 'Extension version uploaded successfully!'}, + ) + def post(self, request, *args, **kwargs): + serializer = ExtensionVersionSerializer(data=request.data) + if not serializer.is_valid(): + return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) + + user = request.user + extension_id = serializer.validated_data['extension_id'] + version_file = serializer.validated_data['version_file'] + release_notes = serializer.validated_data.get('release_notes', '') + + extension = Extension.objects.filter(extension_id=extension_id).first() + if not extension: + return Response( + { + 'message': f'Extension "{extension_id}" not found', + }, + status=status.HTTP_404_NOT_FOUND, + ) + + if not extension.has_maintainer(user): + return Response( + { + 'message': f'Extension "{extension_id}" not maintained by user "{user}"', + }, + status=status.HTTP_403_FORBIDDEN, + ) + + try: + # Create a NewVersionView instance to handle file creation + new_version_view = NewVersionView(request=request, extension=extension) + + # Pass the version_file to the form + form = new_version_view.get_form(FileFormSkipAgreed) + form.fields['source'].initial = version_file + + if not form.is_valid(): + return Response({'message': form.errors}, status=status.HTTP_400_BAD_REQUEST) + + # Create the file instance + file_instance = form.save(commit=False) + file_instance.user = user + file_instance.save() + + # Create the version from the file + version = Version.objects.update_or_create( + extension=extension, + file=file_instance, + release_notes=release_notes, + **file_instance.parsed_version_fields, + )[0] + + return Response( + { + 'message': 'Extension version uploaded successfully!', + 'extension_id': extension_id, + 'version_file': version_file.name, + 'release_notes': version.release_notes, + 'version_id': version.id, + }, + status=status.HTTP_201_CREATED, + ) + except Exception as e: + return Response( + { + 'message': str(e), + }, + status=status.HTTP_500_INTERNAL_SERVER_ERROR, + ) diff --git a/files/forms.py b/files/forms.py index a2798787..ae1be39d 100644 --- a/files/forms.py +++ b/files/forms.py @@ -167,6 +167,16 @@ class FileForm(forms.ModelForm): return self.cleaned_data +class FileFormSkipAgreed(FileForm): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.fields['agreed_with_terms'].required = False + + def clean(self): + self.cleaned_data['agreed_with_terms'] = True + super().clean() + + class BaseMediaFileForm(forms.ModelForm): class Meta: model = files.models.File -- 2.30.2 From 3b1cbdbb00e694bc86ccbf49aed179608454d011 Mon Sep 17 00:00:00 2001 From: Dalai Felinto Date: Tue, 21 May 2024 23:34:33 +0200 Subject: [PATCH 02/12] From review: wrap database calls with transaction.atomic() --- extensions/views/api.py | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/extensions/views/api.py b/extensions/views/api.py index 9a561bd8..e5981aba 100644 --- a/extensions/views/api.py +++ b/extensions/views/api.py @@ -7,6 +7,7 @@ from rest_framework.views import APIView from rest_framework.permissions import IsAuthenticated from drf_spectacular.utils import OpenApiParameter, extend_schema from django.core.exceptions import ValidationError +from django.db import transaction from common.compare import is_in_version_range, version from extensions.models import Extension, Platform, Version @@ -207,18 +208,19 @@ class UploadExtensionVersionView(APIView): if not form.is_valid(): return Response({'message': form.errors}, status=status.HTTP_400_BAD_REQUEST) - # Create the file instance - file_instance = form.save(commit=False) - file_instance.user = user - file_instance.save() + with transaction.atomic(): + # Create the file instance + file_instance = form.save(commit=False) + file_instance.user = user + file_instance.save() - # Create the version from the file - version = Version.objects.update_or_create( - extension=extension, - file=file_instance, - release_notes=release_notes, - **file_instance.parsed_version_fields, - )[0] + # Create the version from the file + version = Version.objects.update_or_create( + extension=extension, + file=file_instance, + release_notes=release_notes, + **file_instance.parsed_version_fields, + )[0] return Response( { -- 2.30.2 From f5f50c3c38bb9b3818ad6ba791b94f0277abbea8 Mon Sep 17 00:00:00 2001 From: Dalai Felinto Date: Tue, 21 May 2024 23:34:46 +0200 Subject: [PATCH 03/12] From review: do not expose internal IDs --- extensions/views/api.py | 1 - 1 file changed, 1 deletion(-) diff --git a/extensions/views/api.py b/extensions/views/api.py index e5981aba..1ca54fda 100644 --- a/extensions/views/api.py +++ b/extensions/views/api.py @@ -228,7 +228,6 @@ class UploadExtensionVersionView(APIView): 'extension_id': extension_id, 'version_file': version_file.name, 'release_notes': version.release_notes, - 'version_id': version.id, }, status=status.HTTP_201_CREATED, ) -- 2.30.2 From 1baf3b9bb8945e3a60c74d81e62f903192ffb0f6 Mon Sep 17 00:00:00 2001 From: Dalai Felinto Date: Tue, 21 May 2024 23:37:53 +0200 Subject: [PATCH 04/12] From review: Remove big try/catch --- extensions/views/api.py | 64 ++++++++++++++++++----------------------- 1 file changed, 28 insertions(+), 36 deletions(-) diff --git a/extensions/views/api.py b/extensions/views/api.py index 1ca54fda..a7ff49d3 100644 --- a/extensions/views/api.py +++ b/extensions/views/api.py @@ -197,44 +197,36 @@ class UploadExtensionVersionView(APIView): status=status.HTTP_403_FORBIDDEN, ) - try: - # Create a NewVersionView instance to handle file creation - new_version_view = NewVersionView(request=request, extension=extension) + # Create a NewVersionView instance to handle file creation + new_version_view = NewVersionView(request=request, extension=extension) - # Pass the version_file to the form - form = new_version_view.get_form(FileFormSkipAgreed) - form.fields['source'].initial = version_file + # Pass the version_file to the form + form = new_version_view.get_form(FileFormSkipAgreed) + form.fields['source'].initial = version_file - if not form.is_valid(): - return Response({'message': form.errors}, status=status.HTTP_400_BAD_REQUEST) + if not form.is_valid(): + return Response({'message': form.errors}, status=status.HTTP_400_BAD_REQUEST) - with transaction.atomic(): - # Create the file instance - file_instance = form.save(commit=False) - file_instance.user = user - file_instance.save() + with transaction.atomic(): + # Create the file instance + file_instance = form.save(commit=False) + file_instance.user = user + file_instance.save() - # Create the version from the file - version = Version.objects.update_or_create( - extension=extension, - file=file_instance, - release_notes=release_notes, - **file_instance.parsed_version_fields, - )[0] + # Create the version from the file + version = Version.objects.update_or_create( + extension=extension, + file=file_instance, + release_notes=release_notes, + **file_instance.parsed_version_fields, + )[0] - return Response( - { - 'message': 'Extension version uploaded successfully!', - 'extension_id': extension_id, - 'version_file': version_file.name, - 'release_notes': version.release_notes, - }, - status=status.HTTP_201_CREATED, - ) - except Exception as e: - return Response( - { - 'message': str(e), - }, - status=status.HTTP_500_INTERNAL_SERVER_ERROR, - ) + return Response( + { + 'message': 'Extension version uploaded successfully!', + 'extension_id': extension_id, + 'version_file': version_file.name, + 'release_notes': version.release_notes, + }, + status=status.HTTP_201_CREATED, + ) -- 2.30.2 From 2ca34ac8c25f3c01e600fd01cddb581b4661610b Mon Sep 17 00:00:00 2001 From: Dalai Felinto Date: Sat, 25 May 2024 13:44:58 +0200 Subject: [PATCH 05/12] Fix authentication to include timezone --- apitokens/authentication.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apitokens/authentication.py b/apitokens/authentication.py index 857b7bf8..4f43cd23 100644 --- a/apitokens/authentication.py +++ b/apitokens/authentication.py @@ -1,4 +1,4 @@ -import datetime +from django.utils import timezone from rest_framework.authentication import BaseAuthentication from rest_framework.exceptions import AuthenticationFailed @@ -27,7 +27,7 @@ class UserTokenAuthentication(BaseAuthentication): raise AuthenticationFailed('Invalid token') token.ip_address_last_access = clean_ip_address(request) - token.date_last_access = datetime.datetime.now() + token.date_last_access = timezone.now() token.save(update_fields={'ip_address_last_access', 'date_last_access'}) return (token.user, token) -- 2.30.2 From 81d4bfd3b9e1f15e6093e6bc76ddcad2e731acc0 Mon Sep 17 00:00:00 2001 From: Dalai Felinto Date: Sat, 25 May 2024 13:49:40 +0200 Subject: [PATCH 06/12] From review: Fix unittests --- extensions/tests/test_api.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/extensions/tests/test_api.py b/extensions/tests/test_api.py index 61eb8564..e842da1f 100644 --- a/extensions/tests/test_api.py +++ b/extensions/tests/test_api.py @@ -8,7 +8,7 @@ from common.tests.factories.users import UserFactory from common.tests.factories.extensions import create_approved_version from extensions.models import Version -from tokens.models import UserToken +from apitokens.models import UserToken TEST_FILES_DIR = Path(__file__).resolve().parent / 'files' @@ -17,7 +17,12 @@ TEST_FILES_DIR = Path(__file__).resolve().parent / 'files' class VersionUploadAPITest(APITestCase): def setUp(self): self.user = UserFactory() - self.token = UserToken.objects.create(user=self.user) + self.token_key = UserToken.generate_token_key() + self.token = UserToken.objects.create( + user=self.user, + token_prefix=UserToken.generate_token_prefix(self.token_key), + token_hash=UserToken.generate_hash(self.token_key), + ) self.client = APIClient() self.version = create_approved_version( @@ -44,7 +49,7 @@ class VersionUploadAPITest(APITestCase): 'release_notes': 'These are the release notes', }, format='multipart', - HTTP_AUTHORIZATION=f'Bearer {self.token.token}', + HTTP_AUTHORIZATION=f'Bearer {self.token_key}', ) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) @@ -64,7 +69,7 @@ class VersionUploadAPITest(APITestCase): 'release_notes': 'These are the release notes', }, format='multipart', - HTTP_AUTHORIZATION=f'Bearer {self.token.token}', + HTTP_AUTHORIZATION=f'Bearer {self.token_key}', ) self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) @@ -81,9 +86,8 @@ class VersionUploadAPITest(APITestCase): 'release_notes': 'These are the release notes', }, format='multipart', - HTTP_AUTHORIZATION=f'Bearer {self.token.token}', + HTTP_AUTHORIZATION=f'Bearer {self.token_key}', ) self.assertEqual(response.status_code, status.HTTP_201_CREATED) - self.assertIn('version_id', response.data) self.assertEqual(Version.objects.filter(extension=self.extension).count(), 2) -- 2.30.2 From 9ac16a529fbca3272cc6cbe51dd88995eba4819e Mon Sep 17 00:00:00 2001 From: Dalai Felinto Date: Sat, 25 May 2024 14:01:14 +0200 Subject: [PATCH 07/12] Unify creation of token with hash/prefix --- apitokens/models.py | 10 ++++++++++ apitokens/tests/test_user_token.py | 8 +------- extensions/tests/test_api.py | 7 +------ 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/apitokens/models.py b/apitokens/models.py index 13e9e25f..dc6c1fc9 100644 --- a/apitokens/models.py +++ b/apitokens/models.py @@ -6,6 +6,8 @@ from django.urls import reverse from users.models import User +from typing import Tuple + class UserToken(models.Model): user = models.ForeignKey(User, on_delete=models.CASCADE, related_name='tokens') @@ -34,3 +36,11 @@ class UserToken(models.Model): def generate_token_prefix(cls, token_key: str) -> str: token_prefix_length = cls._meta.get_field('token_prefix').max_length return token_key[:token_prefix_length] + + @classmethod + def create_with_token(cls, *args, **kwargs) -> Tuple['UserToken', str]: + token_key = cls.generate_token_key() + kwargs['token_hash'] = cls.generate_hash(token_key) + kwargs['token_prefix'] = cls.generate_token_prefix(token_key) + token = cls.objects.create(*args, **kwargs) + return token, token_key diff --git a/apitokens/tests/test_user_token.py b/apitokens/tests/test_user_token.py index b763f356..ad5ab831 100644 --- a/apitokens/tests/test_user_token.py +++ b/apitokens/tests/test_user_token.py @@ -44,13 +44,7 @@ class UserTokenTest(TestCase): self.assertNotContains(response, token_key) def test_list_page_does_not_display_full_token_value(self): - token_key = UserToken.generate_token_key() - - token_prefix = UserToken.generate_token_prefix(token_key) - token_hash = UserToken.generate_hash(token_key) - token = UserToken.objects.create( - user=self.user, name='Test Token', token_prefix=token_prefix, token_hash=token_hash - ) + token, token_key = UserToken.create_with_token(user=self.user, name='Test Token') response = self.client.get(reverse('apitokens:list')) self.assertContains(response, str(token.token_prefix)) diff --git a/extensions/tests/test_api.py b/extensions/tests/test_api.py index e842da1f..a050940e 100644 --- a/extensions/tests/test_api.py +++ b/extensions/tests/test_api.py @@ -17,12 +17,7 @@ TEST_FILES_DIR = Path(__file__).resolve().parent / 'files' class VersionUploadAPITest(APITestCase): def setUp(self): self.user = UserFactory() - self.token_key = UserToken.generate_token_key() - self.token = UserToken.objects.create( - user=self.user, - token_prefix=UserToken.generate_token_prefix(self.token_key), - token_hash=UserToken.generate_hash(self.token_key), - ) + self.token, self.token_key = UserToken.create_with_token(user=self.user) self.client = APIClient() self.version = create_approved_version( -- 2.30.2 From 26bfe61f5396977a6485f5227435ac8a0ce7d74b Mon Sep 17 00:00:00 2001 From: Dalai Felinto Date: Sat, 25 May 2024 14:10:48 +0200 Subject: [PATCH 08/12] From review: Test for a non-authenticated request --- extensions/tests/test_api.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/extensions/tests/test_api.py b/extensions/tests/test_api.py index a050940e..a9348eca 100644 --- a/extensions/tests/test_api.py +++ b/extensions/tests/test_api.py @@ -29,6 +29,20 @@ class VersionUploadAPITest(APITestCase): self.upload_url = reverse('extensions:upload-extension-version') self.file_path = TEST_FILES_DIR / "amaranth-1.0.8.zip" + def test_version_upload_unauthenticated(self): + with open(self.file_path, 'rb') as version_file: + response = self.client.post( + self.upload_url, + { + 'version_file': version_file, + 'extension_id': self.extension.extension_id, + 'release_notes': 'These are the release notes', + }, + format='multipart', + ) + + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + def test_version_upload_extension_not_maintained_by_user(self): other_user = UserFactory() other_extension = create_approved_version( -- 2.30.2 From 8c11df9e5d4f78a4c860993e520525ef17d7f3e7 Mon Sep 17 00:00:00 2001 From: Dalai Felinto Date: Mon, 27 May 2024 12:49:43 +0200 Subject: [PATCH 09/12] From review: new API url --- extensions/tests/test_api.py | 18 +++++++++--------- extensions/urls.py | 2 +- extensions/views/api.py | 4 +--- 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/extensions/tests/test_api.py b/extensions/tests/test_api.py index a9348eca..fd01084e 100644 --- a/extensions/tests/test_api.py +++ b/extensions/tests/test_api.py @@ -26,16 +26,19 @@ class VersionUploadAPITest(APITestCase): file__user=self.user, ) self.extension = self.version.extension - self.upload_url = reverse('extensions:upload-extension-version') self.file_path = TEST_FILES_DIR / "amaranth-1.0.8.zip" + @staticmethod + def _get_upload_url(extension_id): + upload_url = reverse('extensions:upload-extension-version', args=(extension_id,)) + return upload_url + def test_version_upload_unauthenticated(self): with open(self.file_path, 'rb') as version_file: response = self.client.post( - self.upload_url, + self._get_upload_url(self.extension.extension_id), { 'version_file': version_file, - 'extension_id': self.extension.extension_id, 'release_notes': 'These are the release notes', }, format='multipart', @@ -51,10 +54,9 @@ class VersionUploadAPITest(APITestCase): with open(self.file_path, 'rb') as version_file: response = self.client.post( - self.upload_url, + self._get_upload_url(other_extension.extension_id), { 'version_file': version_file, - 'extension_id': other_extension.extension_id, 'release_notes': 'These are the release notes', }, format='multipart', @@ -71,10 +73,9 @@ class VersionUploadAPITest(APITestCase): extension_name = 'extension_do_not_exist' with open(self.file_path, 'rb') as version_file: response = self.client.post( - self.upload_url, + self._get_upload_url(extension_name), { 'version_file': version_file, - 'extension_id': extension_name, 'release_notes': 'These are the release notes', }, format='multipart', @@ -88,10 +89,9 @@ class VersionUploadAPITest(APITestCase): self.assertEqual(Version.objects.filter(extension=self.extension).count(), 1) with open(self.file_path, 'rb') as version_file: response = self.client.post( - self.upload_url, + self._get_upload_url(self.extension.extension_id), { 'version_file': version_file, - 'extension_id': self.extension.extension_id, 'release_notes': 'These are the release notes', }, format='multipart', diff --git a/extensions/urls.py b/extensions/urls.py index cd28a25a..e3531431 100644 --- a/extensions/urls.py +++ b/extensions/urls.py @@ -17,7 +17,7 @@ urlpatterns = [ # API path('api/v1/extensions/', api.ExtensionsAPIView.as_view(), name='api'), path( - 'api/v1/version-upload/', + 'api/v1/extensions//versions/new/', api.UploadExtensionVersionView.as_view(), name='upload-extension-version', ), diff --git a/extensions/views/api.py b/extensions/views/api.py index a7ff49d3..8db7fb06 100644 --- a/extensions/views/api.py +++ b/extensions/views/api.py @@ -158,7 +158,6 @@ class ExtensionsAPIView(APIView): class ExtensionVersionSerializer(serializers.Serializer): - extension_id = serializers.CharField(max_length=255) version_file = serializers.FileField() release_notes = serializers.CharField(max_length=1024, required=False) @@ -170,13 +169,12 @@ class UploadExtensionVersionView(APIView): request=ExtensionVersionSerializer, responses={201: 'Extension version uploaded successfully!'}, ) - def post(self, request, *args, **kwargs): + def post(self, request, extension_id, *args, **kwargs): serializer = ExtensionVersionSerializer(data=request.data) if not serializer.is_valid(): return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) user = request.user - extension_id = serializer.validated_data['extension_id'] version_file = serializer.validated_data['version_file'] release_notes = serializer.validated_data.get('release_notes', '') -- 2.30.2 From f72b6c217c2664d0a4435c5a0be5385d52c452a1 Mon Sep 17 00:00:00 2001 From: Dalai Felinto Date: Mon, 27 May 2024 14:50:36 +0200 Subject: [PATCH 10/12] From review: move create_with_token to tests.utils --- apitokens/models.py | 10 ---------- apitokens/tests/test_user_token.py | 3 ++- common/tests/utils.py | 12 ++++++++++++ extensions/tests/test_api.py | 4 ++-- 4 files changed, 16 insertions(+), 13 deletions(-) diff --git a/apitokens/models.py b/apitokens/models.py index dc6c1fc9..13e9e25f 100644 --- a/apitokens/models.py +++ b/apitokens/models.py @@ -6,8 +6,6 @@ from django.urls import reverse from users.models import User -from typing import Tuple - class UserToken(models.Model): user = models.ForeignKey(User, on_delete=models.CASCADE, related_name='tokens') @@ -36,11 +34,3 @@ class UserToken(models.Model): def generate_token_prefix(cls, token_key: str) -> str: token_prefix_length = cls._meta.get_field('token_prefix').max_length return token_key[:token_prefix_length] - - @classmethod - def create_with_token(cls, *args, **kwargs) -> Tuple['UserToken', str]: - token_key = cls.generate_token_key() - kwargs['token_hash'] = cls.generate_hash(token_key) - kwargs['token_prefix'] = cls.generate_token_prefix(token_key) - token = cls.objects.create(*args, **kwargs) - return token, token_key diff --git a/apitokens/tests/test_user_token.py b/apitokens/tests/test_user_token.py index ad5ab831..10503562 100644 --- a/apitokens/tests/test_user_token.py +++ b/apitokens/tests/test_user_token.py @@ -6,6 +6,7 @@ from django.urls import reverse from apitokens.models import UserToken from common.tests.factories.users import UserFactory +from common.tests.utils import create_user_token class UserTokenTest(TestCase): @@ -44,7 +45,7 @@ class UserTokenTest(TestCase): self.assertNotContains(response, token_key) def test_list_page_does_not_display_full_token_value(self): - token, token_key = UserToken.create_with_token(user=self.user, name='Test Token') + token, token_key = create_user_token(user=self.user, name='Test Token') response = self.client.get(reverse('apitokens:list')) self.assertContains(response, str(token.token_prefix)) diff --git a/common/tests/utils.py b/common/tests/utils.py index 194778b2..e95e4bad 100644 --- a/common/tests/utils.py +++ b/common/tests/utils.py @@ -1,9 +1,13 @@ import itertools +from typing import Tuple import django.urls as urls from django.utils.functional import cached_property from django.utils.regex_helper import normalize +from apitokens.models import UserToken + + try: # Django 2.0 url_resolver_types = (urls.URLResolver,) DJANGO_2 = True @@ -109,3 +113,11 @@ class CheckFilePropertiesMixin: self.assertEqual(file.original_name, kwargs.get('original_name')) if 'size_bytes' in kwargs: self.assertEqual(file.size_bytes, kwargs.get('size_bytes')) + + +def create_user_token(*args, **kwargs) -> Tuple['UserToken', str]: + token_key = UserToken.generate_token_key() + kwargs['token_hash'] = UserToken.generate_hash(token_key) + kwargs['token_prefix'] = UserToken.generate_token_prefix(token_key) + token = UserToken.objects.create(*args, **kwargs) + return token, token_key diff --git a/extensions/tests/test_api.py b/extensions/tests/test_api.py index fd01084e..88ee5f49 100644 --- a/extensions/tests/test_api.py +++ b/extensions/tests/test_api.py @@ -6,9 +6,9 @@ from rest_framework.test import APITestCase, APIClient from common.tests.factories.users import UserFactory from common.tests.factories.extensions import create_approved_version +from common.tests.utils import create_user_token from extensions.models import Version -from apitokens.models import UserToken TEST_FILES_DIR = Path(__file__).resolve().parent / 'files' @@ -17,7 +17,7 @@ TEST_FILES_DIR = Path(__file__).resolve().parent / 'files' class VersionUploadAPITest(APITestCase): def setUp(self): self.user = UserFactory() - self.token, self.token_key = UserToken.create_with_token(user=self.user) + self.token, self.token_key = create_user_token(user=self.user) self.client = APIClient() self.version = create_approved_version( -- 2.30.2 From f0f4e808a03f1436a509c38b04541d3a15db24b1 Mon Sep 17 00:00:00 2001 From: Dalai Felinto Date: Mon, 27 May 2024 15:31:39 +0200 Subject: [PATCH 11/12] From review: unittest to check if date_last_access is updated --- extensions/tests/test_api.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/extensions/tests/test_api.py b/extensions/tests/test_api.py index 88ee5f49..365e9db0 100644 --- a/extensions/tests/test_api.py +++ b/extensions/tests/test_api.py @@ -100,3 +100,18 @@ class VersionUploadAPITest(APITestCase): self.assertEqual(response.status_code, status.HTTP_201_CREATED) self.assertEqual(Version.objects.filter(extension=self.extension).count(), 2) + + def test_date_last_access(self): + self.assertIsNone(self.token.date_last_access) + with open(self.file_path, 'rb') as version_file: + response = self.client.post( + self._get_upload_url(self.extension.extension_id), + { + 'version_file': version_file, + 'release_notes': 'These are the release notes', + }, + format='multipart', + HTTP_AUTHORIZATION=f'Bearer {self.token_key}', + ) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + self.assertIsNotNone(self.token.date_last_access) -- 2.30.2 From de05ae8d57f5de12b67846e5499d4768ef61ec90 Mon Sep 17 00:00:00 2001 From: Dalai Felinto Date: Mon, 27 May 2024 15:49:03 +0200 Subject: [PATCH 12/12] Fix unittest: It required refresh_from_db() --- extensions/tests/test_api.py | 1 + 1 file changed, 1 insertion(+) diff --git a/extensions/tests/test_api.py b/extensions/tests/test_api.py index 365e9db0..54d66b96 100644 --- a/extensions/tests/test_api.py +++ b/extensions/tests/test_api.py @@ -114,4 +114,5 @@ class VersionUploadAPITest(APITestCase): HTTP_AUTHORIZATION=f'Bearer {self.token_key}', ) self.assertEqual(response.status_code, status.HTTP_201_CREATED) + self.token.refresh_from_db() self.assertIsNotNone(self.token.date_last_access) -- 2.30.2