From 4a9002cee3c80a9de24f01a5eb313ff17d2e4931 Mon Sep 17 00:00:00 2001 From: rpereira2 Date: Wed, 17 Apr 2019 15:16:11 +0530 Subject: [PATCH] Inherit from BaseService Change MetricsDashboard::Service to inherit from BaseService so that it can reuse methods like initialize, success, error. --- .../projects/environments_controller.rb | 14 ++++++++++++-- lib/gitlab/metrics_dashboard/service.rb | 12 +++++------- .../projects/environments_controller_spec.rb | 4 ++-- spec/lib/gitlab/metrics_dashboard/service_spec.rb | 10 ++++++---- 4 files changed, 25 insertions(+), 15 deletions(-) diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb index 69ceeab5b99..25815b92a0f 100644 --- a/app/controllers/projects/environments_controller.rb +++ b/app/controllers/projects/environments_controller.rb @@ -162,9 +162,19 @@ class Projects::EnvironmentsController < Projects::ApplicationController respond_to do |format| format.json do - dashboard = Gitlab::MetricsDashboard::Service.new(@project).get_dashboard + result = Gitlab::MetricsDashboard::Service.new(@project).get_dashboard - render json: dashboard, status: :ok + if result[:status] == :success + render status: :ok, json: { + status: :success, + dashboard: result[:dashboard] + } + else + render status: result[:http_status] || :bad_request, json: { + message: result[:message], + status: result[:status] + } + end end end end diff --git a/lib/gitlab/metrics_dashboard/service.rb b/lib/gitlab/metrics_dashboard/service.rb index 22e1771cbea..521a3914e9e 100644 --- a/lib/gitlab/metrics_dashboard/service.rb +++ b/lib/gitlab/metrics_dashboard/service.rb @@ -3,19 +3,17 @@ # Fetches the metrics dashboard layout and supplemented the output with DB info. module Gitlab module MetricsDashboard - class Service + class Service < ::BaseService SYSTEM_DASHBOARD_NAME = 'common_metrics' SYSTEM_DASHBOARD_PATH = Rails.root.join('config', 'prometheus', "#{SYSTEM_DASHBOARD_NAME}.yml") - def initialize(project) - @project = project - end - # Returns a DB-supplemented json representation of a dashboard config file. def get_dashboard - dashboard = Rails.cache.fetch(cache_key) { system_dashboard } + dashboard_string = Rails.cache.fetch(cache_key) { system_dashboard } - process_dashboard(dashboard) + dashboard = JSON.parse(process_dashboard(dashboard_string)) + + success(dashboard: dashboard) end private diff --git a/spec/controllers/projects/environments_controller_spec.rb b/spec/controllers/projects/environments_controller_spec.rb index 02441385da0..6c3aad55622 100644 --- a/spec/controllers/projects/environments_controller_spec.rb +++ b/spec/controllers/projects/environments_controller_spec.rb @@ -479,8 +479,8 @@ describe Projects::EnvironmentsController do get :metrics_dashboard, params: environment_params(format: :json) expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to include('dashboard', 'order', 'panel_groups') - expect(json_response['panel_groups']).to all( include('group', 'priority', 'panels') ) + expect(json_response.keys).to contain_exactly('dashboard', 'status') + expect(json_response['dashboard']).to be_an_instance_of(Hash) end end end diff --git a/spec/lib/gitlab/metrics_dashboard/service_spec.rb b/spec/lib/gitlab/metrics_dashboard/service_spec.rb index 416b84687a9..f9574889314 100644 --- a/spec/lib/gitlab/metrics_dashboard/service_spec.rb +++ b/spec/lib/gitlab/metrics_dashboard/service_spec.rb @@ -7,11 +7,13 @@ describe Gitlab::MetricsDashboard::Service, :use_clean_rails_memory_store_cachin describe 'get_dashboard' do it 'returns a json representation of the environment dashboard' do - dashboard = described_class.new(project).get_dashboard - json = JSON.parse(dashboard, symbolize_names: true) + result = described_class.new(project).get_dashboard - expect(json).to include(:dashboard, :order, :panel_groups) - expect(json[:panel_groups]).to all( include(:group, :priority, :panels) ) + expect(result.keys).to contain_exactly(:dashboard, :status) + expect(result[:status]).to eq(:success) + + expect(result[:dashboard]).to include('dashboard', 'order', 'panel_groups') + expect(result[:dashboard]['panel_groups']).to all( include('group', 'priority', 'panels') ) end it 'caches the dashboard for subsequent calls' do