Merge branch 'rpereira2/cache-only-custom-dashboard-paths' into 'master'

Cache only user defined metrics  dashboard paths

See merge request gitlab-org/gitlab!37689

app/helpers/environments_helper.rb

@@ -111,7 +111,7 @@ module EnvironmentsHelper
       'empty-no-data-svg-path'           => image_path('illustrations/monitoring/no_data.svg'),
       'empty-no-data-small-svg-path'     => image_path('illustrations/chart-empty-state-small.svg'),
       'empty-unable-to-connect-svg-path' => image_path('illustrations/monitoring/unable_to_connect.svg'),
-      'custom-dashboard-base-path'       => Metrics::Dashboard::CustomDashboardService::DASHBOARD_ROOT
+      'custom-dashboard-base-path'       => Gitlab::Metrics::Dashboard::RepoDashboardFinder::DASHBOARD_ROOT
     }
   end
 end

app/models/repository.rb

@@ -43,7 +43,7 @@ class Repository
                       gitlab_ci_yml branch_names tag_names branch_count
                       tag_count avatar exists? root_ref merged_branch_names
                       has_visible_content? issue_template_names merge_request_template_names
-                      metrics_dashboard_paths xcode_project?).freeze
+                      user_defined_metrics_dashboard_paths xcode_project?).freeze
 
   # Methods that use cache_method but only memoize the value
   MEMOIZED_CACHED_METHODS = %i(license).freeze
@@ -61,7 +61,7 @@ class Repository
     avatar: :avatar,
     issue_template: :issue_template_names,
     merge_request_template: :merge_request_template_names,
-    metrics_dashboard: :metrics_dashboard_paths,
+    metrics_dashboard: :user_defined_metrics_dashboard_paths,
     xcode_config: :xcode_project?
   }.freeze
 
@@ -576,10 +576,10 @@ class Repository
   end
   cache_method :merge_request_template_names, fallback: []
 
-  def metrics_dashboard_paths
-    Gitlab::Metrics::Dashboard::Finder.find_all_paths_from_source(project)
+  def user_defined_metrics_dashboard_paths
+    Gitlab::Metrics::Dashboard::RepoDashboardFinder.list_dashboards(project)
   end
-  cache_method :metrics_dashboard_paths
+  cache_method :user_defined_metrics_dashboard_paths, fallback: []
 
   def readme
     head_tree&.readme

app/services/metrics/dashboard/clone_dashboard_service.rb

@@ -9,7 +9,7 @@ module Metrics
       include Gitlab::Utils::StrongMemoize
 
       ALLOWED_FILE_TYPE = '.yml'
-      USER_DASHBOARDS_DIR = ::Metrics::Dashboard::CustomDashboardService::DASHBOARD_ROOT
+      USER_DASHBOARDS_DIR = ::Gitlab::Metrics::Dashboard::RepoDashboardFinder::DASHBOARD_ROOT
       SEQUENCES = {
         ::Metrics::Dashboard::SystemDashboardService::DASHBOARD_PATH => [
           ::Gitlab::Metrics::Dashboard::Stages::CommonMetricsInserter,

app/services/metrics/dashboard/custom_dashboard_service.rb

@@ -6,16 +6,13 @@
 module Metrics
   module Dashboard
     class CustomDashboardService < ::Metrics::Dashboard::BaseService
-      DASHBOARD_ROOT = ".gitlab/dashboards"
-
       class << self
         def valid_params?(params)
           params[:dashboard_path].present?
         end
 
         def all_dashboard_paths(project)
-          file_finder(project)
-            .list_files_for(DASHBOARD_ROOT)
+          project.repository.user_defined_metrics_dashboard_paths
             .map do |filepath|
               {
                 path: filepath,
@@ -27,13 +24,9 @@ module Metrics
             end
         end
 
-        def file_finder(project)
-          Gitlab::Template::Finders::RepoTemplateFinder.new(project, DASHBOARD_ROOT, '.yml')
-        end
-
         # Grabs the filepath after the base directory.
         def name_for_path(filepath)
-          filepath.delete_prefix("#{DASHBOARD_ROOT}/")
+          filepath.delete_prefix("#{Gitlab::Metrics::Dashboard::RepoDashboardFinder::DASHBOARD_ROOT}/")
         end
       end
 
@@ -41,7 +34,7 @@ module Metrics
 
       # Searches the project repo for a custom-defined dashboard.
       def get_raw_dashboard
-        yml = self.class.file_finder(project).read(dashboard_path)
+        yml = Gitlab::Metrics::Dashboard::RepoDashboardFinder.read_dashboard(project, dashboard_path)
 
         load_yaml(yml)
       end

app/services/metrics/dashboard/update_dashboard_service.rb

@@ -7,7 +7,7 @@ module Metrics
       include Stepable
 
       ALLOWED_FILE_TYPE = '.yml'
-      USER_DASHBOARDS_DIR = ::Metrics::Dashboard::CustomDashboardService::DASHBOARD_ROOT
+      USER_DASHBOARDS_DIR = ::Gitlab::Metrics::Dashboard::RepoDashboardFinder::DASHBOARD_ROOT
 
       steps :check_push_authorized,
         :check_branch_name,

lib/gitlab/metrics/dashboard/finder.rb

@@ -72,14 +72,6 @@ module Gitlab
           #                              display_name: String,
           #                              default: Boolean }]
           def find_all_paths(project)
-            project.repository.metrics_dashboard_paths
-          end
-
-          # Summary of all known dashboards. Used to populate repo cache.
-          # Prefer #find_all_paths.
-          def find_all_paths_from_source(project)
-            Gitlab::Metrics::Dashboard::Cache.delete_all!
-
             user_facing_dashboard_services(project).flat_map do |service|
               service.all_dashboard_paths(project)
             end

lib/gitlab/metrics/dashboard/repo_dashboard_finder.rb

+# frozen_string_literal: true
+
+# Provides methods to list and read dashboard yaml files from a project's repository.
+module Gitlab
+  module Metrics
+    module Dashboard
+      class RepoDashboardFinder
+        DASHBOARD_ROOT = ".gitlab/dashboards"
+        DASHBOARD_EXTENSION = '.yml'
+
+        class << self
+          # Returns list of all user-defined dashboard paths. Used to populate
+          # Repository model cache (Repository#user_defined_metrics_dashboard_paths).
+          # Also deletes all dashboard cache entries.
+          # @return [Array] ex) ['.gitlab/dashboards/dashboard1.yml']
+          def list_dashboards(project)
+            Gitlab::Metrics::Dashboard::Cache.delete_all!
+
+            file_finder(project).list_files_for(DASHBOARD_ROOT)
+          end
+
+          # Reads the given dashboard from repository, and returns the content as a string.
+          # @return [String]
+          def read_dashboard(project, dashboard_path)
+            file_finder(project).read(dashboard_path)
+          end
+
+          private
+
+          def file_finder(project)
+            Gitlab::Template::Finders::RepoTemplateFinder.new(project, DASHBOARD_ROOT, DASHBOARD_EXTENSION)
+          end
+        end
+      end
+    end
+  end
+end

spec/helpers/environments_helper_spec.rb

@@ -42,7 +42,7 @@ RSpec.describe EnvironmentsHelper do
         'custom-metrics-available' => 'true',
         'alerts-endpoint' => project_prometheus_alerts_path(project, environment_id: environment.id, format: :json),
         'prometheus-alerts-available' => 'true',
-        'custom-dashboard-base-path' => Metrics::Dashboard::CustomDashboardService::DASHBOARD_ROOT,
+        'custom-dashboard-base-path' => Gitlab::Metrics::Dashboard::RepoDashboardFinder::DASHBOARD_ROOT,
         'operations-settings-path' => project_settings_operations_path(project),
         'can-access-operations-settings' => 'true'
       )

spec/lib/gitlab/metrics/dashboard/repo_dashboard_finder_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Metrics::Dashboard::RepoDashboardFinder do
+  include MetricsDashboardHelpers
+
+  let_it_be(:project) { create(:project) }
+
+  describe '.list_dashboards' do
+    it 'deletes dashboard cache entries' do
+      expect(Gitlab::Metrics::Dashboard::Cache).to receive(:delete_all!).and_call_original
+
+      described_class.list_dashboards(project)
+    end
+
+    it 'returns empty array when there are no dashboards' do
+      expect(described_class.list_dashboards(project)).to eq([])
+    end
+
+    context 'when there are project dashboards available' do
+      let_it_be(:dashboard_path) { '.gitlab/dashboards/test.yml' }
+      let_it_be(:project) { project_with_dashboard(dashboard_path) }
+
+      it 'returns the dashboard list' do
+        expect(described_class.list_dashboards(project)).to contain_exactly(dashboard_path)
+      end
+    end
+  end
+
+  describe '.read_dashboard' do
+    it 'raises error when dashboard does not exist' do
+      dashboard_path = '.gitlab/dashboards/test.yml'
+
+      expect { described_class.read_dashboard(project, dashboard_path) }.to raise_error(
+        Gitlab::Metrics::Dashboard::Errors::NOT_FOUND_ERROR
+      )
+    end
+
+    context 'when there are project dashboards available' do
+      let_it_be(:dashboard_path) { '.gitlab/dashboards/test.yml' }
+      let_it_be(:project) { project_with_dashboard(dashboard_path) }
+
+      it 'reads dashboard' do
+        expect(described_class.read_dashboard(project, dashboard_path)).to eq(
+          fixture_file('lib/gitlab/metrics/dashboard/sample_dashboard.yml')
+        )
+      end
+    end
+  end
+end

spec/models/repository_spec.rb

@@ -1926,7 +1926,7 @@ RSpec.describe Repository do
         :has_visible_content?,
         :issue_template_names,
         :merge_request_template_names,
-        :metrics_dashboard_paths,
+        :user_defined_metrics_dashboard_paths,
         :xcode_project?
       ])
 

spec/services/metrics/dashboard/custom_dashboard_service_spec.rb

@@ -104,6 +104,16 @@ RSpec.describe Metrics::Dashboard::CustomDashboardService, :use_clean_rails_memo
           }]
         )
       end
+
+      it 'caches repo file list' do
+        expect(Gitlab::Metrics::Dashboard::RepoDashboardFinder).to receive(:list_dashboards)
+          .with(project)
+          .once
+          .and_call_original
+
+        described_class.all_dashboard_paths(project)
+        described_class.all_dashboard_paths(project)
+      end
     end
   end