From 5398de264d6b40120b20eb123faff68a13f9de30 Mon Sep 17 00:00:00 2001 From: Timur Sufiev Date: Thu, 20 Aug 2015 18:36:51 +0300 Subject: [PATCH] Enable Launch Instance NG for the case when Nova quotas are disabled Do this by representing every float('inf') python numeric as a 1e+999 JSON token - this way Javascript JSON.parse() correctly transforms it to the Javascript Infinity object. The infinity value corresponds to disabled limits. The logic of representing infinity values as 1e+999 tokens is implemented by means of custom `NaNJSONEncoder` which overrides `iterencode` method of its `json.JSONEncoder` ancestor. Due to the fact that the ancestor method could take an advantage of `c_make_encoder` function (which is implemented in C) - and thus could be much faster, I'm using the custom encoder only where it's really needed, to not hamper Horizon performance. Rewrite pie-chart directive rendering according to https://invis.io/A4445BDSF Change-Id: If498d9ccd5f0144c9e78ae58583ede2c1cf83f0b Closes-Bug: #1482705 --- .../widgets/charts/pie-chart.directive.js | 97 ++++++++++++------- .../framework/widgets/charts/pie-chart.html | 10 +- .../framework/widgets/charts/pie-chart.scss | 29 ++++++ .../widgets/charts/pie-chart.spec.js | 51 ++++++++-- openstack_dashboard/api/rest/json_encoder.py | 76 +++++++++++++++ openstack_dashboard/api/rest/nova.py | 3 +- openstack_dashboard/api/rest/utils.py | 10 +- .../test/api_tests/rest_util_tests.py | 82 ++++++++++++++++ 8 files changed, 304 insertions(+), 54 deletions(-) create mode 100644 openstack_dashboard/api/rest/json_encoder.py diff --git a/horizon/static/framework/widgets/charts/pie-chart.directive.js b/horizon/static/framework/widgets/charts/pie-chart.directive.js index 3df9480e2a..232fd31c41 100644 --- a/horizon/static/framework/widgets/charts/pie-chart.directive.js +++ b/horizon/static/framework/widgets/charts/pie-chart.directive.js @@ -113,7 +113,16 @@ return directive; function link(scope, element) { + function updateChartVisibility() { + var showChart = scope.chartData.maxLimit !== Infinity; + scope.chartData.showChart = showChart; + scope.chartData.chartless = showChart ? '' : 'chartless'; + return showChart; + } + var settings = {}; + var showChart = updateChartVisibility(); + // if chartSettings is defined via the attribute value, use it if (angular.isObject(scope.chartSettings)) { settings = scope.chartSettings; @@ -132,15 +141,17 @@ } }; - var d3Elt = d3.select(element[0]); + if (showChart) { + var d3Elt = d3.select(element[0]); - var arc = d3.svg.arc() - .outerRadius(settings.outerRadius) - .innerRadius(settings.innerRadius); + var arc = d3.svg.arc() + .outerRadius(settings.outerRadius) + .innerRadius(settings.innerRadius); - var pie = d3.layout.pie() - .sort(null) - .value(function (d) { return d.value; }); + var pie = d3.layout.pie() + .sort(null) + .value(function (d) { return d.value; }); + } var unwatch = scope.$watch('chartData', updateChart); scope.$on('$destroy', unwatch); @@ -148,8 +159,18 @@ scope.model = model; function updateChart() { + var showChart = updateChartVisibility(); + angular.forEach(scope.chartData.data, function(item) { + if (item.value === Infinity) { + item.hideKey = true; + } + }); + // set labels depending on whether this is a max or total chart - if (angular.isDefined(scope.chartData.maxLimit)) { + if (!showChart) { + scope.model.total = null; + scope.model.totalLabel = gettext('no quota'); + } else if (angular.isDefined(scope.chartData.maxLimit)) { scope.model.total = scope.chartData.maxLimit; scope.model.totalLabel = gettext('Max'); } else { @@ -159,41 +180,43 @@ scope.model.tooltipData.enabled = false; // Generate or update slices - var chart = d3Elt.select('.slices') - .selectAll('path.slice') - .data(pie(scope.chartData.data)); + if (showChart) { + var chart = d3Elt.select('.slices') + .selectAll('path.slice') + .data(pie(scope.chartData.data)); - chart.enter().append('path') - .attr('class', 'slice') - .attr('d', arc); + chart.enter().append('path') + .attr('class', 'slice') + .attr('d', arc); - // Set the color or CSS class for the fill - chart.each(function (d) { - var slice = d3.select(this); - if (d.data.color) { - slice.style('fill', d.data.color); - } else if (d.data.colorClass) { - slice.classed(d.data.colorClass, true); - } - }); + // Set the color or CSS class for the fill + chart.each(function (d) { + var slice = d3.select(this); + if (d.data.color) { + slice.style('fill', d.data.color); + } else if (d.data.colorClass) { + slice.classed(d.data.colorClass, true); + } + }); - chart.on('mouseenter', function (d) { showTooltip(d, this); }) - .on('mouseleave', clearTooltip); + chart.on('mouseenter', function (d) { showTooltip(d, this); }) + .on('mouseleave', clearTooltip); - // Animate the slice rendering - chart.transition() - .duration(500) - .attrTween('d', function animate(d) { - this.lastAngle = this.lastAngle || { startAngle: 0, endAngle: 0 }; - var interpolate = d3.interpolate(this.lastAngle, d); - this.lastAngle = interpolate(0); + // Animate the slice rendering + chart.transition() + .duration(500) + .attrTween('d', function animate(d) { + this.lastAngle = this.lastAngle || { startAngle: 0, endAngle: 0 }; + var interpolate = d3.interpolate(this.lastAngle, d); + this.lastAngle = interpolate(0); - return function (t) { - return arc(interpolate(t)); - }; - }); + return function (t) { + return arc(interpolate(t)); + }; + }); - chart.exit().remove(); + chart.exit().remove(); + } } function showTooltip(d, elt) { diff --git a/horizon/static/framework/widgets/charts/pie-chart.html b/horizon/static/framework/widgets/charts/pie-chart.html index 252b959d5c..1a120b99b2 100644 --- a/horizon/static/framework/widgets/charts/pie-chart.html +++ b/horizon/static/framework/widgets/charts/pie-chart.html @@ -3,10 +3,11 @@
- {$ ::chartData.title $} ({$ model.total $} {$ model.totalLabel $}) + {$ ::chartData.title $} ({$ model.total ? model.total + ' ' : '' $}{$ model.totalLabel $})
-
- {$ slice.value $} {$ slice.label $} +
+
{$ slice.value $}
+
{$ slice.label $}
\ No newline at end of file diff --git a/horizon/static/framework/widgets/charts/pie-chart.scss b/horizon/static/framework/widgets/charts/pie-chart.scss index fdd1be43e8..e88c8f9a7f 100644 --- a/horizon/static/framework/widgets/charts/pie-chart.scss +++ b/horizon/static/framework/widgets/charts/pie-chart.scss @@ -1,6 +1,7 @@ .pie-chart { display: inline-block; position: relative; + margin-left: 10px; .svg-pie-chart { float: left; @@ -51,9 +52,19 @@ font-size: $chart-legend-font-size; line-height: 1em; padding: $chart-legend-padding; + display: table; .slice-legend { padding: $chart-slice-legend-padding; + display: table-row; + + & > :last-child { + padding-left: 5px; + } + + div { + display: table-cell; + } .slice-key { color: transparent; @@ -77,6 +88,24 @@ background-color: $chart-quota-remaining-color; } } + + .chartless { + font-size: x-large; + text-align: right; + padding-top: 10px; + font-weight: bold; + &.usage { + color: $chart-quota-usage-color; + } + + &.added { + color: $chart-quota-added-color; + } + + &.remaining { + color: $chart-quota-remaining-color; + } + } } } } diff --git a/horizon/static/framework/widgets/charts/pie-chart.spec.js b/horizon/static/framework/widgets/charts/pie-chart.spec.js index 210af5a960..35933ed104 100644 --- a/horizon/static/framework/widgets/charts/pie-chart.spec.js +++ b/horizon/static/framework/widgets/charts/pie-chart.spec.js @@ -26,13 +26,17 @@ describe('pie chart directive', function () { var $scope, $elementMax, $elementTotal, $elementOverMax, - donutChartSettings, quotaChartDefaults; + $elementNoQuota, donutChartSettings, quotaChartDefaults; beforeEach(module('templates')); beforeEach(module('horizon.framework')); beforeEach(module('horizon.framework.widgets')); beforeEach(module('horizon.framework.widgets.charts')); + function cleanSpaces(string) { + return string.trim().replace(/\s+/, ' '); + } + beforeEach(inject(function ($injector) { var $compile = $injector.get('$compile'); $scope = $injector.get('$rootScope').$new(); @@ -58,6 +62,7 @@ $scope.testDataMax = {}; $scope.testDataOverMax = {}; + $scope.testDataNoQuota = {}; // Max chart is similar to Total chart data structure // but has an additional 'maxLimit' property angular.copy($scope.testDataTotal, $scope.testDataMax); @@ -68,6 +73,8 @@ $scope.testDataOverMax.data[1].value = 3; $scope.testDataOverMax.data[2].value = 0; $scope.testDataOverMax.overMax = true; + angular.copy($scope.testDataMax, $scope.testDataNoQuota); + $scope.testDataNoQuota.maxLimit = Infinity; $scope.chartSettings = { innerRadius: 24, @@ -100,6 +107,13 @@ $elementTotal = angular.element(markupTotal); $compile($elementTotal)($scope); + // Unlimited quota chart markup + var markupNoQuota = '' + + ''; + $elementNoQuota = angular.element(markupNoQuota); + $compile($elementNoQuota)($scope); + $scope.$apply(); })); @@ -127,6 +141,10 @@ expect($elementTotal.find('svg').length).toBe(1); }); + it('Unlimited quota chart should have no svg element', function () { + expect($elementNoQuota.find('svg').length).toBe(0); + }); + it('Max chart should have 3 path elements', function () { expect($elementMax.find('path.slice').length).toBe(3); }); @@ -175,6 +193,11 @@ expect(title).toBe('Total Instances (8 Total)'); }); + it('Unlimited Quota chart should have title "Total Instances (no quota)"', function () { + var title = $elementNoQuota.find('.pie-chart-title').text().trim(); + expect(title).toBe('Total Instances (no quota)'); + }); + it('Max chart should have a legend', function () { expect($elementMax.find('.pie-chart-legend').length).toBe(1); }); @@ -183,6 +206,10 @@ expect($elementOverMax.find('.pie-chart-legend').length).toBe(1); }); + it('Unlimited quotachart should have a legend', function () { + expect($elementNoQuota.find('.pie-chart-legend').length).toBe(1); + }); + it('Total chart should have a legend', function () { expect($elementTotal.find('.pie-chart-legend').length).toBe(1); }); @@ -193,8 +220,8 @@ var firstKeyLabel = legendKeys[0]; var secondKeyLabel = legendKeys[1]; - expect(firstKeyLabel.textContent.trim()).toBe('1 Current Usage'); - expect(secondKeyLabel.textContent.trim()).toBe('1 Added'); + expect(cleanSpaces(firstKeyLabel.textContent)).toBe('1 Current Usage'); + expect(cleanSpaces(secondKeyLabel.textContent)).toBe('1 Added'); }); it ('OverMax chart should have correct legend keys and labels', function () { @@ -203,8 +230,8 @@ var firstKeyLabel = legendKeys[0]; var secondKeyLabel = legendKeys[1]; - expect(firstKeyLabel.textContent.trim()).toBe('6 Current Usage'); - expect(secondKeyLabel.textContent.trim()).toBe('3 Added'); + expect(cleanSpaces(firstKeyLabel.textContent)).toBe('6 Current Usage'); + expect(cleanSpaces(secondKeyLabel.textContent)).toBe('3 Added'); }); it ('OverMax chart should have "danger" class', function () { @@ -218,8 +245,18 @@ var firstKeyLabel = legendKeys[0]; var secondKeyLabel = legendKeys[1]; - expect(firstKeyLabel.textContent.trim()).toBe('1 Current Usage'); - expect(secondKeyLabel.textContent.trim()).toBe('1 Added'); + expect(cleanSpaces(firstKeyLabel.textContent)).toEqual('1 Current Usage'); + expect(cleanSpaces(secondKeyLabel.textContent)).toEqual('1 Added'); + }); + + it ('Unlimited quota chart should have correct legend keys and labels', function () { + var legendKeys = $elementNoQuota.find('.pie-chart-legend .slice-legend'); + + var firstKeyLabel = legendKeys[0]; + var secondKeyLabel = legendKeys[1]; + + expect(cleanSpaces(firstKeyLabel.textContent)).toEqual('1 Current Usage'); + expect(cleanSpaces(secondKeyLabel.textContent)).toEqual('1 Added'); }); }); diff --git a/openstack_dashboard/api/rest/json_encoder.py b/openstack_dashboard/api/rest/json_encoder.py new file mode 100644 index 0000000000..73f7cd59c1 --- /dev/null +++ b/openstack_dashboard/api/rest/json_encoder.py @@ -0,0 +1,76 @@ +# Copyright (c) 2015 Mirantis, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. +import json +import json.encoder as encoder + +from django.utils.translation import ugettext_lazy as _ + + +class NaNJSONEncoder(json.JSONEncoder): + def __init__(self, nan_str='NaN', inf_str='1e+999', **kwargs): + self.nan_str = nan_str + self.inf_str = inf_str + super(NaNJSONEncoder, self).__init__(**kwargs) + + def iterencode(self, o, _one_shot=False): + """The sole purpose of defining a custom JSONEncoder class is to + override floatstr() inner function, or more specifically the + representation of NaN and +/-float('inf') values in a JSON. Although + Infinity values are not supported by JSON standard, we still can + convince Javascript JSON.parse() to create a Javascript Infinity + object if we feed a token `1e+999` to it. + """ + if self.check_circular: + markers = {} + else: + markers = None + + if self.ensure_ascii: + _encoder = encoder.encode_basestring_ascii + else: + _encoder = encoder.encode_basestring + + if self.encoding != 'utf-8': + def _encoder(o, _orig_encoder=_encoder, _encoding=self.encoding): + if isinstance(o, str): + o = o.decode(_encoding) + return _orig_encoder(o) + + def floatstr(o, allow_nan=self.allow_nan, _repr=encoder.FLOAT_REPR, + _inf=encoder.INFINITY, _neginf=-encoder.INFINITY): + # Check for specials. Note that this type of test is processor + # and/or platform-specific, so do tests which don't depend on the + # internals. + + if o != o: + text = self.nan_str + elif o == _inf: + text = self.inf_str + elif o == _neginf: + text = '-' + self.inf_str + else: + return _repr(o) + + if not allow_nan: + raise ValueError( + _("Out of range float values are not JSON compliant: %r") % + o) + + return text + + _iterencode = json.encoder._make_iterencode( + markers, self.default, _encoder, self.indent, floatstr, + self.key_separator, self.item_separator, self.sort_keys, + self.skipkeys, _one_shot) + return _iterencode(o, 0) diff --git a/openstack_dashboard/api/rest/nova.py b/openstack_dashboard/api/rest/nova.py index 7ab90c3446..66946ba040 100644 --- a/openstack_dashboard/api/rest/nova.py +++ b/openstack_dashboard/api/rest/nova.py @@ -19,6 +19,7 @@ from django.utils import http as utils_http from django.views import generic from openstack_dashboard import api +from openstack_dashboard.api.rest import json_encoder from openstack_dashboard.api.rest import urls from openstack_dashboard.api.rest import utils as rest_utils @@ -91,7 +92,7 @@ class Limits(generic.View): """ url_regex = r'nova/limits/$' - @rest_utils.ajax() + @rest_utils.ajax(json_encoder=json_encoder.NaNJSONEncoder) def get(self, request): """Get an object describing the current project limits. diff --git a/openstack_dashboard/api/rest/utils.py b/openstack_dashboard/api/rest/utils.py index da7a720187..9797037dea 100644 --- a/openstack_dashboard/api/rest/utils.py +++ b/openstack_dashboard/api/rest/utils.py @@ -49,11 +49,12 @@ class CreatedResponse(http.HttpResponse): class JSONResponse(http.HttpResponse): - def __init__(self, data, status=200): + def __init__(self, data, status=200, json_encoder=json.JSONEncoder): if status == 204: content = '' else: - content = jsonutils.dumps(data, sort_keys=settings.DEBUG) + content = jsonutils.dumps(data, sort_keys=settings.DEBUG, + cls=json_encoder) super(JSONResponse, self).__init__( status=status, @@ -62,7 +63,8 @@ class JSONResponse(http.HttpResponse): ) -def ajax(authenticated=True, data_required=False): +def ajax(authenticated=True, data_required=False, + json_encoder=json.JSONEncoder): '''Provide a decorator to wrap a view method so that it may exist in an entirely AJAX environment: @@ -116,7 +118,7 @@ def ajax(authenticated=True, data_required=False): return data elif data is None: return JSONResponse('', status=204) - return JSONResponse(data) + return JSONResponse(data, json_encoder=json_encoder) except http_errors as e: # exception was raised with a specific HTTP status if hasattr(e, 'http_status'): diff --git a/openstack_dashboard/test/api_tests/rest_util_tests.py b/openstack_dashboard/test/api_tests/rest_util_tests.py index fc72ff929d..97be934915 100644 --- a/openstack_dashboard/test/api_tests/rest_util_tests.py +++ b/openstack_dashboard/test/api_tests/rest_util_tests.py @@ -11,6 +11,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +from openstack_dashboard.api.rest import json_encoder from openstack_dashboard.api.rest import utils from openstack_dashboard.test import helpers as test @@ -166,3 +167,84 @@ class RestUtilsTestCase(test.TestCase): request) self.assertDictEqual({}, output_kwargs) self.assertDictEqual({}, output_filters) + + +class JSONEncoderTestCase(test.TestCase): + # NOTE(tsufiev): NaN numeric is "conventional" in a sense that the custom + # NaNJSONEncoder encoder translates it to the same token that the standard + # JSONEncoder encoder does + conventional_data = {'key1': 'string', 'key2': 10, 'key4': [1, 'some'], + 'key5': {'subkey': 7}, 'nanKey': float('nan')} + data_nan = float('nan') + data_inf = float('inf') + data_neginf = -float('inf') + + def test_custom_encoder_on_nan(self): + @utils.ajax(json_encoder=json_encoder.NaNJSONEncoder) + def f(self, request): + return self.data_nan + + request = self.mock_rest_request() + response = f(self, request) + request.user.is_authenticated.assert_called_once_with() + self.assertStatusCode(response, 200) + self.assertEqual(response['content-type'], 'application/json') + self.assertEqual(response.content, 'NaN') + + def test_custom_encoder_on_infinity(self): + @utils.ajax(json_encoder=json_encoder.NaNJSONEncoder) + def f(self, request): + return self.data_inf + + request = self.mock_rest_request() + response = f(self, request) + request.user.is_authenticated.assert_called_once_with() + self.assertStatusCode(response, 200) + self.assertEqual(response['content-type'], 'application/json') + self.assertEqual(response.content, '1e+999') + + def test_custom_encoder_on_negative_infinity(self): + @utils.ajax(json_encoder=json_encoder.NaNJSONEncoder) + def f(self, request): + return self.data_neginf + + request = self.mock_rest_request() + response = f(self, request) + request.user.is_authenticated.assert_called_once_with() + self.assertStatusCode(response, 200) + self.assertEqual(response['content-type'], 'application/json') + self.assertEqual(response.content, '-1e+999') + + def test_custom_encoder_yields_standard_json_for_conventional_data(self): + @utils.ajax() + def f(self, request): + return self.conventional_data + + @utils.ajax(json_encoder=json_encoder.NaNJSONEncoder) + def g(self, request): + return self.conventional_data + + request = self.mock_rest_request() + default_encoder_response = f(self, request) + custom_encoder_response = g(self, request) + + self.assertEqual(default_encoder_response.content, + custom_encoder_response.content) + + def test_custom_encoder_yields_different_json_for_enhanced_data(self): + @utils.ajax() + def f(self, request): + return dict(tuple(self.conventional_data.items()) + + (('key3', self.data_inf),)) + + @utils.ajax(json_encoder=json_encoder.NaNJSONEncoder) + def g(self, request): + return dict(tuple(self.conventional_data.items()) + + (('key3', self.data_inf),)) + + request = self.mock_rest_request() + default_encoder_response = f(self, request) + custom_encoder_response = g(self, request) + + self.assertNotEqual(default_encoder_response.content, + custom_encoder_response.content)