From fa6770b8749f92e6f0fa3ed440d7cc5c731e3f02 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 18:32:13 +0200
Subject: [PATCH] wrote all the unit tests + admin panel works

---
 Dockerfile                        |  3 +-
 README.md                         | 34 +++++++----
 agent/admin.py                    |  7 ++-
 agent/migrations/0001_initial.py  |  7 ++-
 agent/models.py                   |  4 ++
 counting/cron.py                  | 74 ++++++++++++++++++++++++
 counting/models.py                |  2 +-
 docker-compose.yml                |  4 +-
 netguru/settings.py               | 17 +++++-
 netguru/urls.py                   | 10 ++--
 requirements.txt                  |  1 +
 shares/admin.py                   | 28 ++++++++-
 shares/cron.py                    | 40 -------------
 shares/migrations/0001_initial.py | 13 +++--
 shares/models.py                  | 77 +++++++++++++++++++++++--
 shares/static/js/clipboard.js     | 53 ++++++++---------
 shares/templates/share/add.html   | 19 ++++--
 shares/templates/share/view.html  | 24 +++++---
 shares/tests.py                   | 39 ++++++++++++-
 shares/views.py                   | 96 ++++++++++++++++---------------
 templates/base.html               |  6 +-
 test.sh                           |  2 +-
 22 files changed, 391 insertions(+), 169 deletions(-)
 create mode 100644 counting/cron.py
 delete mode 100644 shares/cron.py

diff --git a/Dockerfile b/Dockerfile
index 9084a98..d2520ee 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -13,7 +13,8 @@ ADD counting /app/counting
 ADD agent /app/agent
 ADD test.sh /app/test.sh
 RUN python manage.py collectstatic && \
-    chmod ugo+x /app/test.sh
+    chmod ugo+x /app/test.sh && \
+    mkdir -p /data
 
 VOLUME /data
 
diff --git a/README.md b/README.md
index f36d79c..b0cde87 100644
--- a/README.md
+++ b/README.md
@@ -5,18 +5,35 @@ This contains a solution of the Netguru's recruiment task.
 
 # Local development
 
-To unit test locally, just run:
+To install the application, you check it out from the repo and do:
 ```bash
-docker-compose up -d unittest
+docker-compose up --build -d run_local
+docker exec -it netguru_run_local_1 bash
+python manage.py migrate
+python manage.py createsuperuser --username admin --email admin@example.com
 ```
 
-To run locally just run:
+This will load schema for PostgreSQL and create a superuser. 
+Don't worry
+if error messages appear about not being able to bind to the socket 
+- this is perfectly fine and normal. Provide your own password
+and you're good to go!
+
+Just take care that your generated links will contain
+the https schema - you should substitute it with http.
+I did it because you shouldn't expose non-secured HTTP to the world,
+and you should always terminate SSL there (get a free cert from Let's Encrypt
+if you're short on cash).
+
+This will expose both ports 80 (for TCP traffic)
+and ports 81 (for metrics) on localhost.
+
+To unit test locally, just run:
 ```bash
-docker-compose up -d run_local
+docker-compose up -d unittest
 ```
 
-This will expose both ports 80 (for TCP traffic)
-and ports 81 (for metrics).
+# Production
 
 I expect you to terminate SSL at a reverse proxy.
 Please watch out for generated links, as they will
@@ -24,11 +41,6 @@ invariable link to SSL, because I've got no easy way
 to detect if I've been called with SSL already, or is
 it just a reverse proxy handing over the requests.
 
-# Production
-
-## Links
-
-
 ## Deployment
 
 Deployment is handled automatically via a
diff --git a/agent/admin.py b/agent/admin.py
index 8c38f3f..06ae671 100644
--- a/agent/admin.py
+++ b/agent/admin.py
@@ -1,3 +1,8 @@
 from django.contrib import admin
 
-# Register your models here.
+from .models import UserAgentStorage
+
+
+@admin.register(UserAgentStorage)
+class UAAdmin(admin.ModelAdmin):
+    readonly_fields = 'user', 'ua'
diff --git a/agent/migrations/0001_initial.py b/agent/migrations/0001_initial.py
index fd5b309..e7105d1 100644
--- a/agent/migrations/0001_initial.py
+++ b/agent/migrations/0001_initial.py
@@ -6,7 +6,6 @@ import django.db.models.deletion
 
 
 class Migration(migrations.Migration):
-
     initial = True
 
     dependencies = [
@@ -17,9 +16,11 @@ class Migration(migrations.Migration):
         migrations.CreateModel(
             name='UserAgentStorage',
             fields=[
-                ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+                ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False,
+                                           verbose_name='ID')),
                 ('ua', models.TextField(blank=True, null=True, verbose_name='User agent')),
-                ('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL, verbose_name='User')),
+                ('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE,
+                                           to=settings.AUTH_USER_MODEL, verbose_name='User')),
             ],
         ),
     ]
diff --git a/agent/models.py b/agent/models.py
index 2b80242..6174cc9 100644
--- a/agent/models.py
+++ b/agent/models.py
@@ -5,3 +5,7 @@ class UserAgentStorage(models.Model):
     user = models.ForeignKey('auth.User', verbose_name='User', db_index=True,
                              on_delete=models.CASCADE)
     ua = models.TextField(verbose_name='User agent', null=True, blank=True)
+
+    def __str__(self):
+        return f'{self.user} - {self.ua}'
+
diff --git a/counting/cron.py b/counting/cron.py
new file mode 100644
index 0000000..8597d58
--- /dev/null
+++ b/counting/cron.py
@@ -0,0 +1,74 @@
+import enum
+import logging
+
+from datetime import datetime, timedelta
+from django_cron import CronJobBase, Schedule
+from satella.instrumentation.metrics import getMetric
+
+from counting.models import StoryOfADay
+
+from shares.models import Share, SHARE_FILE
+
+logger = logging.getLogger(__name__)
+entries_compiled = getMetric('entries.compiled', 'counter')
+
+
+class DayType(enum.IntEnum):
+    # a day that has already been processed
+    DAY_WITH_HISTORY = 0
+
+    # a day whose links will have fully expired, that is not yet in history
+    DAY_UNPROCESSED_BUT_PROCESSABLE = 1
+
+    # a day for which the statistics need to be calculated in real time
+    DAY_LIVE = 2
+
+
+class ReaperJob(CronJobBase):
+    """
+    Reaper's job is to collect dead links within the database and compile a history out of them.
+    It will delete only links from previous days, only if they have already expired.
+
+    Let's talk a moment about it's logic - days can be divided into one of 3 categories
+    displayed above in DayType
+    """
+    RUN_EVERY_MINS = 24*60  # once each day
+
+    schedule = Schedule(run_every_mins=RUN_EVERY_MINS)
+    code = 'reaper-job'  # a unique code
+
+    def get_day_type(self, date: datetime.date) -> DayType:
+        if date >= datetime.date.today() - datetime.timedelta(days=2):
+            return DayType.DAY_LIVE
+
+        try:
+            StoryOfADay.objects.get(date=date)
+            return DayType.DAY_WITH_HISTORY
+        except StoryOfADay.DoesNotExist:
+            if all(item.expired for item in Share.objects.get_for_day(date)):
+                return DayType.DAY_UNPROCESSED_BUT_PROCESSABLE
+            else:
+                return DayType.DAY_LIVE
+
+    def do(self):
+        try:
+            cur_day = Share.objects.order_by('created_on')[0].creation_date
+        except IndexError:
+            return  # no links to process
+
+        while self.get_day_type(cur_day) != DayType.LIVE:
+            links_visited, files_visited = 0, 0
+            for share in Share.objects.get_for_day(cur_day):
+                if share.times_used:
+                    if share.share_type == SHARE_FILE:
+                        files_visited += 1
+                    else:
+                        links_visited += 1
+                sod = StoryOfADay(day=cur_day, links_visited=links_visited,
+                                  files_visited=files_visited)
+                logger.info('Historic info for %s compiled, %s files visited, %s links visited',
+                            cur_day, files_visited, links_visited)
+                entries_compiled.runtime(+1)
+                sod.save()
+                share.delete()
+            cur_day = cur_day + timedelta(days=1)
diff --git a/counting/models.py b/counting/models.py
index 2a8760c..5266213 100644
--- a/counting/models.py
+++ b/counting/models.py
@@ -2,6 +2,6 @@ from django.db import models
 
 
 class StoryOfADay(models.Model):
-    day = models.DateField(verbose_name='Date')
+    day = models.DateField(verbose_name='Date', primary_key=True)
     links_visited = models.IntegerField(verbose_name='Links visited')
     files_visited = models.IntegerField(verbose_name='Files visited')
diff --git a/docker-compose.yml b/docker-compose.yml
index cc0ad77..e846369 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -12,13 +12,15 @@ services:
     command: "./test.sh"
     build: .
     environment:
-      - DEBUG-1
+      - DEBUG=1
     depends_on:
       - postgres
   run_local:
     build: .
     depends_on:
       - postgres
+    environment:
+      - DEBUG=1
     ports:
       - 80:80
       - 81:81
diff --git a/netguru/settings.py b/netguru/settings.py
index ec3f823..b2ab496 100644
--- a/netguru/settings.py
+++ b/netguru/settings.py
@@ -27,8 +27,21 @@ INSTALLED_APPS = [
     'shares.apps.SharesConfig',
     'agent.apps.AgentConfig',
     'counting.apps.CountingConfig',
+    'django_cron',
+    'rest_framework',
 ]
 
+CRON_CLASSES = [
+    "counting.cron.ReaperJob",
+    # ...
+]
+
+REST_FRAMEWORK = {
+    'DEFAULT_PARSER_CLASSES': [
+        'rest_framework.parsers.JSONParser',
+    ]
+}
+
 MIDDLEWARE = [
     'django_satella_metrics.DjangoSatellaMetricsMiddleware',
     'django_opentracing.OpenTracingMiddleware',
@@ -107,7 +120,7 @@ USE_I18N = True
 
 USE_L10N = True
 
-USE_TZ = True
+USE_TZ = False
 
 # Static files (CSS, JavaScript, Images)
 # https://docs.djangoproject.com/en/3.0/howto/static-files/
@@ -150,5 +163,3 @@ if DEBUG:
     logger.warning('WARNING! You are running in debug mode. Don\'t do it on production!')
 else:
     logging.basicConfig(level=logging.INFO)
-
-
diff --git a/netguru/urls.py b/netguru/urls.py
index 764e36c..ca97135 100644
--- a/netguru/urls.py
+++ b/netguru/urls.py
@@ -5,8 +5,8 @@ from django.urls import path, include, re_path
 from shares import views
 
 urlpatterns = [
-    path('accounts/', include('django.contrib.auth.urls')),
-    path('accounts/profile', views.add_share),
-    re_path(r'shares/(?P<share_id>[0-9]+)$',views.view_share),
-    path('admin/', admin.site.urls),
-] + static('/static/', document_root='/app/media/')
+                  path('accounts/', include('django.contrib.auth.urls')),
+                  path('accounts/profile', views.add_share),
+                  re_path(r'shares/(?P<share_id>[0-9]+)$', views.view_share),
+                  path('admin/', admin.site.urls),
+              ] + static('/static/', document_root='/app/media/')
diff --git a/requirements.txt b/requirements.txt
index 1b1b3ef..e01c6e4 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -5,3 +5,4 @@ django_cron
 satella
 django-satella-metrics
 django_opentracing
+djangorestframework
diff --git a/shares/admin.py b/shares/admin.py
index 8c38f3f..b0e88db 100644
--- a/shares/admin.py
+++ b/shares/admin.py
@@ -1,3 +1,29 @@
+from django import forms
 from django.contrib import admin
 
-# Register your models here.
+from shares.models import Share
+from shares.views import hash_password
+
+
+class ShareForm(forms.ModelForm):
+
+    class Meta:
+        model = Share
+        exclude = ['pwd_hash']
+
+    password = forms.CharField(label='New password', required=False)
+
+    def save(self, *args, **kwargs):
+        v = super().save(*args, **kwargs)
+        if self.cleaned_data.get('password'):
+            self.instance.pwd_hash = hash_password(self.cleaned_data['password'])
+            self.instance.save()
+        return v
+
+
+@admin.register(Share)
+class ShareAdmin(admin.ModelAdmin):
+    form = ShareForm
+    readonly_fields = 'share_type',
+
+
diff --git a/shares/cron.py b/shares/cron.py
deleted file mode 100644
index 6018a96..0000000
--- a/shares/cron.py
+++ /dev/null
@@ -1,40 +0,0 @@
-import logging
-import os
-from datetime import datetime, timedelta
-from django_cron import CronJobBase, Schedule
-from satella.instrumentation.metrics import getMetric
-
-from netguru.settings import FILE_SAVE_LOCATION
-from .models import Share, SHARE_FILE
-
-logger = logging.getLogger(__name__)
-links_deleted = getMetric('deleted.links', 'counter')
-files_deleted = getMetric('deleted.files', 'counter')
-
-
-class ClearExpiredLinks(CronJobBase):
-    # the links might be removed late by 59 minutes, but that shouldn't be a big deal
-    # I could if the handler to check exactly for expiration, but I won't do that.
-    RUN_EVERY_MINS = 60  # every hour
-
-    schedule = Schedule(run_every_mins=RUN_EVERY_MINS)
-    code = 'clear-expired-links'  # a unique code
-
-    def do(self):
-        shares = Share.objects.filter(created_on__lt=datetime.now() - timedelta.days(1))
-        for share in shares:
-            if share.share_type == SHARE_FILE:
-                path = os.path.join(FILE_SAVE_LOCATION, share.resource.split('.', 1)[0])
-                if os.path.exists(path):
-                    # we might have deleted in earlier and this call might have aborted with
-                    # exception, the transaction won't proceed so we might have already deleted this
-                    # file. I realize that EAFP would be more Pythonic, but who cares?
-                    logger.info('File %s deleted', path)
-                    os.unlink(path)
-                    files_deleted.runtime(+1)   # I'm typing +1 on purpose, since it will
-                                                # increment the counter
-                else:
-                    links_deleted.runtime(+1)
-                    logger.info('A link was deleted')
-            share.delete()
-        logger.info('Deleted expired shares')
diff --git a/shares/migrations/0001_initial.py b/shares/migrations/0001_initial.py
index 8f2c551..b661939 100644
--- a/shares/migrations/0001_initial.py
+++ b/shares/migrations/0001_initial.py
@@ -7,7 +7,6 @@ import django.db.models.deletion
 
 
 class Migration(migrations.Migration):
-
     initial = True
 
     dependencies = [
@@ -18,13 +17,17 @@ class Migration(migrations.Migration):
         migrations.CreateModel(
             name='Share',
             fields=[
-                ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
-                ('share_type', models.IntegerField(choices=[(0, 'URL'), (1, 'File')], verbose_name='Share type')),
-                ('created_on', models.DateTimeField(db_index=True, default=datetime.datetime.now, verbose_name='Created on')),
+                ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False,
+                                           verbose_name='ID')),
+                ('share_type',
+                 models.IntegerField(choices=[(0, 'URL'), (1, 'File')], verbose_name='Share type')),
+                ('created_on', models.DateTimeField(db_index=True, default=datetime.datetime.now,
+                                                    verbose_name='Created on')),
                 ('pwd_hash', models.CharField(max_length=64, verbose_name='Hashed password salt')),
                 ('resource', models.TextField(verbose_name='Resource')),
                 ('times_used', models.IntegerField(default=0, verbose_name='Visit count')),
-                ('creator', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL, verbose_name='Creator')),
+                ('creator', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE,
+                                              to=settings.AUTH_USER_MODEL, verbose_name='Creator')),
             ],
         ),
     ]
diff --git a/shares/models.py b/shares/models.py
index d88d3a5..2748ede 100644
--- a/shares/models.py
+++ b/shares/models.py
@@ -1,13 +1,18 @@
+import typing as tp
+import mimetypes
 import os
-from datetime import datetime
+import datetime
 
 from django.db import models
 
-# Create your models here.
 from satella.coding import silence_excs
+from satella.instrumentation.metrics import getMetric
 
 from netguru.settings import FILE_SAVE_LOCATION
 
+files_deleted = getMetric('deleted.files', 'counter')
+links_deleted = getMetric('deleted.links', 'counter')
+
 SHARE_URL = 0
 SHARE_FILE = 1
 
@@ -17,6 +22,12 @@ SHARE_TYPES = [
 ]
 
 
+class ShareManager(models.Manager):
+    def get_for_day(self, date: datetime.date):
+        return super().get_queryset().filter(created_on__ge=date,
+                                             created_on__lt=date+datetime.timedelta(days=1))
+
+
 class Share(models.Model):
     """
     A share is a single shared resource.
@@ -25,13 +36,65 @@ class Share(models.Model):
     """
     creator = models.ForeignKey('auth.User', verbose_name='Creator', on_delete=models.CASCADE)
     share_type = models.IntegerField(choices=SHARE_TYPES, verbose_name='Share type')
-    created_on = models.DateTimeField(default=datetime.now, verbose_name='Created on',
+    created_on = models.DateTimeField(default=datetime.datetime.now,
+                                      verbose_name='Created on',
                                       db_index=True)
     pwd_hash = models.CharField(max_length=64, verbose_name='Hashed password salt')
-    # this is either an URL or a f'{UUID}.real_file_name' in /data
-    resource = models.TextField(verbose_name='Resource')
+    resource = models.TextField(verbose_name='Resource',
+                                help_text='URL if this is an URL. Otherwise it will be '
+                                          'name of the file as it is on filesystem, then a dot,'
+                                          'and then the file name the user submitted it as')
     times_used = models.IntegerField(verbose_name='Visit count', default=0)
 
+    objects = ShareManager()
+
+    def get_absolute_url(self) -> str:
+        return f'/shares/{self.id}'
+
+    def __str__(self):
+        if self.share_type == SHARE_URL:
+            return f'Link to {self.resource} no {self.id}'
+        else:
+            return f'File {self.file_name} no {self.id}'
+
+    @property
+    def creation_date(self) -> datetime.date:
+        return datetime.date(year=self.created_on.year,
+                             month=self.created_on.month,
+                             day=self.created_on.day)
+
+    @property
+    def expired(self) -> bool:
+        """Has the link expired?"""
+        return self.created_on < datetime.datetime.now() - datetime.timedelta(days=1)
+
+    @property
+    def mimetype(self) -> str:
+        """Guessed mimetype or application/octet-stream"""
+        return mimetypes.guess_type(self.file_name)[0] or 'application/octet-stream'
+
+    def get_kwargs_for_s_http_response(self) -> tp.Dict:
+        """
+        Return self as set of kwargs for streaming http response
+        """
+        return {
+            'streaming_content': self.file_iterator,
+            'content_type': self.mimetype,
+            'headers': {
+                'Content-Disposition': f'attachment; filename="{self.file_name}"'
+            }
+        }
+
+    @property
+    def file_iterator(self) -> tp.Iterator[bytes]:
+        """
+        Return a file iterator.
+        Must be a SHARE_FILE
+        """
+        with open(self.path, 'rb') as f_in:
+            while p := f_in.read(1024):
+                yield p
+
     @property
     def file_name(self) -> str:
         """
@@ -61,4 +124,8 @@ class Share(models.Model):
             # besides it's a great showcase of my Satella
             with silence_excs(FileNotFoundError):
                 os.unlink(path)
+            files_deleted.runtime(+1)
+        else:
+            links_deleted.runtime(+1)
         super().delete(*args, **kwargs)
+
diff --git a/shares/static/js/clipboard.js b/shares/static/js/clipboard.js
index 2c14791..e3a82c9 100644
--- a/shares/static/js/clipboard.js
+++ b/shares/static/js/clipboard.js
@@ -1,34 +1,35 @@
 function fallbackCopyTextToClipboard(text) {
-  var textArea = document.createElement("textarea");
-  textArea.value = text;
+    var textArea = document.createElement("textarea");
+    textArea.value = text;
 
-  // Avoid scrolling to bottom
-  textArea.style.top = "0";
-  textArea.style.left = "0";
-  textArea.style.position = "fixed";
+    // Avoid scrolling to bottom
+    textArea.style.top = "0";
+    textArea.style.left = "0";
+    textArea.style.position = "fixed";
 
-  document.body.appendChild(textArea);
-  textArea.focus();
-  textArea.select();
+    document.body.appendChild(textArea);
+    textArea.focus();
+    textArea.select();
 
-  try {
-    var successful = document.execCommand('copy');
-    var msg = successful ? 'successful' : 'unsuccessful';
-    console.log('Fallback: Copying text command was ' + msg);
-  } catch (err) {
-    console.error('Fallback: Oops, unable to copy', err);
-  }
+    try {
+        var successful = document.execCommand('copy');
+        var msg = successful ? 'successful' : 'unsuccessful';
+        console.log('Fallback: Copying text command was ' + msg);
+    } catch (err) {
+        console.error('Fallback: Oops, unable to copy', err);
+    }
 
-  document.body.removeChild(textArea);
+    document.body.removeChild(textArea);
 }
+
 function copyTextToClipboard(text) {
-  if (!navigator.clipboard) {
-    fallbackCopyTextToClipboard(text);
-    return;
-  }
-  navigator.clipboard.writeText(text).then(function() {
-    console.log('Async: Copying to clipboard was successful!');
-  }, function(err) {
-    console.error('Async: Could not copy text: ', err);
-  });
+    if (!navigator.clipboard) {
+        fallbackCopyTextToClipboard(text);
+        return;
+    }
+    navigator.clipboard.writeText(text).then(function () {
+        console.log('Async: Copying to clipboard was successful!');
+    }, function (err) {
+        console.error('Async: Could not copy text: ', err);
+    });
 }
diff --git a/shares/templates/share/add.html b/shares/templates/share/add.html
index ada7993..d9e6d8d 100644
--- a/shares/templates/share/add.html
+++ b/shares/templates/share/add.html
@@ -8,15 +8,22 @@
     {% if added %}
         <div><em>Resource successfully added:</em><br/>
             Your password is <em id="link_password">{{ added.password }}</em>
-            <button onclick="copyTextToClipboard('{{ added.password }}')">Copy the link to clipboard</button><br>
+            <button onclick="copyTextToClipboard('{{ added.password }}')">Copy the password to
+                clipboard
+            </button>
+            <br>
             Your link is: <span><a href="{{ added.url }}" id="link_url">{{ added.url }}</a></span>
-            <button onclick="copyTextToClipboard('{{ added.url }}')">Copy the password to clipboard</button><br>
+            <button onclick="copyTextToClipboard('{{ added.url }}')">Copy the link to
+                clipboard
+            </button>
+            <br>
         </div>
     {% endif %}
-    <form class="form-group text-left" role="form" action="/accounts/profile" method="POST" enctype="multipart/form-data">
-            {% csrf_token %}
+    <form class="form-group text-left" role="form" action="/accounts/profile" method="POST"
+          enctype="multipart/form-data">
+        {% csrf_token %}
         {{ form.as_p }}
-    <p>
-        <input type="submit" value="Submit">
+        <p>
+            <input type="submit" value="Submit">
     </form>
 {% endblock %}
diff --git a/shares/templates/share/view.html b/shares/templates/share/view.html
index 3f03d24..24aabc1 100644
--- a/shares/templates/share/view.html
+++ b/shares/templates/share/view.html
@@ -4,14 +4,20 @@
 {% block head %}
 {% endblock %}
 {% block body %}
-    {% if invalid_password %}
-        <em>The password that you provided was invalid!</em>
-    {% endif %}
-    <form class="form-group text-left" role="form" action="/shares/{{ share_id }}" method="POST">
+    {% if not_found %}
+        <em>The requested link was not found. It might not have ever existed,
+            or it could have expired.</em>
+    {% else %}
+        {% if invalid_password %}
+            <em>The password that you provided was invalid!</em>
+        {% endif %}
+        <form class="form-group text-left" role="form" action="/shares/{{ share_id }}"
+              method="POST">
             {% csrf_token %}
-        {{ form.as_p }}
-    <p>
-        <input type="submit" value="Submit">
-    </p>
-    </form>
+            {{ form.as_p }}
+            <p>
+                <input type="submit" value="Submit">
+            </p>
+        </form>
+    {% endif %}
 {% endblock %}
diff --git a/shares/tests.py b/shares/tests.py
index c526eec..d27c440 100644
--- a/shares/tests.py
+++ b/shares/tests.py
@@ -9,6 +9,7 @@
 from django.contrib.auth.models import User
 from django.test import TestCase, Client
 from django.test.html import parse_html
+from satella.files import write_to_file
 
 
 def find_element(document, field, value):
@@ -33,6 +34,40 @@ class TestShares(TestCase):
         self.client = Client()  # May be you have missed this line
         self.client.force_login(user)
 
+    def test_add_file(self):
+        """
+        Test that a file can be added and that it can be visited
+        """
+        FILE_CONTENTS = b'somedata'
+        write_to_file('test', FILE_CONTENTS)
+        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']
+        with open('test', 'rb') as f_in:
+            response = self.client.post('http://127.0.0.1/accounts/profile', {
+                'file': f_in,
+                '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')
+        password = find_element(html, 'id', 'link_password').children[0]
+
+        response = self.client.get(link)
+        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(link, data={
+            'password': password,
+            'csrfmiddlewaretoken': csrf_token
+        })
+        resp = response.streaming_content
+        self.assertEqual(b''.join(resp), FILE_CONTENTS)
+        self.assertEqual(response.status_code, 200)
+
     def test_add_url(self):
         """
         Test that an URL can be added and that it can be visited
@@ -43,7 +78,7 @@ class TestShares(TestCase):
         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', data={
+        response = self.client.post('http://127.0.0.1/accounts/profile', {
             'url': 'https://example.com',
             'csrfmiddlewaretoken': csrf_token
         })
@@ -57,7 +92,7 @@ class TestShares(TestCase):
         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(link, data={
+        response = self.client.post(link, {
             'password': password,
             'csrfmiddlewaretoken': csrf_token
         })
diff --git a/shares/views.py b/shares/views.py
index f687466..a5ffd8f 100644
--- a/shares/views.py
+++ b/shares/views.py
@@ -1,4 +1,3 @@
-import datetime
 import mimetypes
 import typing as tp
 import os
@@ -10,7 +9,7 @@ from django import forms
 from django.core.validators import URLValidator
 from django.forms import fields
 from django.core.exceptions import ValidationError
-from django.http import Http404, StreamingHttpResponse, HttpResponse
+from django.http import Http404, StreamingHttpResponse as SHTTPResponse
 from django.shortcuts import render, get_object_or_404, redirect
 from django.contrib.auth.decorators import login_required
 from django.utils.safestring import mark_safe
@@ -27,6 +26,11 @@ links_visited = getMetric('visited.links', 'counter')
 logger = logging.getLogger(__name__)
 
 
+#  ==============
+#  Add share part
+#  ==============
+
+
 def hash_password(password: str) -> str:
     return hashlib.sha256(password.encode('utf-8') + SECRET_KEY.encode('utf-8')).hexdigest()
 
@@ -39,14 +43,14 @@ def generate_correct_uuid() -> str:
     # I realize that it's susceptible to race conditions, but then again what is the chance?
     # The chance that UUID will repeat during the 24 hours a link is active is much higher
     # than the chance that it will repeat during the same 2 second window
-    while os.path.exists(FILE_SAVE_LOCATION, f):
+    while os.path.exists(os.path.join(FILE_SAVE_LOCATION, f)):
         f = uuid.uuid4().hex
     return f
 
 
 # Cut off O, I and S so that the user doesn't confuse them with 0, 1 and 5 respectively
 CHARACTERS_TO_USE = string.ascii_uppercase.replace('O', '') \
-                          .replace('I', '').replace('S', '') + string.digits
+                        .replace('I', '').replace('S', '') + string.digits
 
 
 def random_password() -> str:
@@ -96,59 +100,61 @@ def add_share(request):
                               pwd_hash=pwd_hash,
                               share_type=SHARE_FILE)
                 files_created.runtime(+1)
-            data_added['url'] = mark_safe(f'https://{request.get_host()}/shares/{share.id}')
             share.save()
+            data_added['url'] = mark_safe(f'https://{request.get_host()}/shares/{share.id}')
     else:
         form = AddShareURLForm()
 
     return render(request, 'share/add.html', {'form': form, 'added': data_added})
 
 
+#  ===============
+#  View share part
+#  ===============
+
 class PasswordForm(forms.Form):
     password = fields.CharField(label='Password', widget=forms.PasswordInput)
 
 
+class InvalidPassword(Exception):
+    ...
+
+
 def view_share(request, share_id: str):
-    share = get_object_or_404(Share, id=share_id)
 
-    if share.created_on < datetime.datetime.now() - datetime.timedelta(days=1):
-        raise Http404()
+    def render_me(arg, **kwargs):
+        return render(request, 'share/view.html', arg, **kwargs)
 
-    if request.method == 'POST':
-        form = PasswordForm(request.POST)
-        if form.is_valid():
-            pwd = form.cleaned_data['password']
-            if hash_password(pwd) != share.pwd_hash:
-                return HttpResponse(status=401)
-            try:
-                if share.share_type == SHARE_URL:
-                    links_visited.runtime(+1)
-                    return render(request, 'share/view.html', {'form': form, 'share_id': share_id,
-                                                               'invalid_password': True})
-                else:
-                    if not os.path.exists(share.path):
-                        raise Http404()
-
-                    def file_iterator(path: str) -> tp.Iterator[bytes]:
-                        with open(path, 'rb') as f:
-                            p = f.read(1024)
-                            while p:
-                                yield p
-                                p = f.read(1024)
-
-                    # Django will have the UTF-8 mumbo jumbo for us
-                    cd = f'attachment; filename="{share.file_name}"'
-                    mimetype = mimetypes.guess_type(share.file_name)[0] or 'application/octet-stream'
-                    files_visited.runtime(+1)
-                    return StreamingHttpResponse(file_iterator(share.path),
-                                                 content_type=mimetype,
-                                                 headers={
-                                                     'Content-Disposition': cd
-                                                 })
-            finally:
-                share.times_used += 1
-                share.save()
-    else:
-        form = PasswordForm()
+    try:
+        share = get_object_or_404(Share, id=share_id)
+
+        if share.expired:
+            raise Http404()
 
-    return render(request, 'share/view.html', {'form': form, 'share_id': share_id})
+        form = PasswordForm()
+        if request.method == 'POST':
+            form = PasswordForm(request.POST)
+            if form.is_valid():
+                pwd = form.cleaned_data['password']
+                if hash_password(pwd) != share.pwd_hash:
+                    raise InvalidPassword()
+                try:
+                    if share.share_type == SHARE_URL:
+                        links_visited.runtime(+1)
+                        return redirect(share.resource)
+                    else:
+                        if not os.path.exists(share.path):
+                            raise Http404()
+
+                        # Django will have the UTF-8 mumbo jumbo for us
+                        files_visited.runtime(+1)
+                        return SHTTPResponse(**share.get_kwargs_for_s_http_response())
+                finally:
+                    share.times_used += 1
+                    share.save()
+    except Http404:
+        return render_me({'not_found': True, 'share_id': share_id}, status=404)
+    except InvalidPassword:
+        return render_me({'form': form, 'invalid_password': True, 'share_id': share_id}, status=401)
+
+    return render_me({'form': form, 'share_id': share_id})
diff --git a/templates/base.html b/templates/base.html
index e2c255c..1f61742 100644
--- a/templates/base.html
+++ b/templates/base.html
@@ -8,9 +8,9 @@
     <meta name="author" content="Piotr Maślanka">
     <title>{% block title %}A file and URL sharing facility{% endblock %}</title>
     <script
-      src="https://code.jquery.com/jquery-1.12.4.min.js"
-      integrity="sha256-ZosEbRLbNQzLpnKIkEdrPv7lOy9C27hHQ+Xp8a4MxAQ="
-      crossorigin="anonymous"></script>
+            src="https://code.jquery.com/jquery-1.12.4.min.js"
+            integrity="sha256-ZosEbRLbNQzLpnKIkEdrPv7lOy9C27hHQ+Xp8a4MxAQ="
+            crossorigin="anonymous"></script>
     <link href="https://stackpath.bootstrapcdn.com/bootstrap/4.1.3/css/bootstrap.min.css"
           rel="stylesheet">
     <script src="https://stackpath.bootstrapcdn.com/bootstrap/4.1.3/js/bootstrap.min.js"></script>
diff --git a/test.sh b/test.sh
index a84325e..507ad9a 100644
--- a/test.sh
+++ b/test.sh
@@ -1,6 +1,6 @@
 #!/bin/bash
 set -e
 
-sleep 3   # wait for postgres to start up
+sleep 3 # wait for postgres to start up
 python manage.py makemigrations shares
 python manage.py test --no-input
-- 
GitLab