Bug 1287501 - Fix /jobs/ endpoint to preserve `last_modified` param (#1704)

A prior commit removed the ability to use "-Infinity" for the last_modified
query param.  However, the fix accidentally stripped the param entirely.
This change ensures that the value is a valid date string.  The range is
not limited.

This also adds some new tests to ensure the param of `last_modified` is
working correctly when included.

tests/jobs_test.json

@@ -73,16 +73,27 @@
 
                 "host_type":"master_host"
         },
-        "set_jobs_last_modified":{
+        "set_jobs_submit_timestamp": {
             "sql":"UPDATE `job` SET `submit_timestamp` = ?",
 
             "host_type":"master_host"
         },
-        "set_one_job_last_modified_timestamp":{
+        "set_one_job_submit_timestamp": {
             "sql":"UPDATE `job` SET `submit_timestamp` = ? WHERE id = 1",
 
             "host_type":"master_host"
         },
+        "set_jobs_last_modified": {
+            "sql":"UPDATE `job` SET `last_modified` = ?
+                   WHERE id in (REP0)",
+
+            "host_type":"master_host"
+        },
+        "set_one_job_last_modified": {
+            "sql":"UPDATE `job` SET `last_modified` = ? WHERE id = ?",
+
+            "host_type":"master_host"
+        },
         "set_job_result":{
             "sql":"UPDATE `job` SET `result` = ? WHERE id = ?",
 

tests/model/derived/test_jobs_model.py

@@ -423,7 +423,7 @@ def test_cycle_all_data(jm, sample_data,
     cycle_date_ts = time_now - 7 * 24 * 3600
 
     jm.execute(
-        proc="jobs_test.updates.set_jobs_last_modified",
+        proc="jobs_test.updates.set_jobs_submit_timestamp",
         placeholders=[cycle_date_ts]
     )
 
@@ -482,12 +482,12 @@ def test_cycle_one_job(jm, sample_data,
     cycle_date_ts = int(time_now - 7 * 24 * 3600)
 
     jm.execute(
-        proc="jobs_test.updates.set_jobs_last_modified",
+        proc="jobs_test.updates.set_jobs_submit_timestamp",
         placeholders=[time_now]
     )
 
     jm.execute(
-        proc="jobs_test.updates.set_one_job_last_modified_timestamp",
+        proc="jobs_test.updates.set_one_job_submit_timestamp",
         placeholders=[cycle_date_ts]
     )
 
@@ -542,7 +542,7 @@ def test_cycle_all_data_in_chunks(jm, sample_data,
     cycle_date_ts = int(time_now - 7 * 24 * 3600)
 
     jm.execute(
-        proc="jobs_test.updates.set_jobs_last_modified",
+        proc="jobs_test.updates.set_jobs_submit_timestamp",
         placeholders=[cycle_date_ts]
     )
 

tests/webapp/api/test_jobs_api.py

@@ -1,5 +1,9 @@
+import datetime
+
 import pytest
+from dateutil import parser
 from django.core.urlresolvers import reverse
+from rest_framework.status import HTTP_400_BAD_REQUEST
 from rest_framework.test import APIClient
 
 from treeherder.model.models import (ExclusionProfile,
@@ -388,3 +392,36 @@ def test_job_create(webapp, test_repository, test_user, eleven_job_blobs, monkey
     assert resp.status_code == 200
     test_job_list(webapp, None, test_repository)
     test_job_detail(webapp, None, None, jm)
+
+
+@pytest.mark.parametrize('lm_key,lm_value,exp_status, exp_job_count', [
+    ("last_modified__gt",  "2016-07-18T22:16:58.000", 200, 8),
+    ("last_modified__lt",  "2016-07-18T22:16:58.000", 200, 3),
+    ("last_modified__gt",  "-Infinity", HTTP_400_BAD_REQUEST, 0),
+    ("last_modified__gt",  "whatever", HTTP_400_BAD_REQUEST, 0),
+    ])
+def test_last_modified(webapp, jm, eleven_jobs_stored, test_project,
+                       lm_key, lm_value, exp_status, exp_job_count):
+    try:
+        param_date = parser.parse(lm_value)
+        newer_date = param_date - datetime.timedelta(minutes=10)
+
+        jobs = jm.get_job_list(0, 11)
+        gt_ids = [str(x["id"]) for x in jobs[:3]]
+        # modify job last_modified
+        jm.execute(
+            proc="jobs_test.updates.set_jobs_last_modified",
+            placeholders=[str(newer_date)],
+            replace=[",".join(gt_ids)]
+        )
+    except ValueError:
+        # no problem.  these params are the wrong
+        pass
+
+    url = reverse("jobs-list", kwargs={"project": test_project})
+    final_url = url + ("?{}={}".format(lm_key, lm_value))
+
+    resp = webapp.get(final_url, expect_errors=(exp_status != 200))
+    assert resp.status_int == exp_status
+    if exp_status == 200:
+        assert len(resp.json["results"]) == exp_job_count

treeherder/webapp/api/jobs.py

@@ -1,5 +1,3 @@
-import datetime
-
 import django_filters
 from dateutil import parser
 from rest_framework import (filters,
@@ -94,27 +92,23 @@ class JobsViewSet(viewsets.ViewSet):
         - return_type (dict)
         """
         MAX_JOBS_COUNT = 2000
-        LAST_MODIFIED_WINDOW = 30  # minutes
 
         filter = UrlQueryFilter(request.query_params)
 
         offset = int(filter.pop("offset", 0))
         count = int(filter.pop("count", 10))
+
         if "last_modified" in filter.conditions:
-            datestr = filter.get("last_modified")[1]
+            # could be more than one, this is a set
-            try:
+            for lm in filter.conditions["last_modified"]:
-                last_modified = parser.parse(datestr)
+                datestr = lm[1]
-                if last_modified < datetime.datetime.utcnow() - \
+                try:
-                   datetime.timedelta(minutes=LAST_MODIFIED_WINDOW):
+                    # ensure last_modified is a date
+                    parser.parse(datestr)
+                except ValueError:
                     return Response(
-                        "`last_modified` of {} is not within last {} minutes".format(
+                        "Invalid date value for `last_modified`: {}".format(datestr),
-                            datestr,
-                            LAST_MODIFIED_WINDOW),
                         status=HTTP_400_BAD_REQUEST)
-            except ValueError:
-                return Response(
-                    "Invalid value for `last_modified`: ".format(datestr),
-                    status=HTTP_400_BAD_REQUEST)
 
         if count > MAX_JOBS_COUNT:
             msg = "Specified count exceeds API MAX_JOBS_COUNT value: {}".format(MAX_JOBS_COUNT)

treeherder/webapp/api/utils.py

@@ -67,7 +67,7 @@ class UrlQueryFilter(object):
         if key in self.conditions:
             value = self.conditions[key]
             if len(value) == 1:
-                value = value.pop()
+                value = next(iter(value))
                 if value[0] == "=":
                     value = value[1]
             return value