Merge branch '7968-case-insensitive-match' into 'master'

Perform case insensitive diff on license names

See merge request gitlab-org/gitlab!16335

ee/app/assets/javascripts/vue_shared/license_management/store/utils.js

@@ -87,41 +87,33 @@ export const parseLicenseReportMetrics = (headMetrics, baseMetrics, managedLicen
   const baseLicenses = baseMetrics.licenses || [];
   const managedLicenseList = managedLicenses || [];
 
-  if (headLicenses.length > 0 && headDependencies.length > 0) {
-    const report = [];
-    const knownLicenses = baseLicenses.map(license => license.name);
-
-    headLicenses.forEach(license => {
-      const { name, count } = license;
-
-      if (!knownLicenses.includes(name)) {
-        const { id, approvalStatus } = getLicenseStatusByName(managedLicenseList, name);
-
-        const dependencies = getDependenciesByLicenseName(headDependencies, name);
-
-        const url =
-          (dependencies &&
-            dependencies[0] &&
-            dependencies[0].license &&
-            dependencies[0].license.url) ||
-          '';
-
-        report.push({
-          name,
-          count,
-          url,
-          packages: dependencies.map(dependencyItem => dependencyItem.dependency),
-          status: getIssueStatusFromLicenseStatus(approvalStatus),
-          approvalStatus,
-          id,
-        });
-      }
-    });
-
-    return report.sort(byLicenseNameComparator);
-  }
+  if (!headLicenses.length && !headDependencies.length) return [];
+
+  const knownLicenses = baseLicenses.map(license => license.name.toLowerCase());
+  const identityMap = license => knownLicenses.includes(license.name.toLowerCase());
+  const mapper = license => {
+    const { name, count } = license;
+    const { id, approvalStatus } = getLicenseStatusByName(managedLicenseList, name);
+    const dependencies = getDependenciesByLicenseName(headDependencies, name);
+    const url =
+      (dependencies && dependencies[0] && dependencies[0].license && dependencies[0].license.url) ||
+      '';
+
+    return {
+      name,
+      count,
+      url,
+      packages: dependencies.map(dependencyItem => dependencyItem.dependency),
+      status: getIssueStatusFromLicenseStatus(approvalStatus),
+      approvalStatus,
+      id,
+    };
+  };
 
-  return [];
+  return headLicenses
+    .filter(license => !identityMap(license))
+    .map(mapper)
+    .sort(byLicenseNameComparator);
 };
 
 export const getPackagesString = (packages, truncate, maxPackages) => {

ee/changelogs/unreleased/7968-case-insensitive-match.yml

+---
+title: Perform case insensitive diff on license names
+merge_request: 16335
+author:
+type: fixed

ee/lib/gitlab/ci/reports/license_management/report.rb

@@ -28,6 +28,28 @@ module Gitlab
           def violates?(software_license_policies)
             software_license_policies.blacklisted.with_license_by_name(license_names).exists?
           end
+
+          def diff_with(other_report)
+            base = self.license_names.map { |name| canonicalize(name) }
+            head = other_report.license_names.map { |name| canonicalize(name) }
+
+            {
+              added: other_report.find_by_names(head - base),
+              unchanged: find_by_names(base & head),
+              removed: find_by_names(base - head)
+            }
+          end
+
+          def find_by_names(names)
+            names = names.map { |name| canonicalize(name) }
+            licenses.select { |license| names.include?(canonicalize(license.name)) }
+          end
+
+          private
+
+          def canonicalize(name)
+            name.downcase
+          end
         end
       end
     end

ee/lib/gitlab/ci/reports/license_management/reports_comparer.rb

@@ -15,23 +15,22 @@ module Gitlab
           end
 
           def new_licenses
-            strong_memoize(:new_licenses) do
-              names = @head_report.license_names - @base_report.license_names
-              @head_report.licenses.select { |license| names.include?(license.name) }
-            end
+            diff[:added]
           end
 
           def existing_licenses
-            strong_memoize(:existing_licenses) do
-              names = @base_report.license_names & @head_report.license_names
-              @head_report.licenses.select { |license| names.include?(license.name) }
-            end
+            diff[:unchanged]
           end
 
           def removed_licenses
-            strong_memoize(:removed_licenses) do
-              names = @base_report.license_names - @head_report.license_names
-              @base_report.licenses.select { |license| names.include?(license.name) }
+            diff[:removed]
+          end
+
+          private
+
+          def diff
+            strong_memoize(:diff) do
+              base_report.diff_with(head_report)
             end
           end
         end

ee/spec/javascripts/license_management/store/utils_spec.js

@@ -61,6 +61,14 @@ describe('utils', () => {
       expect(result[1].approvalStatus).toBe(blacklistedLicense.approvalStatus);
       expect(result[1].id).toBe(blacklistedLicense.id);
     });
+
+    it('matches using a case insensitive match on license name', () => {
+      const headReport = { licenses: [{ count: 1, name: 'BSD' }], dependencies: [] };
+      const baseReport = { licenses: [{ count: 1, name: 'bsd' }], dependencies: [] };
+      const result = parseLicenseReportMetrics(headReport, baseReport, []);
+
+      expect(result.length).toBe(0);
+    });
   });
 
   describe('byLicenseNameComparator', () => {

ee/spec/lib/gitlab/ci/reports/license_management/report_spec.rb

@@ -41,4 +41,23 @@ describe Gitlab::Ci::Reports::LicenseManagement::Report do
       specify { expect(subject.violates?(project.software_license_policies)).to be(false) }
     end
   end
+
+  describe "#diff_with" do
+    let(:report_1) { build(:ci_reports_license_management_report, :report_1) }
+    let(:report_2) { build(:ci_reports_license_management_report, :report_2) }
+    subject { report_1.diff_with(report_2) }
+
+    before do
+      report_1.add_dependency('BSD', 1, 'https://opensource.org/licenses/0BSD', 'Library1')
+      report_2.add_dependency('bsd', 1, 'https://opensource.org/licenses/0BSD', 'Library1')
+    end
+
+    def names_from(licenses)
+      licenses.map(&:name)
+    end
+
+    it { expect(names_from(subject[:added])).to contain_exactly('Apache 2.0') }
+    it { expect(names_from(subject[:unchanged])).to contain_exactly('MIT', 'BSD') }
+    it { expect(names_from(subject[:removed])).to contain_exactly('WTFPL') }
+  end
 end

ee/spec/lib/gitlab/ci/reports/license_management/reports_comparer_spec.rb

@@ -7,30 +7,30 @@ describe Gitlab::Ci::Reports::LicenseManagement::ReportsComparer do
   let(:report_2) { build :ci_reports_license_management_report, :report_2 }
   let(:report_comparer) { described_class.new(report_1, report_2) }
 
+  before do
+    report_1.add_dependency('BSD', 1, 'https://opensource.org/licenses/0BSD', 'Library1')
+    report_2.add_dependency('bsd', 1, 'https://opensource.org/licenses/0BSD', 'Library1')
+  end
+
+  def names_from(licenses)
+    licenses.map(&:name)
+  end
+
   describe '#new_licenses' do
     subject { report_comparer.new_licenses }
 
-    it 'reports new licenses' do
-      expect(subject.count).to eq 1
-      expect(subject[0].name).to eq 'Apache 2.0'
-    end
+    it { expect(names_from(subject)).to contain_exactly('Apache 2.0') }
   end
 
   describe '#existing_licenses' do
     subject { report_comparer.existing_licenses }
 
-    it 'reports existing licenses' do
-      expect(subject.count).to eq 1
-      expect(subject[0].name).to eq 'MIT'
-    end
+    it { expect(names_from(subject)).to contain_exactly('MIT', 'BSD') }
   end
 
   describe '#removed_licenses' do
     subject { report_comparer.removed_licenses }
 
-    it 'reports removed licenses' do
-      expect(subject.count).to eq 1
-      expect(subject[0].name).to eq 'WTFPL'
-    end
+    it { expect(names_from(subject)).to contain_exactly('WTFPL') }
   end
 end