Merge branch 'rc/fix-tags-api' into 'master'

Fix the /projects/:id/repository/tags endpoint to handle dots in the tag name…

See merge request !13368

4 files changed, 348 insertions, 160 deletions

diff --git a/spec/fixtures/api/schemas/public_api/v4/release.json b/spec/fixtures/api/schemas/public_api/v4/release.json
new file mode 100644
index 00000000000..6612c2a9911
--- /dev/null
+++ b/spec/fixtures/api/schemas/public_api/v4/release.json
@@ -0,0 +1,12 @@
+{
+  "type": "object",
+  "required" : [
+    "tag_name",
+    "description"
+  ],
+  "properties" : {
+    "tag_name": { "type": ["string", "null"] },
+    "description": { "type": "string" }
+  },
+  "additionalProperties": false
+}
diff --git a/spec/fixtures/api/schemas/public_api/v4/tag.json b/spec/fixtures/api/schemas/public_api/v4/tag.json
new file mode 100644
index 00000000000..52cfe86aeeb
--- /dev/null
+++ b/spec/fixtures/api/schemas/public_api/v4/tag.json
@@ -0,0 +1,21 @@
+{
+  "type": "object",
+  "required" : [
+    "name",
+    "message",
+    "commit",
+    "release"
+  ],
+  "properties" : {
+    "name": { "type": "string" },
+    "message": { "type": ["string", "null"] },
+    "commit": { "$ref": "commit/basic.json" },
+    "release": {
+      "oneOf": [
+        { "type": "null" },
+        { "$ref": "release.json" }
+      ]
+    }
+  },
+  "additionalProperties": false
+}
diff --git a/spec/fixtures/api/schemas/public_api/v4/tags.json b/spec/fixtures/api/schemas/public_api/v4/tags.json
new file mode 100644
index 00000000000..eae352e7f87
--- /dev/null
+++ b/spec/fixtures/api/schemas/public_api/v4/tags.json
@@ -0,0 +1,4 @@
+{
+  "type": "array",
+  "items": { "$ref": "tag.json" }
+}
diff --git a/spec/requests/api/tags_spec.rb b/spec/requests/api/tags_spec.rb
index ef7d0c3ee41..9884c1ec206 100644
--- a/spec/requests/api/tags_spec.rb
+++ b/spec/requests/api/tags_spec.rb
@@ -1,66 +1,85 @@
 require 'spec_helper'
-require 'mime/types'
 
 describe API::Tags do
-  include RepoHelpers
-
   let(:user) { create(:user) }
-  let(:user2) { create(:user) }
-  let!(:project) { create(:project, :repository, creator: user) }
-  let!(:master) { create(:project_member, :master, user: user, project: project) }
-  let!(:guest) { create(:project_member, :guest, user: user2, project: project) }
+  let(:guest) { create(:user).tap { |u| project.add_guest(u) } }
+  let(:project) { create(:project, :repository, creator: user, path: 'my.project') }
+  let(:tag_name) { project.repository.find_tag('v1.1.0').name }
 
-  describe "GET /projects/:id/repository/tags" do
-    let(:tag_name) { project.repository.tag_names.sort.reverse.first }
-    let(:description) { 'Awesome release!' }
+  let(:project_id) { project.id }
+  let(:current_user) { nil }
+
+  before do
+    project.add_master(user)
+  end
+
+  describe 'GET /projects/:id/repository/tags' do
+    let(:route) { "/projects/#{project_id}/repository/tags" }
 
     shared_examples_for 'repository tags' do
       it 'returns the repository tags' do
-        get api("/projects/#{project.id}/repository/tags", current_user)
+        get api(route, current_user)
 
-        expect(response).to have_http_status(200)
+        expect(response).to have_gitlab_http_status(200)
+        expect(response).to match_response_schema('public_api/v4/tags')
         expect(response).to include_pagination_headers
-        expect(json_response).to be_an Array
         expect(json_response.first['name']).to eq(tag_name)
       end
-    end
 
-    context 'when unauthenticated' do
-      it_behaves_like 'repository tags' do
-        let(:project) { create(:project, :public, :repository) }
-        let(:current_user) { nil }
+      context 'when repository is disabled' do
+        include_context 'disabled repository'
+
+        it_behaves_like '403 response' do
+          let(:request) { get api(route, current_user) }
+        end
       end
     end
 
-    context 'when authenticated' do
-      it_behaves_like 'repository tags' do
-        let(:current_user) { user }
+    context 'when unauthenticated', 'and project is public' do
+      let(:project) { create(:project, :public, :repository) }
+
+      it_behaves_like 'repository tags'
+    end
+
+    context 'when unauthenticated', 'and project is private' do
+      it_behaves_like '404 response' do
+        let(:request) { get api(route) }
+        let(:message) { '404 Project Not Found' }
       end
     end
 
-    context 'without releases' do
-      it "returns an array of project tags" do
-        get api("/projects/#{project.id}/repository/tags", user)
+    context 'when authenticated', 'as a master' do
+      let(:current_user) { user }
 
-        expect(response).to have_http_status(200)
-        expect(response).to include_pagination_headers
-        expect(json_response).to be_an Array
-        expect(json_response.first['name']).to eq(tag_name)
+      it_behaves_like 'repository tags'
+
+      context 'requesting with the escaped project full path' do
+        let(:project_id) { CGI.escape(project.full_path) }
+
+        it_behaves_like 'repository tags'
+      end
+    end
+
+    context 'when authenticated', 'as a guest' do
+      it_behaves_like '403 response' do
+        let(:request) { get api(route, guest) }
       end
     end
 
     context 'with releases' do
+      let(:description) { 'Awesome release!' }
+
       before do
         release = project.releases.find_or_initialize_by(tag: tag_name)
         release.update_attributes(description: description)
       end
 
-      it "returns an array of project tags with release info" do
-        get api("/projects/#{project.id}/repository/tags", user)
+      it 'returns an array of project tags with release info' do
+        get api(route, user)
 
-        expect(response).to have_http_status(200)
+        expect(response).to have_gitlab_http_status(200)
+        expect(response).to match_response_schema('public_api/v4/tags')
         expect(response).to include_pagination_headers
-        expect(json_response).to be_an Array
         expect(json_response.first['name']).to eq(tag_name)
         expect(json_response.first['message']).to eq('Version 1.1.0')
         expect(json_response.first['release']['description']).to eq(description)
@@ -69,210 +88,342 @@ describe API::Tags do
   end
 
   describe 'GET /projects/:id/repository/tags/:tag_name' do
-    let(:tag_name) { project.repository.tag_names.sort.reverse.first }
+    let(:route) { "/projects/#{project_id}/repository/tags/#{tag_name}" }
 
     shared_examples_for 'repository tag' do
-      it 'returns the repository tag' do
-        get api("/projects/#{project.id}/repository/tags/#{tag_name}", current_user)
-
-        expect(response).to have_http_status(200)
+      it 'returns the repository branch' do
+        get api(route, current_user)
 
+        expect(response).to have_gitlab_http_status(200)
+        expect(response).to match_response_schema('public_api/v4/tag')
         expect(json_response['name']).to eq(tag_name)
       end
 
-      it 'returns 404 for an invalid tag name' do
-        get api("/projects/#{project.id}/repository/tags/foobar", current_user)
+      context 'when tag does not exist' do
+        let(:tag_name) { 'unknown' }
 
-        expect(response).to have_http_status(404)
+        it_behaves_like '404 response' do
+          let(:request) { get api(route, current_user) }
+          let(:message) { '404 Tag Not Found' }
+        end
+      end
+
+      context 'when repository is disabled' do
+        include_context 'disabled repository'
+
+        it_behaves_like '403 response' do
+          let(:request) { get api(route, current_user) }
+        end
       end
     end
 
-    context 'when unauthenticated' do
-      it_behaves_like 'repository tag' do
-        let(:project) { create(:project, :public, :repository) }
-        let(:current_user) { nil }
+    context 'when unauthenticated', 'and project is public' do
+      let(:project) { create(:project, :public, :repository) }
+
+      it_behaves_like 'repository tag'
+    end
+
+    context 'when unauthenticated', 'and project is private' do
+      it_behaves_like '404 response' do
+        let(:request) { get api(route) }
+        let(:message) { '404 Project Not Found' }
       end
     end
 
-    context 'when authenticated' do
-      it_behaves_like 'repository tag' do
-        let(:current_user) { user }
+    context 'when authenticated', 'as a master' do
+      let(:current_user) { user }
+
+      it_behaves_like 'repository tag'
+
+      context 'requesting with the escaped project full path' do
+        let(:project_id) { CGI.escape(project.full_path) }
+
+        it_behaves_like 'repository tag'
+      end
+    end
+
+    context 'when authenticated', 'as a guest' do
+      it_behaves_like '403 response' do
+        let(:request) { get api(route, guest) }
       end
     end
   end
 
   describe 'POST /projects/:id/repository/tags' do
-    context 'lightweight tags' do
+    let(:tag_name) { 'new_tag' }
+    let(:route) { "/projects/#{project_id}/repository/tags" }
+
+    shared_examples_for 'repository new tag' do
       it 'creates a new tag' do
-        post api("/projects/#{project.id}/repository/tags", user),
-             tag_name: 'v7.0.1',
-             ref: 'master'
+        post api(route, current_user), tag_name: tag_name, ref: 'master'
 
-        expect(response).to have_http_status(201)
-        expect(json_response['name']).to eq('v7.0.1')
+        expect(response).to have_gitlab_http_status(201)
+        expect(response).to match_response_schema('public_api/v4/tag')
+        expect(json_response['name']).to eq(tag_name)
       end
-    end
 
-    context 'lightweight tags with release notes' do
-      it 'creates a new tag' do
-        post api("/projects/#{project.id}/repository/tags", user),
-             tag_name: 'v7.0.1',
-             ref: 'master',
-             release_description: 'Wow'
+      context 'when repository is disabled' do
+        include_context 'disabled repository'
 
-        expect(response).to have_http_status(201)
-        expect(json_response['name']).to eq('v7.0.1')
-        expect(json_response['release']['description']).to eq('Wow')
+        it_behaves_like '403 response' do
+          let(:request) { post api(route, current_user) }
+        end
       end
     end
 
-    describe 'DELETE /projects/:id/repository/tags/:tag_name' do
-      let(:tag_name) { project.repository.tag_names.sort.reverse.first }
+    context 'when unauthenticated', 'and project is private' do
+      it_behaves_like '404 response' do
+        let(:request) { post api(route) }
+        let(:message) { '404 Project Not Found' }
+      end
+    end
 
-      before do
-        allow_any_instance_of(Repository).to receive(:rm_tag).and_return(true)
+    context 'when authenticated', 'as a guest' do
+      it_behaves_like '403 response' do
+        let(:request) { post api(route, guest) }
       end
+    end
+
+    context 'when authenticated', 'as a master' do
+      let(:current_user) { user }
+
+      context "when a protected branch doesn't already exist" do
+        it_behaves_like 'repository new tag'
 
-      context 'delete tag' do
-        it 'deletes an existing tag' do
-          delete api("/projects/#{project.id}/repository/tags/#{tag_name}", user)
+        context 'when tag contains a dot' do
+          let(:tag_name) { 'v7.0.1' }
 
-          expect(response).to have_http_status(204)
+          it_behaves_like 'repository new tag'
         end
 
-        it 'raises 404 if the tag does not exist' do
-          delete api("/projects/#{project.id}/repository/tags/foobar", user)
-          expect(response).to have_http_status(404)
+        context 'requesting with the escaped project full path' do
+          let(:project_id) { CGI.escape(project.full_path) }
+
+          it_behaves_like 'repository new tag'
+
+          context 'when tag contains a dot' do
+            let(:tag_name) { 'v7.0.1' }
+
+            it_behaves_like 'repository new tag'
+          end
         end
       end
-    end
 
-    context 'annotated tag' do
-      it 'creates a new annotated tag' do
-        # Identity must be set in .gitconfig to create annotated tag.
-        repo_path = project.repository.path_to_repo
-        system(*%W(#{Gitlab.config.git.bin_path} --git-dir=#{repo_path} config user.name #{user.name}))
-        system(*%W(#{Gitlab.config.git.bin_path} --git-dir=#{repo_path} config user.email #{user.email}))
+      it 'returns 400 if tag name is invalid' do
+        post api(route, current_user), tag_name: 'new design', ref: 'master'
+
+        expect(response).to have_gitlab_http_status(400)
+        expect(json_response['message']).to eq('Tag name invalid')
+      end
+
+      it 'returns 400 if tag already exists' do
+        post api(route, current_user), tag_name: 'new_design1', ref: 'master'
 
-        post api("/projects/#{project.id}/repository/tags", user),
-             tag_name: 'v7.1.0',
-             ref: 'master',
-             message: 'Release 7.1.0'
+        expect(response).to have_gitlab_http_status(201)
+        expect(response).to match_response_schema('public_api/v4/tag')
 
-        expect(response).to have_http_status(201)
-        expect(json_response['name']).to eq('v7.1.0')
-        expect(json_response['message']).to eq('Release 7.1.0')
+        post api(route, current_user), tag_name: 'new_design1', ref: 'master'
+
+        expect(response).to have_gitlab_http_status(400)
+        expect(json_response['message']).to eq('Tag new_design1 already exists')
       end
-    end
 
-    it 'denies for user without push access' do
-      post api("/projects/#{project.id}/repository/tags", user2),
-           tag_name: 'v1.9.0',
-           ref: '621491c677087aa243f165eab467bfdfbee00be1'
-      expect(response).to have_http_status(403)
+      it 'returns 400 if ref name is invalid' do
+        post api(route, current_user), tag_name: 'new_design3', ref: 'foo'
+
+        expect(response).to have_gitlab_http_status(400)
+        expect(json_response['message']).to eq('Target foo is invalid')
+      end
+
+      context 'lightweight tags with release notes' do
+        it 'creates a new tag' do
+          post api(route, current_user), tag_name: tag_name, ref: 'master', release_description: 'Wow'
+
+          expect(response).to have_gitlab_http_status(201)
+          expect(response).to match_response_schema('public_api/v4/tag')
+          expect(json_response['name']).to eq(tag_name)
+          expect(json_response['release']['description']).to eq('Wow')
+        end
+      end
+
+      context 'annotated tag' do
+        it 'creates a new annotated tag' do
+          # Identity must be set in .gitconfig to create annotated tag.
+          repo_path = project.repository.path_to_repo
+          system(*%W(#{Gitlab.config.git.bin_path} --git-dir=#{repo_path} config user.name #{user.name}))
+          system(*%W(#{Gitlab.config.git.bin_path} --git-dir=#{repo_path} config user.email #{user.email}))
+
+          post api(route, current_user), tag_name: 'v7.1.0', ref: 'master', message: 'Release 7.1.0'
+
+          expect(response).to have_gitlab_http_status(201)
+          expect(response).to match_response_schema('public_api/v4/tag')
+          expect(json_response['name']).to eq('v7.1.0')
+          expect(json_response['message']).to eq('Release 7.1.0')
+        end
+      end
     end
+  end
+
+  describe 'DELETE /projects/:id/repository/tags/:tag_name' do
+    let(:route) { "/projects/#{project_id}/repository/tags/#{tag_name}" }
 
-    it 'returns 400 if tag name is invalid' do
-      post api("/projects/#{project.id}/repository/tags", user),
-           tag_name: 'v 1.0.0',
-           ref: 'master'
-      expect(response).to have_http_status(400)
-      expect(json_response['message']).to eq('Tag name invalid')
+    before do
+      allow_any_instance_of(Repository).to receive(:rm_tag).and_return(true)
     end
 
-    it 'returns 400 if tag already exists' do
-      post api("/projects/#{project.id}/repository/tags", user),
-           tag_name: 'v8.0.0',
-           ref: 'master'
-      expect(response).to have_http_status(201)
-      post api("/projects/#{project.id}/repository/tags", user),
-           tag_name: 'v8.0.0',
-           ref: 'master'
-      expect(response).to have_http_status(400)
-      expect(json_response['message']).to eq('Tag v8.0.0 already exists')
+    shared_examples_for 'repository delete tag' do
+      it 'deletes a tag' do
+        delete api(route, current_user)
+
+        expect(response).to have_gitlab_http_status(204)
+      end
+
+      context 'when tag does not exist' do
+        let(:tag_name) { 'unknown' }
+
+        it_behaves_like '404 response' do
+          let(:request) { delete api(route, current_user) }
+          let(:message) { 'No such tag' }
+        end
+      end
+
+      context 'when repository is disabled' do
+        include_context 'disabled repository'
+
+        it_behaves_like '403 response' do
+          let(:request) { delete api(route, current_user) }
+        end
+      end
     end
 
-    it 'returns 400 if ref name is invalid' do
-      post api("/projects/#{project.id}/repository/tags", user),
-           tag_name: 'mytag',
-           ref: 'foo'
-      expect(response).to have_http_status(400)
-      expect(json_response['message']).to eq('Target foo is invalid')
+    context 'when authenticated', 'as a master' do
+      let(:current_user) { user }
+
+      it_behaves_like 'repository delete tag'
+
+      context 'requesting with the escaped project full path' do
+        let(:project_id) { CGI.escape(project.full_path) }
+
+        it_behaves_like 'repository delete tag'
+      end
     end
   end
 
   describe 'POST /projects/:id/repository/tags/:tag_name/release' do
-    let(:tag_name) { project.repository.tag_names.first }
+    let(:route) { "/projects/#{project_id}/repository/tags/#{tag_name}/release" }
     let(:description) { 'Awesome release!' }
 
-    it 'creates description for existing git tag' do
-      post api("/projects/#{project.id}/repository/tags/#{tag_name}/release", user),
-        description: description
+    shared_examples_for 'repository new release' do
+      it 'creates description for existing git tag' do
+        post api(route, user), description: description
 
-      expect(response).to have_http_status(201)
-      expect(json_response['tag_name']).to eq(tag_name)
-      expect(json_response['description']).to eq(description)
-    end
+        expect(response).to have_gitlab_http_status(201)
+        expect(response).to match_response_schema('public_api/v4/release')
+        expect(json_response['tag_name']).to eq(tag_name)
+        expect(json_response['description']).to eq(description)
+      end
+
+      context 'when tag does not exist' do
+        let(:tag_name) { 'unknown' }
+
+        it_behaves_like '404 response' do
+          let(:request) { post api(route, current_user), description: description }
+          let(:message) { 'Tag does not exist' }
+        end
+      end
 
-    it 'returns 404 if the tag does not exist' do
-      post api("/projects/#{project.id}/repository/tags/foobar/release", user),
-        description: description
+      context 'when repository is disabled' do
+        include_context 'disabled repository'
 
-      expect(response).to have_http_status(404)
-      expect(json_response['message']).to eq('Tag does not exist')
+        it_behaves_like '403 response' do
+          let(:request) { post api(route, current_user), description: description }
+        end
+      end
     end
 
-    context 'on tag with existing release' do
-      before do
-        release = project.releases.find_or_initialize_by(tag: tag_name)
-        release.update_attributes(description: description)
+    context 'when authenticated', 'as a master' do
+      let(:current_user) { user }
+
+      it_behaves_like 'repository new release'
+
+      context 'requesting with the escaped project full path' do
+        let(:project_id) { CGI.escape(project.full_path) }
+
+        it_behaves_like 'repository new release'
       end
 
-      it 'returns 409 if there is already a release' do
-        post api("/projects/#{project.id}/repository/tags/#{tag_name}/release", user),
-          description: description
+      context 'on tag with existing release' do
+        before do
+          release = project.releases.find_or_initialize_by(tag: tag_name)
+          release.update_attributes(description: description)
+        end
+
+        it 'returns 409 if there is already a release' do
+          post api(route, user), description: description
 
-        expect(response).to have_http_status(409)
-        expect(json_response['message']).to eq('Release already exists')
+          expect(response).to have_gitlab_http_status(409)
+          expect(json_response['message']).to eq('Release already exists')
+        end
       end
     end
   end
 
   describe 'PUT id/repository/tags/:tag_name/release' do
-    let(:tag_name) { project.repository.tag_names.first }
+    let(:route) { "/projects/#{project_id}/repository/tags/#{tag_name}/release" }
     let(:description) { 'Awesome release!' }
     let(:new_description) { 'The best release!' }
 
-    context 'on tag with existing release' do
-      before do
-        release = project.releases.find_or_initialize_by(tag: tag_name)
-        release.update_attributes(description: description)
+    shared_examples_for 'repository update release' do
+      context 'on tag with existing release' do
+        before do
+          release = project.releases.find_or_initialize_by(tag: tag_name)
+          release.update_attributes(description: description)
+        end
+
+        it 'updates the release description' do
+          put api(route, current_user), description: new_description
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(json_response['tag_name']).to eq(tag_name)
+          expect(json_response['description']).to eq(new_description)
+        end
       end
 
-      it 'updates the release description' do
-        put api("/projects/#{project.id}/repository/tags/#{tag_name}/release", user),
-          description: new_description
+      context 'when tag does not exist' do
+        let(:tag_name) { 'unknown' }
 
-        expect(response).to have_http_status(200)
-        expect(json_response['tag_name']).to eq(tag_name)
-        expect(json_response['description']).to eq(new_description)
+        it_behaves_like '404 response' do
+          let(:request) { put api(route, current_user), description: new_description }
+          let(:message) { 'Tag does not exist' }
+        end
       end
-    end
 
-    it 'returns 404 if the tag does not exist' do
-      put api("/projects/#{project.id}/repository/tags/foobar/release", user),
-        description: new_description
+      context 'when repository is disabled' do
+        include_context 'disabled repository'
 
-      expect(response).to have_http_status(404)
-      expect(json_response['message']).to eq('Tag does not exist')
+        it_behaves_like '403 response' do
+          let(:request) { put api(route, current_user), description: new_description }
+        end
+      end
     end
 
-    it 'returns 404 if the release does not exist' do
-      put api("/projects/#{project.id}/repository/tags/#{tag_name}/release", user),
-        description: new_description
+    context 'when authenticated', 'as a master' do
+      let(:current_user) { user }
+
+      it_behaves_like 'repository update release'
 
-      expect(response).to have_http_status(404)
-      expect(json_response['message']).to eq('Release does not exist')
+      context 'requesting with the escaped project full path' do
+        let(:project_id) { CGI.escape(project.full_path) }
+
+        it_behaves_like 'repository update release'
+      end
+
+      context 'when release does not exist' do
+        it_behaves_like '404 response' do
+          let(:request) { put api(route, current_user), description: new_description }
+          let(:message) { 'Release does not exist' }
+        end
+      end
     end
   end
 end