From fa70da031fe1faaa1abcff7aa1d7c4eafe58a4a2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= <sybren@stuvel.eu>
Date: Thu, 26 Jan 2017 17:44:40 +0100
Subject: [PATCH] Server: fixed Last-Modified HTTP header format

---
 .../src/flamenco-manager/flamenco/http.go     |  3 +-
 .../flamenco-manager/flamenco/http_test.go    |  4 +--
 packages/flamenco/flamenco/managers/api.py    | 33 +++++++++++++++++--
 packages/flamenco/tests/test_depsgraph.py     | 11 +++++--
 4 files changed, 44 insertions(+), 7 deletions(-)

diff --git a/packages/flamenco-manager-go/src/flamenco-manager/flamenco/http.go b/packages/flamenco-manager-go/src/flamenco-manager/flamenco/http.go
index 0027177d..4bb63463 100644
--- a/packages/flamenco-manager-go/src/flamenco-manager/flamenco/http.go
+++ b/packages/flamenco-manager-go/src/flamenco-manager/flamenco/http.go
@@ -8,13 +8,14 @@ import (
 	"io/ioutil"
 	"net/http"
 	"net/url"
+	"time"
 
 	log "github.com/Sirupsen/logrus"
 )
 
 // For timestamp parsing
 const IsoFormat = "2006-01-02T15:04:05-0700"
-const LastModifiedHeaderFormat = "2006-01-02 15:04:05-07:00"
+const LastModifiedHeaderFormat = time.RFC1123
 
 /**
  * Decodes JSON and writes a Bad Request status if it fails.
diff --git a/packages/flamenco-manager-go/src/flamenco-manager/flamenco/http_test.go b/packages/flamenco-manager-go/src/flamenco-manager/flamenco/http_test.go
index b16bb188..94792456 100644
--- a/packages/flamenco-manager-go/src/flamenco-manager/flamenco/http_test.go
+++ b/packages/flamenco-manager-go/src/flamenco-manager/flamenco/http_test.go
@@ -14,8 +14,8 @@ var _ = check.Suite(&HttpTestSuite{})
 
 func (s *HttpTestSuite) TestParseDates(c *check.C) {
 	parsed_iso, err1 := time.Parse(IsoFormat, "2017-01-23T13:04:05+0200")
-	parsed_ms, err2 := time.Parse(LastModifiedHeaderFormat, "2017-01-23 13:04:05+02:00")
+	parsed_http, err2 := time.Parse(LastModifiedHeaderFormat, "Mon, 23 Jan 2017 13:04:05 CEST")
 	assert.Nil(c, err1)
 	assert.Nil(c, err2)
-	assert.Equal(c, parsed_iso, parsed_ms)
+	assert.Equal(c, parsed_iso, parsed_http)
 }
diff --git a/packages/flamenco/flamenco/managers/api.py b/packages/flamenco/flamenco/managers/api.py
index 0b4a301a..1644d0b4 100644
--- a/packages/flamenco/flamenco/managers/api.py
+++ b/packages/flamenco/flamenco/managers/api.py
@@ -229,10 +229,10 @@ def get_depsgraph(manager_id, request_json):
     that have been modified since that timestamp.
     """
 
+    import dateutil.parser
     from pillar.api.utils import jsonify, bsonify
     from flamenco import current_flamenco
     from flamenco.utils import report_duration
-    import dateutil.parser
 
     modified_since = request.headers.get('If-Modified-Since')
 
@@ -264,6 +264,7 @@ def get_depsgraph(manager_id, request_json):
             modified_since = dateutil.parser.parse(modified_since)
             task_query['_updated'] = {'$gt': modified_since}
             task_query['status'] = {'$in': DEPSGRAPH_MODIFIED_SINCE_TASK_STATUSES}
+            log.debug('Querying all tasks changed since %s', modified_since)
 
         cursor = tasks_coll.find(task_query)
         depsgraph = list(cursor)
@@ -294,9 +295,37 @@ def get_depsgraph(manager_id, request_json):
 
     if depsgraph:
         last_modification = max(task['_updated'] for task in depsgraph)
-        resp.headers['Last-Modified'] = last_modification
+        log.debug('Last modification was %s', last_modification)
+        resp.headers['Last-Modified'] = format_http_date(last_modification)
     return resp
 
 
+def format_http_date(last_modification):
+    """Format the given timestamp into RFC 1123 format.
+
+    :param last_modification: datetime in UTC timezone
+    :type last_modification: datetime.datetime
+    """
+
+    import time
+    import email.utils
+
+    # This incorrectly represents 'stamp' in local time, instead of the UTC we get
+    # from the database.
+    stamp = time.mktime(last_modification.timetuple())
+
+    # Subtract the UTC to local time offset
+    timediff = time.mktime(time.gmtime(0))
+    stamp -= timediff
+
+    http_date = email.utils.formatdate(
+        timeval=stamp,
+        localtime=False,
+        usegmt=True
+    )  # --> Wed, 22 Oct 2008 10:55:46 GMT
+
+    return http_date
+
+
 def setup_app(app):
     app.register_api_blueprint(api_blueprint, url_prefix='/flamenco/managers')
diff --git a/packages/flamenco/tests/test_depsgraph.py b/packages/flamenco/tests/test_depsgraph.py
index c4b5ee3e..7a08db3e 100644
--- a/packages/flamenco/tests/test_depsgraph.py
+++ b/packages/flamenco/tests/test_depsgraph.py
@@ -95,7 +95,7 @@ class DepsgraphTest(AbstractFlamencoTest):
         last_modified = parse(resp.headers['Last-Modified'])
         with self.app.test_request_context():
             task0 = self.flamenco.db('tasks').find_one({'_id': self.task_ids[0]})
-        self.assertEqual(task0['_updated'], last_modified)
+        self.assert_equal_to_second(task0['_updated'], last_modified)
 
         # The tasks in the database, as well as the response, should be set to claimed-by-manager
         with self.app.test_request_context():
@@ -163,9 +163,16 @@ class DepsgraphTest(AbstractFlamencoTest):
         with self.app.test_request_context():
             task0 = self.flamenco.db('tasks').find_one({'_id': self.task_ids[0]})
             task2 = self.flamenco.db('tasks').find_one({'_id': self.task_ids[2]})
-        self.assertEqual(task2['_updated'], last_modified)
+        # They should be equal to second precision
+        self.assert_equal_to_second(task2['_updated'], last_modified)
 
         self.assertEqual(task0['status'], u'claimed-by-manager')
         self.assertEqual(task2['status'], u'claimed-by-manager')
         self.assertEqual(2 * [u'claimed-by-manager'],
                          [task['status'] for task in depsgraph])
+
+    def assert_equal_to_second(self, actual, expected):
+        import datetime
+
+        diff = datetime.timedelta(microseconds=actual.microsecond)
+        self.assertEqual(actual - diff, expected)
-- 
GitLab