diff --git a/app/finders/releases_finder.rb b/app/finders/releases_finder.rb index fcc216f213b5bd9318fdb633725fdf0abda34485..e95228430ed46589183440e18391f20851eee5c9 100644 --- a/app/finders/releases_finder.rb +++ b/app/finders/releases_finder.rb @@ -2,6 +2,7 @@ class ReleasesFinder include Gitlab::Utils::StrongMemoize + include UpdatedAtFilter attr_reader :parent, :current_user, :params @@ -20,6 +21,7 @@ def execute(preload: true) releases = params[:latest] ? get_latest_releases : get_releases releases = by_tag(releases) + releases = by_updated_at(releases) releases = releases.preloaded if preload order_releases(releases) end diff --git a/app/models/release.rb b/app/models/release.rb index 6f32b0e50d5eab79d9ad54823338478b0eada785..13adbf203a90fd81bf2a3a76f86cc0a72199d572 100644 --- a/app/models/release.rb +++ b/app/models/release.rb @@ -7,6 +7,7 @@ class Release < ApplicationRecord include Gitlab::Utils::StrongMemoize include EachBatch include FromUnion + include UpdatedAtFilterable cache_markdown_field :description diff --git a/db/post_migrate/20240125211243_index_releases_on_project_id_and_updated_at_and_released_at.rb b/db/post_migrate/20240125211243_index_releases_on_project_id_and_updated_at_and_released_at.rb new file mode 100644 index 0000000000000000000000000000000000000000..5ac8c41aa17a85958f16e2ac6f81535db24a1d0f --- /dev/null +++ b/db/post_migrate/20240125211243_index_releases_on_project_id_and_updated_at_and_released_at.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class IndexReleasesOnProjectIdAndUpdatedAtAndReleasedAt < Gitlab::Database::Migration[2.2] + disable_ddl_transaction! + milestone '16.9' + + INDEX_NAME = 'index_releases_on_project_id_and_updated_at_and_released_at' + + def up + add_concurrent_index :releases, [:project_id, :updated_at, :released_at], name: INDEX_NAME + end + + def down + remove_concurrent_index_by_name :releases, INDEX_NAME + end +end diff --git a/db/schema_migrations/20240125211243 b/db/schema_migrations/20240125211243 new file mode 100644 index 0000000000000000000000000000000000000000..fcb7e915abab9d090216c7f4a09dbcfb67f8b313 --- /dev/null +++ b/db/schema_migrations/20240125211243 @@ -0,0 +1 @@ +49fef4dc551f78c9f5bbd85dcb0fdbfbfbb77099dbb7e93bc5808125bf990f88 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index ad06a31783c83a9c4f69cf279aa06a5fcc589d83..2104f68e06978451e6353b79bf540d29ce57dce1 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -35233,6 +35233,8 @@ CREATE INDEX index_releases_on_author_id_id_created_at ON releases USING btree ( CREATE INDEX index_releases_on_project_id_and_released_at_and_id ON releases USING btree (project_id, released_at, id); +CREATE INDEX index_releases_on_project_id_and_updated_at_and_released_at ON releases USING btree (project_id, updated_at, released_at); + CREATE INDEX index_releases_on_project_id_id ON releases USING btree (project_id, id); CREATE UNIQUE INDEX index_releases_on_project_tag_unique ON releases USING btree (project_id, tag); diff --git a/lib/api/releases.rb b/lib/api/releases.rb index 83085b5b7e39b97aeb8c893d00664292a0f652a5..77a8a3b85d7bc4c7ce2189795b30d203278ce8e0 100644 --- a/lib/api/releases.rb +++ b/lib/api/releases.rb @@ -93,10 +93,13 @@ class Releases < ::API::Base optional :include_html_description, type: Boolean, desc: 'If `true`, a response includes HTML rendered markdown of the release description' + + optional :updated_before, type: DateTime, desc: 'Return releases updated before the specified datetime. Format: ISO 8601 YYYY-MM-DDTHH:MM:SSZ' + optional :updated_after, type: DateTime, desc: 'Return releases updated after the specified datetime. Format: ISO 8601 YYYY-MM-DDTHH:MM:SSZ' end route_setting :authentication, job_token_allowed: true get ':id/releases' do - releases = ::ReleasesFinder.new(user_project, current_user, declared_params.slice(:order_by, :sort)).execute + releases = ::ReleasesFinder.new(user_project, current_user, declared_params.slice(:order_by, :sort, :updated_before, :updated_after)).execute # We cache the serialized payload per user in order to avoid repeated renderings. # Since the cached result could contain sensitive information, diff --git a/spec/finders/releases_finder_spec.rb b/spec/finders/releases_finder_spec.rb index 2603a205c429dcaeaf3bcf014c21a612882003ba..43ad9ef2a12154b63e540e21b94c91808a6c8f1e 100644 --- a/spec/finders/releases_finder_spec.rb +++ b/spec/finders/releases_finder_spec.rb @@ -9,7 +9,7 @@ let(:params) { {} } let(:args) { {} } let(:repository) { project.repository } - let_it_be(:v1_0_0) { create(:release, project: project, tag: 'v1.0.0') } + let_it_be(:v1_0_0) { create(:release, project: project, tag: 'v1.0.0', updated_at: 4.days.ago) } let_it_be(:v1_1_0) { create(:release, project: project, tag: 'v1.1.0') } shared_examples_for 'when the user is not authorized' do @@ -170,6 +170,52 @@ it { is_expected.to eq([v2_1_0, v2_0_0, v1_1_0, v1_0_0]) } end end + + context 'filtered by updated_at' do + before do + v1_0_0.update!(updated_at: 4.days.ago) + end + + context 'when only updated_before is present' do + let(:params) { { updated_before: 2.days.ago } } + + it { is_expected.to contain_exactly(v1_0_0) } + end + + context 'when only updated_after is present' do + let(:params) { { updated_after: 2.days.ago } } + + it { is_expected.not_to include(v1_0_0) } + end + + context 'when both updated_before and updated_after are present' do + let(:params) { { updated_before: 2.days.ago, updated_after: 6.days.ago } } + + it { is_expected.to contain_exactly(v1_0_0) } + + context 'when updated_after > updated_before' do + let(:params) { { updated_before: 6.days.ago, updated_after: 2.days.ago } } + + it { is_expected.to be_empty } + end + + context 'when updated_after equals updated_before' do + let(:params) { { updated_after: v1_0_0.updated_at, updated_before: v1_0_0.updated_at } } + + it 'allows an exact match' do + expect(subject).to contain_exactly(v1_0_0) + end + end + + context 'when arguments are invalid datetimes' do + let(:params) { { updated_after: 'invalid', updated_before: 'invalid' } } + + it 'does not filter by updated_at' do + expect(subject).to include(v1_0_0) + end + end + end + end end end diff --git a/spec/requests/api/releases_spec.rb b/spec/requests/api/releases_spec.rb index 0c811a21fb052c3a953e73c90c25828278cd8d9a..f5f47e063d393fefabe0068117545fd42acb419a 100644 --- a/spec/requests/api/releases_spec.rb +++ b/spec/requests/api/releases_spec.rb @@ -152,6 +152,16 @@ expect(json_response.first['upcoming_release']).to eq(false) end + it 'filters by updated_at' do + create(:release, project: project, tag: 'v0.1', author: maintainer, released_at: 1.day.ago, updated_at: 1.day.ago) + release = create(:release, project: project, tag: 'v0.2', author: maintainer, released_at: 1.day.ago, updated_at: 4.days.ago) + + get api("/projects/#{project.id}/releases", maintainer), params: { updated_before: 2.days.ago } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response.map { |p| p['tag_name'] }).to match([release.tag]) + end + it 'avoids N+1 queries', :use_sql_query_cache do create(:release, :with_evidence, project: project, tag: 'v0.1', author: maintainer) create(:release_link, release: project.releases.first)