Response HTTP 404 if param value is not found

Previously all some of wrong values were silently ignored but some
resulted in HTTP 500 Server Error.
New policy is to validate all values and return HTTP 404 Not Found
if there are any wrong values.

Closes bug 1373980
Closes bug 1361647

Change-Id: Ib2968b2188070863b2ea6df25c35ad729a72ede5

stackalytics/dashboard/decorators.py

@@ -35,6 +35,35 @@ from stackalytics import version as stackalytics_version
 LOG = logging.getLogger(__name__)
 
 
+def _check_param_in(params, name, collection, allow_all=False):
+    for single in (params.get(name) or []):
+        single = single.lower()
+        if allow_all and single == 'all':
+            continue
+        if single not in collection:
+            params[name] = []
+            flask.abort(404)
+
+
+def _validate_params(params):
+    vault_inst = vault.get_vault()
+    memory_storage_inst = vault.get_memory_storage()
+
+    _check_param_in(params, 'release', vault_inst['releases'], True)
+    _check_param_in(params, 'project_type', vault_inst['project_types_index'])
+    _check_param_in(params, 'module', vault_inst['module_id_index'])
+    _check_param_in(params, 'company',
+                    memory_storage_inst.get_companies_lower())
+    _check_param_in(params, 'user_id', memory_storage_inst.get_user_ids())
+    _check_param_in(params, 'metric', parameters.METRIC_TO_RECORD_TYPE, True)
+
+
+def _get_single(params):
+    if params:
+        return params[0]
+    return None
+
+
 def _prepare_params(kwargs, ignore):
     params = kwargs.get('_params')
 
@@ -50,6 +79,7 @@ def _prepare_params(kwargs, ignore):
             params['end_date'] = [utils.round_timestamp_to_day(
                 params['end_date'][0])]
 
+        _validate_params(params)
         kwargs['_params'] = params
 
     if ignore:
@@ -178,8 +208,7 @@ def record_filter(ignore=None):
 
             if 'tm_marks' in metric:
                 filtered_ids = []
-                review_nth = int(parameters.get_parameter(
-                    kwargs, 'review_nth')[0])
+                review_nth = int(parameters.get_parameter('review_nth')[0])
                 for record in memory_storage_inst.get_records(record_ids):
                     parent = memory_storage_inst.get_record_by_primary_key(
                         record['review_id'])
@@ -371,50 +400,45 @@ def templated(template=None, return_code=200):
             if ctx is None:
                 ctx = {}
 
+            try:
+                _prepare_params(kwargs, [])
+            except Exception:
+                if return_code == 200:
+                    raise  # do not re-raise on error page
+
             # put parameters into template
-            metric = flask.request.args.get('metric')
-            if metric not in parameters.METRIC_LABELS:
-                metric = None
-            ctx['metric'] = metric or parameters.get_default('metric')
+            ctx['metric'] = parameters.get_single_parameter(
+                kwargs, 'metric', use_default=True)
             ctx['metric_label'] = parameters.METRIC_LABELS[ctx['metric']]
 
-            project_type = flask.request.args.get('project_type')
-            if not vault.is_project_type_valid(project_type):
-                project_type = parameters.get_default('project_type')
+            project_type = parameters.get_single_parameter(
+                kwargs, 'project_type', use_default=True)
             ctx['project_type'] = project_type
 
-            release = flask.request.args.get('release')
-            releases = vault_inst['releases']
-            if release:
-                release = release.lower()
-                if release != 'all':
-                    if release not in releases:
-                        release = None
-                    else:
-                        release = releases[release]['release_name']
-            ctx['release'] = (release or
-                              parameters.get_default('release')).lower()
-            ctx['review_nth'] = (flask.request.args.get('review_nth') or
-                                 parameters.get_default('review_nth'))
+            ctx['release'] = parameters.get_single_parameter(
+                kwargs, 'release', use_default=True)
 
-            ctx['company'] = parameters.get_single_parameter(kwargs, 'company')
-            ctx['company_original'] = (
-                vault.get_memory_storage().get_original_company_name(
-                    ctx['company']))
+            company = parameters.get_single_parameter(kwargs, 'company')
+            ctx['company'] = company
+            if company:
+                ctx['company_original'] = (
+                    vault.get_memory_storage().get_original_company_name(
+                        ctx['company']))
 
             module = parameters.get_single_parameter(kwargs, 'module')
             ctx['module'] = module
-            module_name = None
             if module and module in vault_inst['module_id_index']:
                 ctx['module_inst'] = vault_inst['module_id_index'][module]
-                module_name = ctx['module_inst']['module_group_name']
 
             ctx['user_id'] = parameters.get_single_parameter(kwargs, 'user_id')
             if ctx['user_id']:
                 ctx['user_inst'] = vault.get_user_from_runtime_storage(
                     ctx['user_id'])
+
             ctx['page_title'] = helpers.make_page_title(
-                ctx['company'], ctx['user_id'], module_name, ctx['release'])
+                ctx.get('release'), ctx.get('module_inst'),
+                ctx.get('company_original'), ctx.get('user_inst'))
+
             ctx['stackalytics_version'] = (
                 stackalytics_version.version_info.version_string())
             ctx['stackalytics_release'] = (
@@ -422,6 +446,10 @@ def templated(template=None, return_code=200):
             ctx['runtime_storage_update_time'] = (
                 vault_inst['runtime_storage_update_time'])
 
+            # deprecated -- top mentor report
+            ctx['review_nth'] = parameters.get_single_parameter(
+                kwargs, 'review_nth')
+
             return flask.render_template(template_name, **ctx), return_code
 
         return templated_decorated_function

stackalytics/dashboard/helpers.py

@@ -237,7 +237,7 @@ def make_link(title, uri=None, options=None):
                    'metric')
     param_values = {}
     for param_name in param_names:
-        v = parameters.get_parameter({}, param_name, param_name)
+        v = parameters.get_parameter({}, param_name)
         if v:
             param_values[param_name] = ','.join(v)
     if options:
@@ -274,13 +274,10 @@ def make_commit_message(record):
     return s
 
 
-def make_page_title(company, user_id, module, release):
-    if company:
-        memory_storage = vault.get_memory_storage()
-        company = memory_storage.get_original_company_name(company)
-    if company or user_id:
-        if user_id:
-            s = vault.get_user_from_runtime_storage(user_id)['user_name']
+def make_page_title(release, module_inst, company, user_inst):
+    if company or user_inst:
+        if user_inst:
+            s = user_inst['user_name']
             if company:
                 s += ' (%s)' % company
         else:
@@ -288,8 +285,8 @@ def make_page_title(company, user_id, module, release):
     else:
         s = 'OpenStack community'
     s += ' contribution'
-    if module:
-        s += ' to %s' % module
+    if module_inst:
+        s += ' to %s' % module_inst['module_group_name']
     if release != 'all':
         s += ' in %s release' % release.capitalize()
     else:

stackalytics/dashboard/memory_storage.py

@@ -174,6 +174,9 @@ class CachedMemoryStorage(MemoryStorage):
     def get_companies(self):
         return self.company_index.keys()
 
+    def get_companies_lower(self):
+        return self.company_name_mapping.keys()
+
     def get_modules(self):
         return self.module_index.keys()
 

stackalytics/dashboard/parameters.py

@@ -85,25 +85,29 @@ def get_default(param_name):
         return None
 
 
-def get_parameter(kwargs, singular_name, plural_name=None, use_default=True):
-    if singular_name in kwargs:
-        p = kwargs[singular_name]
+def get_parameter(kwargs, name, plural_name=None, use_default=True):
+    processed_params = kwargs.get('_params') or {}
+    if name in processed_params:
+        return processed_params[name]
+
+    if name in kwargs:
+        p = kwargs[name]
     else:
-        p = flask.request.args.get(singular_name)
+        p = flask.request.args.get(name)
         if (not p) and plural_name:
             p = flask.request.args.get(plural_name)
     if p:
         return parse.unquote_plus(p).split(',')
     elif use_default:
-        default = get_default(singular_name)
+        default = get_default(name)
         return [default] if default else []
     else:
         return []
 
 
-def get_single_parameter(kwargs, singular_name, use_default=True):
-    param = get_parameter(kwargs, singular_name, use_default)
+def get_single_parameter(kwargs, name, use_default=True):
+    param = get_parameter(kwargs, name, use_default)
     if param:
         return param[0]
     else:
-        return ''
+        return None

stackalytics/dashboard/web.py

@@ -60,12 +60,6 @@ def widget():
     return flask.render_template('widget.html')
 
 
-@app.errorhandler(404)
-@decorators.templated('404.html', 404)
-def page_not_found(e):
-    pass
-
-
 # AJAX Handlers ---------
 
 def _get_aggregated_stats(records, metric_filter, keys, param_id,
@@ -167,7 +161,7 @@ def get_core_engineer_branch(user, modules):
 @decorators.record_filter()
 @decorators.aggregate_filter()
 def get_engineers(records, metric_filter, finalize_handler, **kwargs):
-    modules_names = parameters.get_parameter({}, 'module', 'modules')
+    modules_names = parameters.get_parameter(kwargs, 'module')
     modules = set([m for m, r in vault.resolve_modules(modules_names, [''])])
 
     def postprocessing(record):
@@ -190,7 +184,7 @@ def get_engineers(records, metric_filter, finalize_handler, **kwargs):
 @decorators.jsonify('stats')
 @decorators.record_filter(ignore=['metric'])
 def get_engineers_extended(records, **kwargs):
-    modules_names = parameters.get_parameter({}, 'module', 'modules')
+    modules_names = parameters.get_parameter(kwargs, 'module')
     modules = set([m for m, r in vault.resolve_modules(modules_names, [''])])
 
     def postprocessing(record):
@@ -299,7 +293,7 @@ def get_companies_json(record_ids, **kwargs):
 def get_modules_json(record_ids, **kwargs):
     module_id_index = vault.get_vault()['module_id_index']
 
-    tags = parameters.get_parameter({}, 'tag', 'tags')
+    tags = parameters.get_parameter(kwargs, 'tag', plural_name='tags')
 
     # all modules mentioned in records
     module_ids = vault.get_memory_storage().get_index_keys_by_record_ids(
@@ -414,7 +408,7 @@ def get_bpd(records, **kwargs):
 @decorators.jsonify()
 @decorators.record_filter(ignore=['user_id'])
 def get_users_json(record_ids, **kwargs):
-    core_in = parameters.get_single_parameter({}, 'core_in') or None
+    core_in = parameters.get_single_parameter(kwargs, 'core_in') or None
     valid_modules = set()
     if core_in:
         core_in = set(core_in.split(','))

tests/api/test_companies.py

@@ -81,7 +81,20 @@ class TestAPICompanies(test_api.TestAPI):
                      'uri': 'git://git.openstack.org/openstack/nova.git'},
                     {'module': 'glance', 'project_type': 'openstack',
                      'organization': 'openstack',
-                     'uri': 'git://git.openstack.org/openstack/glance.git'}]},
+                     'uri': 'git://git.openstack.org/openstack/glance.git'}],
+                 'module_groups': {
+                     'nova': test_api.make_module('nova'),
+                     'glance': test_api.make_module('glance'),
+                 },
+                 'releases': [{'release_name': 'prehistory',
+                               'end_date': 1234567890},
+                              {'release_name': 'icehouse',
+                               'end_date': 1234567890}],
+                 'project_types': [
+                     {'id': 'all', 'title': 'All',
+                      'modules': ['nova', 'glance', 'nova-cli']},
+                     {'id': 'openstack', 'title': 'OpenStack',
+                      'modules': ['nova', 'glance']}]},
                 test_api.make_records(record_type=['commit'],
                                       loc=[10, 20, 30],
                                       module=['glance'],

tests/api/test_modules.py

@@ -83,7 +83,16 @@ class TestAPIModules(test_api.TestAPI):
                                     'tag': 'group'},
                      'nova': test_api.make_module('nova'),
                      'nova-cli': test_api.make_module('nova-cli'),
-                 }},
+                 },
+                 'releases': [{'release_name': 'prehistory',
+                               'end_date': 1234567890},
+                              {'release_name': 'icehouse',
+                               'end_date': 1234567890}],
+                 'project_types': [
+                     {'id': 'all', 'title': 'All',
+                      'modules': ['nova', 'glance', 'nova-cli']},
+                     {'id': 'openstack', 'title': 'OpenStack',
+                      'modules': ['nova', 'glance']}]},
                 test_api.make_records(record_type=['commit'])):
 
             response = self.app.get('/api/1.0/modules/nova')

tests/api/test_releases.py

@@ -23,7 +23,12 @@ class TestAPIReleases(test_api.TestAPI):
                 {'releases': [
                     {'release_name': 'prehistory', 'end_date': 1365033600},
                     {'release_name': 'havana', 'end_date': 1381968000},
-                    {'release_name': 'icehouse', 'end_date': 1397692800}]},
+                    {'release_name': 'icehouse', 'end_date': 1397692800}],
+                 'project_types': [
+                     {'id': 'all', 'title': 'All',
+                      'modules': ['nova', 'glance', 'nova-cli']},
+                     {'id': 'openstack', 'title': 'OpenStack',
+                      'modules': ['nova', 'glance']}]},
                 test_api.make_records(record_type=['commit'])):
             response = self.app.get('/api/1.0/releases')
             releases = test_api.load_json(response)['data']

tests/unit/test_web_utils.py

@@ -86,25 +86,23 @@ Implements Blueprint ''' + (
     @mock.patch('stackalytics.dashboard.vault.get_vault')
     @mock.patch('stackalytics.dashboard.vault.get_user_from_runtime_storage')
     def test_make_page_title(self, user_patch, vault_patch):
-        memory_storage_mock = mock.Mock()
-        memory_storage_mock.get_original_company_name = mock.Mock(
-            return_value='Mirantis'
-        )
-        vault_patch.return_value = {'memory_storage': memory_storage_mock}
-        user_patch.return_value = {'user_name': 'John Doe'}
+        user_inst = {'user_name': 'John Doe'}
+        module_inst = {'module_group_name': 'neutron'}
 
         self.assertEqual('OpenStack community contribution in all releases',
-                         helpers.make_page_title('', '', '', 'all'))
+                         helpers.make_page_title('all', None, None, None))
         self.assertEqual('OpenStack community contribution in Havana release',
-                         helpers.make_page_title('', '', '', 'Havana'))
+                         helpers.make_page_title('Havana', None, None, None))
         self.assertEqual('Mirantis contribution in Havana release',
-                         helpers.make_page_title('Mirantis', '', '', 'Havana'))
+                         helpers.make_page_title(
+                             'Havana', None, 'Mirantis', None))
         self.assertEqual('John Doe contribution in Havana release',
-                         helpers.make_page_title('', 'john_doe', '', 'Havana'))
+                         helpers.make_page_title(
+                             'Havana', None, None, user_inst))
         self.assertEqual(
             'John Doe (Mirantis) contribution to neutron in Havana release',
             helpers.make_page_title(
-                'Mirantis', 'John Doe', 'neutron', 'Havana'))
+                'Havana', module_inst, 'Mirantis', user_inst))
 
     @mock.patch('flask.request')
     @mock.patch('stackalytics.dashboard.parameters.get_default')