Merge branch '42639-move-custom-metrics-controller' into 'master'

Move metrics controller EE methods into Core

See merge request gitlab-org/gitlab!28452

app/controllers/projects/prometheus/metrics_controller.rb

@@ -20,6 +20,85 @@ module Projects
         end
       end
 
+      def validate_query
+        respond_to do |format|
+          format.json do
+            result = prometheus_adapter.query(:validate, params[:query])
+
+            if result
+              render json: result
+            else
+              head :accepted
+            end
+          end
+        end
+      end
+
+      def new
+        @metric = project.prometheus_metrics.new
+      end
+
+      def index
+        respond_to do |format|
+          format.json do
+            metrics = ::PrometheusMetricsFinder.new(
+              project: project,
+              ordered: true
+            ).execute.to_a
+
+            response = {}
+            if metrics.any?
+              response[:metrics] = ::PrometheusMetricSerializer
+                                     .new(project: project)
+                                     .represent(metrics)
+            end
+
+            render json: response
+          end
+        end
+      end
+
+      def create
+        @metric = project.prometheus_metrics.create(
+          metrics_params.to_h.symbolize_keys
+        )
+
+        if @metric.persisted?
+          redirect_to edit_project_service_path(project, ::PrometheusService),
+                      notice: _('Metric was successfully added.')
+        else
+          render 'new'
+        end
+      end
+
+      def update
+        @metric = update_metrics_service(prometheus_metric).execute
+
+        if @metric.persisted?
+          redirect_to edit_project_service_path(project, ::PrometheusService),
+                      notice: _('Metric was successfully updated.')
+        else
+          render 'edit'
+        end
+      end
+
+      def edit
+        @metric = prometheus_metric
+      end
+
+      def destroy
+        destroy_metrics_service(prometheus_metric).execute
+
+        respond_to do |format|
+          format.html do
+            redirect_to edit_project_service_path(project, ::PrometheusService), status: :see_other
+          end
+          format.json do
+            head :ok
+          end
+        end
+      end
+
       private
 
       def prometheus_adapter
@@ -29,8 +108,22 @@ module Projects
       def require_prometheus_metrics!
         render_404 unless prometheus_adapter&.can_query?
       end
+
+      def prometheus_metric
+        @prometheus_metric ||= ::PrometheusMetricsFinder.new(id: params[:id]).execute.first
       end
+
+      def update_metrics_service(metric)
+        ::Projects::Prometheus::Metrics::UpdateService.new(metric, metrics_params)
       end
-end
 
-Projects::Prometheus::MetricsController.prepend_if_ee('EE::Projects::Prometheus::MetricsController')
+      def destroy_metrics_service(metric)
+        ::Projects::Prometheus::Metrics::DestroyService.new(metric)
+      end
+
+      def metrics_params
+        params.require(:prometheus_metric).permit(:title, :query, :y_label, :unit, :legend, :group)
+      end
+    end
+  end
+end

config/routes/project.rb

@@ -356,6 +356,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
 
         resources :metrics, constraints: { id: %r{[^\/]+} }, only: [:index, :new, :create, :edit, :update, :destroy] do
           get :active_common, on: :collection
+          post :validate_query, on: :collection
         end
       end
 

ee/app/controllers/ee/projects/prometheus/metrics_controller.rb

-# frozen_string_literal: true
-
-module EE
-  module Projects
-    module Prometheus
-      module MetricsController
-        extend ActiveSupport::Concern
-
-        def validate_query
-          respond_to do |format|
-            format.json do
-              result = prometheus_adapter.query(:validate, params[:query])
-
-              if result
-                render json: result
-              else
-                head :accepted
-              end
-            end
-          end
-        end
-
-        def new
-          @metric = project.prometheus_metrics.new # rubocop:disable Gitlab/ModuleWithInstanceVariables
-        end
-
-        def index
-          respond_to do |format|
-            format.json do
-              metrics = ::PrometheusMetricsFinder.new(
-                project: project,
-                ordered: true
-              ).execute.to_a
-
-              response = {}
-              if metrics.any?
-                response[:metrics] = ::PrometheusMetricSerializer
-                                       .new(project: project)
-                                       .represent(metrics)
-              end
-
-              render json: response
-            end
-          end
-        end
-
-        def create
-          @metric = project.prometheus_metrics.create( # rubocop:disable Gitlab/ModuleWithInstanceVariables
-            metrics_params.to_h.symbolize_keys
-          )
-
-          if @metric.persisted? # rubocop:disable Gitlab/ModuleWithInstanceVariables
-            redirect_to edit_project_service_path(project, ::PrometheusService),
-                        notice: _('Metric was successfully added.')
-          else
-            render 'new'
-          end
-        end
-
-        def update
-          @metric = update_metrics_service(prometheus_metric).execute # rubocop:disable Gitlab/ModuleWithInstanceVariables
-
-          if @metric.persisted? # rubocop:disable Gitlab/ModuleWithInstanceVariables
-            redirect_to edit_project_service_path(project, ::PrometheusService),
-                        notice: _('Metric was successfully updated.')
-          else
-            render 'edit'
-          end
-        end
-
-        def edit
-          @metric = prometheus_metric # rubocop:disable Gitlab/ModuleWithInstanceVariables
-        end
-
-        def destroy
-          destroy_metrics_service(prometheus_metric).execute
-
-          respond_to do |format|
-            format.html do
-              redirect_to edit_project_service_path(project, ::PrometheusService), status: :see_other
-            end
-            format.json do
-              head :ok
-            end
-          end
-        end
-
-        private
-
-        def prometheus_metric
-          @prometheus_metric ||= ::PrometheusMetricsFinder.new(id: params[:id]).execute.first
-        end
-
-        def update_metrics_service(metric)
-          ::Projects::Prometheus::Metrics::UpdateService.new(metric, metrics_params)
-        end
-
-        def destroy_metrics_service(metric)
-          ::Projects::Prometheus::Metrics::DestroyService.new(metric)
-        end
-
-        def metrics_params
-          params.require(:prometheus_metric).permit(:title, :query, :y_label, :unit, :legend, :group)
-        end
-      end
-    end
-  end
-end

ee/config/routes/project.rb

@@ -130,12 +130,6 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
         end
       end
 
-      namespace :prometheus do
-        resources :metrics, constraints: { id: %r{[^\/]+} }, only: [] do
-          post :validate_query, on: :collection
-        end
-      end
-
       resource :tracing, only: [:show]
 
       resources :web_ide_terminals, path: :ide_terminals, only: [:create, :show], constraints: { id: /\d+/, format: :json } do

ee/spec/controllers/projects/prometheus/metrics_controller_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-describe Projects::Prometheus::MetricsController do
-  let(:user) { create(:user) }
-  let(:project) { create(:prometheus_project) }
-
-  let(:prometheus_adapter) { double('prometheus_adapter', can_query?: true) }
-
-  before do
-    allow(controller).to receive(:project).and_return(project)
-    allow(controller).to receive(:prometheus_adapter).and_return(prometheus_adapter)
-
-    project.add_maintainer(user)
-    sign_in(user)
-  end
-
-  describe 'POST #validate_query' do
-    before do
-      allow(prometheus_adapter).to receive(:query).with(:validate, query) { validation_result }
-    end
-
-    let(:query) { 'avg(metric)' }
-
-    context 'validation information is ready' do
-      let(:validation_result) { { valid: true } }
-
-      it 'validation data is returned' do
-        post :validate_query, params: project_params(format: :json, query: query)
-
-        expect(json_response).to eq('valid' => true)
-      end
-    end
-
-    context 'validation information is not ready' do
-      let(:validation_result) { nil }
-
-      it 'validation data is returned' do
-        post :validate_query, params: project_params(format: :json, query: query)
-
-        expect(response).to have_gitlab_http_status(:accepted)
-      end
-    end
-  end
-
-  describe 'GET #index' do
-    context 'with custom metric present' do
-      let!(:prometheus_metric) { create(:prometheus_metric, project: project) }
-
-      it 'returns a list of metrics' do
-        get :index, params: project_params(format: :json)
-
-        expect(response).to have_gitlab_http_status(:ok)
-        expect(response).to match_response_schema('prometheus/metrics', dir: 'ee')
-      end
-    end
-
-    context 'without custom metrics ' do
-      it 'returns an empty json' do
-        get :index, params: project_params(format: :json)
-
-        expect(response).to have_gitlab_http_status(:ok)
-        expect(json_response).to eq({})
-      end
-    end
-  end
-
-  describe 'POST #create' do
-    context 'metric is valid' do
-      let(:valid_metric) { { prometheus_metric: { title: 'title', query: 'query', group: 'business', y_label: 'label', unit: 'u', legend: 'legend' } } }
-
-      it 'shows a success flash message' do
-        post :create, params: project_params(valid_metric)
-
-        expect(flash[:notice]).to include('Metric was successfully added.')
-
-        expect(response).to redirect_to(edit_project_service_path(project, PrometheusService))
-      end
-    end
-
-    context 'metric is invalid' do
-      let(:invalid_metric) { { prometheus_metric: { title: 'title' } } }
-
-      it 'renders new metric page' do
-        post :create, params: project_params(invalid_metric)
-
-        expect(response).to have_gitlab_http_status(:ok)
-        expect(response).to render_template('new')
-      end
-    end
-  end
-
-  describe 'DELETE #destroy' do
-    context 'format html' do
-      let!(:metric) { create(:prometheus_metric, project: project) }
-
-      it 'destroys the metric' do
-        delete :destroy, params: project_params(id: metric.id)
-
-        expect(response).to redirect_to(edit_project_service_path(project, PrometheusService))
-        expect(PrometheusMetric.find_by(id: metric.id)).to be_nil
-      end
-    end
-
-    context 'format json' do
-      let!(:metric) { create(:prometheus_metric, project: project) }
-
-      it 'destroys the metric' do
-        delete :destroy, params: project_params(id: metric.id, format: :json)
-
-        expect(response).to have_gitlab_http_status(:ok)
-        expect(PrometheusMetric.find_by(id: metric.id)).to be_nil
-      end
-    end
-  end
-
-  def project_params(opts = {})
-    opts.reverse_merge(namespace_id: project.namespace, project_id: project)
-  end
-end

spec/controllers/projects/prometheus/metrics_controller_spec.rb

@@ -3,8 +3,8 @@
 require 'spec_helper'
 
 describe Projects::Prometheus::MetricsController do
-  let(:user) { create(:user) }
-  let(:project) { create(:project) }
+  let_it_be(:user) { create(:user) }
+  let_it_be(:project) { create(:prometheus_project) }
 
   let(:prometheus_adapter) { double('prometheus_adapter', can_query?: true) }
 
@@ -71,6 +71,8 @@ describe Projects::Prometheus::MetricsController do
     end
 
     context 'when prometheus_adapter is disabled' do
+      let(:project) { create(:project) }
+
       it 'renders 404' do
         get :active_common, params: project_params(format: :json)
 
@@ -79,6 +81,106 @@ describe Projects::Prometheus::MetricsController do
     end
   end
 
+  describe 'POST #validate_query' do
+    before do
+      allow(controller).to receive(:prometheus_adapter).and_return(prometheus_adapter)
+      allow(prometheus_adapter).to receive(:query).with(:validate, query) { validation_result }
+    end
+
+    let(:query) { 'avg(metric)' }
+
+    context 'validation information is ready' do
+      let(:validation_result) { { valid: true } }
+
+      it 'validation data is returned' do
+        post :validate_query, params: project_params(format: :json, query: query)
+
+        expect(json_response).to eq('valid' => true)
+      end
+    end
+
+    context 'validation information is not ready' do
+      let(:validation_result) { nil }
+
+      it 'validation data is returned' do
+        post :validate_query, params: project_params(format: :json, query: query)
+
+        expect(response).to have_gitlab_http_status(:accepted)
+      end
+    end
+  end
+
+  describe 'GET #index' do
+    context 'with custom metric present' do
+      let!(:prometheus_metric) { create(:prometheus_metric, project: project) }
+
+      it 'returns a list of metrics' do
+        get :index, params: project_params(format: :json)
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(response).to match_response_schema('prometheus/metrics')
+      end
+    end
+
+    context 'without custom metrics ' do
+      it 'returns an empty json' do
+        get :index, params: project_params(format: :json)
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(json_response).to eq({})
+      end
+    end
+  end
+
+  describe 'POST #create' do
+    context 'metric is valid' do
+      let(:valid_metric) { { prometheus_metric: { title: 'title', query: 'query', group: 'business', y_label: 'label', unit: 'u', legend: 'legend' } } }
+
+      it 'shows a success flash message' do
+        post :create, params: project_params(valid_metric)
+
+        expect(flash[:notice]).to include('Metric was successfully added.')
+
+        expect(response).to redirect_to(edit_project_service_path(project, PrometheusService))
+      end
+    end
+
+    context 'metric is invalid' do
+      let(:invalid_metric) { { prometheus_metric: { title: 'title' } } }
+
+      it 'renders new metric page' do
+        post :create, params: project_params(invalid_metric)
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(response).to render_template('new')
+      end
+    end
+  end
+
+  describe 'DELETE #destroy' do
+    context 'format html' do
+      let!(:metric) { create(:prometheus_metric, project: project) }
+
+      it 'destroys the metric' do
+        delete :destroy, params: project_params(id: metric.id)
+
+        expect(response).to redirect_to(edit_project_service_path(project, PrometheusService))
+        expect(PrometheusMetric.find_by(id: metric.id)).to be_nil
+      end
+    end
+
+    context 'format json' do
+      let!(:metric) { create(:prometheus_metric, project: project) }
+
+      it 'destroys the metric' do
+        delete :destroy, params: project_params(id: metric.id, format: :json)
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(PrometheusMetric.find_by(id: metric.id)).to be_nil
+      end
+    end
+  end
+
   describe '#prometheus_adapter' do
     before do
       allow(controller).to receive(:project).and_return(project)