diff --git a/stackalytics/dashboard/decorators.py b/stackalytics/dashboard/decorators.py index 65c2f8e7e..bc4bd0c35 100644 --- a/stackalytics/dashboard/decorators.py +++ b/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 diff --git a/stackalytics/dashboard/helpers.py b/stackalytics/dashboard/helpers.py index 49fc19ccd..828acf5c2 100644 --- a/stackalytics/dashboard/helpers.py +++ b/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: diff --git a/stackalytics/dashboard/memory_storage.py b/stackalytics/dashboard/memory_storage.py index 8b954b21f..6000a893c 100644 --- a/stackalytics/dashboard/memory_storage.py +++ b/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() diff --git a/stackalytics/dashboard/parameters.py b/stackalytics/dashboard/parameters.py index 1b35abd65..5b59bf98a 100644 --- a/stackalytics/dashboard/parameters.py +++ b/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 diff --git a/stackalytics/dashboard/web.py b/stackalytics/dashboard/web.py index 17822c3f7..db4728bc7 100644 --- a/stackalytics/dashboard/web.py +++ b/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(',')) diff --git a/tests/api/test_companies.py b/tests/api/test_companies.py index 4a713a680..9d7896280 100644 --- a/tests/api/test_companies.py +++ b/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'], diff --git a/tests/api/test_modules.py b/tests/api/test_modules.py index a47c68637..3c6386bcd 100644 --- a/tests/api/test_modules.py +++ b/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') diff --git a/tests/api/test_releases.py b/tests/api/test_releases.py index 60676c330..9b31d5774 100644 --- a/tests/api/test_releases.py +++ b/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'] diff --git a/tests/unit/test_web_utils.py b/tests/unit/test_web_utils.py index d83724c98..d64a675b7 100644 --- a/tests/unit/test_web_utils.py +++ b/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')