From 5019cad90723de1aebd1bbc9d4313e37923da27f Mon Sep 17 00:00:00 2001
From: Nikhil B <nikhilb@juniper.net>
Date: Thu, 28 Jul 2016 11:31:40 +0530
Subject: [PATCH] move parameter validation for stats queries from QE to
 opserver

Now, with stats table schema removed from viz.sandesh, it is no longer
available to QE for parameter validation. These parameter validation steps
should instead be done in Opserver now.
Closes-Bug:1602130

Change-Id: I8d7eaa8fa9faa45de62715a6d002e0e6981fca9a
---
 src/opserver/opserver.py         | 58 ++++++++++++++++++++++++++++++++
 src/query_engine/stats_select.cc | 35 +++----------------
 2 files changed, 62 insertions(+), 31 deletions(-)

diff --git a/src/opserver/opserver.py b/src/opserver/opserver.py
index 94450af961a..bd46bd56917 100644
--- a/src/opserver/opserver.py
+++ b/src/opserver/opserver.py
@@ -1168,6 +1168,58 @@ def _query_chunk(self, request, qid, chunk_id):
                 % (qid, chunk_id, time.time()))
     # end _query_chunk
 
+    def _is_valid_stats_table_query(self, request, tabn):
+        isT_ = False
+        isT = False
+        for key, value in request.iteritems():
+            if key == "select_fields":
+                for select_field in value:
+                    if select_field == STAT_TIME_FIELD:
+                        isT = True
+                    elif select_field.find(STAT_TIMEBIN_FIELD) == 0:
+                        isT_ = True
+                    else:
+                        agg_field = select_field.split('(')
+                        if len(agg_field) == 2:
+                            oper = agg_field[0]
+                            field = agg_field[1].split(')')[0]
+                            if oper != "COUNT":
+                                if field == STAT_TIME_FIELD:
+                                    isT = True
+                                elif field == STAT_TIMEBIN_FIELD:
+                                    isT_ = True
+                                else:
+                                    field_found = False
+                                    for column in self._VIRTUAL_TABLES[tabn].schema.columns:
+                                        if column.name == field:
+                                            if column.datatype != "":
+                                                field_found = True
+                                    if field_found == False:
+                                        reply = bottle.HTTPError(_ERRORS[errno.EINVAL], \
+                                                            'Unknown field %s' %field)
+                                        return reply
+                            elif field != tabl.split('.')[2]:
+                                reply = bottle.HTTPError(_ERRORS[errno.EINVAL], \
+                                            'Invalid COUNT field %s' %field)
+                                return reply
+                        elif len(agg_field) == 1:
+                            field_found = False
+                            for column in self._VIRTUAL_TABLES[tabn].schema.columns:
+                                if column.name == select_field:
+                                    if column.datatype != "":
+                                        field_found = True
+                            if field_found == False:
+                                reply = bottle.HTTPError(_ERRORS[errno.EINVAL], \
+                                            'Invalid field %s' %select_field)
+                                return reply
+
+                    if isT and isT_:
+                        reply = bottle.HTTPError(_ERRORS[errno.EINVAL], \
+                                    "Stats query cannot have both T and T=")
+                        return reply
+        return None
+    # end _is_valid_stats_table_query
+
     def _query(self, request):
         reply = {}
         try:
@@ -1189,6 +1241,12 @@ def _query(self, request):
                 if self._VIRTUAL_TABLES[i].name == tabl:
                     tabn = i
 
+            if (tabn is not None) and (tabl.find("StatTable") == 0):
+                query_err = self._is_valid_stats_table_query(request.json, tabn)
+                if query_err is not None:
+                    yield query_err
+                    return
+
             if (tabn is not None):
                 tabtypes = {}
                 for cols in self._VIRTUAL_TABLES[tabn].schema.columns:
diff --git a/src/query_engine/stats_select.cc b/src/query_engine/stats_select.cc
index bdb3fe694e9..b38bdd889c1 100644
--- a/src/query_engine/stats_select.cc
+++ b/src/query_engine/stats_select.cc
@@ -219,8 +219,7 @@ void StatsSelect::Merge(const MapBufT& input, MapBufT& output) {
 StatsSelect::StatsSelect(AnalyticsQuery * m_query,
 		const std::vector<std::string> & select_fields) :
 			main_query(m_query), select_fields_(select_fields),
-			ts_period_(0), isT_(false),
-                        isTC_(false), isTBC_(false), count_field_() {
+			ts_period_(0), isT_(false), count_field_() {
 
     QE_ASSERT(main_query->is_stat_table_query(main_query->table()));
     status_ = false;
@@ -228,7 +227,7 @@ StatsSelect::StatsSelect(AnalyticsQuery * m_query,
     for (size_t j=0; j<select_fields_.size(); j++) {
 
         if (select_fields_[j] == g_viz_constants.STAT_TIME_FIELD) {
-            if (ts_period_ || isTBC_) {
+            if (ts_period_) {
                 QE_LOG(INFO, "StatsSelect cannot accept both T and T="); 
                 return;
             }
@@ -238,7 +237,7 @@ StatsSelect::StatsSelect(AnalyticsQuery * m_query,
 
         } else if (select_fields_[j].substr(0, g_viz_constants.STAT_TIMEBIN_FIELD.size()) ==
                 g_viz_constants.STAT_TIMEBIN_FIELD) {
-            if (isT_ || isTC_) { 
+            if (isT_) {
                 QE_LOG(INFO, "StatsSelect cannot accept both T and T="); 
                 return;
             }
@@ -254,35 +253,9 @@ StatsSelect::StatsSelect(AnalyticsQuery * m_query,
             std::string sfield = select_fields_[j];
             QEOpServerProxy::AggOper agg =
                 StatsQuery::ParseAgg(select_fields_[j], sfield);
-            if (main_query->stats().is_stat_table_static()) {
-                if (agg != QEOpServerProxy::COUNT) {
-                    StatsQuery::column_t c = main_query->stats().get_column_desc(sfield);
-                    if (sfield == g_viz_constants.STAT_TIME_FIELD) {
-                        if (ts_period_ || isTBC_) { 
-                            QE_TRACE(DEBUG, "StatsSelect cannot aggregate " <<
-                                sfield << "with T=");
-                        }
-                        isTC_ = true;
-                    } else if (sfield == g_viz_constants.STAT_TIMEBIN_FIELD) {
-                        if (isT_ || isTC_) { 
-                            QE_TRACE(DEBUG, "StatsSelect cannot aggregate " <<
-                                sfield << " with T");
-                        }
-                        isTBC_ = true;
-                    } else if (c.datatype == QEOpServerProxy::BLANK) {
-                        QE_TRACE(DEBUG, "StatsSelect unknown field " << select_fields_[j]);
-                        return;
-                    }
-                } else {
-                    if (sfield != main_query->stats().attr()) {
-                        QE_TRACE(DEBUG,"StatsSelect Invalid COUNT field " << sfield);
-                        return;
-                    }
-                }
-            }
             if (agg == QEOpServerProxy::INVALID) {
                 unik_cols_.insert(select_fields_[j]);
-                QE_TRACE(DEBUG, "StatsSelect unik field " << select_fields_[j]);                 
+                QE_TRACE(DEBUG, "StatsSelect unik field " << select_fields_[j]);
             } else if (agg == QEOpServerProxy::COUNT) {
                 count_field_ = sfield;
                 QE_TRACE(DEBUG, "StatsSelect COUNT " << sfield);