diff options
author | Paco Guzman <pacoguzmanp@gmail.com> | 2016-04-22 15:07:25 +0300 |
---|---|---|
committer | Paco Guzman <pacoguzmanp@gmail.com> | 2016-04-29 10:26:52 +0300 |
commit | c4b9bd041321df25764ad1de90f89b1f0dda9f33 (patch) | |
tree | 2517d2fdcf07838902a7eb7fedb788bcc5fa786d | |
parent | a792427eed95570da22844a06a09227730443189 (diff) |
API support for the 'since' and 'until' operators on commit requests
- Parameter validation as ISO8601 format
-rw-r--r-- | CHANGELOG | 1 | ||||
-rw-r--r-- | Gemfile.lock | 2 | ||||
-rw-r--r-- | app/controllers/projects/commits_controller.rb | 2 | ||||
-rw-r--r-- | app/controllers/projects/graphs_controller.rb | 4 | ||||
-rw-r--r-- | app/models/repository.rb | 6 | ||||
-rw-r--r-- | doc/api/commits.md | 2 | ||||
-rw-r--r-- | lib/api/commits.rb | 8 | ||||
-rw-r--r-- | lib/api/helpers.rb | 16 | ||||
-rw-r--r-- | lib/gitlab/push_data_builder.rb | 2 | ||||
-rw-r--r-- | spec/models/repository_spec.rb | 2 | ||||
-rw-r--r-- | spec/requests/api/commits_spec.rb | 35 |
11 files changed, 71 insertions, 9 deletions
diff --git a/CHANGELOG b/CHANGELOG index e52e52691c2..c9215954378 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -8,6 +8,7 @@ v 8.8.0 (unreleased) - Replace Devise Async with Devise ActiveJob integration. !3902 (Connor Shea) - Allow "NEWS" and "CHANGES" as alternative names for CHANGELOG. !3768 (Connor Shea) - Added button to toggle whitespaces changes on diff view + - API support for the 'since' and 'until' operators on commit requests (Paco Guzman) v 8.7.1 (unreleased) - Throttle the update of `project.last_activity_at` to 1 minute. !3848 diff --git a/Gemfile.lock b/Gemfile.lock index 91d89b4875a..02ccbdd8fc3 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -351,7 +351,7 @@ GEM posix-spawn (~> 0.3) gitlab_emoji (0.3.1) gemojione (~> 2.2, >= 2.2.1) - gitlab_git (10.0.0) + gitlab_git (10.0.2) activesupport (~> 4.0) charlock_holmes (~> 0.7.3) github-linguist (~> 4.7.0) diff --git a/app/controllers/projects/commits_controller.rb b/app/controllers/projects/commits_controller.rb index 1420b96840c..a52c614b259 100644 --- a/app/controllers/projects/commits_controller.rb +++ b/app/controllers/projects/commits_controller.rb @@ -15,7 +15,7 @@ class Projects::CommitsController < Projects::ApplicationController if search.present? @repository.find_commits_by_message(search, @ref, @path, @limit, @offset).compact else - @repository.commits(@ref, @path, @limit, @offset) + @repository.commits(@ref, path: @path, limit: @limit, offset: @offset) end @note_counts = project.notes.where(commit_id: @commits.map(&:id)). diff --git a/app/controllers/projects/graphs_controller.rb b/app/controllers/projects/graphs_controller.rb index d13ea9f34b6..092ef32e6e3 100644 --- a/app/controllers/projects/graphs_controller.rb +++ b/app/controllers/projects/graphs_controller.rb @@ -17,7 +17,7 @@ class Projects::GraphsController < Projects::ApplicationController end def commits - @commits = @project.repository.commits(@ref, nil, 2000, 0, true) + @commits = @project.repository.commits(@ref, limit: 2000, skip_merges: true) @commits_graph = Gitlab::Graphs::Commits.new(@commits) @commits_per_week_days = @commits_graph.commits_per_week_days @commits_per_time = @commits_graph.commits_per_time @@ -55,7 +55,7 @@ class Projects::GraphsController < Projects::ApplicationController private def fetch_graph - @commits = @project.repository.commits(@ref, nil, 6000, 0, true) + @commits = @project.repository.commits(@ref, limit: 6000, skip_merges: true) @log = [] @commits.each do |commit| diff --git a/app/models/repository.rb b/app/models/repository.rb index d495c8d18f5..292acf9044f 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -87,13 +87,15 @@ class Repository nil end - def commits(ref, path = nil, limit = nil, offset = nil, skip_merges = false) + def commits(ref, path: nil, limit: nil, offset: nil, skip_merges: false, after: nil, before: nil) options = { repo: raw_repository, ref: ref, path: path, limit: limit, offset: offset, + after: after, + before: before, # --follow doesn't play well with --skip. See: # https://gitlab.com/gitlab-org/gitlab-ce/issues/3574#note_3040520 follow: false, @@ -575,7 +577,7 @@ class Repository end def contributors - commits = self.commits(nil, nil, 2000, 0, true) + commits = self.commits(nil, limit: 2000, offset: 0, skip_merges: true) commits.group_by(&:author_email).map do |email, commits| contributor = Gitlab::Contributor.new diff --git a/doc/api/commits.md b/doc/api/commits.md index 6341440c58b..57c2e1d9b87 100644 --- a/doc/api/commits.md +++ b/doc/api/commits.md @@ -12,6 +12,8 @@ GET /projects/:id/repository/commits | --------- | ---- | -------- | ----------- | | `id` | integer | yes | The ID of a project | | `ref_name` | string | no | The name of a repository branch or tag or if not given the default branch | +| `since` | string | no | Only commits after or in this date will be returned in ISO 8601 format YYYY-MM-DDTHH:MM:SSZ | +| `until` | string | no | Only commits before or in this date will be returned in ISO 8601 format YYYY-MM-DDTHH:MM:SSZ | ```bash curl -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/5/repository/commits" diff --git a/lib/api/commits.rb b/lib/api/commits.rb index 4544a41b1e3..93a3a5ce089 100644 --- a/lib/api/commits.rb +++ b/lib/api/commits.rb @@ -12,14 +12,20 @@ module API # Parameters: # id (required) - The ID of a project # ref_name (optional) - The name of a repository branch or tag, if not given the default branch is used + # since (optional) - Only commits after or in this date will be returned + # until (optional) - Only commits before or in this date will be returned # Example Request: # GET /projects/:id/repository/commits get ":id/repository/commits" do + datetime_attributes! :since, :until + page = (params[:page] || 0).to_i per_page = (params[:per_page] || 20).to_i ref = params[:ref_name] || user_project.try(:default_branch) || 'master' + after = params[:since] + before = params[:until] - commits = user_project.repository.commits(ref, nil, per_page, page * per_page) + commits = user_project.repository.commits(ref, limit: per_page, offset: page * per_page, after: after, before: before) present commits, with: Entities::RepoCommit end diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 5bbf721321d..40c967453fb 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -183,6 +183,22 @@ module API Gitlab::Access.options_with_owner.values.include? level.to_i end + # Checks the occurrences of datetime attributes, each attribute if present in the params hash must be in ISO 8601 + # format (YYYY-MM-DDTHH:MM:SSZ) or a Bad Request error is invoked. + # + # Parameters: + # keys (required) - An array consisting of elements that must be parseable as dates from the params hash + def datetime_attributes!(*keys) + keys.each do |key| + begin + params[key] = Time.xmlschema(params[key]) if params[key].present? + rescue ArgumentError + message = "\"" + key.to_s + "\" must be a timestamp in ISO 8601 format: YYYY-MM-DDTHH:MM:SSZ" + render_api_error!(message, 400) + end + end + end + def issuable_order_by if params["order_by"] == 'updated_at' 'updated_at' diff --git a/lib/gitlab/push_data_builder.rb b/lib/gitlab/push_data_builder.rb index 67622f321a6..c8f12577112 100644 --- a/lib/gitlab/push_data_builder.rb +++ b/lib/gitlab/push_data_builder.rb @@ -66,7 +66,7 @@ module Gitlab # This method provide a sample data generated with # existing project and commits to test webhooks def build_sample(project, user) - commits = project.repository.commits(project.default_branch, nil, 3) + commits = project.repository.commits(project.default_branch, limit: 3) ref = "#{Gitlab::Git::BRANCH_REF_PREFIX}#{project.default_branch}" build(project, user, commits.last.id, commits.first.id, ref, commits) end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index a306cc4aef8..802b4e579e1 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -561,7 +561,7 @@ describe Repository, models: true do end describe :skip_merged_commit do - subject { repository.commits(Gitlab::Git::BRANCH_REF_PREFIX + "'test'", nil, 100, 0, true).map{ |k| k.id } } + subject { repository.commits(Gitlab::Git::BRANCH_REF_PREFIX + "'test'", limit: 100, skip_merges: true).map{ |k| k.id } } it { is_expected.not_to include('e56497bb5f03a90a51293fc6d516788730953899') } end diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb index e28998d51b5..cb82ca7802d 100644 --- a/spec/requests/api/commits_spec.rb +++ b/spec/requests/api/commits_spec.rb @@ -32,6 +32,41 @@ describe API::API, api: true do expect(response.status).to eq(401) end end + + context "since optional parameter" do + it "should return project commits since provided parameter" do + commits = project.repository.commits("master") + since = commits.second.created_at + + get api("/projects/#{project.id}/repository/commits?since=#{since.utc.iso8601}", user) + + expect(json_response.size).to eq 2 + expect(json_response.first["id"]).to eq(commits.first.id) + expect(json_response.second["id"]).to eq(commits.second.id) + end + end + + context "until optional parameter" do + it "should return project commits until provided parameter" do + commits = project.repository.commits("master") + before = commits.second.created_at + + get api("/projects/#{project.id}/repository/commits?until=#{before.utc.iso8601}", user) + + expect(json_response.size).to eq(commits.size - 1) + expect(json_response.first["id"]).to eq(commits.second.id) + expect(json_response.second["id"]).to eq(commits.third.id) + end + end + + context "invalid xmlschema date parameters" do + it "should return an invalid parameter error message" do + get api("/projects/#{project.id}/repository/commits?since=invalid-date", user) + + expect(response.status).to eq(400) + expect(json_response['message']).to include "\"since\" must be a timestamp in ISO 8601 format" + end + end end describe "GET /projects:id/repository/commits/:sha" do |