Merge branch 'ag-code-hotspots-finder' into 'master'

Add models, finders for code hotspots MVP

See merge request gitlab-org/gitlab!17277

db/migrate/20191004133612_create_analytics_repository_file_commits.rb

+# frozen_string_literal: true
+
+class CreateAnalyticsRepositoryFileCommits < ActiveRecord::Migration[5.2]
+  DOWNTIME = false
+
+  def change
+    create_table :analytics_repository_file_commits do |t|
+      t.references :analytics_repository_file, index: { name: 'index_analytics_repository_file_commits_file_id' }, foreign_key: { on_delete: :cascade }, null: false
+      t.references :project, index: false, foreign_key: { on_delete: :cascade }, null: false
+      t.date :committed_date, null: false
+      t.integer :commit_count, limit: 2, null: false
+    end
+
+    add_index :analytics_repository_file_commits,
+      [:project_id, :committed_date, :analytics_repository_file_id],
+      name: 'index_file_commits_on_committed_date_file_id_and_project_id',
+      unique: true
+  end
+end

db/post_migrate/20191004134055_drop_unused_analytics_repository_file_edits.rb

+# frozen_string_literal: true
+
+class DropUnusedAnalyticsRepositoryFileEdits < ActiveRecord::Migration[5.2]
+  DOWNTIME = false
+
+  def up
+    # The table was never used, there is no app code that writes or reads the table. Safe to remove.
+    drop_table :analytics_repository_file_edits
+  end
+
+  def down
+    create_table :analytics_repository_file_edits do |t|
+      t.references :project,
+        index: true,
+        foreign_key: { on_delete: :cascade }, null: false
+      t.references :analytics_repository_file,
+        index: false,
+        foreign_key: { on_delete: :cascade },
+        null: false
+      t.date :committed_date,
+        null: false
+      t.integer :num_edits,
+        null: false,
+        default: 0
+    end
+
+    add_index :analytics_repository_file_edits,
+      [:analytics_repository_file_id, :committed_date, :project_id],
+      name: 'index_file_edits_on_committed_date_file_id_and_project_id',
+      unique: true
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2019_09_30_025655) do
+ActiveRecord::Schema.define(version: 2019_10_04_134055) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_trgm"
@@ -93,13 +93,13 @@ ActiveRecord::Schema.define(version: 2019_09_30_025655) do
     t.index ["project_id"], name: "analytics_repository_languages_on_project_id"
   end
 
-  create_table "analytics_repository_file_edits", force: :cascade do |t|
-    t.bigint "project_id", null: false
+  create_table "analytics_repository_file_commits", force: :cascade do |t|
     t.bigint "analytics_repository_file_id", null: false
+    t.bigint "project_id", null: false
     t.date "committed_date", null: false
-    t.integer "num_edits", default: 0, null: false
-    t.index ["analytics_repository_file_id", "committed_date", "project_id"], name: "index_file_edits_on_committed_date_file_id_and_project_id", unique: true
-    t.index ["project_id"], name: "index_analytics_repository_file_edits_on_project_id"
+    t.integer "commit_count", limit: 2, null: false
+    t.index ["analytics_repository_file_id"], name: "index_analytics_repository_file_commits_file_id"
+    t.index ["project_id", "committed_date", "analytics_repository_file_id"], name: "index_file_commits_on_committed_date_file_id_and_project_id", unique: true
   end
 
   create_table "analytics_repository_files", force: :cascade do |t|
@@ -3894,8 +3894,8 @@ ActiveRecord::Schema.define(version: 2019_09_30_025655) do
   add_foreign_key "analytics_cycle_analytics_project_stages", "projects", on_delete: :cascade
   add_foreign_key "analytics_language_trend_repository_languages", "programming_languages", on_delete: :cascade
   add_foreign_key "analytics_language_trend_repository_languages", "projects", on_delete: :cascade
-  add_foreign_key "analytics_repository_file_edits", "analytics_repository_files", on_delete: :cascade
-  add_foreign_key "analytics_repository_file_edits", "projects", on_delete: :cascade
+  add_foreign_key "analytics_repository_file_commits", "analytics_repository_files", on_delete: :cascade
+  add_foreign_key "analytics_repository_file_commits", "projects", on_delete: :cascade
   add_foreign_key "analytics_repository_files", "projects", on_delete: :cascade
   add_foreign_key "application_settings", "namespaces", column: "custom_project_templates_group_id", on_delete: :nullify
   add_foreign_key "application_settings", "projects", column: "file_template_project_id", name: "fk_ec757bd087", on_delete: :nullify

ee/app/finders/analytics/code_analytics_finder.rb

+# frozen_string_literal: true
+
+module Analytics
+  class CodeAnalyticsFinder
+    def initialize(project:, from:, to:, file_count: nil)
+      @project = project
+      @from = from
+      @to = to
+      @file_count = file_count
+    end
+
+    def execute
+      Analytics::CodeAnalytics::RepositoryFileCommit.top_files(
+        project: @project,
+        from: @from,
+        to: @to,
+        file_count: @file_count
+      )
+    end
+  end
+end

ee/app/models/analytics/code_analytics/repository_file.rb

+# frozen_string_literal: true
+
+module Analytics
+  module CodeAnalytics
+    class RepositoryFile < ApplicationRecord
+      self.table_name = 'analytics_repository_files'
+
+      belongs_to :project
+    end
+  end
+end

ee/app/models/analytics/code_analytics/repository_file_commit.rb

+# frozen_string_literal: true
+
+module Analytics
+  module CodeAnalytics
+    class RepositoryFileCommit < ApplicationRecord
+      DEFAULT_FILE_COUNT = 100
+      MAX_FILE_COUNT = 500
+
+      TopFilesLimitError = Class.new(StandardError)
+
+      belongs_to :project
+      belongs_to :analytics_repository_file, class_name: 'Analytics::CodeAnalytics::RepositoryFile'
+
+      self.table_name = 'analytics_repository_file_commits'
+
+      def self.files_table
+        Analytics::CodeAnalytics::RepositoryFile.arel_table
+      end
+
+      def self.top_files(project:, from:, to:, file_count: DEFAULT_FILE_COUNT)
+        file_count ||= DEFAULT_FILE_COUNT
+
+        raise TopFilesLimitError if file_count > MAX_FILE_COUNT
+
+        joins(:analytics_repository_file)
+          .select(files_table[:file_path])
+          .where(project_id: project.id)
+          .where(arel_table[:committed_date].gteq(from))
+          .where(arel_table[:committed_date].lteq(to))
+          .group(files_table[:file_path])
+          .order(arel_table[:commit_count].sum)
+          .limit(file_count)
+          .sum(arel_table[:commit_count])
+      end
+
+      private_class_method :files_table
+    end
+  end
+end

ee/changelogs/unreleased/ag-code-hotspots-finder.yml

+---
+title: Add new table for recording commit counts per file
+merge_request: 17277
+author:
+type: added

ee/spec/factories/analytics/repository_file.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :analytics_repository_file, class: 'Analytics::CodeAnalytics::RepositoryFile' do
+    project
+    file_path { 'app/db/migrate/file.rb' }
+  end
+end

ee/spec/factories/analytics/repository_file_commits.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :analytics_repository_file_commit, class: 'Analytics::CodeAnalytics::RepositoryFileCommit' do
+    commit_count { 5 }
+    committed_date { Date.today }
+    project
+    analytics_repository_file
+  end
+end

ee/spec/finders/analytics/code_analytics_finder_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Analytics::CodeAnalyticsFinder do
+  describe '#execute' do
+    let_it_be(:project) { create(:project) }
+    let_it_be(:gemfile) { create(:analytics_repository_file, project: project, file_path: 'Gemfile') }
+    let_it_be(:user_model) { create(:analytics_repository_file, project: project, file_path: 'app/models/user.rb') }
+    let_it_be(:app_controller) { create(:analytics_repository_file, project: project, file_path: 'app/controllers/application_controller.rb') }
+
+    let_it_be(:date1) { Date.new(2018, 3, 5) }
+    let_it_be(:date2) { Date.new(2018, 10, 20) }
+    let_it_be(:date_outside_of_range) { Date.new(2019, 12, 1) }
+
+    let_it_be(:gemfile_commit) { create(:analytics_repository_file_commit, project: project, analytics_repository_file: gemfile, committed_date: date1, commit_count: 2) }
+    let_it_be(:gemfile_commit_other_day) { create(:analytics_repository_file_commit, project: project, analytics_repository_file: gemfile, committed_date: date2, commit_count: 1) }
+    let_it_be(:user_model_commit) { create(:analytics_repository_file_commit, project: project, analytics_repository_file: user_model, committed_date: date1, commit_count: 5) }
+    let_it_be(:controller_outside_of_range) { create(:analytics_repository_file_commit, project: project, analytics_repository_file: app_controller, committed_date: date_outside_of_range) }
+
+    let(:params) { { project: project } }
+
+    subject { described_class.new(params).execute }
+
+    context 'with no commits in the given date range' do
+      before do
+        params[:from] = 5.years.ago
+        params[:to] = 4.years.ago
+      end
+
+      it 'returns empty hash' do
+        expect(subject).to eq({})
+      end
+    end
+
+    context 'with commits in the given date range' do
+      before do
+        params[:from] = date1
+        params[:to] = date2
+      end
+
+      it 'sums up the gemfile commits' do
+        expect(subject[gemfile.file_path]).to eq(3)
+      end
+
+      it 'includes the user model commit' do
+        expect(subject[user_model.file_path]).to eq(5)
+      end
+
+      it 'verifies that the out of range record is persisted' do
+        expect(controller_outside_of_range).to be_persisted
+        expect(controller_outside_of_range.committed_date).to eq(date_outside_of_range)
+      end
+
+      it 'does not include items outside of the date range' do
+        expect(subject).not_to have_key(app_controller.file_path)
+      end
+
+      it 'orders the results by commit count' do
+        expect(subject.keys).to eq([gemfile.file_path, user_model.file_path])
+      end
+
+      context 'when `file_count` is given' do
+        before do
+          params[:file_count] = 1
+        end
+
+        it 'limits the number of files' do
+          expect(subject.size).to eq(1)
+        end
+      end
+    end
+  end
+end

ee/spec/models/analytics/code_analytics/repository_file_commit_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Analytics::CodeAnalytics::RepositoryFileCommit do
+  it { is_expected.to belong_to(:project) }
+  it { is_expected.to belong_to(:analytics_repository_file) }
+
+  describe '.top_files' do
+    let_it_be(:project) { create(:project) }
+
+    subject { described_class.top_files(project: project, from: 10.days.ago, to: Date.today) }
+
+    context 'when no records matching the query' do
+      it 'returns empty hash' do
+        expect(subject).to eq({})
+      end
+    end
+
+    context 'returns file with the commit count' do
+      let(:file) { create(:analytics_repository_file, project: project) }
+      let!(:file_commit1) { create(:analytics_repository_file_commit, { project: project, analytics_repository_file: file, committed_date: 1.day.ago, commit_count: 2 }) }
+      let!(:file_commit2) { create(:analytics_repository_file_commit, { project: project, analytics_repository_file: file, committed_date: 2.days.ago, commit_count: 2 }) }
+
+      it { expect(subject[file.file_path]).to eq(4) }
+    end
+
+    context 'when the `file_count` is higher than allowed' do
+      it 'raises error' do
+        max_files = Analytics::CodeAnalytics::RepositoryFileCommit::MAX_FILE_COUNT
+
+        expect do
+          described_class.top_files(project: project, from: 10.days.ago, to: Date.today, file_count: max_files + 1)
+        end.to raise_error(Analytics::CodeAnalytics::RepositoryFileCommit::TopFilesLimitError)
+      end
+    end
+  end
+end

ee/spec/models/analytics/code_analytics/repository_file_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Analytics::CodeAnalytics::RepositoryFile do
+  it { is_expected.to belong_to(:project) }
+end