Merge branch 'security-id-potential-denial-languages' into 'master'

Return cached languages if they've been detected before

See merge request gitlab/gitlabhq!2998

13 files changed, 130 insertions, 18 deletions

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 dda0445e3f2..eee38651b33 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"
@@ -1739,6 +1739,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 a0aab9fcbaf..d5a291dfa78 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
     - :tag_list
   namespaces:
     - :runners_token
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