Remove unecessary endpoint from repository, add compatibility endpoints for v3...

Remove unecessary endpoint from repository, add compatibility endpoints for v3 and several improvements

doc/api/repositories.md

@@ -15,7 +15,7 @@ Parameters:
 - `id` (required) - The ID of a project
 - `path` (optional) - The path inside repository. Used to get contend of subdirectories
-- `ref_name` (optional) - The name of a repository branch or tag or if not given the default branch
+- `ref` (optional) - The name of a repository branch or tag or if not given the default branch
 - `recursive` (optional) - Boolean value used to get a recursive tree (false by default)
 ```json
@@ -72,10 +72,10 @@ Parameters:
 ]
 ```
-## Get file from repository
-Allows you to receive information about file in repository like size and
-content. Note that file content is Base64 encoded. This endpoint can be accessed
+## Get a blob from repository
+Allows you to receive information about blob in repository like size and
+content. Note that blob content is Base64 encoded. This endpoint can be accessed
 without authentication if the repository is publicly accessible.
 ```

doc/api/v3_to_v4.md

@@ -72,6 +72,7 @@ changes are in V4:
 - API uses merge request `IID`s (internal ID, as in the web UI) rather than `ID`s. This affects the merge requests, award emoji, todos, and time tracking APIs. [!9530](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9530)
 - API uses issue `IID`s (internal ID, as in the web UI) rather than `ID`s. This affects the issues, award emoji, todos, and time tracking APIs. [!9530](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9530)
 - Update endpoints for repository files [!9637](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9637)
-  - Moved `/projects/:id/repository/files` to `/projects/:id/repository/files/:filepath` (`:filepath` should be URL-encoded)
-  - Moved `/projects/:id/repository/blobs/:sha` to `/projects/:id/repository/files/:filepath/raw`
-  - Moved `/projects/:id/repository/raw_blobs/:sha` to `/projects/:id/repository/blobs/:sha/raw`
+  - Moved `/projects/:id/repository/files?file_path=:file_path` to `/projects/:id/repository/files/:file_path` (`:file_path` should be URL-encoded)
+  - `/projects/:id/repository/blobs/:sha` now returns JSON attributes for the blob identified by `:sha`, instead of finding the commit identified by `:sha` and returning the raw content of the blob in that commit identified by the required `?filepath=:filepath`
+  - Moved `/projects/:id/repository/commits/:sha/blob?file_path=:file_path`  and `/projects/:id/repository/blobs/:sha?file_path=:file_path`  to `/projects/:id/repository/files/:file_path/raw?ref=:sha`
+  - `/projects/:id/repository/tree` parameter `ref_name` has been renamed to `ref` for consistency

lib/api/files.rb

@@ -15,13 +15,13 @@ module API
       end
       def assign_file_vars!
+        authorize! :download_code, user_project
+
         @commit = user_project.commit(params[:ref])
         not_found!('Commit') unless @commit
-        @repo      = user_project.repository
-        @file_path = params[:file_path]
-        @file_path = [params[:file_path], params[:format]].join('.') if params[:format].present?
-        @blob      = @repo.blob_at(@commit.sha, @file_path)
+        @repo = user_project.repository
+        @blob = @repo.blob_at(@commit.sha, params[:file_path])
         not_found!('File') unless @blob
         @blob.load_all_data!(@repo)
@@ -35,7 +35,7 @@ module API
       end
       params :simple_file_params do
-        requires :file_path, type: String, desc: 'The path to the file. Ex. lib/class.rb'
+        requires :file_path, type: String, desc: 'The url encoded path to the file. Ex. lib%2Fclass%2Erb'
         requires :branch, type: String, desc: 'The name of branch'
         requires :commit_message, type: String, desc: 'Commit Message'
         optional :author_email, type: String, desc: 'The email of the author'
@@ -53,31 +53,25 @@ module API
       requires :id, type: String, desc: 'The project ID'
     end
     resource :projects do
-      desc 'Get a file from repository in raw format'
+      desc 'Get raw file contents from the repository'
       params do
-        requires :ref, type: String, desc: 'The name of branch, tag, or commit'
+        requires :file_path, type: String, desc: 'The url encoded path to the file. Ex. lib%2Fclass%2Erb'
+        requires :ref, type: String, desc: 'The name of branch, tag commit'
       end
       get ":id/repository/files/:file_path/raw" do
-        authorize! :download_code, user_project
-
         assign_file_vars!
-        status(200)
-
         send_git_blob @repo, @blob
       end
-      desc 'Get a file from repository in base64 format'
+      desc 'Get a file from the repository'
       params do
-        requires :ref, type: String, desc: 'The name of branch, tag, or commit'
+        requires :file_path, type: String, desc: 'The url encoded path to the file. Ex. lib%2Fclass%2Erb'
+        requires :ref, type: String, desc: 'The name of branch, tag or commit'
       end
-      get ":id/repository/files/:file_path" do
-        authorize! :download_code, user_project
-
+      get ":id/repository/files/:file_path", requirements: { file_path: /.+/ } do
         assign_file_vars!
-        status(200)
-
         {
           file_name: @blob.name,
           file_path: @blob.path,
@@ -87,7 +81,7 @@ module API
           ref: params[:ref],
           blob_id: @blob.id,
           commit_id: @commit.id,
-          last_commit_id: @repo.last_commit_id_for_path(@commit.sha, @file_path)
+          last_commit_id: @repo.last_commit_id_for_path(@commit.sha, params[:file_path])
         }
       end

lib/api/repositories.rb

@@ -19,10 +19,13 @@ module API
         end
         def assign_blob_vars!
+          authorize! :download_code, user_project
+
           @repo = user_project.repository
           begin
             @blob = Gitlab::Git::Blob.raw(@repo, params[:sha])
+            @blob.load_all_data!(@repo)
           rescue
             not_found! 'Blob'
           end
@@ -35,13 +38,13 @@ module API
         success Entities::RepoTreeObject
       end
       params do
-        optional :ref_name, type: String, desc: 'The name of a repository branch or tag, if not given the default branch is used'
+        optional :ref, type: String, desc: 'The name of a repository branch or tag, if not given the default branch is used'
         optional :path, type: String, desc: 'The path of the tree'
         optional :recursive, type: Boolean, default: false, desc: 'Used to get a recursive tree'
         use :pagination
       end
       get ':id/repository/tree' do
-        ref = params[:ref_name] || user_project.try(:default_branch) || 'master'
+        ref = params[:ref] || user_project.try(:default_branch) || 'master'
         path = params[:path] || nil
         commit = user_project.commit(ref)
@@ -52,44 +55,23 @@ module API
         present paginate(entries), with: Entities::RepoTreeObject
       end
-      desc 'Get a raw file contents'
+      desc 'Get raw blob contents from the repository'
       params do
         requires :sha, type: String, desc: 'The commit, branch name, or tag name'
-        requires :filepath, type: String, desc: 'The path to the file to display'
-      end
-      get ":id/repository/commits/:sha/blob" do
-        repo = user_project.repository
-
-        commit = repo.commit(params[:sha])
-        not_found! "Commit" unless commit
-
-        blob = Gitlab::Git::Blob.find(repo, commit.id, params[:filepath])
-        not_found! "File" unless blob
-
-        send_git_blob repo, blob
       end
-
-      desc 'Get raw blob by sha'
-      params do
-        requires :sha, type: String, desc: 'The commit, branch name, or tag name'
-      end
-      get ':id/repository/blobs/:sha/:raw' do
+      get ':id/repository/blobs/:sha/raw' do
         assign_blob_vars!
-        status(200)
-
         send_git_blob @repo, @blob
       end
-      desc 'Get blob base4 encoded content by sha'
+      desc 'Get a blob from the repository'
       params do
         requires :sha, type: String, desc: 'The commit, branch name, or tag name'
       end
       get ':id/repository/blobs/:sha' do
         assign_blob_vars!
-        status(200)
-
         {
           size: @blob.size,
           encoding: "base64",

lib/api/v3/repositories.rb

@@ -38,6 +38,60 @@ module API
           present tree.sorted_entries, with: ::API::Entities::RepoTreeObject
         end
+        desc 'Get a raw file contents'
+        params do
+          requires :sha, type: String, desc: 'The commit, branch name, or tag name'
+          requires :filepath, type: String, desc: 'The path to the file to display'
+        end
+        get [":id/repository/blobs/:sha", ":id/repository/commits/:sha/blob"] do
+          repo = user_project.repository
+          commit = repo.commit(params[:sha])
+          not_found! "Commit" unless commit
+          blob = Gitlab::Git::Blob.find(repo, commit.id, params[:filepath])
+          not_found! "File" unless blob
+          send_git_blob repo, blob
+        end
+
+        desc 'Get a raw blob contents by blob sha'
+        params do
+          requires :sha, type: String, desc: 'The commit, branch name, or tag name'
+        end
+        get ':id/repository/raw_blobs/:sha' do
+          repo = user_project.repository
+          begin
+            blob = Gitlab::Git::Blob.raw(repo, params[:sha])
+          rescue
+            not_found! 'Blob'
+          end
+          not_found! 'Blob' unless blob
+          send_git_blob repo, blob
+        end
+
+        desc 'Get an archive of the repository'
+        params do
+          optional :sha, type: String, desc: 'The commit sha of the archive to be downloaded'
+          optional :format, type: String, desc: 'The archive format'
+        end
+        get ':id/repository/archive', requirements: { format: Gitlab::Regex.archive_formats_regex } do
+          begin
+            send_git_archive user_project.repository, ref: params[:sha], format: params[:format]
+          rescue
+            not_found!('File')
+          end
+        end
+
+        desc 'Compare two branches, tags, or commits' do
+          success ::API::Entities::Compare
+        end
+        params do
+          requires :from, type: String, desc: 'The commit, branch name, or tag name to start comparison'
+          requires :to, type: String, desc: 'The commit, branch name, or tag name to stop comparison'
+        end
+        get ':id/repository/compare' do
+          compare = Gitlab::Git::Compare.new(user_project.repository.raw_repository, params[:from], params[:to])
+          present compare, with: ::API::Entities::Compare
+        end
+
         desc 'Get repository contributors' do
           success ::API::Entities::Contributor
         end

spec/requests/api/files_spec.rb

@@ -45,6 +45,18 @@ describe API::Files, api: true  do
         expect(Base64.decode64(json_response['content']).lines.first).to eq("require 'fileutils'\n")
       end
+      it 'returns file by commit sha' do
+        # This file is deleted on HEAD
+        file_path = "files%2Fjs%2Fcommit%2Ejs%2Ecoffee"
+        params[:ref] = "6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9"
+
+        get api(route(file_path), current_user), params
+
+        expect(response).to have_http_status(200)
+        expect(json_response['file_name']).to eq('commit.js.coffee')
+        expect(Base64.decode64(json_response['content']).lines.first).to eq("class Commit\n")
+      end
+
       it 'returns raw file info' do
         url = route(file_path) + "/raw"
         expect(Gitlab::Workhorse).to receive(:send_git_blob)
@@ -116,6 +128,17 @@ describe API::Files, api: true  do
         expect(response).to have_http_status(200)
       end
+      it 'returns file by commit sha' do
+        # This file is deleted on HEAD
+        file_path = "files%2Fjs%2Fcommit%2Ejs%2Ecoffee"
+        params[:ref] = "6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9"
+        expect(Gitlab::Workhorse).to receive(:send_git_blob)
+
+        get api(route(file_path) + "/raw", current_user), params
+
+        expect(response).to have_http_status(200)
+      end
+
       context 'when mandatory params are not given' do
         it_behaves_like '400 response' do
           let(:request) { get api(route("any%2Ffile"), current_user) }

spec/requests/api/repositories_spec.rb

@@ -30,7 +30,7 @@ describe API::Repositories, api: true  do
       context 'when ref does not exist' do
         it_behaves_like '404 response' do
-          let(:request) { get api("#{route}?ref_name=foo", current_user) }
+          let(:request) { get api("#{route}?ref=foo", current_user) }
           let(:message) { '404 Tree Not Found' }
         end
       end
@@ -66,7 +66,7 @@ describe API::Repositories, api: true  do
         context 'when ref does not exist' do
           it_behaves_like '404 response' do
-            let(:request) { get api("#{route}?recursive=1&ref_name=foo", current_user) }
+            let(:request) { get api("#{route}?recursive=1&ref=foo", current_user) }
             let(:message) { '404 Tree Not Found' }
           end
         end
@@ -100,72 +100,6 @@ describe API::Repositories, api: true  do
     end
   end
-  describe "GET /projects/:id/repository/commits/:sha/blob" do
-    let(:route) { "/projects/#{project.id}/repository/commits/master/blob?filepath=README.md" }
-
-    shared_examples_for 'repository blob' do
-      it 'returns the repository blob' do
-        get api(route, current_user)
-
-        expect(response).to have_http_status(200)
-      end
-
-      context 'when sha does not exist' do
-        it_behaves_like '404 response' do
-          let(:request) { get api(route.sub('master', 'invalid_branch_name'), current_user) }
-          let(:message) { '404 Commit Not Found' }
-        end
-      end
-
-      context 'when filepath does not exist' do
-        it_behaves_like '404 response' do
-          let(:request) { get api(route.sub('README.md', 'README.invalid'), current_user) }
-          let(:message) { '404 File Not Found' }
-        end
-      end
-
-      context 'when no filepath is given' do
-        it_behaves_like '400 response' do
-          let(:request) { get api(route.sub('?filepath=README.md', ''), current_user) }
-        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', 'and project is public' do
-      it_behaves_like 'repository blob' do
-        let(:project) { create(:project, :public, :repository) }
-        let(:current_user) { nil }
-      end
-    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', 'as a developer' do
-      it_behaves_like 'repository blob' do
-        let(:current_user) { user }
-      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 "GET /projects/:id/repository/blobs/:sha" do
     let(:route) { "/projects/#{project.id}/repository/blobs/#{sample_blob.oid}" }

spec/requests/api/v3/repositories_spec.rb

@@ -3,6 +3,8 @@ require 'mime/types'
 describe API::V3::Repositories, api: true  do
   include ApiHelpers
+  include RepoHelpers
+  include WorkhorseHelpers
   let(:user) { create(:user) }
   let(:guest) { create(:user).tap { |u| create(:project_member, :guest, user: u, project: project) } }
@@ -96,6 +98,226 @@ describe API::V3::Repositories, api: true  do
     end
   end
+  {
+    'blobs/:sha' => 'blobs/master',
+    'commits/:sha/blob' => 'commits/master/blob'
+  }.each do |desc_path, example_path|
+    describe "GET /projects/:id/repository/#{desc_path}" do
+      let(:route) { "/projects/#{project.id}/repository/#{example_path}?filepath=README.md" }
+      shared_examples_for 'repository blob' do
+        it 'returns the repository blob' do
+          get v3_api(route, current_user)
+          expect(response).to have_http_status(200)
+        end
+        context 'when sha does not exist' do
+          it_behaves_like '404 response' do
+            let(:request) { get v3_api(route.sub('master', 'invalid_branch_name'), current_user) }
+            let(:message) { '404 Commit Not Found' }
+          end
+        end
+        context 'when filepath does not exist' do
+          it_behaves_like '404 response' do
+            let(:request) { get v3_api(route.sub('README.md', 'README.invalid'), current_user) }
+            let(:message) { '404 File Not Found' }
+          end
+        end
+        context 'when no filepath is given' do
+          it_behaves_like '400 response' do
+            let(:request) { get v3_api(route.sub('?filepath=README.md', ''), current_user) }
+          end
+        end
+        context 'when repository is disabled' do
+          include_context 'disabled repository'
+          it_behaves_like '403 response' do
+            let(:request) { get v3_api(route, current_user) }
+          end
+        end
+      end
+      context 'when unauthenticated', 'and project is public' do
+        it_behaves_like 'repository blob' do
+          let(:project) { create(:project, :public, :repository) }
+          let(:current_user) { nil }
+        end
+      end
+      context 'when unauthenticated', 'and project is private' do
+        it_behaves_like '404 response' do
+          let(:request) { get v3_api(route) }
+          let(:message) { '404 Project Not Found' }
+        end
+      end
+      context 'when authenticated', 'as a developer' do
+        it_behaves_like 'repository blob' do
+          let(:current_user) { user }
+        end
+      end
+      context 'when authenticated', 'as a guest' do
+        it_behaves_like '403 response' do
+          let(:request) { get v3_api(route, guest) }
+        end
+      end
+    end
+  end
+  describe "GET /projects/:id/repository/raw_blobs/:sha" do
+    let(:route) { "/projects/#{project.id}/repository/raw_blobs/#{sample_blob.oid}" }
+    shared_examples_for 'repository raw blob' do
+      it 'returns the repository raw blob' do
+        get v3_api(route, current_user)
+        expect(response).to have_http_status(200)
+      end
+      context 'when sha does not exist' do
+        it_behaves_like '404 response' do
+          let(:request) { get v3_api(route.sub(sample_blob.oid, '123456'), current_user) }
+          let(:message) { '404 Blob Not Found' }
+        end
+      end
+      context 'when repository is disabled' do
+        include_context 'disabled repository'
+        it_behaves_like '403 response' do
+          let(:request) { get v3_api(route, current_user) }
+        end
+      end
+    end
+    context 'when unauthenticated', 'and project is public' do
+      it_behaves_like 'repository raw blob' do
+        let(:project) { create(:project, :public, :repository) }
+        let(:current_user) { nil }
+      end
+    end
+    context 'when unauthenticated', 'and project is private' do
+      it_behaves_like '404 response' do
+        let(:request) { get v3_api(route) }
+        let(:message) { '404 Project Not Found' }
+      end
+    end
+    context 'when authenticated', 'as a developer' do
+      it_behaves_like 'repository raw blob' do
+        let(:current_user) { user }
+      end
+    end
+    context 'when authenticated', 'as a guest' do
+      it_behaves_like '403 response' do
+        let(:request) { get v3_api(route, guest) }
+      end
+    end
+  end
+  describe "GET /projects/:id/repository/archive(.:format)?:sha" do
+    let(:route) { "/projects/#{project.id}/repository/archive" }
+    shared_examples_for 'repository archive' do
+      it 'returns the repository archive' do
+        get v3_api(route, current_user)
+        expect(response).to have_http_status(200)
+        repo_name = project.repository.name.gsub("\.git", "")
+        type, params = workhorse_send_data
+        expect(type).to eq('git-archive')
+        expect(params['ArchivePath']).to match(/#{repo_name}\-[^\.]+\.tar.gz/)
+      end
+      it 'returns the repository archive archive.zip' do
+        get v3_api("/projects/#{project.id}/repository/archive.zip", user)
+        expect(response).to have_http_status(200)
+        repo_name = project.repository.name.gsub("\.git", "")
+        type, params = workhorse_send_data
+        expect(type).to eq('git-archive')
+        expect(params['ArchivePath']).to match(/#{repo_name}\-[^\.]+\.zip/)
+      end
+      it 'returns the repository archive archive.tar.bz2' do
+        get v3_api("/projects/#{project.id}/repository/archive.tar.bz2", user)
+        expect(response).to have_http_status(200)
+        repo_name = project.repository.name.gsub("\.git", "")
+        type, params = workhorse_send_data
+        expect(type).to eq('git-archive')
+        expect(params['ArchivePath']).to match(/#{repo_name}\-[^\.]+\.tar.bz2/)
+      end
+      context 'when sha does not exist' do
+        it_behaves_like '404 response' do
+          let(:request) { get v3_api("#{route}?sha=xxx", current_user) }
+          let(:message) { '404 File Not Found' }
+        end
+      end
+    end
+    context 'when unauthenticated', 'and project is public' do
+      it_behaves_like 'repository archive' do
+        let(:project) { create(:project, :public, :repository) }
+        let(:current_user) { nil }
+      end
+    end
+    context 'when unauthenticated', 'and project is private' do
+      it_behaves_like '404 response' do
+        let(:request) { get v3_api(route) }
+        let(:message) { '404 Project Not Found' }
+      end
+    end
+    context 'when authenticated', 'as a developer' do
+      it_behaves_like 'repository archive' do
+        let(:current_user) { user }
+      end
+    end
+    context 'when authenticated', 'as a guest' do
+      it_behaves_like '403 response' do
+        let(:request) { get v3_api(route, guest) }
+      end
+    end
+  end
+
+  describe 'GET /projects/:id/repository/compare' do
+    let(:route) { "/projects/#{project.id}/repository/compare" }
+    shared_examples_for 'repository compare' do
+      it "compares branches" do
+        get v3_api(route, current_user), from: 'master', to: 'feature'
+        expect(response).to have_http_status(200)
+        expect(json_response['commits']).to be_present
+        expect(json_response['diffs']).to be_present
+      end
+      it "compares tags" do
+        get v3_api(route, current_user), from: 'v1.0.0', to: 'v1.1.0'
+        expect(response).to have_http_status(200)
+        expect(json_response['commits']).to be_present
+        expect(json_response['diffs']).to be_present
+      end
+      it "compares commits" do
+        get v3_api(route, current_user), from: sample_commit.id, to: sample_commit.parent_id
+        expect(response).to have_http_status(200)
+        expect(json_response['commits']).to be_empty
+        expect(json_response['diffs']).to be_empty
+        expect(json_response['compare_same_ref']).to be_falsey
+      end
+      it "compares commits in reverse order" do
+        get v3_api(route, current_user), from: sample_commit.parent_id, to: sample_commit.id
+        expect(response).to have_http_status(200)
+        expect(json_response['commits']).to be_present
+        expect(json_response['diffs']).to be_present
+      end
+      it "compares same refs" do
+        get v3_api(route, current_user), from: 'master', to: 'master'
+        expect(response).to have_http_status(200)
+        expect(json_response['commits']).to be_empty
+        expect(json_response['diffs']).to be_empty
+        expect(json_response['compare_same_ref']).to be_truthy
+      end
+    end
+    context 'when unauthenticated', 'and project is public' do
+      it_behaves_like 'repository compare' do
+        let(:project) { create(:project, :public, :repository) }
+        let(:current_user) { nil }
+      end
+    end
+    context 'when unauthenticated', 'and project is private' do
+      it_behaves_like '404 response' do
+        let(:request) { get v3_api(route) }
+        let(:message) { '404 Project Not Found' }
+      end
+    end
+    context 'when authenticated', 'as a developer' do
+      it_behaves_like 'repository compare' do
+        let(:current_user) { user }
+      end
+    end
+    context 'when authenticated', 'as a guest' do
+      it_behaves_like '403 response' do
+        let(:request) { get v3_api(route, guest) }
+      end
+    end
+  end
+
   describe 'GET /projects/:id/repository/contributors' do
     let(:route) { "/projects/#{project.id}/repository/contributors" }