Merge branch 'da-refactor-standard-vulnerability' into 'master'

Refactor StandardVulnerability to be less cryptic

See merge request gitlab-org/gitlab-ee!8794

ee/app/views/vulnerabilities/issue_description.md.erb

@@ -8,11 +8,8 @@
 <% if vulnerability.confidence.present? %>
 * Confidence: <%= vulnerability.confidence %>
 <% end %>
-<% if defined?(vulnerability.file) && vulnerability.file.present?
-     location_text = [vulnerability.file, vulnerability.line].compact.join(':')
-     location_link = [vulnerability.file, vulnerability.line].compact.join('#L')
-%>
-* Location: [<%= location_text %>](<%= location_link %>)
+<% if defined?(vulnerability.file) && vulnerability.file.present? %>
+* Location: [<%= vulnerability.location_text %>](<%= vulnerability.location_link %>)
 <% end %>
 
 <% if vulnerability.solution.present? %>

ee/lib/gitlab/vulnerabilities/standard_vulnerability.rb

+# frozen_string_literal: true
+
 module Gitlab
   module Vulnerabilities
     class StandardVulnerability < BaseVulnerability
@@ -20,11 +22,23 @@ module Gitlab
       end
 
       def file
-        @data[:file].presence || @data[:location]&.[](:file)
+        @data[:file].presence || @data.dig(:location, :file)
       end
 
       def line
-        @data[:line].presence || @data[:location]&.[](:start_line)
+        @data[:line].presence || @data.dig(:location, :start_line)
+      end
+
+      def location_text
+        return file unless line
+
+        "#{file}:#{line}"
+      end
+
+      def location_link
+        return file unless line
+
+        "#{file}#L#{line}"
       end
     end
   end

ee/spec/lib/gitlab/vulnerabilities/standard_vulnerability_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Vulnerabilities::StandardVulnerability do
+  let(:title) { 'Predictable pseudorandom number generator' }
+  let(:description) { 'Description of Predictable pseudorandom number generator' }
+  let(:severity) { 'Low' }
+  let(:confidence) { 'High' }
+  let(:solution) { 'Please do something!' }
+  let(:file) { 'subdir/src/main/java/com/gitlab/security_products/tests/App.java' }
+  let(:line) { 15 }
+
+  let(:location) do
+    { file: file, start_line: line }
+  end
+
+  let(:identifiers) do
+    [{
+      type: 'CVE',
+      name: 'CVE-2017-15650',
+      value: 'CVE-2017-15650',
+      url: 'https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-15650'
+    }]
+  end
+
+  let(:links) do
+    [
+      { name: 'Awesome-security blog post', url: 'https;//example.com/blog-post' },
+      { url: 'https://example.com/another-link' }
+    ]
+  end
+
+  it 'inherits from Gitlab::Vulnerabilities::BaseVulnerability' do
+    vulnerability = described_class.new(foo: 'bar')
+
+    expect(vulnerability).to be_kind_of(Gitlab::Vulnerabilities::BaseVulnerability)
+  end
+
+  describe '#title' do
+    context 'when title is present' do
+      it 'returns title' do
+        vulnerability = described_class.new(title: title)
+
+        expect(vulnerability.title).to eq title
+      end
+    end
+
+    context 'when title is not set' do
+      it 'returns nil' do
+        vulnerability = described_class.new(foo: 'bar')
+
+        expect(vulnerability.title).to be_nil
+      end
+    end
+  end
+
+  describe '#severity' do
+    context 'when severity is present' do
+      it 'returns severity' do
+        vulnerability = described_class.new(severity: severity)
+
+        expect(vulnerability.severity).to eq severity
+      end
+    end
+
+    context 'when severity is not set' do
+      it 'returns nil' do
+        vulnerability = described_class.new(foo: 'bar')
+
+        expect(vulnerability.severity).to be_nil
+      end
+    end
+  end
+
+  describe '#confidence' do
+    context 'when confidence is present' do
+      it 'returns confidence' do
+        vulnerability = described_class.new(confidence: confidence)
+
+        expect(vulnerability.confidence).to eq confidence
+      end
+    end
+
+    context 'when confidence is not set' do
+      it 'returns nil' do
+        vulnerability = described_class.new(foo: 'bar')
+
+        expect(vulnerability.confidence).to be_nil
+      end
+    end
+  end
+
+  describe '#solution' do
+    context 'when solution is present' do
+      it 'returns solution' do
+        vulnerability = described_class.new(solution: solution)
+
+        expect(vulnerability.solution).to eq solution
+      end
+    end
+
+    context 'when solution is not set' do
+      it 'returns nil' do
+        vulnerability = described_class.new(foo: 'bar')
+
+        expect(vulnerability.solution).to be_nil
+      end
+    end
+  end
+
+  describe '#identifiers' do
+    context 'when identifiers is present' do
+      it 'returns identifiers' do
+        vulnerability = described_class.new(identifiers: identifiers)
+
+        expect(vulnerability.identifiers).to eq identifiers
+      end
+    end
+
+    context 'when identifiers is not set' do
+      it 'returns nil' do
+        vulnerability = described_class.new(foo: 'bar')
+
+        expect(vulnerability.identifiers).to be_nil
+      end
+    end
+  end
+
+  describe '#links' do
+    context 'when links is present' do
+      it 'returns links' do
+        vulnerability = described_class.new(links: links)
+
+        expect(vulnerability.links).to eq links
+      end
+    end
+
+    context 'when links is not set' do
+      it 'returns nil' do
+        vulnerability = described_class.new(foo: 'bar')
+
+        expect(vulnerability.links).to be_nil
+      end
+    end
+  end
+
+  describe '#description' do
+    context 'when description is present' do
+      it 'returns description' do
+        vulnerability = described_class.new(title: title, description: description)
+
+        expect(vulnerability.description).to eq description
+      end
+    end
+
+    context 'when description is not set' do
+      it 'fallbacks to title' do
+        vulnerability = described_class.new(title: title)
+
+        expect(vulnerability.description).to eq title
+      end
+    end
+
+    context 'when title and description are not set' do
+      it 'returns nil' do
+        vulnerability = described_class.new(foo: 'bar')
+
+        expect(vulnerability.description).to be_nil
+      end
+    end
+  end
+
+  describe '#file' do
+    context 'when file is present' do
+      it 'returns file' do
+        vulnerability = described_class.new(file: file, location: location)
+
+        expect(vulnerability.file).to eq file
+      end
+    end
+
+    context 'when file is not set' do
+      it 'fallbacks to location' do
+        vulnerability = described_class.new(location: location)
+
+        expect(vulnerability.file).to eq location[:file]
+      end
+    end
+
+    context 'when file and location are not set' do
+      it 'returns nil' do
+        vulnerability = described_class.new(foo: 'bar')
+
+        expect(vulnerability.file).to be_nil
+      end
+    end
+  end
+
+  describe '#line' do
+    context 'when line is present' do
+      it 'returns line' do
+        vulnerability = described_class.new(line: line, location: location)
+
+        expect(vulnerability.line).to eq line
+      end
+    end
+
+    context 'when line is not set' do
+      it 'fallbacks to location' do
+        vulnerability = described_class.new(location: location)
+
+        expect(vulnerability.line).to eq location[:start_line]
+      end
+    end
+
+    context 'when line and location are not set' do
+      it 'returns nil' do
+        vulnerability = described_class.new(foo: 'bar')
+
+        expect(vulnerability.line).to be_nil
+      end
+    end
+  end
+
+  describe '#location_text' do
+    context 'when line is nil' do
+      it 'returns a string with file' do
+        vulnerability = described_class.new(file: file)
+
+        expect(vulnerability.location_text).to eq file
+      end
+    end
+
+    context 'when line is present' do
+      it 'returns a string with file and line' do
+        vulnerability = described_class.new(file: file, line: line)
+
+        expect(vulnerability.location_text).to eq "#{file}:#{line}"
+      end
+    end
+  end
+
+  describe '#location_link' do
+    context 'when line is nil' do
+      it 'returns a string with file' do
+        vulnerability = described_class.new(file: file)
+
+        expect(vulnerability.location_link).to eq file
+      end
+    end
+
+    context 'when line is present' do
+      it 'returns a string with file and line' do
+        vulnerability = described_class.new(file: file, line: line)
+
+        expect(vulnerability.location_link).to eq "#{file}#L#{line}"
+      end
+    end
+  end
+end