From 4acf107b1e09e96f9c1f0d20b024540229e76e55 Mon Sep 17 00:00:00 2001 From: Igor Drozdov Date: Wed, 20 Mar 2019 20:23:23 +0300 Subject: Return cached languages if they've been detected before --- app/controllers/projects/graphs_controller.rb | 8 +--- .../detect_repository_languages_service.rb | 10 ++++- .../projects/repository_languages_service.rb | 24 +++++++++++ .../security-id-potential-denial-languages.yml | 5 +++ ...dd_detected_repository_languages_to_projects.rb | 12 ++++++ db/schema.rb | 3 +- lib/api/projects.rb | 8 ++-- lib/gitlab/import_export/import_export.yml | 1 + .../controllers/projects/graphs_controller_spec.rb | 1 + spec/features/projects/graph_spec.rb | 2 + spec/requests/api/projects_spec.rb | 16 +++++--- .../detect_repository_languages_service_spec.rb | 10 +++++ .../projects/repository_languages_service_spec.rb | 48 ++++++++++++++++++++++ 13 files changed, 130 insertions(+), 18 deletions(-) create mode 100644 app/services/projects/repository_languages_service.rb create mode 100644 changelogs/unreleased/security-id-potential-denial-languages.yml create mode 100644 db/migrate/20190312071108_add_detected_repository_languages_to_projects.rb create mode 100644 spec/services/projects/repository_languages_service_spec.rb diff --git a/app/controllers/projects/graphs_controller.rb b/app/controllers/projects/graphs_controller.rb index c80fce513f6..67d3f49af18 100644 --- a/app/controllers/projects/graphs_controller.rb +++ b/app/controllers/projects/graphs_controller.rb @@ -46,12 +46,8 @@ class Projects::GraphsController < Projects::ApplicationController def get_languages @languages = - if @project.repository_languages.present? - @project.repository_languages.map do |lang| - { value: lang.share, label: lang.name, color: lang.color, highlight: lang.color } - end - else - @project.repository.languages + ::Projects::RepositoryLanguagesService.new(@project, current_user).execute.map do |lang| + { value: lang.share, label: lang.name, color: lang.color, highlight: lang.color } end end diff --git a/app/services/projects/detect_repository_languages_service.rb b/app/services/projects/detect_repository_languages_service.rb index 4a837a4fb6a..b020e4d9088 100644 --- a/app/services/projects/detect_repository_languages_service.rb +++ b/app/services/projects/detect_repository_languages_service.rb @@ -2,7 +2,7 @@ module Projects class DetectRepositoryLanguagesService < BaseService - attr_reader :detected_repository_languages, :programming_languages + attr_reader :programming_languages # rubocop: disable CodeReuse/ActiveRecord def execute @@ -25,6 +25,8 @@ module Projects RepositoryLanguage.table_name, detection.insertions(matching_programming_languages) ) + + set_detected_repository_languages end project.repository_languages.reload @@ -56,5 +58,11 @@ module Projects retry end # rubocop: enable CodeReuse/ActiveRecord + + def set_detected_repository_languages + return if project.detected_repository_languages? + + project.update_column(:detected_repository_languages, true) + end end end diff --git a/app/services/projects/repository_languages_service.rb b/app/services/projects/repository_languages_service.rb new file mode 100644 index 00000000000..e75851c7da4 --- /dev/null +++ b/app/services/projects/repository_languages_service.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +module Projects + class RepositoryLanguagesService < BaseService + def execute + perform_language_detection unless project.detected_repository_languages? + persisted_repository_languages + end + + private + + def perform_language_detection + if persisted_repository_languages.blank? + ::DetectRepositoryLanguagesWorker.perform_async(project.id, current_user.id) + else + project.update_column(:detected_repository_languages, true) + end + end + + def persisted_repository_languages + project.repository_languages + end + end +end diff --git a/changelogs/unreleased/security-id-potential-denial-languages.yml b/changelogs/unreleased/security-id-potential-denial-languages.yml new file mode 100644 index 00000000000..2194ecb97dc --- /dev/null +++ b/changelogs/unreleased/security-id-potential-denial-languages.yml @@ -0,0 +1,5 @@ +--- +title: Return cached languages if they've been detected before +merge_request: +author: +type: security diff --git a/db/migrate/20190312071108_add_detected_repository_languages_to_projects.rb b/db/migrate/20190312071108_add_detected_repository_languages_to_projects.rb new file mode 100644 index 00000000000..5ce0ca19888 --- /dev/null +++ b/db/migrate/20190312071108_add_detected_repository_languages_to_projects.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddDetectedRepositoryLanguagesToProjects < ActiveRecord::Migration[5.0] + DOWNTIME = false + + def change + add_column :projects, :detected_repository_languages, :boolean + end +end diff --git a/db/schema.rb b/db/schema.rb index 59a76e21a5f..24b84f23167 100644 --- a/db/schema.rb +++ b/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: 20190301182457) do +ActiveRecord::Schema.define(version: 20190312071108) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -1738,6 +1738,7 @@ ActiveRecord::Schema.define(version: 20190301182457) do t.bigint "pool_repository_id" t.string "runners_token_encrypted" t.string "bfg_object_map" + t.boolean "detected_repository_languages" t.index ["ci_id"], name: "index_projects_on_ci_id", using: :btree t.index ["created_at"], name: "index_projects_on_created_at", using: :btree t.index ["creator_id"], name: "index_projects_on_creator_id", using: :btree diff --git a/lib/api/projects.rb b/lib/api/projects.rb index 91501ba4d36..22c90e4e83e 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -392,11 +392,9 @@ module API desc 'Get languages in project repository' get ':id/languages' do - if user_project.repository_languages.present? - user_project.repository_languages.map { |l| [l.name, l.share] }.to_h - else - user_project.repository.languages.map { |language| language.values_at(:label, :value) }.to_h - end + ::Projects::RepositoryLanguagesService + .new(user_project, current_user) + .execute.map { |lang| [lang.name, lang.share] }.to_h end desc 'Remove a project' diff --git a/lib/gitlab/import_export/import_export.yml b/lib/gitlab/import_export/import_export.yml index fa54fc17d95..af7798b959f 100644 --- a/lib/gitlab/import_export/import_export.yml +++ b/lib/gitlab/import_export/import_export.yml @@ -117,6 +117,7 @@ excluded_attributes: - :description_html - :repository_languages - :bfg_object_map + - :detected_repository_languages namespaces: - :runners_token - :runners_token_encrypted diff --git a/spec/controllers/projects/graphs_controller_spec.rb b/spec/controllers/projects/graphs_controller_spec.rb index 8decd8f1382..df6a6e00f73 100644 --- a/spec/controllers/projects/graphs_controller_spec.rb +++ b/spec/controllers/projects/graphs_controller_spec.rb @@ -27,6 +27,7 @@ describe Projects::GraphsController do describe 'charts' do context 'when languages were previously detected' do + let(:project) { create(:project, :repository, detected_repository_languages: true) } let!(:repository_language) { create(:repository_language, project: project) } it 'sets the languages properly' do diff --git a/spec/features/projects/graph_spec.rb b/spec/features/projects/graph_spec.rb index 9665f1755d6..e1bc18519a2 100644 --- a/spec/features/projects/graph_spec.rb +++ b/spec/features/projects/graph_spec.rb @@ -6,6 +6,8 @@ describe 'Project Graph', :js do let(:branch_name) { 'master' } before do + ::Projects::DetectRepositoryLanguagesService.new(project, user).execute + project.add_maintainer(user) sign_in(user) diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 60d9d7fed13..fdbb78b8829 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -13,12 +13,18 @@ shared_examples 'languages and percentages JSON response' do ) end - it 'returns expected language values' do - get api("/projects/#{project.id}/languages", user) + context "when the languages haven't been detected yet" do + it 'returns expected language values' do + get api("/projects/#{project.id}/languages", user) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq({}) - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to eq(expected_languages) - expect(json_response.count).to be > 1 + get api("/projects/#{project.id}/languages", user) + + expect(response).to have_gitlab_http_status(:ok) + expect(JSON.parse(response.body)).to eq(expected_languages) + end end context 'when the languages were detected before' do diff --git a/spec/services/projects/detect_repository_languages_service_spec.rb b/spec/services/projects/detect_repository_languages_service_spec.rb index deea1189cdf..b38bd62c9f0 100644 --- a/spec/services/projects/detect_repository_languages_service_spec.rb +++ b/spec/services/projects/detect_repository_languages_service_spec.rb @@ -19,6 +19,10 @@ describe Projects::DetectRepositoryLanguagesService, :clean_gitlab_redis_shared_ expect(names).to eq(%w[Ruby JavaScript HTML CoffeeScript]) end + + it 'updates detected_repository_languages flag' do + expect { subject.execute }.to change(project, :detected_repository_languages).to(true) + end end context 'with a previous detection' do @@ -36,6 +40,12 @@ describe Projects::DetectRepositoryLanguagesService, :clean_gitlab_redis_shared_ expect(repository_languages).to eq(%w[Ruby D]) end + + it "doesn't touch detected_repository_languages flag" do + expect(project).not_to receive(:update_column).with(:detected_repository_languages, true) + + subject.execute + end end context 'when no repository exists' do diff --git a/spec/services/projects/repository_languages_service_spec.rb b/spec/services/projects/repository_languages_service_spec.rb new file mode 100644 index 00000000000..61c1b8c5ec1 --- /dev/null +++ b/spec/services/projects/repository_languages_service_spec.rb @@ -0,0 +1,48 @@ +require 'spec_helper' + +describe Projects::RepositoryLanguagesService do + let(:service) { described_class.new(project, project.owner) } + + context 'when detected_repository_languages flag is set' do + let(:project) { create(:project) } + + context 'when a project is without detected programming languages' do + it 'schedules a worker and returns the empty result' do + expect(::DetectRepositoryLanguagesWorker).to receive(:perform_async).with(project.id, project.owner.id) + expect(service.execute).to eq([]) + end + end + + context 'when a project is with detected programming languages' do + let!(:repository_language) { create(:repository_language, project: project) } + + it 'does not schedule a worker and returns the detected languages' do + expect(::DetectRepositoryLanguagesWorker).not_to receive(:perform_async).with(project.id, project.owner.id) + + languages = service.execute + + expect(languages.size).to eq(1) + expect(languages.last.attributes.values).to eq( + [project.id, repository_language.programming_language_id, repository_language.share] + ) + end + + it 'sets detected_repository_languages flag' do + expect { service.execute }.to change(project, :detected_repository_languages).from(nil).to(true) + end + end + end + + context 'when detected_repository_languages flag is not set' do + let!(:repository_language) { create(:repository_language, project: project) } + let(:project) { create(:project, detected_repository_languages: true) } + let(:languages) { service.execute } + + it 'returns repository languages' do + expect(languages.size).to eq(1) + expect(languages.last.attributes.values).to eq( + [project.id, repository_language.programming_language_id, repository_language.share] + ) + end + end +end -- cgit v1.2.3