Remove old `project` parameter, use `gl_repository` instead86-follow-up-from-use-gl_repository-if-present-when-enqueing-sidekiq-postreceive-jobs

6 files changed, 24 insertions, 44 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 6b543ec..8ad71a3 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,3 +1,6 @@
+v5.0.6
+  - Remove old `project` parameter, use `gl_repository` instead
+
 v5.0.5
   - Use gl_repository if present when enqueing Sidekiq PostReceive jobs
 
diff --git a/lib/gitlab_net.rb b/lib/gitlab_net.rb
index 66e9fd5..faf8680 100644
--- a/lib/gitlab_net.rb
+++ b/lib/gitlab_net.rb
@@ -67,11 +67,13 @@ class GitlabNet
     JSON.parse(resp.body) rescue {}
   end
 
-  def merge_request_urls(gl_repository, repo_path, changes)
+  def merge_request_urls(gl_repository, changes)
     changes = changes.join("\n") unless changes.kind_of?(String)
     changes = changes.encode('UTF-8', 'ASCII', invalid: :replace, replace: '')
-    url = "#{host_v3}/merge_request_urls?project=#{URI.escape(repo_path)}&changes=#{URI.escape(changes)}"
-    url += "&gl_repository=#{URI.escape(gl_repository)}" if gl_repository
+    url = "#{host_v3}/merge_request_urls?" \
+      "gl_repository=#{URI.escape(gl_repository)}&" \
+      "changes=#{URI.escape(changes)}"
+
     resp = get(url)
     JSON.parse(resp.body) rescue []
   end
@@ -96,9 +98,8 @@ class GitlabNet
     {}
   end
 
-  def notify_post_receive(gl_repository, repo_path)
-    params = { gl_repository: gl_repository, project: repo_path }
-    resp = post("#{host}/notify_post_receive", params)
+  def notify_post_receive(gl_repository)
+    resp = post("#{host}/notify_post_receive", gl_repository: gl_repository)
 
     resp.code == '200'
   rescue
diff --git a/lib/gitlab_post_receive.rb b/lib/gitlab_post_receive.rb
index 00a1b1b..ac2ee5e 100644
--- a/lib/gitlab_post_receive.rb
+++ b/lib/gitlab_post_receive.rb
@@ -33,11 +33,11 @@ class GitlabPostReceive
       end
 
       merge_request_urls = GitlabMetrics.measure("merge-request-urls") do
-        api.merge_request_urls(@gl_repository, @repo_path, @changes)
+        api.merge_request_urls(gl_repository, changes)
       end
       print_merge_request_links(merge_request_urls)
 
-      api.notify_post_receive(gl_repository, repo_path)
+      api.notify_post_receive(gl_repository)
     rescue GitlabNet::ApiUnreachableError
       nil
     end
@@ -107,14 +107,11 @@ class GitlabPostReceive
   def update_redis
     # Encode changes as base64 so we don't run into trouble with non-UTF-8 input.
     changes = Base64.encode64(@changes)
-    # TODO: Change to `@gl_repository` in next release.
-    # See https://gitlab.com/gitlab-org/gitlab-shell/merge_requests/130#note_28747613
-    project_identifier = @gl_repository || @repo_path
 
     queue = "#{config.redis_namespace}:queue:post_receive"
     msg = JSON.dump({
       'class' => 'PostReceive',
-      'args' => [project_identifier, @actor, changes],
+      'args' => [@gl_repository, @actor, changes],
       'jid' => @jid,
       'enqueued_at' => Time.now.to_f
     })
diff --git a/spec/gitlab_net_spec.rb b/spec/gitlab_net_spec.rb
index adf1723..a770f2a 100644
--- a/spec/gitlab_net_spec.rb
+++ b/spec/gitlab_net_spec.rb
@@ -95,20 +95,13 @@ describe GitlabNet, vcr: true do
 
   describe :merge_request_urls do
     let(:gl_repository) { "project-1" }
-    let(:repo_path) { "/path/to/my/repo.git" }
     let(:changes) { "123456 789012 refs/heads/test\n654321 210987 refs/tags/tag" }
     let(:encoded_changes) { "123456%20789012%20refs/heads/test%0A654321%20210987%20refs/tags/tag" }
 
     it "sends the given arguments as encoded URL parameters" do
-      gitlab_net.should_receive(:get).with("#{host_v3}/merge_request_urls?project=#{repo_path}&changes=#{encoded_changes}&gl_repository=#{gl_repository}")
+      gitlab_net.should_receive(:get).with("#{host_v3}/merge_request_urls?gl_repository=#{gl_repository}&changes=#{encoded_changes}")
 
-      gitlab_net.merge_request_urls(gl_repository, repo_path, changes)
-    end
-
-    it "omits the gl_repository parameter if it's nil" do
-      gitlab_net.should_receive(:get).with("#{host_v3}/merge_request_urls?project=#{repo_path}&changes=#{encoded_changes}")
-
-      gitlab_net.merge_request_urls(nil, repo_path, changes)
+      gitlab_net.merge_request_urls(gl_repository, changes)
     end
   end
 
@@ -162,21 +155,20 @@ describe GitlabNet, vcr: true do
 
   describe '#notify_post_receive' do
     let(:gl_repository) { 'project-1' }
-    let(:repo_path) { '/path/to/my/repo.git' }
     let(:params) do
-      { gl_repository: gl_repository, project: repo_path }
+      { gl_repository: gl_repository }
     end
 
     it 'sets the arguments as form parameters' do
       VCR.use_cassette('notify-post-receive') do
         Net::HTTP::Post.any_instance.should_receive(:set_form_data).with(hash_including(params))
-        gitlab_net.notify_post_receive(gl_repository, repo_path)
+        gitlab_net.notify_post_receive(gl_repository)
       end
     end
 
     it 'returns true if notification was succesful' do
       VCR.use_cassette('notify-post-receive') do
-        expect(gitlab_net.notify_post_receive(gl_repository, repo_path)).to be_true
+        expect(gitlab_net.notify_post_receive(gl_repository)).to be_true
       end
     end
   end
diff --git a/spec/gitlab_post_receive_spec.rb b/spec/gitlab_post_receive_spec.rb
index 69e19e6..7cebf29 100644
--- a/spec/gitlab_post_receive_spec.rb
+++ b/spec/gitlab_post_receive_spec.rb
@@ -19,7 +19,7 @@ describe GitlabPostReceive do
   before do
     GitlabConfig.any_instance.stub(repos_path: repository_path)
     GitlabNet.any_instance.stub(broadcast_message: { })
-    GitlabNet.any_instance.stub(:merge_request_urls).with(gl_repository, repo_path, wrongly_encoded_changes) { [] }
+    GitlabNet.any_instance.stub(:merge_request_urls).with(gl_repository, wrongly_encoded_changes) { [] }
     GitlabNet.any_instance.stub(notify_post_receive: true)
     expect(Time).to receive(:now).and_return(enqueued_at)
   end
@@ -37,7 +37,7 @@ describe GitlabPostReceive do
     context 'Without broad cast message' do
       context 'pushing new branch' do
         before do
-          GitlabNet.any_instance.stub(:merge_request_urls).with(gl_repository, repo_path, wrongly_encoded_changes) do
+          GitlabNet.any_instance.stub(:merge_request_urls).with(gl_repository, wrongly_encoded_changes) do
             [{
               "branch_name" => "new_branch",
               "url" => "http://localhost/dzaporozhets/gitlab-ci/merge_requests/new?merge_request%5Bsource_branch%5D=new_branch",
@@ -64,7 +64,7 @@ describe GitlabPostReceive do
 
       context 'pushing existing branch with merge request created' do
         before do
-          GitlabNet.any_instance.stub(:merge_request_urls).with(gl_repository, repo_path, wrongly_encoded_changes) do
+          GitlabNet.any_instance.stub(:merge_request_urls).with(gl_repository, wrongly_encoded_changes) do
             [{
               "branch_name" => "feature_branch",
               "url" => "http://localhost/dzaporozhets/gitlab-ci/merge_requests/1",
@@ -92,7 +92,7 @@ describe GitlabPostReceive do
 
     context 'show broadcast message and merge request link' do
       before do
-        GitlabNet.any_instance.stub(:merge_request_urls).with(gl_repository, repo_path, wrongly_encoded_changes) do
+        GitlabNet.any_instance.stub(:merge_request_urls).with(gl_repository, wrongly_encoded_changes) do
           [{
             "branch_name" => "new_branch",
             "url" => "http://localhost/dzaporozhets/gitlab-ci/merge_requests/new?merge_request%5Bsource_branch%5D=new_branch",
@@ -146,19 +146,6 @@ describe GitlabPostReceive do
 
         gitlab_post_receive.exec
       end
-
-      context 'when gl_repository is nil' do
-        let(:gl_repository) { nil }
-
-        it "pushes a Sidekiq job with the repository path" do
-          expect(redis_client).to receive(:rpush).with(
-            'resque:gitlab:queue:post_receive',
-             %Q/{"class":"PostReceive","args":["#{repo_path}","#{actor}",#{base64_changes.inspect}],"jid":"#{gitlab_post_receive.jid}","enqueued_at":#{enqueued_at.to_f}}/
-          ).and_return(true)
-
-          gitlab_post_receive.exec
-        end
-      end
     end
 
     context 'reference counter' do
@@ -192,7 +179,7 @@ describe GitlabPostReceive do
     context 'post_receive notification' do
       it 'calls the api to notify the execution of the hook' do
         expect_any_instance_of(GitlabNet).to receive(:notify_post_receive).
-          with(gl_repository, repo_path)
+          with(gl_repository)
 
         gitlab_post_receive.exec
       end
diff --git a/spec/vcr_cassettes/notify-post-receive.yml b/spec/vcr_cassettes/notify-post-receive.yml
index 818e423..43bd1fb 100644
--- a/spec/vcr_cassettes/notify-post-receive.yml
+++ b/spec/vcr_cassettes/notify-post-receive.yml
@@ -5,7 +5,7 @@ http_interactions:
     uri: https://dev.gitlab.org/api/v4/internal/notify_post_receive
     body:
       encoding: US-ASCII
-      string: gl_repository=project-1&repo_path=%2Fpath%2Fto%2Fmy%2Frepo.git&secret_token=a123
+      string: gl_repository=project-1&secret_token=a123
     headers:
       Accept-Encoding:
       - gzip;q=1.0,deflate;q=0.6,identity;q=0.3