diff options
Diffstat (limited to 'spec/finders/deployments_finder_spec.rb')
| -rw-r--r-- | spec/finders/deployments_finder_spec.rb | 225 |
1 files changed, 201 insertions, 24 deletions
diff --git a/spec/finders/deployments_finder_spec.rb b/spec/finders/deployments_finder_spec.rb index 0f659fa1dab..b294f1117f5 100644 --- a/spec/finders/deployments_finder_spec.rb +++ b/spec/finders/deployments_finder_spec.rb @@ -5,6 +5,58 @@ require 'spec_helper' RSpec.describe DeploymentsFinder do subject { described_class.new(params).execute } + describe "validation" do + context 'when both updated_at and finished_at filters are specified' do + let(:params) { { updated_before: 1.day.ago, finished_before: 1.day.ago } } + + it 'raises an error' do + expect { subject }.to raise_error( + described_class::InefficientQueryError, + 'Both `updated_at` filter and `finished_at` filter can not be specified') + end + end + + context 'when updated_at filter and id sorting' do + let(:params) { { updated_before: 1.day.ago, order_by: :id } } + + it 'raises an error' do + expect { subject }.to raise_error( + described_class::InefficientQueryError, + '`updated_at` filter and `updated_at` sorting must be paired') + end + end + + context 'when finished_at filter and id sorting' do + let(:params) { { finished_before: 1.day.ago, order_by: :id } } + + it 'raises an error' do + expect { subject }.to raise_error( + described_class::InefficientQueryError, + '`finished_at` filter and `finished_at` sorting must be paired') + end + end + + context 'when finished_at filter with failed status filter' do + let(:params) { { finished_before: 1.day.ago, order_by: :finished_at, status: :failed } } + + it 'raises an error' do + expect { subject }.to raise_error( + described_class::InefficientQueryError, + '`finished_at` filter must be combined with `success` status filter.') + end + end + + context 'when environment filter with non-project scope' do + let(:params) { { environment: 'production' } } + + it 'raises an error' do + expect { subject }.to raise_error( + described_class::InefficientQueryError, + '`environment` filter must be combined with `project` scope.') + end + end + end + describe "#execute" do context 'when project or group is missing' do let(:params) { {} } @@ -20,13 +72,24 @@ RSpec.describe DeploymentsFinder do describe 'filtering' do context 'when updated_at filters are specified' do - let(:params) { { **base_params, updated_before: 1.day.ago, updated_after: 3.days.ago } } - let!(:deployment_1) { create(:deployment, :success, project: project, updated_at: 2.days.ago) } - let!(:deployment_2) { create(:deployment, :success, project: project, updated_at: 4.days.ago) } - let!(:deployment_3) { create(:deployment, :success, project: project, updated_at: 1.hour.ago) } + let_it_be(:deployment_1) { create(:deployment, :success, project: project, updated_at: 48.hours.ago) } + let_it_be(:deployment_2) { create(:deployment, :success, project: project, updated_at: 47.hours.ago) } + let_it_be(:deployment_3) { create(:deployment, :success, project: project, updated_at: 4.days.ago) } + let_it_be(:deployment_4) { create(:deployment, :success, project: project, updated_at: 1.hour.ago) } + let(:params) { { **base_params, updated_before: 1.day.ago, updated_after: 3.days.ago, order_by: :updated_at } } it 'returns deployments with matched updated_at' do - is_expected.to match_array([deployment_1]) + is_expected.to match_array([deployment_2, deployment_1]) + end + + context 'when deployments_finder_implicitly_enforce_ordering_for_updated_at_filter feature flag is disabled' do + before do + stub_feature_flags(deployments_finder_implicitly_enforce_ordering_for_updated_at_filter: false) + end + + it 'returns deployments with matched updated_at' do + is_expected.to match_array([deployment_1, deployment_2]) + end end end @@ -72,30 +135,34 @@ RSpec.describe DeploymentsFinder do let(:params) { { **base_params, order_by: order_by, sort: sort } } - let!(:deployment_1) { create(:deployment, :success, project: project, iid: 11, ref: 'master', created_at: 2.days.ago, updated_at: Time.now, finished_at: Time.now) } - let!(:deployment_2) { create(:deployment, :success, project: project, iid: 12, ref: 'feature', created_at: 1.day.ago, updated_at: 2.hours.ago, finished_at: 2.hours.ago) } - let!(:deployment_3) { create(:deployment, :success, project: project, iid: 8, ref: 'video', created_at: Time.now, updated_at: 1.hour.ago, finished_at: 1.hour.ago) } + let!(:deployment_1) { create(:deployment, :success, project: project, ref: 'master', created_at: 2.days.ago, updated_at: Time.now, finished_at: Time.now) } + let!(:deployment_2) { create(:deployment, :success, project: project, ref: 'feature', created_at: 1.day.ago, updated_at: 2.hours.ago, finished_at: 2.hours.ago) } + let!(:deployment_3) { create(:deployment, :success, project: project, ref: 'video', created_at: Time.now, updated_at: 1.hour.ago, finished_at: 1.hour.ago) } where(:order_by, :sort, :ordered_deployments) do 'created_at' | 'asc' | [:deployment_1, :deployment_2, :deployment_3] 'created_at' | 'desc' | [:deployment_3, :deployment_2, :deployment_1] 'id' | 'asc' | [:deployment_1, :deployment_2, :deployment_3] 'id' | 'desc' | [:deployment_3, :deployment_2, :deployment_1] - 'iid' | 'asc' | [:deployment_3, :deployment_1, :deployment_2] - 'iid' | 'desc' | [:deployment_2, :deployment_1, :deployment_3] + 'iid' | 'asc' | [:deployment_1, :deployment_2, :deployment_3] + 'iid' | 'desc' | [:deployment_3, :deployment_2, :deployment_1] 'ref' | 'asc' | [:deployment_2, :deployment_1, :deployment_3] 'ref' | 'desc' | [:deployment_3, :deployment_1, :deployment_2] - 'updated_at' | 'asc' | [:deployment_2, :deployment_3, :deployment_1] - 'updated_at' | 'desc' | [:deployment_1, :deployment_3, :deployment_2] - 'finished_at' | 'asc' | [:deployment_2, :deployment_3, :deployment_1] - 'finished_at' | 'desc' | [:deployment_1, :deployment_3, :deployment_2] + 'updated_at' | 'asc' | described_class::InefficientQueryError + 'updated_at' | 'desc' | described_class::InefficientQueryError + 'finished_at' | 'asc' | described_class::InefficientQueryError + 'finished_at' | 'desc' | described_class::InefficientQueryError 'invalid' | 'asc' | [:deployment_1, :deployment_2, :deployment_3] - 'iid' | 'err' | [:deployment_3, :deployment_1, :deployment_2] + 'iid' | 'err' | [:deployment_1, :deployment_2, :deployment_3] end with_them do it 'returns the deployments ordered' do - expect(subject).to eq(ordered_deployments.map { |name| public_send(name) }) + if ordered_deployments == described_class::InefficientQueryError + expect { subject }.to raise_error(described_class::InefficientQueryError) + else + expect(subject).to eq(ordered_deployments.map { |name| public_send(name) }) + end end end end @@ -112,8 +179,20 @@ RSpec.describe DeploymentsFinder do end end - describe 'tie-breaker for `finished_at` sorting' do - let(:params) { { **base_params, order_by: 'updated_at', sort: 'asc' } } + describe 'transform `iid` sorting to `id` sorting' do + let(:params) { { **base_params, order_by: 'iid', sort: 'asc' } } + + it 'sorts by only one column' do + expect(subject.order_values.size).to eq(1) + end + + it 'sorts by `id`' do + expect(subject.order_values.first.to_sql).to eq(Deployment.arel_table[:id].asc.to_sql) + end + end + + describe 'tie-breaker for `updated_at` sorting' do + let(:params) { { **base_params, updated_after: 1.day.ago, order_by: 'updated_at', sort: 'asc' } } it 'sorts by two columns' do expect(subject.order_values.size).to eq(2) @@ -122,17 +201,62 @@ RSpec.describe DeploymentsFinder do it 'adds `id` sorting as the second order column' do order_value = subject.order_values[1] - expect(order_value.to_sql).to eq(Deployment.arel_table[:id].desc.to_sql) + expect(order_value.to_sql).to eq(Deployment.arel_table[:id].asc.to_sql) end - it 'uses the `id DESC` as tie-breaker when ordering' do + it 'uses the `id ASC` as tie-breaker when ordering' do updated_at = Time.now deployment_1 = create(:deployment, :success, project: project, updated_at: updated_at) deployment_2 = create(:deployment, :success, project: project, updated_at: updated_at) deployment_3 = create(:deployment, :success, project: project, updated_at: updated_at) - expect(subject).to eq([deployment_3, deployment_2, deployment_1]) + expect(subject).to eq([deployment_1, deployment_2, deployment_3]) + end + + context 'when sort direction is desc' do + let(:params) { { **base_params, updated_after: 1.day.ago, order_by: 'updated_at', sort: 'desc' } } + + it 'uses the `id DESC` as tie-breaker when ordering' do + updated_at = Time.now + + deployment_1 = create(:deployment, :success, project: project, updated_at: updated_at) + deployment_2 = create(:deployment, :success, project: project, updated_at: updated_at) + deployment_3 = create(:deployment, :success, project: project, updated_at: updated_at) + + expect(subject).to eq([deployment_3, deployment_2, deployment_1]) + end + end + end + + describe 'enforce sorting to `updated_at` sorting' do + let(:params) { { **base_params, updated_before: 1.day.ago, order_by: 'id', sort: 'asc' } } + + before do + allow(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception) + end + + it 'sorts by only one column' do + expect(subject.order_values.size).to eq(2) + end + + it 'sorts by `updated_at`' do + expect(subject.order_values.first.to_sql).to eq(Deployment.arel_table[:updated_at].asc.to_sql) + expect(subject.order_values.second.to_sql).to eq(Deployment.arel_table[:id].asc.to_sql) + end + + context 'when deployments_finder_implicitly_enforce_ordering_for_updated_at_filter feature flag is disabled' do + before do + stub_feature_flags(deployments_finder_implicitly_enforce_ordering_for_updated_at_filter: false) + end + + it 'sorts by only one column' do + expect(subject.order_values.size).to eq(1) + end + + it 'sorts by `id`' do + expect(subject.order_values.first.to_sql).to eq(Deployment.arel_table[:id].asc.to_sql) + end end end @@ -142,23 +266,76 @@ RSpec.describe DeploymentsFinder do let!(:deployment_3) { create(:deployment, :success, project: project, finished_at: 5.hours.ago) } context 'when filtering by finished_after and finished_before' do - let(:params) { { **base_params, finished_after: 3.days.ago, finished_before: 1.day.ago } } + let(:params) { { **base_params, finished_after: 3.days.ago, finished_before: 1.day.ago, status: :success, order_by: :finished_at } } it { is_expected.to match_array([deployment_1]) } end context 'when the finished_before parameter is missing' do - let(:params) { { **base_params, finished_after: 3.days.ago } } + let(:params) { { **base_params, finished_after: 3.days.ago, status: :success, order_by: :finished_at } } it { is_expected.to match_array([deployment_1, deployment_3]) } end context 'when finished_after is missing' do - let(:params) { { **base_params, finished_before: 3.days.ago } } + let(:params) { { **base_params, finished_before: 3.days.ago, status: :success, order_by: :finished_at } } it { is_expected.to match_array([deployment_2]) } end end end + + context 'at group scope' do + let_it_be(:group) { create(:group) } + let_it_be(:subgroup) { create(:group, parent: group) } + + let_it_be(:group_project_1) { create(:project, :public, :test_repo, group: group) } + let_it_be(:group_project_2) { create(:project, :public, :test_repo, group: group) } + let_it_be(:subgroup_project_1) { create(:project, :public, :test_repo, group: subgroup) } + let(:base_params) { { group: group } } + + describe 'ordering' do + using RSpec::Parameterized::TableSyntax + + let(:params) { { **base_params, order_by: order_by, sort: sort } } + + let!(:group_project_1_deployment) { create(:deployment, :success, project: group_project_1, iid: 11, ref: 'master', created_at: 2.days.ago, updated_at: Time.now, finished_at: Time.now) } + let!(:group_project_2_deployment) { create(:deployment, :success, project: group_project_2, iid: 12, ref: 'feature', created_at: 1.day.ago, updated_at: 2.hours.ago, finished_at: 2.hours.ago) } + let!(:subgroup_project_1_deployment) { create(:deployment, :success, project: subgroup_project_1, iid: 8, ref: 'video', created_at: Time.now, updated_at: 1.hour.ago, finished_at: 1.hour.ago) } + + where(:order_by, :sort) do + 'created_at' | 'asc' + 'created_at' | 'desc' + 'id' | 'asc' + 'id' | 'desc' + 'iid' | 'asc' + 'iid' | 'desc' + 'ref' | 'asc' + 'ref' | 'desc' + 'invalid' | 'asc' + 'iid' | 'err' + end + + with_them do + it 'returns the deployments unordered' do + expect(subject.to_a).to contain_exactly(group_project_1_deployment, + group_project_2_deployment, + subgroup_project_1_deployment) + end + end + end + + it 'avoids N+1 queries' do + execute_queries = -> { described_class.new({ group: group }).execute.first } + control_count = ActiveRecord::QueryRecorder.new { execute_queries }.count + + new_project = create(:project, :repository, group: group) + new_env = create(:environment, project: new_project, name: "production") + create_list(:deployment, 2, status: :success, project: new_project, environment: new_env) + group.reload + + expect { execute_queries }.not_to exceed_query_limit(control_count) + end + end end end |