Merge lp:~cprov/uci-engine/webui-auto-refresh into lp:uci-engine

[Celso Providelo]

Fixing the WebUI auto-refresh feature and preventing tempurls from GK to get cached.

Fixing the WebUI auto-refresh feature and preventing tempurls from GK to get cached.

This branch adds the 'Cache-Control: max-age=0, no-cache' header to the GK response container fresh tempurls for all ticket artifacts. This way it won't be cached in clients browsers and every time the webui refreshes they have access to valid tempurls.

I've completely refactored the the auto-refresh feature in webui.js, so it uses a much simpler and reliable mechanism for periodic tasks (Y.later instead of SetTimeout) which helped to simplify the related code a lot.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:623
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/949/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/949/rebuild

review: Approve (continuous-integration)

[Evan (ev)]

This looks good, but I'm confused as to why you kept the UI for configuring auto-refresh. I can't imagine a case where we'd want to disable that or tweak the interval. It was good for when the feature first landed, but we've been running with it for a while now and it's just cluttering up the interface.

review: Needs Information

[Celso Providelo (cprov)]

"auto-refresh" section completely removed from the UI now.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:625
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/953/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/953/rebuild

review: Approve (continuous-integration)

[Evan (ev)]

Looks good.

review: Approve

[Ubuntu CI Bot (uci-bot)]

There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions.

[Ursula Junque (ursinha)]

Looks good to me. Thanks for fixing this!

review: Approve

=== modified file 'gatekeeper/gatekeeper/resources/tests/test_api.py'
--- gatekeeper/gatekeeper/resources/tests/test_api.py	2014-06-16 13:18:53 +0000
+++ gatekeeper/gatekeeper/resources/tests/test_api.py	2014-06-27 13:46:23 +0000
@@ -148,9 +148,12 @@
 
     def test_get_returns_the_list_of_artifacts(self):
         # GET on a ticket URL returns a list to tempurls for its
-        # artifacts as a dictionary (JSON object).
+        # artifacts as a dictionary (JSON object). Since tempurls
+        # expires, this page should not be cached in browsers.
         resp = self.app.get('/api/v1/ticket/1')
         self.assertEqual(200, resp.status_code)
+        self.assertEqual(
+            'max-age=0, no-cache', str(resp.cache_control))
         self.assertEqual('application/json', resp.content_type)
         self.assertEqual(
             {'A_ARTIFACT': 'http://ARTIFACT_TEMPURL'},

=== modified file 'gatekeeper/gatekeeper/resources/v1.py'
--- gatekeeper/gatekeeper/resources/v1.py	2014-06-12 13:26:06 +0000
+++ gatekeeper/gatekeeper/resources/v1.py	2014-06-27 13:46:23 +0000
@@ -143,7 +143,11 @@
             payload = b'{}({})\n'.format(callback, payload)
         else:
             payload = b'{}\n'.format(payload)
-        return http.ok([(b'Content-Type', b'application/json')], payload)
+        headers = [
+            (b'Cache-Control', 'max-age=0, no-cache'),
+            (b'Content-Type', b'application/json')
+        ]
+        return http.ok(headers, payload)
 
     @resource.child('sandbox/{sandbox_identifier}')
     def sandbox_wrapper(self, request, segments, sandbox_identifier):

=== modified file 'webui/tickets/static/tickets/webui.js'
--- webui/tickets/static/tickets/webui.js	2014-06-26 15:08:17 +0000
+++ webui/tickets/static/tickets/webui.js	2014-06-27 13:46:23 +0000
@@ -1,20 +1,24 @@
 "use strict";
 
 // Make jslint happy
-var window;
 var YUI;
 
 YUI.add('webui', function (Y) {
-    var json_error_msg = "The ticket system returned data that could not be understood. " +
-                         "Please try refreshing, or contact support if this problem persists.",
-        refresh_msg = "Refreshing data...";
-    var default_refresh = 15;
-    var static_prefix = "/static/common/";
+    var json_error_msg = (
+        "The ticket system returned data that could not be understood. " +
+        "Please try refreshing, or contact support if this problem " +
+        "persists."),
+        refresh_msg = "Refreshing data...",
+        default_refresh = 60,
+        static_prefix = "/static/common/";
     Y.webui = {
         url_prefix: "/api/v1/",
-        refresh_interval: (parseInt(Y.one('#refresh_rate'), 10) || default_refresh) * 1000,
+        refresh_interval: default_refresh * 1000,
+        refresh_timer: null,
         done_loading: false,
-        log_prefixes: Y.Array(['package_build', 'image_build', 'image_test', 'package_publish']),
+        log_prefixes: Y.Array(
+            ['package_build', 'image_build', 'image_test',
+             'package_publish']),
         log_step_map: {
             "Image building": "image_build",
             "Image testing": "image_test",
@@ -45,54 +49,22 @@
 
             Y.webui.logs[key].push(log_data);
         },
-        add_refresh_div: function (callback) {
-            var refresh,
-                refresh_rate,
-                refresh_enabled,
-                onChange;
-
-            refresh = Y.Node.create('<div id="refresh"></div>');
-
-            refresh.setHTML('<small>Automatically refresh <input type="checkbox" checked="true"' +
-                'id="refresh_enabled"></input> every <input type="text"' +
-                'id="refresh_rate" value="' + default_refresh + '" maxlength="3" size="2"/> seconds.</small>'
-                );
-
-            Y.one('div#main-content').append(refresh);
-
-            Y.webui.refresh_timeout = null;
-            refresh_rate = Y.one("#refresh_rate");
-            refresh_enabled = Y.one("#refresh_enabled");
-            Y.webui.refresh_enabled = refresh_enabled.get('checked');
-
-            onChange = function () {
-                var value = refresh_rate.get('value'),
-                    val;
-
-                val = parseInt(value, 10);
-
-                if (Y.webui.refresh_timeout) {
-                    clearTimeout(Y.webui.refresh_timeout);
-                }
-                if (val !== 0) {
-                    Y.webui.refresh_interval = val * 1000;
-                    callback();
-                }
-            };
-            refresh_rate.on('change', onChange);
-            refresh_enabled.on('change', function () {
-                if (Y.webui.refresh_timeout) {
-                    clearTimeout(Y.webui.refresh_timeout);
-                }
-
-                Y.webui.refresh_enabled = refresh_enabled.get('checked');
-
-                if (Y.webui.refresh_enabled) {
-                    callback();
-                }
-            });
-            onChange();
+
+        /**
+         * Setup page auto-refresh.
+         *
+         * Immediately runs the callback when auto-refresh is set up
+         * (page is loaded) then use Y.later() for periodically calling
+         * the 'callback' at the `Y.webui.refresh_interval` rate.
+         *
+         * @method setup_auto_refresh
+         */
+        setup_auto_refresh: function (callback) {
+            callback();
+            Y.webui.refresh_timer = Y.later(
+                Y.webui.refresh_interval, Y, callback, '', true);
         },
+
         get_workflow_data: function (api_url) {
             var wf_data = {},
                 wf_str = {},
@@ -349,7 +321,7 @@
         },
         artifact_link: function (data, new_name) {
             var name = Y.webui.basename(data.name),
-                link = Y.Node.create("<a/>");
+                link = Y.Node.create('<a/>');
 
             // Lookup a corresponding tempurl.
             var tempurl = Y.webui.tempurls[name];
@@ -671,34 +643,37 @@
                 }
             }
         },
+
+        /**
+         * Returns the ticket ID based on the window URL.
+         *
+         * Checks the known ticket page path (/ticket/<id>), then match
+         * a valid integer ID.
+         *
+         * @method get_ticket_id
+         */
         get_ticket_id: function () {
-            var query = window.location.search.split('?')[1],
-                ticket_id = null,
-                ob;
-
-            if (query) {
-                ob = Y.QueryString.parse(query);
-                ticket_id = ob.ticket_id;
-            }
-
-            return ticket_id;
+            var path = Y.config.win.location.pathname;
+            if (!path.match(/\/ticket\/\d+\//g)) {
+                return null;
+            }
+            var id_match = path.match(/\d+/g);
+            if (!id_match) {
+                return null;
+            }
+            return id_match[0];
         },
+
+        /**
+         * Updates ticket information.
+         *
+         * @method update_ticket
+         */
         update_ticket: function () {
             var ticket_id = Y.webui.get_ticket_id();
-
             if (ticket_id) {
                 Y.webui.get_ticket(ticket_id, Y.one('#container'));
             }
-
-            // we're done so allow more refreshes if needed.
-            if (Y.webui.refresh_timeout) {
-                clearTimeout(Y.webui.refresh_timeout);
-                Y.webui.refresh_timeout = null;
-            }
-
-            if (Y.webui.refresh_enabled && !Y.webui.refresh_timeout && Y.webui.refresh_interval > 0) {
-                Y.webui.refresh_timeout = setTimeout(Y.webui.update_ticket, Y.webui.refresh_interval);
-            }
         },
         has_ticket: function (ticket_ids, id) {
             return ticket_ids[id] === true;
@@ -812,31 +787,9 @@
 
                             Y.webui.main_table.set('data', recordset);
                             Y.webui.main_table.render();
-
-                            // we're done so allow more refreshes if needed.
-                            if (Y.webui.refresh_timeout) {
-                                clearTimeout(Y.webui.refresh_timeout);
-                                Y.webui.refresh_timeout = null;
-                            }
-                            if (Y.webui.refresh_enabled && !Y.webui.refresh_timeout && Y.webui.refresh_interval > 0) {
-                                Y.webui.refresh_timeout = setTimeout(status_callback, Y.webui.refresh_interval);
-                            }
                         },
-                        complete: function () {
-                            if (Y.webui.refresh_enabled && !Y.webui.refresh_timeout && Y.webui.refresh_interval > 0) {
-                                Y.webui.refresh_timeout = setTimeout(status_callback, Y.webui.refresh_interval);
-                            }
-                        }
                     }
                 });
-            } else {
-                if (Y.webui.refresh_timeout) {
-                    clearTimeout(Y.webui.refresh_timeout);
-                    Y.webui.refresh_timeout = null;
-                }
-                if (Y.webui.refresh_enabled && !Y.webui.refresh_timeout && Y.webui.refresh_interval > 0) {
-                    Y.webui.refresh_timeout = setTimeout(status_callback, Y.webui.refresh_interval);
-                }
             }
         },
         add_item: function (item) {

=== modified file 'webui/tickets/static/tickets/webui_complete.js'
--- webui/tickets/static/tickets/webui_complete.js	2014-05-20 14:47:23 +0000
+++ webui/tickets/static/tickets/webui_complete.js	2014-06-27 13:46:23 +0000
@@ -1,7 +1,7 @@
-YUI().use('io', 'jsonp', 'webui', 'datatable-mutable', function(Y) {
+YUI().use( 'datatable-mutable', 'io', 'jsonp', 'webui', function(Y) {
 
   Y.webui.make_main_table('#table');
 
   Y.webui.get_tickets("complete");
-  Y.webui.add_refresh_div(Y.webui.check_complete_statuses);
+  Y.webui.setup_auto_refresh(Y.webui.check_complete_statuses);
 });

=== modified file 'webui/tickets/static/tickets/webui_failed.js'
--- webui/tickets/static/tickets/webui_failed.js	2014-05-20 14:47:23 +0000
+++ webui/tickets/static/tickets/webui_failed.js	2014-06-27 13:46:23 +0000
@@ -1,7 +1,7 @@
-YUI().use('io', 'jsonp', 'webui', 'datatable-mutable', function(Y) {
+YUI().use('datatable-mutable', 'io', 'jsonp', 'webui', function(Y) {
 
   Y.webui.make_main_table('#table');
 
   Y.webui.get_tickets("failed");
-  Y.webui.add_refresh_div(Y.webui.check_failed_statuses);
+  Y.webui.setup_auto_refresh(Y.webui.check_failed_statuses);
 });

=== modified file 'webui/tickets/static/tickets/webui_ticket_detail.js'
--- webui/tickets/static/tickets/webui_ticket_detail.js	2014-05-19 12:40:40 +0000
+++ webui/tickets/static/tickets/webui_ticket_detail.js	2014-06-27 13:46:23 +0000
@@ -1,10 +1,3 @@
-YUI().use('io', 'json-parse', 'querystring', 'node', 'webui', function(Y) {
-
-  var ticket_id = parseInt(Y.one('#ticket_id').getHTML(), 10);
-
-  if (ticket_id) {
-    var ticket = Y.webui.get_ticket(ticket_id, Y.one('#container'));
-  }
-
-  Y.webui.add_refresh_div(Y.webui.update_ticket);
+YUI().use('io', 'json-parse', 'node', 'webui', function(Y) {
+  Y.webui.setup_auto_refresh(Y.webui.update_ticket);
 });

=== modified file 'webui/tickets/static/tickets/webui_ticket_list.js'
--- webui/tickets/static/tickets/webui_ticket_list.js	2014-05-20 14:47:23 +0000
+++ webui/tickets/static/tickets/webui_ticket_list.js	2014-06-27 13:46:23 +0000
@@ -1,7 +1,7 @@
-YUI().use('event', 'node', 'datatable-mutable', 'webui', function(Y) {
+YUI().use('datatable-mutable', 'event', 'node', 'webui', function(Y) {
 
   Y.webui.make_main_table('#table');
 
   Y.webui.get_tickets("open");
-  Y.webui.add_refresh_div(Y.webui.check_open_statuses);
+  Y.webui.setup_auto_refresh(Y.webui.check_open_statuses);
 });