From 634cae5cf15cf17516d6acbfceb2d4a9c0188e56 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Piotr=20Ma=C5=9Blanka?= <piotr.maslanka@henrietta.com.pl>
Date: Thu, 26 Aug 2021 22:45:29 +0200
Subject: [PATCH] everything done
---
NOTES.md | 32 +++++++++++++++---
README.md | 5 +++
counting/tests.py | 22 +++++++++++++
counting/views.py | 10 ++++--
docker-compose.yml | 6 ++++
netguru/settings.py | 11 +++++++
netguru/urls.py | 4 +--
requirements.txt | 1 +
shares/api.py | 36 ++++++++++++--------
shares/tests.py | 80 ++++++++++++++++++++++++++++++++++++++++-----
shares/views.py | 3 --
11 files changed, 175 insertions(+), 35 deletions(-)
create mode 100644 counting/tests.py
diff --git a/NOTES.md b/NOTES.md
index 33b7182..381ddca 100644
--- a/NOTES.md
+++ b/NOTES.md
@@ -16,12 +16,19 @@ as such:
Because keying them together by day is going to make the thing
a lot harder for the frontend programmer to parse.
+I know that I'm ignoring specification, and you are free to call me out
+on that - but as I got some experience with frontend,
+I'd rather do **the right thing**.
+
## Documentation
-I couldn't get the DRF documentation to cooperate with me, and frankly
-while with a bigger project I'd probably stick with drf-yasg,
-the inline pydocs that I've written here will suffice.
+I couldn't get the DRF documentation to cooperate with me (most
+possibly there's no way to do it without an external dependency),
+and frankly while with a bigger project I'd probably stick with drf-yasg,
+the inline pydocs that I've written here will completely suffice.
+A in-depth knowledge of your tools is all good, but knowing when
+to apply them trumps even that.
`/swagger-ui/` is your go-to URL when it comes to listing endpoints.
Submissions are to follow `/api/add`, because DRF happened to generate
@@ -30,8 +37,23 @@ nice documentation there and not for Swagger.
## Authorization
You can authorize your API requests either via Django cookies
-or through a Web-Basic authentication. I care little which you choose.
-Getting the session token for your API requests is also on you.
+or through a Web-Basic authentication.
+Getting the session token for your API requests is also on you,
+but since the API is unit-tested you'll just have to believe me that it works.
Since it was not specifically requested for history endpoint to be
available for admin only, it was made available for any logged in user.
+
+## Test code duplication
+
+I realize that I would be best practice to deduplicate some code contained
+within tests, but since I'm running about 1,5 of full-time equvialents you just have to forgive me
+for sparing the effort to do so. Thanks "from the mountain!"
+
+# The reaper job
+
+I came up with the reaper job trying to think up a reasonable solution
+that wouldn't load the processing server too much. It processes way-past-due
+links into a nice compact representation and stores them in the database.
+Two last days (since the job fires only every 24 hours) are computed on-the-fly,
+and cached as necessary, for up to 5 minutes.
diff --git a/README.md b/README.md
index 68d6da0..416509f 100644
--- a/README.md
+++ b/README.md
@@ -77,6 +77,11 @@ ALLOWED_HOSTS and giving a list of hosts separated
by a comma. If you don't define it, a default
value of `*` will be used
+You can optionally provide the environment variable
+`REDIS_CACHE_HOST` that should contain a hostname:port of
+a Redis cache instance, if you mean to configure Redis caching.
+It will be disabled by default.
+
**WARNING!!** This is not secure, don't do it!
### Volumes
diff --git a/counting/tests.py b/counting/tests.py
new file mode 100644
index 0000000..ccc6b61
--- /dev/null
+++ b/counting/tests.py
@@ -0,0 +1,22 @@
+from django.test import TestCase
+
+from django.contrib.auth.models import User
+from rest_framework.test import APIClient
+
+
+class TestShares(TestCase):
+
+ def setUp(self) -> None:
+ super().setUp()
+ user, _ = User.objects.get_or_create(username='testuser', email='admin@admin.com',
+ password='12345')
+
+ self.api_client = APIClient()
+ self.api_client.force_login(user)
+
+ def test_history(self):
+ """
+ Test that a file can be added and that it can be visited
+ """
+ response = self.client.get('http://127.0.0.1/api/history')
+ self.assertGreaterEqual(len(response.json()), 2)
diff --git a/counting/views.py b/counting/views.py
index 00eb177..7b57dd1 100644
--- a/counting/views.py
+++ b/counting/views.py
@@ -1,9 +1,12 @@
import datetime
# Create your views here.
+from django.utils.decorators import method_decorator
+from django.views.decorators.cache import cache_page
from rest_framework import serializers, status
from rest_framework.response import Response
from rest_framework.views import APIView
+from satella.time import parse_time_string
from counting.cron import ReaperJob, DayType
from counting.models import StoryOfADay
@@ -38,6 +41,8 @@ class GetHistory(APIView):
You must be authorized to access this endpoint.
+ This endpoint is cached for up to 5 minutes.
+
The result will be a list of elements like
{
"day": "YYYY-MM-DD",
@@ -46,9 +51,10 @@ class GetHistory(APIView):
}
"""
+ @method_decorator(cache_page(parse_time_string('5m')))
def get(self, request):
- # if not request.user.is_authenticated:
- # return Response({}, status=status.HTTP_401_UNAUTHORIZED)
+ if not request.user.is_authenticated:
+ return Response({}, status=status.HTTP_401_UNAUTHORIZED)
items = set(StoryOfADay.objects.all())
today = datetime.date.today()
items.add(get_history_for(today))
diff --git a/docker-compose.yml b/docker-compose.yml
index e846369..e2c086c 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -1,5 +1,7 @@
version: '3.4'
services:
+ redis:
+ image: redis
postgres:
image: postgres
environment:
@@ -13,14 +15,18 @@ services:
build: .
environment:
- DEBUG=1
+ - REDIS_CACHE_HOST=redis:6379
depends_on:
- postgres
+ - redis
run_local:
build: .
depends_on:
- postgres
+ - redis
environment:
- DEBUG=1
+ - REDIS_CACHE_HOST=redis:6379
ports:
- 80:80
- 81:81
diff --git a/netguru/settings.py b/netguru/settings.py
index d12d8e4..746d044 100644
--- a/netguru/settings.py
+++ b/netguru/settings.py
@@ -1,5 +1,7 @@
import os
import logging
+
+import django_opentracing
from satella.instrumentation.metrics import getMetric
from satella.instrumentation.metrics.exporters import PrometheusHTTPExporterThread
@@ -141,6 +143,15 @@ STATIC_ROOT = os.path.join(BASE_DIR, 'media')
FILE_SAVE_LOCATION = '/data'
+# Configure cache
+if os.environ.get('REDIS_CACHE_HOST'):
+ CACHES = {
+ 'default': {
+ 'BACKEND': 'redis_cache.RedisCache',
+ 'LOCATION': os.environ['REDIS_CACHE_HOST'],
+ },
+ }
+
# Configure DRF and Swagger
DEFAULT_SCHEMA_CLASS = 'rest_framework.schemas.openapi.AutoSchema'
diff --git a/netguru/urls.py b/netguru/urls.py
index 99f10a6..f566220 100644
--- a/netguru/urls.py
+++ b/netguru/urls.py
@@ -1,6 +1,6 @@
from django.conf.urls.static import static
from django.contrib import admin
-from django.urls import path, include, re_path
+from django.urls import path, include
from django.views.generic import TemplateView
from rest_framework.schemas import get_schema_view
@@ -14,7 +14,7 @@ urlpatterns = [
path('api/add', shares_api.AddShare.as_view()),
path('api/history', counting_views.GetHistory.as_view()),
path('api/get/<int:share_id>', shares_api.GetShare.as_view()),
- re_path(r'shares/(?P<share_id>[0-9]+)$', shares_views.view_share),
+ path('shares/<int:share_id>', shares_views.view_share),
path('admin/', admin.site.urls),
path('', shares_views.add_share),
] + static('/static/', document_root='/app/media/')
diff --git a/requirements.txt b/requirements.txt
index f588852..a8782f9 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -9,3 +9,4 @@ djangorestframework
pyyaml
uritemplate
coreapi
+django-redis-cache
diff --git a/shares/api.py b/shares/api.py
index 8eb2d1f..6a383db 100644
--- a/shares/api.py
+++ b/shares/api.py
@@ -1,5 +1,6 @@
-from django.shortcuts import get_object_or_404, redirect
-from django.http import StreamingHttpResponse as SHTTPResponse
+import logging
+from django.shortcuts import get_object_or_404
+from django.http import StreamingHttpResponse as SHTTPResponse, HttpResponseRedirect
from rest_framework import serializers, status
from rest_framework.parsers import JSONParser, FileUploadParser, MultiPartParser
from rest_framework.response import Response
@@ -8,6 +9,7 @@ from satella.instrumentation.metrics import getMetric
from shares.models import Share, hash_password, SHARE_FILE, add_file, add_url
+logger = logging.getLogger(__name__)
files = getMetric('visited.api.files', 'counter')
links = getMetric('visited.api.links', 'counter')
@@ -25,27 +27,29 @@ class ShareSerializer(serializers.Serializer):
class AddShare(APIView):
"""
- Adds a share
+ Adds a share.
- You have to provide either a field called url or upload a file. If you're using
- multipart, name it file.
+ You have to provide either a field called url or upload a file using multipart.
+ You should name it file.
The result will be a JSON object containing two properties: url and password.
The result will be passed using HTTP 201.
You must be authorized to access this endpoint.
"""
- parser_classes = FileUploadParser, MultiPartParser, JSONParser
+ parser_classes = JSONParser, MultiPartParser, FileUploadParser
def get_serializer(self):
return AddShareSerializer()
- def post(self, request, format=None):
+ def put(self, request):
if not request.user.is_authenticated:
return Response({}, status=status.HTTP_401_UNAUTHORIZED)
- if 'file' in request.data:
- dct = add_file(request.data['file'], request, escape_url=False)
- elif 'url' in request.data:
+ if request.FILES.get('file'):
+ dct = add_file(request.FILES['file'], request, escape_url=False)
+ logger.info('Created a file via API')
+ elif request.data.get('url'):
dct = add_url(request.data['url'], request, escape_url=False)
+ logger.info('Created an URL via API')
else:
return Response({}, status=status.HTTP_400_BAD_REQUEST)
@@ -64,15 +68,19 @@ class GetShare(APIView):
You will receive either a 302 Redirect or a 200 OK with the file contents
"""
- def get_serializer(self, *args):
- return GetShareSerializer(*args)
+ def get_serializer(self, **kwargs):
+ return GetShareSerializer(**kwargs)
def post(self, request, share_id: str, format=None):
- serializer = self.get_serializer(request.data)
+ serializer = self.get_serializer(data=request.data)
if not serializer.is_valid():
return Response({}, status=status.HTTP_400_BAD_REQUEST)
share = get_object_or_404(Share, id=share_id)
+
+ if share.expired:
+ return Response({}, status=status.HTTP_404_NOT_FOUND)
+
if hash_password(serializer.data['password']) != share.pwd_hash:
return Response({}, status=status.HTTP_401_UNAUTHORIZED)
@@ -82,7 +90,7 @@ class GetShare(APIView):
return SHTTPResponse(**share.get_kwargs_for_s_http_response())
else:
links.runtime(+1)
- return redirect(share.resource)
+ return HttpResponseRedirect(redirect_to=share.resource)
finally:
share.times_used += 1
share.save()
diff --git a/shares/tests.py b/shares/tests.py
index d27c440..154d1b6 100644
--- a/shares/tests.py
+++ b/shares/tests.py
@@ -1,16 +1,13 @@
-#
-# APOLOGIES IN ADVANCE!!!
-#
-# I realize that I would be best practice to deduplicate some code contained here
-# but since I'm running about 1,5 of full-time equvialents you just have to forgive me
-# for sparing the effort to do so. Thanks "from the mountain!"
-#
+import datetime
from django.contrib.auth.models import User
from django.test import TestCase, Client
from django.test.html import parse_html
+from rest_framework.test import APIClient
from satella.files import write_to_file
+from shares.models import Share
+
def find_element(document, field, value):
if isinstance(document, str):
@@ -34,7 +31,10 @@ class TestShares(TestCase):
self.client = Client() # May be you have missed this line
self.client.force_login(user)
- def test_add_file(self):
+ self.api_client = APIClient()
+ self.api_client.force_login(user)
+
+ def test_add_file_form(self):
"""
Test that a file can be added and that it can be visited
"""
@@ -68,7 +68,46 @@ class TestShares(TestCase):
self.assertEqual(b''.join(resp), FILE_CONTENTS)
self.assertEqual(response.status_code, 200)
- def test_add_url(self):
+ def test_add_url_api_expired(self):
+ response = self.api_client.put('http://127.0.0.1/api/add', {'url': 'http://example.com'},
+ format='json')
+ self.assertEqual(response.status_code, 201)
+ share = Share.objects.get(id=response.json()['url'].rsplit('/', 1)[-1])
+ share.created_on = datetime.datetime.now() - datetime.timedelta(days=2)
+ share.save()
+ response = self.api_client.post(f'http://127.0.0.1/api/get/{share.id}', {'password': response.json()['password']},
+ format='json')
+ self.assertEqual(response.status_code, 404)
+
+ def test_add_url_api(self):
+ response = self.api_client.put('http://127.0.0.1/api/add', {'url': 'http://example.com'},
+ format='json')
+ self.assertEqual(response.status_code, 201)
+
+ id_ = response.json()['url'].rsplit('/', 1)[-1]
+ url = f'http://127.0.0.1/api/get/{id_}'
+
+ response = self.api_client.post(url, {'password': response.json()['password']},
+ format='json')
+ self.assertEqual(response.status_code, 302)
+
+ def test_add_file_api(self):
+ FILE_CONTENTS = b'somedata'
+ write_to_file('test', FILE_CONTENTS)
+ with open('test', 'rb') as f_in:
+ response = self.api_client.put('http://127.0.0.1/api/add', {'file': f_in},
+ format='multipart')
+ self.assertEqual(response.status_code, 201)
+
+ id_ = response.json()['url'].rsplit('/', 1)[-1]
+ url = f'http://127.0.0.1/api/get/{id_}'
+
+ response = self.api_client.post(url, {'password': response.json()['password']},
+ format='json')
+ self.assertEqual(response.status_code, 200)
+ self.assertEqual(b''.join(response.streaming_content), FILE_CONTENTS)
+
+ def test_add_url_form(self):
"""
Test that an URL can be added and that it can be visited
"""
@@ -97,3 +136,26 @@ class TestShares(TestCase):
'csrfmiddlewaretoken': csrf_token
})
self.assertEqual(response.status_code, 302)
+
+ def test_add_url_form_expired(self):
+ response = self.client.get('http://127.0.0.1/accounts/profile')
+
+ self.assertEqual(response.status_code, 200)
+ html = parse_html(response.content.decode('utf-8'))
+ csrf_token = find_element(html, 'name', 'csrfmiddlewaretoken').attributes['value']
+
+ response = self.client.post('http://127.0.0.1/accounts/profile', {
+ 'url': 'https://example.com',
+ 'csrfmiddlewaretoken': csrf_token
+ })
+ self.assertEqual(response.status_code, 200)
+
+ html = parse_html(response.content.decode('utf-8'))
+ link = find_element(html, 'id', 'link_url').children[0].replace('https', 'http')
+
+ share = Share.objects.get(id=link.rsplit('/', 1)[-1])
+ share.created_on = datetime.datetime.now() - datetime.timedelta(days=2)
+ share.save()
+
+ response = self.client.get(link)
+ self.assertEqual(response.status_code, 404)
diff --git a/shares/views.py b/shares/views.py
index 2c28bcd..7c95042 100644
--- a/shares/views.py
+++ b/shares/views.py
@@ -49,11 +49,8 @@ def add_share(request):
if request.method == 'POST':
form = AddShareURLForm(request.POST, request.FILES)
if form.is_valid():
-
- pwd_hash = hash_password(data_added['password'])
data = form.cleaned_data
if data.get('url'):
- # Create a URL resource
data_added = add_url(data['url'], request)
logger.info('Created a link via form')
else:
--
GitLab