Merge pull request #365 from EverythingMe/fix/mget_error

Fix: when no queries are being run, cleanup job fails with error

rd_ui/app/index.html

@@ -126,6 +126,7 @@
 <script src="/scripts/ui-bootstrap-tpls-0.5.0.min.js"></script>
 <script src="/bower_components/bucky/bucky.js"></script>
 <script src="/bower_components/pace/pace.js"></script>
+<script src="/bower_components/mustache/mustache.js"></script>
 <!-- endbuild -->
 
 <!-- build:js({.tmp,app}) /scripts/scripts.js -->

rd_ui/app/scripts/controllers/dashboard.js

@@ -16,7 +16,7 @@
             var w = new Widget(widget);
 
             if (w.visualization) {
-              promises.push(w.getQuery().getQueryResultPromise());
+              promises.push(w.getQuery().getQueryResult().toPromise());
             }
 
             return w;
@@ -104,7 +104,7 @@
     };
   };
 
-  var WidgetCtrl = function($scope, Events, Query) {
+  var WidgetCtrl = function($scope, $location, Events, Query) {
     $scope.deleteWidget = function() {
       if (!confirm('Are you sure you want to remove "' + $scope.widget.getName() + '" from the dashboard?')) {
         return;
@@ -128,7 +128,9 @@
       Events.record(currentUser, "view", "visualization", $scope.widget.visualization.id);
 
       $scope.query = $scope.widget.getQuery();
-      $scope.queryResult = $scope.query.getQueryResult();
+      var parameters = Query.collectParamsFromQueryString($location, $scope.query);
+      var maxAge = $location.search()['maxAge'];
+      $scope.queryResult = $scope.query.getQueryResult(maxAge, parameters);
       $scope.nextUpdateTime = moment(new Date(($scope.query.updated_at + $scope.query.ttl + $scope.query.runtime + 300) * 1000)).fromNow();
 
       $scope.type = 'visualization';
@@ -139,6 +141,6 @@
 
   angular.module('redash.controllers')
     .controller('DashboardCtrl', ['$scope', 'Events', 'Widget', '$routeParams', '$location', '$http', '$timeout', '$q', 'Dashboard', DashboardCtrl])
-    .controller('WidgetCtrl', ['$scope', 'Events', 'Query', WidgetCtrl])
+    .controller('WidgetCtrl', ['$scope', '$location', 'Events', 'Query', WidgetCtrl])
 
 })();

rd_ui/app/scripts/controllers/query_source.js

@@ -14,22 +14,7 @@
     var isNewQuery = !$scope.query.id,
         queryText = $scope.query.query,
         // ref to QueryViewCtrl.saveQuery
-        saveQuery = $scope.saveQuery,
-        shortcuts = {
-          'meta+s': function () {
-            if ($scope.canEdit) {
-              $scope.saveQuery();
-            }
-          },
-          // Cmd+Enter for Mac
-          'meta+enter': function () {
-            $scope.executeQuery();
-          },
-          // Ctrl+Enter for PC
-          'ctrl+enter': function () {
-            $scope.executeQuery();
-          }
-        };
+        saveQuery = $scope.saveQuery;
 
     $scope.sourceMode = true;
     $scope.canEdit = currentUser.canEdit($scope.query);
@@ -44,8 +29,22 @@
       }
     });
 
-
-    KeyboardShortcuts.bind(shortcuts);
+    KeyboardShortcuts.bind({
+      'meta+s': function () {
+        if ($scope.canEdit) {
+          $scope.saveQuery();
+        }
+      },
+      'ctrl+s': function () {
+        if ($scope.canEdit) {
+          $scope.saveQuery();
+        }
+      },
+      // Cmd+Enter for Mac
+      'meta+enter': $scope.executeQuery,
+      // Ctrl+Enter for PC
+      'ctrl+enter': $scope.executeQuery
+    });
 
     // @override
     $scope.saveQuery = function(options, data) {

rd_ui/app/scripts/controllers/query_view.js

@@ -4,9 +4,18 @@
   function QueryViewCtrl($scope, Events, $route, $location, notifications, growl, Query, DataSource) {
     var DEFAULT_TAB = 'table';
 
+    var getQueryResult = function(ttl) {
+      // Collect params, and getQueryResult with params; getQueryResult merges it into the query
+      var parameters = Query.collectParamsFromQueryString($location, $scope.query);
+      if (ttl == undefined) {
+        ttl = $location.search()['maxAge'];
+      }
+      $scope.queryResult = $scope.query.getQueryResult(ttl, parameters);
+    }
+
     $scope.query = $route.current.locals.query;
     Events.record(currentUser, 'view', 'query', $scope.query.id);
-    $scope.queryResult = $scope.query.getQueryResult();
+    getQueryResult();
     $scope.queryExecuting = false;
 
     $scope.isQueryOwner = currentUser.id === $scope.query.user.id;
@@ -57,7 +66,7 @@
     };
 
     $scope.executeQuery = function() {
-      $scope.queryResult = $scope.query.getQueryResult(0);
+      getQueryResult(0);
       $scope.lockButton(true);
       $scope.cancelling = false;
       Events.record(currentUser, 'execute', 'query', $scope.query.id);

rd_ui/app/scripts/services/resources.js

@@ -399,15 +399,54 @@
       });
     };
 
+    Query.collectParamsFromQueryString = function($location, query) {
+      var parameterNames = query.getParameters();
+      var parameters = {};
+
+      var queryString = $location.search();
+      _.each(parameterNames, function(param, i) {
+        var qsName = "p_" + param;
+        if (qsName in queryString) {
+          parameters[param] = queryString[qsName];
+        }
+      });
+
+      return parameters;
+    };
+
     Query.prototype.getSourceLink = function () {
       return '/queries/' + this.id + '/source';
     };
 
-    Query.prototype.getQueryResult = function (ttl) {
+    Query.prototype.getQueryResult = function (ttl, parameters) {
       if (ttl == undefined) {
         ttl = this.ttl;
       }
 
+      var queryText = this.query;
+
+      var queryParameters = this.getParameters();
+      var paramsRequired = !_.isEmpty(queryParameters);
+
+      var missingParams = parameters === undefined ? queryParameters : _.difference(queryParameters, _.keys(parameters));
+
+      if (paramsRequired && missingParams.length > 0) {
+        var paramsWord = "parameter";
+        if (missingParams.length > 1) {
+          paramsWord = "parameters";
+        }
+
+        return new QueryResult({job: {error: "Missing values for " + missingParams.join(', ')  + " "+paramsWord+".", status: 4}});
+      }
+
+      if (parameters !== undefined) {
+        queryText = Mustache.render(queryText, parameters);
+
+        // Need to clear latest results, to make sure we don't used results for different params.
+        this.latest_query_data = null;
+        this.latest_query_data_id = null;
+      }
+
       if (this.latest_query_data && ttl != 0) {
         if (!this.queryResult) {
           this.queryResult = new QueryResult({'query_result': this.latest_query_data});
@@ -417,7 +456,7 @@
           this.queryResult = QueryResult.getById(this.latest_query_data_id);
         }
       } else if (this.data_source_id) {
-        this.queryResult = QueryResult.get(this.data_source_id, this.query, ttl);
+        this.queryResult = QueryResult.get(this.data_source_id, queryText, ttl);
       }
 
       return this.queryResult;
@@ -425,6 +464,18 @@
 
     Query.prototype.getQueryResultPromise = function() {
       return this.getQueryResult().toPromise();
+    };
+
+    Query.prototype.getParameters = function() {
+      var parts = Mustache.parse(this.query);
+      var parameters = [];
+      _.each(parts, function(part) {
+        if (part[0] == 'name') {
+          parameters.push(part[1]);
+        }
+      });
+
+      return parameters;
     }
 
     return Query;

rd_ui/bower.json

@@ -27,7 +27,8 @@
     "bucky": "~0.2.6",
     "pace": "~0.5.1",
     "angular-ui-select": "0.8.2",
-    "font-awesome": "~4.2.0"
+    "font-awesome": "~4.2.0",
+    "mustache": "~1.0.0"
   },
   "devDependencies": {
     "angular-mocks": "1.2.18",

rd_ui/test/karma.conf.js

@@ -55,6 +55,7 @@ module.exports = function(config) {
       'app/scripts/ui-bootstrap-tpls-0.5.0.min.js',
       'app/bower_components/bucky/bucky.js',
       'app/bower_components/pace/pace.js',
+      'app/bower_components/mustache/mustache.js',
 
       'app/scripts/app.js',
       'app/scripts/services/services.js',

redash/controllers.py

@@ -349,7 +349,7 @@ class QueryAPI(BaseResource):
             if q.user.id == self.current_user.id or self.current_user.has_permission('admin'):
                 q.archive()
             else:
-                self.delete_others_query(query_id)
+                abort(403)
         else:
             abort(404, message="Query not found.")
 

redash/models.py

@@ -452,7 +452,6 @@ class Dashboard(BaseModel):
         if with_widgets:
             widgets = Widget.select(Widget, Visualization, Query, User)\
                 .where(Widget.dashboard == self.id)\
-                .where(Query.is_archived == False)\
                 .join(Visualization, join_type=peewee.JOIN_LEFT_OUTER)\
                 .join(Query, join_type=peewee.JOIN_LEFT_OUTER)\
                 .join(User, join_type=peewee.JOIN_LEFT_OUTER)

redash/tasks.py

@@ -171,8 +171,10 @@ def refresh_queries():
 def cleanup_tasks():
     # in case of cold restart of the workers, there might be jobs that still have their "lock" object, but aren't really
     # going to run. this job removes them.
-
     lock_keys = redis_connection.keys("query_hash_job:*") # TODO: use set instead of keys command
+    if not lock_keys:
+        return
+    
     query_tasks = [QueryTask(job_id=j) for j in redis_connection.mget(lock_keys)]
 
     logger.info("Found %d locks", len(query_tasks))