Merge branch '15470-fix-unknown-license-not-shown' into 'master'

Fix license detection to detect all license files, not only known licenses

Fixes #15470.

See merge request !3878

CHANGELOG

@@ -4,8 +4,9 @@ v 8.8.0 (unreleased)
   - Remove future dates from contribution calendar graph.
 
 v 8.7.1 (unreleased)
-  - Use the `can?` helper instead of `current_user.can?`
-  - Fix .gitlab-ci.yml parsing issue when hidde job is a template without script definition
+  - Fix .gitlab-ci.yml parsing issue when hidde job is a template without script definition. !3849
+  - Fix license detection to detect all license files, not only known licenses. !3878
+  - Use the `can?` helper instead of `current_user.can?`. !3882
 
 v 8.7.0
   - Gitlab::GitAccess and Gitlab::GitAccessWiki are now instrumented

app/helpers/projects_helper.rb

@@ -123,6 +123,18 @@ module ProjectsHelper
     end
   end
 
+  def license_short_name(project)
+    no_license_key = project.repository.license_key.nil? ||
+      # Back-compat if cache contains 'no-license', can be removed in a few weeks
+      project.repository.license_key == 'no-license'
+
+    return 'LICENSE' if no_license_key
+
+    license = Licensee::License.new(project.repository.license_key)
+
+    license.nickname || license.name
+  end
+
   private
 
   def get_project_nav_tabs(project, current_user)
@@ -320,14 +332,6 @@ module ProjectsHelper
     @ref || @repository.try(:root_ref)
   end
 
-  def license_short_name(project)
-    license = Licensee::License.new(project.repository.license_key)
-
-    license.nickname || license.name
-  end
-
-  private
-
   def filename_path(project, filename)
     if project && blob = project.repository.send(filename)
       namespace_project_blob_path(

app/models/repository.rb

@@ -466,8 +466,8 @@ class Repository
     return nil if !exists? || empty?
 
     cache.fetch(:license_blob) do
-      if licensee_project.license
-        blob_at_branch(root_ref, licensee_project.matched_file.filename)
+      tree(:head).blobs.find do |file|
+        file.name =~ /\A(licen[sc]e|copying)(\..+|\z)/i
       end
     end
   end
@@ -476,7 +476,7 @@ class Repository
     return nil if !exists? || empty?
 
     cache.fetch(:license_key) do
-      licensee_project.license.try(:key) || 'no-license'
+      Licensee.license(path).try(:key)
     end
   end
 
@@ -959,8 +959,4 @@ class Repository
   def cache
     @cache ||= RepositoryCache.new(path_with_namespace)
   end
-
-  def licensee_project
-    @licensee_project ||= Licensee.project(path)
-  end
 end

spec/helpers/projects_helper_spec.rb

@@ -105,4 +105,30 @@ describe ProjectsHelper do
       end
     end
   end
+
+  describe '#license_short_name' do
+    let(:project) { create(:project) }
+
+    context 'when project.repository has a license_key' do
+      it 'returns the nickname of the license if present' do
+        allow(project.repository).to receive(:license_key).and_return('agpl-3.0')
+
+        expect(helper.license_short_name(project)).to eq('GNU AGPLv3')
+      end
+
+      it 'returns the name of the license if nickname is not present' do
+        allow(project.repository).to receive(:license_key).and_return('mit')
+
+        expect(helper.license_short_name(project)).to eq('MIT License')
+      end
+    end
+
+    context 'when project.repository has no license_key but a license_blob' do
+      it 'returns LICENSE' do
+        allow(project.repository).to receive(:license_key).and_return(nil)
+
+        expect(helper.license_short_name(project)).to eq('LICENSE')
+      end
+    end
+  end
 end

spec/models/repository_spec.rb

@@ -132,7 +132,6 @@ describe Repository, models: true do
         it { expect(subject.basename).to eq('a/b/c') }
       end
     end
-
   end
 
   describe '#license_blob' do
@@ -148,39 +147,18 @@ describe Repository, models: true do
       expect(repository.license_blob).to be_nil
     end
 
-    it 'favors license file with no extension' do
-      repository.commit_file(user, 'LICENSE', Licensee::License.new('mit').content, 'Add LICENSE', 'master', false)
-      repository.commit_file(user, 'LICENSE.md', Licensee::License.new('mit').content, 'Add LICENSE.md', 'master', false)
-
-      expect(repository.license_blob.name).to eq('LICENSE')
-    end
-
-    it 'favors .md file to .txt' do
-      repository.commit_file(user, 'LICENSE.md', Licensee::License.new('mit').content, 'Add LICENSE.md', 'master', false)
-      repository.commit_file(user, 'LICENSE.txt', Licensee::License.new('mit').content, 'Add LICENSE.txt', 'master', false)
-
-      expect(repository.license_blob.name).to eq('LICENSE.md')
-    end
-
-    it 'favors LICENCE to LICENSE' do
-      repository.commit_file(user, 'LICENSE', Licensee::License.new('mit').content, 'Add LICENSE', 'master', false)
-      repository.commit_file(user, 'LICENCE', Licensee::License.new('mit').content, 'Add LICENCE', 'master', false)
-
-      expect(repository.license_blob.name).to eq('LICENCE')
-    end
-
-    it 'favors LICENSE to COPYING' do
-      repository.commit_file(user, 'LICENSE', Licensee::License.new('mit').content, 'Add LICENSE', 'master', false)
-      repository.commit_file(user, 'COPYING', Licensee::License.new('mit').content, 'Add COPYING', 'master', false)
+    it 'detects license file with no recognizable open-source license content' do
+      repository.commit_file(user, 'LICENSE', 'Copyright!', 'Add LICENSE', 'master', false)
 
       expect(repository.license_blob.name).to eq('LICENSE')
     end
 
-    it 'favors LICENCE to COPYING' do
-      repository.commit_file(user, 'LICENCE', Licensee::License.new('mit').content, 'Add LICENCE', 'master', false)
-      repository.commit_file(user, 'COPYING', Licensee::License.new('mit').content, 'Add COPYING', 'master', false)
+    %w[LICENSE LICENCE LiCensE LICENSE.md LICENSE.foo COPYING COPYING.md].each do |filename|
+      it "detects '#{filename}'" do
+        repository.commit_file(user, filename, Licensee::License.new('mit').content, "Add #{filename}", 'master', false)
 
-      expect(repository.license_blob.name).to eq('LICENCE')
+        expect(repository.license_blob.name).to eq(filename)
+      end
     end
   end
 
@@ -190,8 +168,14 @@ describe Repository, models: true do
       repository.remove_file(user, 'LICENSE', 'Remove LICENSE', 'master')
     end
 
-    it 'returns "no-license" when no license is detected' do
-      expect(repository.license_key).to eq('no-license')
+    it 'returns nil when no license is detected' do
+      expect(repository.license_key).to be_nil
+    end
+
+    it 'detects license file with no recognizable open-source license content' do
+      repository.commit_file(user, 'LICENSE', 'Copyright!', 'Add LICENSE', 'master', false)
+
+      expect(repository.license_key).to be_nil
     end
 
     it 'returns the license key' do