Merge remote-tracking branch 'upstream/master' into pipeline-emails

* upstream/master: (292 commits)
  Deletes extra empty line breaking the build
  Optimize the `award_user_list` helper spec
  Fix typo and add he MWBS accronym for "Merge When Build Succeeds"
  Added missing content and improved layout
  ExpireBuildArtifactsWorker query builds table without ordering enqueuing one job per build to cleanup
  Improve the contribution and MR review guide
  Updates test in order to look for link
  Make projects API docs match parameter style
  Fix Event#reset_project_activity updates
  Update user whitelist reject message
  Call ensure_secret_token! in secret token test's before block since it would be called in an initializer.
  Add a CHANGELOG for CacheMarkdownField
  Enable CacheMarkdownField for the remaining models
  Make search results use the markdown cache columns, treating them consistently
  Use CacheMarkdownField for notes
  Add markdown cache columns to the database, but don't use them yet
  Update issue board spec
  Link to Registry docs from project settings
  Truncate long labels with ellipsis in labels page
  Improve issue load time performance by avoiding ORDER BY in find_by call
  ...

18 files changed, 566 insertions, 52 deletions

diff --git a/spec/lib/banzai/filter/html_entity_filter_spec.rb b/spec/lib/banzai/filter/html_entity_filter_spec.rb
new file mode 100644
index 00000000000..6dc4a970071
--- /dev/null
+++ b/spec/lib/banzai/filter/html_entity_filter_spec.rb
@@ -0,0 +1,14 @@
+require 'spec_helper'
+
+describe Banzai::Filter::HTMLEntityFilter, lib: true do
+  include FilterSpecHelper
+
+  let(:unescaped) { 'foo <strike attr="foo">&&&</strike>' }
+  let(:escaped) { 'foo &lt;strike attr=&quot;foo&quot;&gt;&amp;&amp;&amp;&lt;/strike&gt;' }
+
+  it 'converts common entities to their HTML-escaped equivalents' do
+    output = filter(unescaped)
+
+    expect(output).to eq(escaped)
+  end
+end
diff --git a/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb b/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb
index b1370bca833..d265d29ee86 100644
--- a/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb
+++ b/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb
@@ -6,21 +6,21 @@ describe Banzai::Filter::SyntaxHighlightFilter, lib: true do
   context "when no language is specified" do
     it "highlights as plaintext" do
       result = filter('<pre><code>def fun end</code></pre>')
-      expect(result.to_html).to eq('<pre class="code highlight js-syntax-highlight plaintext"><code>def fun end</code></pre>')
+      expect(result.to_html).to eq('<pre class="code highlight js-syntax-highlight plaintext" v-pre="true"><code>def fun end</code></pre>')
     end
   end
 
   context "when a valid language is specified" do
     it "highlights as that language" do
       result = filter('<pre><code class="ruby">def fun end</code></pre>')
-      expect(result.to_html).to eq('<pre class="code highlight js-syntax-highlight ruby"><code><span class="k">def</span> <span class="nf">fun</span> <span class="k">end</span></code></pre>')
+      expect(result.to_html).to eq('<pre class="code highlight js-syntax-highlight ruby" v-pre="true"><code><span class="k">def</span> <span class="nf">fun</span> <span class="k">end</span></code></pre>')
     end
   end
 
   context "when an invalid language is specified" do
     it "highlights as plaintext" do
       result = filter('<pre><code class="gnuplot">This is a test</code></pre>')
-      expect(result.to_html).to eq('<pre class="code highlight js-syntax-highlight plaintext"><code>This is a test</code></pre>')
+      expect(result.to_html).to eq('<pre class="code highlight js-syntax-highlight plaintext" v-pre="true"><code>This is a test</code></pre>')
     end
   end
 
@@ -31,7 +31,7 @@ describe Banzai::Filter::SyntaxHighlightFilter, lib: true do
 
     it "highlights as plaintext" do
       result = filter('<pre><code class="ruby">This is a test</code></pre>')
-      expect(result.to_html).to eq('<pre class="code highlight"><code>This is a test</code></pre>')
+      expect(result.to_html).to eq('<pre class="code highlight" v-pre="true"><code>This is a test</code></pre>')
     end
   end
 end
diff --git a/spec/lib/banzai/filter/user_reference_filter_spec.rb b/spec/lib/banzai/filter/user_reference_filter_spec.rb
index fdbdb21eac1..729e77fd43f 100644
--- a/spec/lib/banzai/filter/user_reference_filter_spec.rb
+++ b/spec/lib/banzai/filter/user_reference_filter_spec.rb
@@ -31,13 +31,16 @@ describe Banzai::Filter::UserReferenceFilter, lib: true do
     end
 
     it 'supports a special @all mention' do
+      project.team << [user, :developer]
       doc = reference_filter("Hey #{reference}", author: user)
+
       expect(doc.css('a').length).to eq 1
       expect(doc.css('a').first.attr('href'))
         .to eq urls.namespace_project_url(project.namespace, project)
     end
 
     it 'includes a data-author attribute when there is an author' do
+      project.team << [user, :developer]
       doc = reference_filter(reference, author: user)
 
       expect(doc.css('a').first.attr('data-author')).to eq(user.id.to_s)
@@ -48,6 +51,12 @@ describe Banzai::Filter::UserReferenceFilter, lib: true do
 
       expect(doc.css('a').first.has_attribute?('data-author')).to eq(false)
     end
+
+    it 'ignores reference to all when the user is not a project member' do
+      doc = reference_filter("Hey #{reference}", author: user)
+
+      expect(doc.css('a').length).to eq 0
+    end
   end
 
   context 'mentioning a user' do
diff --git a/spec/lib/banzai/note_renderer_spec.rb b/spec/lib/banzai/note_renderer_spec.rb
index 98f76f36fd5..49556074278 100644
--- a/spec/lib/banzai/note_renderer_spec.rb
+++ b/spec/lib/banzai/note_renderer_spec.rb
@@ -12,8 +12,7 @@ describe Banzai::NoteRenderer do
         with(project, user,
              requested_path: 'foo',
              project_wiki: wiki,
-             ref: 'bar',
-             pipeline: :note).
+             ref: 'bar').
         and_call_original
 
       expect_any_instance_of(Banzai::ObjectRenderer).
diff --git a/spec/lib/banzai/object_renderer_spec.rb b/spec/lib/banzai/object_renderer_spec.rb
index bcdb95250ca..f5ff236105e 100644
--- a/spec/lib/banzai/object_renderer_spec.rb
+++ b/spec/lib/banzai/object_renderer_spec.rb
@@ -4,10 +4,18 @@ describe Banzai::ObjectRenderer do
   let(:project) { create(:empty_project) }
   let(:user) { project.owner }
 
+  def fake_object(attrs = {})
+    object = double(attrs.merge("new_record?": true, "destroyed?": true))
+    allow(object).to receive(:markdown_cache_field_for).with(:note).and_return(:note_html)
+    allow(object).to receive(:banzai_render_context).with(:note).and_return(project: nil, author: nil)
+    allow(object).to receive(:update_column).with(:note_html, anything).and_return(true)
+    object
+  end
+
   describe '#render' do
     it 'renders and redacts an Array of objects' do
       renderer = described_class.new(project, user)
-      object = double(:object, note: 'hello', note_html: nil)
+      object = fake_object(note: 'hello', note_html: nil)
 
       expect(renderer).to receive(:render_objects).with([object], :note).
         and_call_original
@@ -16,7 +24,7 @@ describe Banzai::ObjectRenderer do
         with(an_instance_of(Array)).
         and_call_original
 
-      expect(object).to receive(:note_html=).with('<p>hello</p>')
+      expect(object).to receive(:redacted_note_html=).with('<p>hello</p>')
       expect(object).to receive(:user_visible_reference_count=).with(0)
 
       renderer.render([object], :note)
@@ -25,7 +33,7 @@ describe Banzai::ObjectRenderer do
 
   describe '#render_objects' do
     it 'renders an Array of objects' do
-      object = double(:object, note: 'hello')
+      object = fake_object(note: 'hello', note_html: nil)
 
       renderer = described_class.new(project, user)
 
@@ -57,49 +65,29 @@ describe Banzai::ObjectRenderer do
   end
 
   describe '#context_for' do
-    let(:object) { double(:object, note: 'hello') }
+    let(:object) { fake_object(note: 'hello') }
     let(:renderer) { described_class.new(project, user) }
 
     it 'returns a Hash' do
       expect(renderer.context_for(object, :note)).to be_an_instance_of(Hash)
     end
 
-    it 'includes the cache key' do
+    it 'includes the banzai render context for the object' do
+      expect(object).to receive(:banzai_render_context).with(:note).and_return(foo: :bar)
       context = renderer.context_for(object, :note)
-
-      expect(context[:cache_key]).to eq([object, :note])
-    end
-
-    context 'when the object responds to "author"' do
-      it 'includes the author in the context' do
-        expect(object).to receive(:author).and_return('Alice')
-
-        context = renderer.context_for(object, :note)
-
-        expect(context[:author]).to eq('Alice')
-      end
-    end
-
-    context 'when the object does not respond to "author"' do
-      it 'does not include the author in the context' do
-        context = renderer.context_for(object, :note)
-
-        expect(context.key?(:author)).to eq(false)
-      end
+      expect(context).to have_key(:foo)
+      expect(context[:foo]).to eq(:bar)
     end
   end
 
   describe '#render_attributes' do
     it 'renders the attribute of a list of objects' do
-      objects = [double(:doc, note: 'hello'), double(:doc, note: 'bye')]
-      renderer = described_class.new(project, user, pipeline: :note)
+      objects = [fake_object(note: 'hello', note_html: nil), fake_object(note: 'bye', note_html: nil)]
+      renderer = described_class.new(project, user)
 
-      expect(Banzai).to receive(:cache_collection_render).
-        with([
-          { text: 'hello', context: renderer.context_for(objects[0], :note) },
-          { text: 'bye', context: renderer.context_for(objects[1], :note) }
-        ]).
-        and_call_original
+      objects.each do |object|
+        expect(Banzai).to receive(:render_field).with(object, :note).and_call_original
+      end
 
       docs = renderer.render_attributes(objects, :note)
 
@@ -114,17 +102,13 @@ describe Banzai::ObjectRenderer do
       objects = []
       renderer = described_class.new(project, user, pipeline: :note)
 
-      expect(Banzai).to receive(:cache_collection_render).
-        with([]).
-        and_call_original
-
       expect(renderer.render_attributes(objects, :note)).to eq([])
     end
   end
 
   describe '#base_context' do
     let(:context) do
-      described_class.new(project, user, pipeline: :note).base_context
+      described_class.new(project, user, foo: :bar).base_context
     end
 
     it 'returns a Hash' do
@@ -132,7 +116,7 @@ describe Banzai::ObjectRenderer do
     end
 
     it 'includes the custom attributes' do
-      expect(context[:pipeline]).to eq(:note)
+      expect(context[:foo]).to eq(:bar)
     end
 
     it 'includes the current user' do
diff --git a/spec/lib/banzai/renderer_spec.rb b/spec/lib/banzai/renderer_spec.rb
new file mode 100644
index 00000000000..aaa6b12e67e
--- /dev/null
+++ b/spec/lib/banzai/renderer_spec.rb
@@ -0,0 +1,74 @@
+require 'spec_helper'
+
+describe Banzai::Renderer do
+  def expect_render(project = :project)
+    expected_context = { project: project }
+    expect(renderer).to receive(:cacheless_render) { :html }.with(:markdown, expected_context)
+  end
+
+  def expect_cache_update
+    expect(object).to receive(:update_column).with("field_html", :html)
+  end
+
+  def fake_object(*features)
+    markdown = :markdown if features.include?(:markdown)
+    html = :html if features.include?(:html)
+
+    object = double(
+      "object",
+      banzai_render_context: { project: :project },
+      field: markdown,
+      field_html: html
+    )
+
+    allow(object).to receive(:markdown_cache_field_for).with(:field).and_return("field_html")
+    allow(object).to receive(:new_record?).and_return(features.include?(:new))
+    allow(object).to receive(:destroyed?).and_return(features.include?(:destroyed))
+
+    object
+  end
+
+  describe "#render_field" do
+    let(:renderer) { Banzai::Renderer }
+    let(:subject) { renderer.render_field(object, :field) }
+
+    context "with an empty cache" do
+      let(:object) { fake_object(:markdown) }
+      it "caches and returns the result" do
+        expect_render
+        expect_cache_update
+        expect(subject).to eq(:html)
+      end
+    end
+
+    context "with a filled cache" do
+      let(:object) { fake_object(:markdown, :html) }
+
+      it "uses the cache" do
+        expect_render.never
+        expect_cache_update.never
+        should eq(:html)
+      end
+    end
+
+    context "new object" do
+      let(:object) { fake_object(:new, :markdown) }
+
+      it "doesn't cache the result" do
+        expect_render
+        expect_cache_update.never
+        expect(subject).to eq(:html)
+      end
+    end
+
+    context "destroyed object" do
+      let(:object) { fake_object(:destroyed, :markdown) }
+
+      it "doesn't cache the result" do
+        expect_render
+        expect_cache_update.never
+        expect(subject).to eq(:html)
+      end
+    end
+  end
+end
diff --git a/spec/lib/constraints/group_url_constrainer_spec.rb b/spec/lib/constraints/group_url_constrainer_spec.rb
new file mode 100644
index 00000000000..f0b75a664f2
--- /dev/null
+++ b/spec/lib/constraints/group_url_constrainer_spec.rb
@@ -0,0 +1,10 @@
+require 'spec_helper'
+
+describe GroupUrlConstrainer, lib: true do
+  let!(:username) { create(:group, path: 'gitlab-org') }
+
+  describe '#find_resource' do
+    it { expect(!!subject.find_resource('gitlab-org')).to be_truthy }
+    it { expect(!!subject.find_resource('gitlab-com')).to be_falsey }
+  end
+end
diff --git a/spec/lib/constraints/namespace_url_constrainer_spec.rb b/spec/lib/constraints/namespace_url_constrainer_spec.rb
new file mode 100644
index 00000000000..a5feaacb8ee
--- /dev/null
+++ b/spec/lib/constraints/namespace_url_constrainer_spec.rb
@@ -0,0 +1,25 @@
+require 'spec_helper'
+
+describe NamespaceUrlConstrainer, lib: true do
+  let!(:group) { create(:group, path: 'gitlab') }
+
+  describe '#matches?' do
+    context 'existing namespace' do
+      it { expect(subject.matches?(request '/gitlab')).to be_truthy }
+      it { expect(subject.matches?(request '/gitlab.atom')).to be_truthy }
+      it { expect(subject.matches?(request '/gitlab/')).to be_truthy }
+      it { expect(subject.matches?(request '//gitlab/')).to be_truthy }
+    end
+
+    context 'non-existing namespace' do
+      it { expect(subject.matches?(request '/gitlab-ce')).to be_falsey }
+      it { expect(subject.matches?(request '/gitlab.ce')).to be_falsey }
+      it { expect(subject.matches?(request '/g/gitlab')).to be_falsey }
+      it { expect(subject.matches?(request '/.gitlab')).to be_falsey }
+    end
+  end
+
+  def request(path)
+    OpenStruct.new(path: path)
+  end
+end
diff --git a/spec/lib/constraints/user_url_constrainer_spec.rb b/spec/lib/constraints/user_url_constrainer_spec.rb
new file mode 100644
index 00000000000..4b26692672f
--- /dev/null
+++ b/spec/lib/constraints/user_url_constrainer_spec.rb
@@ -0,0 +1,10 @@
+require 'spec_helper'
+
+describe UserUrlConstrainer, lib: true do
+  let!(:username) { create(:user, username: 'dz') }
+
+  describe '#find_resource' do
+    it { expect(!!subject.find_resource('dz')).to be_truthy }
+    it { expect(!!subject.find_resource('john')).to be_falsey }
+  end
+end
diff --git a/spec/lib/event_filter_spec.rb b/spec/lib/event_filter_spec.rb
new file mode 100644
index 00000000000..a6d8e6927e0
--- /dev/null
+++ b/spec/lib/event_filter_spec.rb
@@ -0,0 +1,49 @@
+require 'spec_helper'
+
+describe EventFilter, lib: true do
+  describe '#apply_filter' do
+    let(:source_user) { create(:user) }
+    let!(:public_project) { create(:project, :public) }
+
+    let!(:push_event) { create(:event, action: Event::PUSHED, project: public_project, target: public_project, author: source_user) }
+    let!(:merged_event) { create(:event, action: Event::MERGED, project: public_project, target: public_project, author: source_user) }
+    let!(:comments_event) { create(:event, action: Event::COMMENTED, project: public_project, target: public_project, author: source_user) }
+    let!(:joined_event) { create(:event, action: Event::JOINED, project: public_project, target: public_project, author: source_user) }
+    let!(:left_event) { create(:event, action: Event::LEFT, project: public_project, target: public_project, author: source_user) }
+
+    it 'applies push filter' do
+      events = EventFilter.new(EventFilter.push).apply_filter(Event.all)
+      expect(events).to contain_exactly(push_event)
+    end
+
+    it 'applies merged filter' do
+      events = EventFilter.new(EventFilter.merged).apply_filter(Event.all)
+      expect(events).to contain_exactly(merged_event)
+    end
+
+    it 'applies comments filter' do
+      events = EventFilter.new(EventFilter.comments).apply_filter(Event.all)
+      expect(events).to contain_exactly(comments_event)
+    end
+
+    it 'applies team filter' do
+      events = EventFilter.new(EventFilter.team).apply_filter(Event.all)
+      expect(events).to contain_exactly(joined_event, left_event)
+    end
+
+    it 'applies all filter' do
+      events = EventFilter.new(EventFilter.all).apply_filter(Event.all)
+      expect(events).to contain_exactly(push_event, merged_event, comments_event, joined_event, left_event)
+    end
+
+    it 'applies no filter' do
+      events = EventFilter.new(nil).apply_filter(Event.all)
+      expect(events).to contain_exactly(push_event, merged_event, comments_event, joined_event, left_event)
+    end
+
+    it 'applies unknown filter' do
+      events = EventFilter.new('').apply_filter(Event.all)
+      expect(events).to contain_exactly(push_event, merged_event, comments_event, joined_event, left_event)
+    end
+  end
+end
diff --git a/spec/lib/gitlab/backend/shell_spec.rb b/spec/lib/gitlab/backend/shell_spec.rb
index 07407f212aa..f826d0d1b04 100644
--- a/spec/lib/gitlab/backend/shell_spec.rb
+++ b/spec/lib/gitlab/backend/shell_spec.rb
@@ -22,15 +22,15 @@ describe Gitlab::Shell, lib: true do
 
   it { expect(gitlab_shell.url_to_repo('diaspora')).to eq(Gitlab.config.gitlab_shell.ssh_path_prefix + "diaspora.git") }
 
-  describe 'generate_and_link_secret_token' do
+  describe 'memoized secret_token' do
     let(:secret_file) { 'tmp/tests/.secret_shell_test' }
     let(:link_file) { 'tmp/tests/shell-secret-test/.gitlab_shell_secret' }
 
     before do
-      allow(Gitlab.config.gitlab_shell).to receive(:path).and_return('tmp/tests/shell-secret-test')
       allow(Gitlab.config.gitlab_shell).to receive(:secret_file).and_return(secret_file)
+      allow(Gitlab.config.gitlab_shell).to receive(:path).and_return('tmp/tests/shell-secret-test')
       FileUtils.mkdir('tmp/tests/shell-secret-test')
-      gitlab_shell.generate_and_link_secret_token
+      Gitlab::Shell.ensure_secret_token!
     end
 
     after do
@@ -39,7 +39,10 @@ describe Gitlab::Shell, lib: true do
     end
 
     it 'creates and links the secret token file' do
+      secret_token = Gitlab::Shell.secret_token
+
       expect(File.exist?(secret_file)).to be(true)
+      expect(File.read(secret_file).chomp).to eq(secret_token)
       expect(File.symlink?(link_file)).to be(true)
       expect(File.readlink(link_file)).to eq(secret_file)
     end
diff --git a/spec/lib/gitlab/badge/coverage/report_spec.rb b/spec/lib/gitlab/badge/coverage/report_spec.rb
index ab0cce6e091..1547bd3228c 100644
--- a/spec/lib/gitlab/badge/coverage/report_spec.rb
+++ b/spec/lib/gitlab/badge/coverage/report_spec.rb
@@ -100,7 +100,7 @@ describe Gitlab::Badge::Coverage::Report do
 
     create(:ci_pipeline, opts).tap do |pipeline|
       yield pipeline
-      pipeline.build_updated
+      pipeline.update_status
     end
   end
 end
diff --git a/spec/lib/gitlab/github_import/project_creator_spec.rb b/spec/lib/gitlab/github_import/project_creator_spec.rb
index ab06b7bc5bb..a73b1f4ff5d 100644
--- a/spec/lib/gitlab/github_import/project_creator_spec.rb
+++ b/spec/lib/gitlab/github_import/project_creator_spec.rb
@@ -33,7 +33,7 @@ describe Gitlab::GithubImport::ProjectCreator, lib: true do
       expect(project.import_data.credentials).to eq(user: 'asdffg', password: nil)
     end
 
-    context 'when Github project is private' do
+    context 'when GitHub project is private' do
       it 'sets project visibility to private' do
         repo.private = true
 
@@ -43,7 +43,7 @@ describe Gitlab::GithubImport::ProjectCreator, lib: true do
       end
     end
 
-    context 'when Github project is public' do
+    context 'when GitHub project is public' do
       before do
         allow_any_instance_of(ApplicationSetting).to receive(:default_project_visibility).and_return(Gitlab::VisibilityLevel::INTERNAL)
       end
@@ -56,5 +56,25 @@ describe Gitlab::GithubImport::ProjectCreator, lib: true do
         expect(project.visibility_level).to eq(Gitlab::VisibilityLevel::INTERNAL)
       end
     end
+
+    context 'when GitHub project has wiki' do
+      it 'does not create the wiki repository' do
+        allow(repo).to receive(:has_wiki?).and_return(true)
+
+        project = service.execute
+
+        expect(project.wiki.repository_exists?).to eq false
+      end
+    end
+
+    context 'when GitHub project does not have wiki' do
+      it 'creates the wiki repository' do
+        allow(repo).to receive(:has_wiki?).and_return(false)
+
+        project = service.execute
+
+        expect(project.wiki.repository_exists?).to eq true
+      end
+    end
   end
 end
diff --git a/spec/lib/gitlab/identifier_spec.rb b/spec/lib/gitlab/identifier_spec.rb
new file mode 100644
index 00000000000..47d6f1007d1
--- /dev/null
+++ b/spec/lib/gitlab/identifier_spec.rb
@@ -0,0 +1,123 @@
+require 'spec_helper'
+
+describe Gitlab::Identifier do
+  let(:identifier) do
+    Class.new { include Gitlab::Identifier }.new
+  end
+
+  let(:project) { create(:empty_project) }
+  let(:user) { create(:user) }
+  let(:key) { create(:key, user: user) }
+
+  describe '#identify' do
+    context 'without an identifier' do
+      it 'identifies the user using a commit' do
+        expect(identifier).to receive(:identify_using_commit).
+          with(project, '123')
+
+        identifier.identify('', project, '123')
+      end
+    end
+
+    context 'with a user identifier' do
+      it 'identifies the user using a user ID' do
+        expect(identifier).to receive(:identify_using_user).
+          with("user-#{user.id}")
+
+        identifier.identify("user-#{user.id}", project, '123')
+      end
+    end
+
+    context 'with an SSH key identifier' do
+      it 'identifies the user using an SSH key ID' do
+        expect(identifier).to receive(:identify_using_ssh_key).
+          with("key-#{key.id}")
+
+        identifier.identify("key-#{key.id}", project, '123')
+      end
+    end
+  end
+
+  describe '#identify_using_commit' do
+    it "returns the User for an existing commit author's Email address" do
+      commit = double(:commit, author_email: user.email)
+
+      expect(project).to receive(:commit).with('123').and_return(commit)
+
+      expect(identifier.identify_using_commit(project, '123')).to eq(user)
+    end
+
+    it 'returns nil when no user could be found' do
+      allow(project).to receive(:commit).with('123').and_return(nil)
+
+      expect(identifier.identify_using_commit(project, '123')).to be_nil
+    end
+
+    it 'returns nil when the commit does not have an author Email' do
+      commit = double(:commit, author_email: nil)
+
+      expect(project).to receive(:commit).with('123').and_return(commit)
+
+      expect(identifier.identify_using_commit(project, '123')).to be_nil
+    end
+
+    it 'caches the found users per Email' do
+      commit = double(:commit, author_email: user.email)
+
+      expect(project).to receive(:commit).with('123').twice.and_return(commit)
+      expect(User).to receive(:find_by).once.and_call_original
+
+      2.times do
+        expect(identifier.identify_using_commit(project, '123')).to eq(user)
+      end
+    end
+  end
+
+  describe '#identify_using_user' do
+    it 'returns the User for an existing ID in the identifier' do
+      found = identifier.identify_using_user("user-#{user.id}")
+
+      expect(found).to eq(user)
+    end
+
+    it 'returns nil for a non existing user ID' do
+      found = identifier.identify_using_user('user--1')
+
+      expect(found).to be_nil
+    end
+
+    it 'caches the found users per ID' do
+      expect(User).to receive(:find_by).once.and_call_original
+
+      2.times do
+        found = identifier.identify_using_user("user-#{user.id}")
+
+        expect(found).to eq(user)
+      end
+    end
+  end
+
+  describe '#identify_using_ssh_key' do
+    it 'returns the User for an existing SSH key' do
+      found = identifier.identify_using_ssh_key("key-#{key.id}")
+
+      expect(found).to eq(user)
+    end
+
+    it 'returns nil for an invalid SSH key' do
+      found = identifier.identify_using_ssh_key('key--1')
+
+      expect(found).to be_nil
+    end
+
+    it 'caches the found users per key' do
+      expect(User).to receive(:find_by_ssh_key_id).once.and_call_original
+
+      2.times do
+        found = identifier.identify_using_ssh_key("key-#{key.id}")
+
+        expect(found).to eq(user)
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb b/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb
new file mode 100644
index 00000000000..b8e7932eb4a
--- /dev/null
+++ b/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb
@@ -0,0 +1,34 @@
+require 'spec_helper'
+
+describe Gitlab::ImportExport::AttributeCleaner, lib: true do
+  let(:unsafe_hash) do
+    {
+      'service_id' => 99,
+      'moved_to_id' => 99,
+      'namespace_id' => 99,
+      'ci_id' => 99,
+      'random_project_id' => 99,
+      'random_id' => 99,
+      'milestone_id' => 99,
+      'project_id' => 99,
+      'user_id' => 99,
+      'random_id_in_the_middle' => 99,
+      'notid' => 99
+    }
+  end
+
+  let(:post_safe_hash) do
+    {
+      'project_id' => 99,
+      'user_id' => 99,
+      'random_id_in_the_middle' => 99,
+      'notid' => 99
+    }
+  end
+
+  it 'removes unwanted attributes from the hash' do
+    described_class.clean!(relation_hash: unsafe_hash)
+
+    expect(unsafe_hash).to eq(post_safe_hash)
+  end
+end
diff --git a/spec/lib/gitlab/import_export/attribute_configuration_spec.rb b/spec/lib/gitlab/import_export/attribute_configuration_spec.rb
index 2e19d590d83..ea65a5dfed1 100644
--- a/spec/lib/gitlab/import_export/attribute_configuration_spec.rb
+++ b/spec/lib/gitlab/import_export/attribute_configuration_spec.rb
@@ -26,10 +26,11 @@ describe 'Import/Export attribute configuration', lib: true do
   it 'has no new columns' do
     relation_names.each do |relation_name|
       relation_class = relation_class_for_name(relation_name)
+      relation_attributes = relation_class.new.attributes.keys
 
       expect(safe_model_attributes[relation_class.to_s]).not_to be_nil, "Expected exported class #{relation_class} to exist in safe_model_attributes"
 
-      current_attributes = parsed_attributes(relation_name, relation_class.attribute_names)
+      current_attributes = parsed_attributes(relation_name, relation_attributes)
       safe_attributes = safe_model_attributes[relation_class.to_s]
       new_attributes = current_attributes - safe_attributes
 
diff --git a/spec/lib/gitlab/import_export/relation_factory_spec.rb b/spec/lib/gitlab/import_export/relation_factory_spec.rb
new file mode 100644
index 00000000000..3aa492a8ab1
--- /dev/null
+++ b/spec/lib/gitlab/import_export/relation_factory_spec.rb
@@ -0,0 +1,125 @@
+require 'spec_helper'
+
+describe Gitlab::ImportExport::RelationFactory, lib: true do
+  let(:project) { create(:empty_project) }
+  let(:members_mapper) { double('members_mapper').as_null_object }
+  let(:user) { create(:user) }
+  let(:created_object) do
+    described_class.create(relation_sym: relation_sym,
+                           relation_hash: relation_hash,
+                           members_mapper: members_mapper,
+                           user: user,
+                           project_id: project.id)
+  end
+
+  context 'hook object' do
+    let(:relation_sym) { :hooks }
+    let(:id) { 999 }
+    let(:service_id) { 99 }
+    let(:original_project_id) { 8 }
+    let(:token) { 'secret' }
+
+    let(:relation_hash) do
+      {
+        'id' => id,
+        'url' => 'https://example.json',
+        'project_id' => original_project_id,
+        'created_at' => '2016-08-12T09:41:03.462Z',
+        'updated_at' => '2016-08-12T09:41:03.462Z',
+        'service_id' => service_id,
+        'push_events' => true,
+        'issues_events' => false,
+        'merge_requests_events' => true,
+        'tag_push_events' => false,
+        'note_events' => true,
+        'enable_ssl_verification' => true,
+        'build_events' => false,
+        'wiki_page_events' => true,
+        'token' => token
+      }
+    end
+
+    it 'does not have the original ID' do
+      expect(created_object.id).not_to eq(id)
+    end
+
+    it 'does not have the original service_id' do
+      expect(created_object.service_id).not_to eq(service_id)
+    end
+
+    it 'does not have the original project_id' do
+      expect(created_object.project_id).not_to eq(original_project_id)
+    end
+
+    it 'has the new project_id' do
+      expect(created_object.project_id).to eq(project.id)
+    end
+
+    it 'has a token' do
+      expect(created_object.token).to eq(token)
+    end
+
+    context 'original service exists' do
+      let(:service_id) { Service.create(project: project).id }
+
+      it 'does not have the original service_id' do
+        expect(created_object.service_id).not_to eq(service_id)
+      end
+    end
+  end
+
+  # Mocks an ActiveRecordish object with the dodgy columns
+  class FooModel
+    include ActiveModel::Model
+
+    def initialize(params)
+      params.each { |key, value| send("#{key}=", value) }
+    end
+
+    def values
+      instance_variables.map { |ivar| instance_variable_get(ivar) }
+    end
+  end
+
+  # `project_id`, `described_class.USER_REFERENCES`, noteable_id, target_id, and some project IDs are already
+  # re-assigned by described_class.
+  context 'Potentially hazardous foreign keys' do
+    let(:relation_sym) { :hazardous_foo_model }
+    let(:relation_hash) do
+      {
+        'service_id' => 99,
+        'moved_to_id' => 99,
+        'namespace_id' => 99,
+        'ci_id' => 99,
+        'random_project_id' => 99,
+        'random_id' => 99,
+        'milestone_id' => 99,
+        'project_id' => 99,
+        'user_id' => 99,
+      }
+    end
+
+    class HazardousFooModel < FooModel
+      attr_accessor :service_id, :moved_to_id, :namespace_id, :ci_id, :random_project_id, :random_id, :milestone_id, :project_id
+    end
+
+    it 'does not preserve any foreign key IDs' do
+      expect(created_object.values).not_to include(99)
+    end
+  end
+
+  context 'Project references' do
+    let(:relation_sym) { :project_foo_model }
+    let(:relation_hash) do
+      Gitlab::ImportExport::RelationFactory::PROJECT_REFERENCES.map { |ref| { ref => 99 } }.inject(:merge)
+    end
+
+    class ProjectFooModel < FooModel
+      attr_accessor(*Gitlab::ImportExport::RelationFactory::PROJECT_REFERENCES)
+    end
+
+    it 'does not preserve any project foreign key IDs' do
+      expect(created_object.values).not_to include(99)
+    end
+  end
+end
diff --git a/spec/lib/gitlab/redis_spec.rb b/spec/lib/gitlab/redis_spec.rb
index cb54c020b31..74ff85e132a 100644
--- a/spec/lib/gitlab/redis_spec.rb
+++ b/spec/lib/gitlab/redis_spec.rb
@@ -88,6 +88,34 @@ describe Gitlab::Redis do
     end
   end
 
+  describe '.with' do
+    before { clear_pool }
+    after { clear_pool }
+
+    context 'when running not on sidekiq workers' do
+      before { allow(Sidekiq).to receive(:server?).and_return(false) }
+
+      it 'instantiates a connection pool with size 5' do
+        expect(ConnectionPool).to receive(:new).with(size: 5).and_call_original
+
+        described_class.with { |_redis| true }
+      end
+    end
+
+    context 'when running on sidekiq workers' do
+      before do
+        allow(Sidekiq).to receive(:server?).and_return(true)
+        allow(Sidekiq).to receive(:options).and_return({ concurrency: 18 })
+      end
+
+      it 'instantiates a connection pool with a size based on the concurrency of the worker' do
+        expect(ConnectionPool).to receive(:new).with(size: 18 + 5).and_call_original
+
+        described_class.with { |_redis| true }
+      end
+    end
+  end
+
   describe '#raw_config_hash' do
     it 'returns default redis url when no config file is present' do
       expect(subject).to receive(:fetch_config) { false }
@@ -114,4 +142,10 @@ describe Gitlab::Redis do
   rescue NameError
     # raised if @_raw_config was not set; ignore
   end
+
+  def clear_pool
+    described_class.remove_instance_variable(:@pool)
+  rescue NameError
+    # raised if @pool was not set; ignore
+  end
 end