Merge pull request #308 from tadhg-ohiggins/tadhg-api

Set of small tweaks from #307

regparser/web/jobs/utils.py

@@ -107,9 +107,9 @@ def get_host():
     """
     hostname = getattr(settings, "CANONICAL_HOSTNAME", "")
     hostport = getattr(settings, "CANONICAL_PORT", "")
-    if hostport and hostport is not "80":
+    if hostport and hostport not in ("80", "443"):
         hostport = ":%s" % hostport
-    elif hostport is "80":
+    elif hostport in ("80", "443"):
         hostport = ""
     return "%s%s" % (hostname, hostport)
 

regparser/web/jobs/views.py

@@ -206,8 +206,10 @@ def build_eregs_args(self, validated_data):
         Overrides the method from ``BaseViewList`` in order to pass the
         arguments appropriate for the ``pipeline`` command.
 
-        Side Effects
-            Runs the ``pipeline`` command.
+        It returns a list of string components that can be passed to the
+        `eregs.py` task runner. For example::
+
+            ["pipeline", "0", "0", "http://some.url/"]
 
         :arg dict validated_data: Incoming data from the POST that's already
         been validated by the serializer.
@@ -242,11 +244,14 @@ def build_eregs_args(self, validated_data):
         Overrides the method from ``BaseViewList`` in order to pass the
         arguments appropriate for the ``proposal_pipeline`` command.
 
+        It returns a list of string components that can be passed to the
+        `eregs.py` task runner. For example::
+
+            ["proposal_pipeline", "/tmp/tmp.xml", "http://some.url/"]
+
         Impure
             Reads the contents of the proposal file from the filesystem (in
-            future, the DB, but impure either way).
-        Side Effects
-            Runs the ``proposal_pipeline`` command.
+            future, likely some other file storage, but impure either way).
 
         :arg dict validated_data: Incoming data from the POST that's already
         been validated by the serializer.
@@ -317,16 +322,15 @@ def create(self, request, *args, **kwargs):
         filename = uploaded_file.name
         url = file_url(hexhash)
 
-        if RegulationFile.objects.filter(hexhash=hexhash).exists():
-            return Response(dict(error="File already present."),
-                            status=status.HTTP_400_BAD_REQUEST)
-        else:
+        if not RegulationFile.objects.filter(hexhash=hexhash).exists():
             serialized.save(contents=contents, file=uploaded_file,
                             filename=filename, hexhash=hexhash, url=url)
-
-        headers = self.get_success_headers(serialized.data)
-        return Response(serialized.data, status=status.HTTP_201_CREATED,
-                        headers=headers)
+            headers = self.get_success_headers(serialized.data)
+            return Response(serialized.data, status=status.HTTP_201_CREATED,
+                            headers=headers)
+        else:
+            return Response(dict(error="File already present."),
+                            status=status.HTTP_400_BAD_REQUEST)
 
     def post(self, request, *args, **kwargs):
         return self.create(request, *args, **kwargs)

tests/test_web_api.py

@@ -1,6 +1,7 @@
 from hashlib import md5
 from mock import patch, Mock
 from os import path as ospath
+from six.moves.urllib.parse import urlparse
 from random import choice
 from regparser.web.jobs.models import job_status_values
 from regparser.web.jobs.utils import (
@@ -17,22 +18,16 @@
 import pytest
 import settings
 
-try:
-    from urllib.parse import urlparse
-except ImportError:
-    from urlparse import urlparse
 
 fake_pipeline_id = uuid4()
 
 
 def _fake_redis_job(cmd, args, timeout=60*30, result_ttl=-1, depends_on=None):
-    return type("FakeRedisJob", (object, ), {"id": fake_pipeline_id})
+    return Mock(id=fake_pipeline_id)
 
 
 def _fake_redis_queue():
-    fq = Mock()
-    fq.fetch_job = Mock(return_value=None)
-    return fq
+    return Mock(fetch_job=Mock(return_value=None))
 
 
 @patch("django_rq.enqueue", _fake_redis_job)
@@ -148,11 +143,8 @@ def __init__(self, *args, **kwargs):
 
     def get_hashed_contents(self):
         if self.hashed_contents is None:
-            try:
-                self.hashed_contents = md5(self.file_contents.encode(
-                    "utf-8")).hexdigest()
-            except AttributeError:
-                self.hashed_contents = md5(self.file_contents).hexdigest()
+            self.hashed_contents = md5(self.file_contents.encode(
+                "utf-8")).hexdigest()
         return self.hashed_contents
 
     def test_create_file(self):
@@ -337,11 +329,9 @@ def _check(port=None):
     with patch.object(settings, "CANONICAL_PORT", "2323"):
         _check(port=2323)
 
-    with patch.object(settings, "CANONICAL_PORT", "80"):
-        _check()
-
-    with patch.object(settings, "CANONICAL_PORT", ""):
-        _check()
+    for port in ("80", "443", ""):
+        with patch.object(settings, "CANONICAL_PORT", port):
+            _check()
 
     with pytest.raises(ValueError) as err:
         status_url("something", "something-without-a-slash")
@@ -359,10 +349,7 @@ def test_file_url():
         for hx in hexes:
             assert file_url(hx) == "%s:2323%s%s/" % (domain, urlpath, hx)
 
-    with patch.object(settings, "CANONICAL_PORT", "80"):
-        for hx in hexes:
-            assert file_url(hx) == "%s%s%s/" % (domain, urlpath, hx)
-
-    with patch.object(settings, "CANONICAL_PORT", ""):
-        for hx in hexes:
-            assert file_url(hx) == "%s%s%s/" % (domain, urlpath, hx)
+    for port in ("80", "443", ""):
+        with patch.object(settings, "CANONICAL_PORT", port):
+            for hx in hexes:
+                assert file_url(hx) == "%s%s%s/" % (domain, urlpath, hx)