From 314266a8af8db04f17cf25088bdcc42c98206add Mon Sep 17 00:00:00 2001 From: Benjamin Dauvergne Date: Thu, 18 Nov 2021 10:21:26 +0100 Subject: [PATCH] misc: report unknown dimensions as errors (#58680) --- bijoe/engine.py | 39 +++++++++++++++++++++++---------- bijoe/schemas.py | 4 ++++ bijoe/templates/bijoe/cube.html | 8 ++++++- bijoe/visualization/utils.py | 23 ++++++++++++++++--- bijoe/visualization/views.py | 12 +++++++++- tests/test_views.py | 9 ++++++++ 6 files changed, 79 insertions(+), 16 deletions(-) diff --git a/bijoe/engine.py b/bijoe/engine.py index 96a8082..a336eff 100644 --- a/bijoe/engine.py +++ b/bijoe/engine.py @@ -163,7 +163,10 @@ class EngineDimension: joins = set(self.join or []) conditions = [] for dimension_name, values in filters: - dimension = self.engine_cube.dimensions[dimension_name] + try: + dimension = self.engine_cube.dimensions[dimension_name] + except KeyError as e: + raise schemas.DimensionNotFound(str(e)) # we build a filter on two sufficient conditions : # * the filter is on the same dimension as the one we currently query for members, # * or, the dimension of the filter has impact on a join shared with the current dimension. @@ -282,11 +285,12 @@ class EngineMeasure: class ProxyList: chain = None - def __init__(self, engine, engine_cube, attribute, cls, chain=None): + def __init__(self, engine, engine_cube, attribute, cls, chain=None, keyerror_class=KeyError): self.engine = engine self.engine_cube = engine_cube self.attribute = attribute self.cls = cls + self.keyerror_class = keyerror_class if chain: self.chain = chain(engine, engine_cube) @@ -299,12 +303,14 @@ class ProxyList: return i def __getitem__(self, name): - for o in getattr(self.engine_cube.cube, self.attribute): - if o.name == name: - return self.cls(self.engine, self.engine_cube, o) - if self.chain: - return self.chain[name] - raise KeyError + try: + for o in getattr(self.engine_cube.cube, self.attribute): + if o.name == name: + return self.cls(self.engine, self.engine_cube, o) + if self.chain: + return self.chain[name] + except KeyError: + raise self.keyerror_class(name) class JSONDimensions: @@ -336,23 +342,34 @@ class JSONDimensions: for name in self.cache: if name == key: return EngineJSONDimension(self.engine, self.engine_cube, name) + raise KeyError(key) class ProxyListDescriptor: - def __init__(self, attribute, cls, chain=None): + def __init__(self, attribute, cls, chain=None, keyerror_class=KeyError): self.attribute = attribute self.cls = cls self.chain = chain + self.keyerror_class = keyerror_class def __get__(self, obj, t=None): key = '_proxy_list_cache_%s' % id(self) if key not in obj.__dict__: - obj.__dict__[key] = ProxyList(obj.engine, obj, self.attribute, self.cls, chain=self.chain) + obj.__dict__[key] = ProxyList( + obj.engine, + obj, + self.attribute, + self.cls, + chain=self.chain, + keyerror_class=self.keyerror_class, + ) return obj.__dict__[key] class EngineCube: - dimensions = ProxyListDescriptor('all_dimensions', EngineDimension, chain=JSONDimensions) + dimensions = ProxyListDescriptor( + 'all_dimensions', EngineDimension, chain=JSONDimensions, keyerror_class=schemas.DimensionNotFound + ) measures = ProxyListDescriptor('measures', EngineMeasure) def __init__(self, warehouse, cube): diff --git a/bijoe/schemas.py b/bijoe/schemas.py index 8911a45..2759254 100644 --- a/bijoe/schemas.py +++ b/bijoe/schemas.py @@ -42,6 +42,10 @@ class SchemaError(Exception): pass +class DimensionNotFound(SchemaError): + pass + + def type_cast(t): if t not in TYPE_MAP: raise SchemaError('invalid type: %r' % t) diff --git a/bijoe/templates/bijoe/cube.html b/bijoe/templates/bijoe/cube.html index e96ef4e..4428831 100644 --- a/bijoe/templates/bijoe/cube.html +++ b/bijoe/templates/bijoe/cube.html @@ -56,7 +56,13 @@ {% endblock %} {% endif %}
- {% if visualization %} + {% if visualization and visualization.errors %} + + {% elif visualization %} {% if visualization.representation == 'table' %} {% include "bijoe/cube_table.html" %} {% else %} diff --git a/bijoe/visualization/utils.py b/bijoe/visualization/utils.py index 7b36546..852b251 100644 --- a/bijoe/visualization/utils.py +++ b/bijoe/visualization/utils.py @@ -30,6 +30,7 @@ from django.utils.encoding import force_bytes, force_text from django.utils.safestring import mark_safe from django.utils.translation import ugettext_lazy as _ +from .. import schemas from ..engine import Engine, MeasureCell, Member from ..utils import get_warehouses from .ods import Workbook @@ -107,13 +108,27 @@ class Visualization: cube = cls.get_cube(d) representation = d['representation'] measure = cube.measures[d['measure']] - drilldown_x = cube.dimensions[d['drilldown_x']] if 'drilldown_x' in d else None - drilldown_y = cube.dimensions[d['drilldown_y']] if 'drilldown_y' in d else None + drilldown_x = None + drilldown_y = None + errors = [] + + if d.get('drilldown_x'): + try: + drilldown_x = cube.dimensions[d.get('drilldown_x')] + except schemas.DimensionNotFound as e: + errors.append(_('Dimension %s does not exist.') % e) + + if d.get('drilldown_y'): + try: + drilldown_y = cube.dimensions[d.get('drilldown_y')] + except schemas.DimensionNotFound as e: + errors.append(_('Dimension %s does not exist.') % e) + filters = d.get('filters', {}) loop = d.get('loop') if loop: loop = cube.dimensions[loop] - return cls( + visualization = cls( cube, representation, measure, @@ -122,6 +137,8 @@ class Visualization: filters=filters, loop=loop, ) + visualization.errors = errors + return visualization @classmethod def from_form(cls, cube, form): diff --git a/bijoe/visualization/views.py b/bijoe/visualization/views.py index af467eb..c5f4fba 100644 --- a/bijoe/visualization/views.py +++ b/bijoe/visualization/views.py @@ -37,11 +37,12 @@ from django.views.generic.edit import CreateView, DeleteView, FormView, UpdateVi from django.views.generic.list import MultipleObjectMixin from django_select2.cache import cache from rest_framework import generics +from rest_framework.exceptions import ValidationError from rest_framework.response import Response from bijoe.utils import export_site, get_warehouses, import_site -from .. import views +from .. import schemas, views from ..engine import Engine from . import forms, models, signature from .utils import Visualization @@ -65,7 +66,11 @@ class WarehouseView(views.AuthorizationMixin, TemplateView): class CubeDisplayMixin: + error = None + def get_context_data(self, **kwargs): + if self.error: + messages.error(self.request, self.error) ctx = super().get_context_data(**kwargs) ctx['warehouse'] = self.warehouse ctx['cube'] = self.cube @@ -313,6 +318,11 @@ class VisualizationJSONView(generics.GenericAPIView): permission_classes = () queryset = models.Visualization.objects.all() + def handle_exception(self, exc): + if isinstance(exc, schemas.DimensionNotFound): + exc = ValidationError(detail={'err': 'dimension-not-found', 'err_desc': str(exc)}) + return super().handle_exception(exc) + def get(self, request, pk, format=None): def cell_value(cell): if cell.measure.type == 'duration' and cell.value is not None: diff --git a/tests/test_views.py b/tests/test_views.py index 8a62d4e..4c6fead 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -329,3 +329,12 @@ def test_mellon_idp_redirections(mocked_resolv_url, mocked_get_idps, app): assert resp.location == 'foo-url?next=http%3A//foo/%3Fbar' resp = app.get('/accounts/logout/', status=302) assert resp.location == 'foo-url' + + +def test_visualization_json_api_dimension_not_found(schema1, app, admin, visualization): + visualization.parameters['filters']['coin'] = '1' + visualization.save() + + login(app, admin) + resp = app.get(reverse('visualization-json', kwargs={'pk': visualization.id}), status=400) + assert resp.json == {'err': 'dimension-not-found', 'err_desc': 'coin'}