diff --git a/app/controllers/concerns/uploads_actions.rb b/app/controllers/concerns/uploads_actions.rb index d2625dcc508..d519818f640 100644 --- a/app/controllers/concerns/uploads_actions.rb +++ b/app/controllers/concerns/uploads_actions.rb @@ -5,6 +5,9 @@ module UploadsActions include Gitlab::Utils::StrongMemoize include SendFileUpload + # Starting with version 2, Markdown upload URLs use project / group IDs instead of paths + ID_BASED_UPLOAD_PATH_VERSION = 2 + UPLOAD_MOUNTS = %w[avatar attachment file logo pwa_icon header_logo favicon screenshot].freeze included do @@ -145,6 +148,12 @@ module UploadsActions action_name == 'show' && embeddable? end + def upload_version_at_least?(version) + return unless uploader && uploader.upload + + uploader.upload.version >= version + end + def target_project nil end diff --git a/app/controllers/groups/uploads_controller.rb b/app/controllers/groups/uploads_controller.rb index d29532f9d6f..a5de78aa5e6 100644 --- a/app/controllers/groups/uploads_controller.rb +++ b/app/controllers/groups/uploads_controller.rb @@ -8,12 +8,18 @@ class Groups::UploadsController < Groups::ApplicationController before_action :authorize_upload_file!, only: [:create, :authorize] before_action :verify_workhorse_api!, only: [:authorize] + before_action :disallow_new_uploads!, only: :show feature_category :portfolio_management urgency :low, [:show] private + # Starting with this version, #show is handled by Banzai::UploadsController#show + def disallow_new_uploads! + render_404 if upload_version_at_least?(ID_BASED_UPLOAD_PATH_VERSION) + end + def upload_model_class Group end diff --git a/app/controllers/projects/uploads_controller.rb b/app/controllers/projects/uploads_controller.rb index 48399e17b25..90bed60971f 100644 --- a/app/controllers/projects/uploads_controller.rb +++ b/app/controllers/projects/uploads_controller.rb @@ -10,11 +10,17 @@ class Projects::UploadsController < Projects::ApplicationController before_action :authorize_upload_file!, only: [:create, :authorize] before_action :verify_workhorse_api!, only: [:authorize] + before_action :disallow_new_uploads!, only: :show feature_category :team_planning private + # Starting with this version, #show is handled by Banzai::UploadsController#show + def disallow_new_uploads! + render_404 if upload_version_at_least?(ID_BASED_UPLOAD_PATH_VERSION) + end + def upload_model_class Project end diff --git a/app/uploaders/records_uploads.rb b/app/uploaders/records_uploads.rb index 8561a72444d..6af60fb8302 100644 --- a/app/uploaders/records_uploads.rb +++ b/app/uploaders/records_uploads.rb @@ -4,6 +4,10 @@ module RecordsUploads module Concern extend ActiveSupport::Concern + # This value is stored in `uploads.version`. Increment this value to have + # functionality that only applies to certain versions of uploads. + VERSION = 2 + attr_accessor :upload included do @@ -60,7 +64,8 @@ module RecordsUploads size: file.size, path: upload_path, model: model, - mount_point: mounted_as + mount_point: mounted_as, + version: VERSION ) end diff --git a/app/views/projects/usage_quotas/index.html.haml b/app/views/projects/usage_quotas/index.html.haml index 2825efe8dbf..687ad9e3d70 100644 --- a/app/views/projects/usage_quotas/index.html.haml +++ b/app/views/projects/usage_quotas/index.html.haml @@ -15,5 +15,6 @@ - content_for :usage_quotas_tabs do #js-storage-usage-app{ data: { project_path: @project.full_path } } = render_if_exists 'projects/usage_quotas/transfer_tab_content' + = render_if_exists 'shared/usage_quotas/tabs_content/observability' = render 'shared/usage_quotas/index' diff --git a/db/docs/batched_background_migrations/backfill_packages_conan_metadata_project_id.yml b/db/docs/batched_background_migrations/backfill_packages_conan_metadata_project_id.yml new file mode 100644 index 00000000000..16183406c37 --- /dev/null +++ b/db/docs/batched_background_migrations/backfill_packages_conan_metadata_project_id.yml @@ -0,0 +1,9 @@ +--- +migration_job_name: BackfillPackagesConanMetadataProjectId +description: Backfills sharding key `packages_conan_metadata.project_id` from `packages_packages`. +feature_category: package_registry +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/156254 +milestone: '17.2' +queued_migration_version: 20240613071715 +finalize_after: '2024-07-22' +finalized_by: # version of the migration that finalized this BBM diff --git a/db/docs/packages_conan_metadata.yml b/db/docs/packages_conan_metadata.yml index fb404b62081..d5ff2cfa642 100644 --- a/db/docs/packages_conan_metadata.yml +++ b/db/docs/packages_conan_metadata.yml @@ -19,3 +19,4 @@ desired_sharding_key: table: packages_packages sharding_key: project_id belongs_to: package +desired_sharding_key_migration_job_name: BackfillPackagesConanMetadataProjectId diff --git a/db/migrate/20240613071711_add_project_id_to_packages_conan_metadata.rb b/db/migrate/20240613071711_add_project_id_to_packages_conan_metadata.rb new file mode 100644 index 00000000000..d2f318e9eb6 --- /dev/null +++ b/db/migrate/20240613071711_add_project_id_to_packages_conan_metadata.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddProjectIdToPackagesConanMetadata < Gitlab::Database::Migration[2.2] + milestone '17.2' + + def change + add_column :packages_conan_metadata, :project_id, :bigint + end +end diff --git a/db/migrate/20240620135128_add_version_to_uploads.rb b/db/migrate/20240620135128_add_version_to_uploads.rb new file mode 100644 index 00000000000..f8837bf2419 --- /dev/null +++ b/db/migrate/20240620135128_add_version_to_uploads.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddVersionToUploads < Gitlab::Database::Migration[2.2] + milestone '17.2' + + def change + add_column :uploads, :version, :integer, null: false, default: 1 + end +end diff --git a/db/post_migrate/20240613071712_index_packages_conan_metadata_on_project_id.rb b/db/post_migrate/20240613071712_index_packages_conan_metadata_on_project_id.rb new file mode 100644 index 00000000000..022fc9b9d40 --- /dev/null +++ b/db/post_migrate/20240613071712_index_packages_conan_metadata_on_project_id.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class IndexPackagesConanMetadataOnProjectId < Gitlab::Database::Migration[2.2] + milestone '17.2' + disable_ddl_transaction! + + INDEX_NAME = 'index_packages_conan_metadata_on_project_id' + + def up + add_concurrent_index :packages_conan_metadata, :project_id, name: INDEX_NAME + end + + def down + remove_concurrent_index_by_name :packages_conan_metadata, INDEX_NAME + end +end diff --git a/db/post_migrate/20240613071713_add_packages_conan_metadata_project_id_fk.rb b/db/post_migrate/20240613071713_add_packages_conan_metadata_project_id_fk.rb new file mode 100644 index 00000000000..186eb566268 --- /dev/null +++ b/db/post_migrate/20240613071713_add_packages_conan_metadata_project_id_fk.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class AddPackagesConanMetadataProjectIdFk < Gitlab::Database::Migration[2.2] + milestone '17.2' + disable_ddl_transaction! + + def up + add_concurrent_foreign_key :packages_conan_metadata, :projects, column: :project_id, on_delete: :cascade + end + + def down + with_lock_retries do + remove_foreign_key :packages_conan_metadata, column: :project_id + end + end +end diff --git a/db/post_migrate/20240613071714_add_packages_conan_metadata_project_id_trigger.rb b/db/post_migrate/20240613071714_add_packages_conan_metadata_project_id_trigger.rb new file mode 100644 index 00000000000..a314e475902 --- /dev/null +++ b/db/post_migrate/20240613071714_add_packages_conan_metadata_project_id_trigger.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +class AddPackagesConanMetadataProjectIdTrigger < Gitlab::Database::Migration[2.2] + milestone '17.2' + + def up + install_sharding_key_assignment_trigger( + table: :packages_conan_metadata, + sharding_key: :project_id, + parent_table: :packages_packages, + parent_sharding_key: :project_id, + foreign_key: :package_id + ) + end + + def down + remove_sharding_key_assignment_trigger( + table: :packages_conan_metadata, + sharding_key: :project_id, + parent_table: :packages_packages, + parent_sharding_key: :project_id, + foreign_key: :package_id + ) + end +end diff --git a/db/post_migrate/20240613071715_queue_backfill_packages_conan_metadata_project_id.rb b/db/post_migrate/20240613071715_queue_backfill_packages_conan_metadata_project_id.rb new file mode 100644 index 00000000000..50d974b7fea --- /dev/null +++ b/db/post_migrate/20240613071715_queue_backfill_packages_conan_metadata_project_id.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +class QueueBackfillPackagesConanMetadataProjectId < Gitlab::Database::Migration[2.2] + milestone '17.2' + restrict_gitlab_migration gitlab_schema: :gitlab_main_cell + + MIGRATION = "BackfillPackagesConanMetadataProjectId" + DELAY_INTERVAL = 2.minutes + BATCH_SIZE = 1000 + SUB_BATCH_SIZE = 100 + + def up + queue_batched_background_migration( + MIGRATION, + :packages_conan_metadata, + :id, + :project_id, + :packages_packages, + :project_id, + :package_id, + job_interval: DELAY_INTERVAL, + batch_size: BATCH_SIZE, + sub_batch_size: SUB_BATCH_SIZE + ) + end + + def down + delete_batched_background_migration( + MIGRATION, + :packages_conan_metadata, + :id, + [ + :project_id, + :packages_packages, + :project_id, + :package_id + ] + ) + end +end diff --git a/db/schema_migrations/20240613071711 b/db/schema_migrations/20240613071711 new file mode 100644 index 00000000000..e64e2396623 --- /dev/null +++ b/db/schema_migrations/20240613071711 @@ -0,0 +1 @@ +fa0b49f89efeed78982d1c8f96b068fa54024eed6a7ab2d847b7c37b9be147e2 \ No newline at end of file diff --git a/db/schema_migrations/20240613071712 b/db/schema_migrations/20240613071712 new file mode 100644 index 00000000000..6ad08be00f4 --- /dev/null +++ b/db/schema_migrations/20240613071712 @@ -0,0 +1 @@ +41d1a6bef277adaf4bfffe31757bcacdb3f12eec6795d80901db329b3bb41e71 \ No newline at end of file diff --git a/db/schema_migrations/20240613071713 b/db/schema_migrations/20240613071713 new file mode 100644 index 00000000000..fb164cf4771 --- /dev/null +++ b/db/schema_migrations/20240613071713 @@ -0,0 +1 @@ +42d0b244f1163155ffd2839796a9f5077cd37fc96d10778d5bbdd17ff8394b42 \ No newline at end of file diff --git a/db/schema_migrations/20240613071714 b/db/schema_migrations/20240613071714 new file mode 100644 index 00000000000..5ba89648795 --- /dev/null +++ b/db/schema_migrations/20240613071714 @@ -0,0 +1 @@ +51abebcfd2ac74b9ad980f2f6374ee49504c88289b6d79a28928bcc914082ef2 \ No newline at end of file diff --git a/db/schema_migrations/20240613071715 b/db/schema_migrations/20240613071715 new file mode 100644 index 00000000000..33c47369f39 --- /dev/null +++ b/db/schema_migrations/20240613071715 @@ -0,0 +1 @@ +ad61d6d55b22a37b7052d6de321ebdd3f0a1543cacebd5fb128e052c2b38ba1c \ No newline at end of file diff --git a/db/schema_migrations/20240620135128 b/db/schema_migrations/20240620135128 new file mode 100644 index 00000000000..74b3c3c2b6d --- /dev/null +++ b/db/schema_migrations/20240620135128 @@ -0,0 +1 @@ +85443e198681c111d6293606fa543e798777e9155cf5bfbd357ae2a7183bb008 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 722e270fea8..d8246476bbf 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -829,6 +829,22 @@ RETURN NEW; END $$; +CREATE FUNCTION trigger_18bc439a6741() RETURNS trigger + LANGUAGE plpgsql + AS $$ +BEGIN +IF NEW."project_id" IS NULL THEN + SELECT "project_id" + INTO NEW."project_id" + FROM "packages_packages" + WHERE "packages_packages"."id" = NEW."package_id"; +END IF; + +RETURN NEW; + +END +$$; + CREATE FUNCTION trigger_1ed40f4d5f4e() RETURNS trigger LANGUAGE plpgsql AS $$ @@ -13739,7 +13755,8 @@ CREATE TABLE packages_conan_metadata ( created_at timestamp with time zone NOT NULL, updated_at timestamp with time zone NOT NULL, package_username character varying(255) NOT NULL, - package_channel character varying(255) NOT NULL + package_channel character varying(255) NOT NULL, + project_id bigint ); CREATE SEQUENCE packages_conan_metadata_id_seq @@ -18096,6 +18113,7 @@ CREATE TABLE uploads ( store integer DEFAULT 1, mount_point character varying, secret character varying, + version integer DEFAULT 1 NOT NULL, CONSTRAINT check_5e9547379c CHECK ((store IS NOT NULL)) ); @@ -27947,6 +27965,8 @@ CREATE UNIQUE INDEX index_packages_conan_file_metadata_on_package_file_id ON pac CREATE UNIQUE INDEX index_packages_conan_metadata_on_package_id_username_channel ON packages_conan_metadata USING btree (package_id, package_username, package_channel); +CREATE INDEX index_packages_conan_metadata_on_project_id ON packages_conan_metadata USING btree (project_id); + CREATE INDEX index_packages_debian_group_component_files_on_component_id ON packages_debian_group_component_files USING btree (component_id); CREATE INDEX index_packages_debian_group_distribution_keys_on_group_id ON packages_debian_group_distribution_keys USING btree (group_id); @@ -31291,6 +31311,8 @@ CREATE TRIGGER trigger_13d4aa8fe3dd BEFORE INSERT OR UPDATE ON draft_notes FOR E CREATE TRIGGER trigger_174b23fa3dfb BEFORE INSERT OR UPDATE ON approval_project_rules_users FOR EACH ROW EXECUTE FUNCTION trigger_174b23fa3dfb(); +CREATE TRIGGER trigger_18bc439a6741 BEFORE INSERT OR UPDATE ON packages_conan_metadata FOR EACH ROW EXECUTE FUNCTION trigger_18bc439a6741(); + CREATE TRIGGER trigger_1ed40f4d5f4e BEFORE INSERT OR UPDATE ON packages_maven_metadata FOR EACH ROW EXECUTE FUNCTION trigger_1ed40f4d5f4e(); CREATE TRIGGER trigger_207005e8e995 BEFORE INSERT OR UPDATE ON operations_strategies FOR EACH ROW EXECUTE FUNCTION trigger_207005e8e995(); @@ -32061,6 +32083,9 @@ ALTER TABLE ONLY subscription_user_add_on_assignments ALTER TABLE ONLY vulnerabilities ADD CONSTRAINT fk_725465b774 FOREIGN KEY (dismissed_by_id) REFERENCES users(id) ON DELETE SET NULL; +ALTER TABLE ONLY packages_conan_metadata + ADD CONSTRAINT fk_7302a29cd9 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; + ALTER TABLE ONLY approval_merge_request_rules ADD CONSTRAINT fk_73fec3d7e5 FOREIGN KEY (approval_policy_rule_id) REFERENCES approval_policy_rules(id) ON DELETE CASCADE; diff --git a/lib/bitbucket/representation/pull_request.rb b/lib/bitbucket/representation/pull_request.rb index 978e34e9183..2fdfab7f9c6 100644 --- a/lib/bitbucket/representation/pull_request.rb +++ b/lib/bitbucket/representation/pull_request.rb @@ -82,7 +82,8 @@ module Bitbucket target_branch_name: target_branch_name, target_branch_sha: target_branch_sha, source_and_target_project_different: source_and_target_project_different, - reviewers: reviewers + reviewers: reviewers, + closed_by: closed_by } end @@ -107,6 +108,10 @@ module Bitbucket def source_and_target_project_different source_repo_uuid != target_repo_uuid end + + def closed_by + raw['closed_by']&.dig('uuid') + end end end end diff --git a/lib/gitlab/background_migration/backfill_packages_conan_metadata_project_id.rb b/lib/gitlab/background_migration/backfill_packages_conan_metadata_project_id.rb new file mode 100644 index 00000000000..8c979dc53b6 --- /dev/null +++ b/lib/gitlab/background_migration/backfill_packages_conan_metadata_project_id.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + class BackfillPackagesConanMetadataProjectId < BackfillDesiredShardingKeyJob + operation_name :backfill_packages_conan_metadata_project_id + feature_category :package_registry + end + end +end diff --git a/lib/gitlab/bitbucket_import/importers/pull_request_importer.rb b/lib/gitlab/bitbucket_import/importers/pull_request_importer.rb index 1d9d2a031e5..3651c1080d3 100644 --- a/lib/gitlab/bitbucket_import/importers/pull_request_importer.rb +++ b/lib/gitlab/bitbucket_import/importers/pull_request_importer.rb @@ -45,6 +45,8 @@ module Gitlab merge_request.reviewer_ids = reviewers merge_request.save! + create_merge_request_metrics(merge_request) + metrics.merge_requests_counter.increment end @@ -85,7 +87,26 @@ module Gitlab end def author_id - user_finder.gitlab_user_id(project, object[:author]) + @author_id ||= user_finder.gitlab_user_id(project, object[:author]) + end + + def create_merge_request_metrics(merge_request) + return if object[:closed_by].nil? + + case object[:state] + when 'merged' + merge_request.metrics.merged_by_id = closed_by_id + when 'closed' + merge_request.metrics.latest_closed_by_id = closed_by_id + else + return + end + + merge_request.metrics.save! + end + + def closed_by_id + user_finder.gitlab_user_id(project, object[:closed_by]) end def reviewers diff --git a/locale/gitlab.pot b/locale/gitlab.pot index b309a9cf3b0..7805583d939 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -48017,9 +48017,6 @@ msgstr "" msgid "SecurityOrchestration|Select frameworks" msgstr "" -msgid "SecurityOrchestration|Select groups" -msgstr "" - msgid "SecurityOrchestration|Select policy" msgstr "" @@ -56883,6 +56880,9 @@ msgstr "" msgid "UsageQuota|No projects to display." msgstr "" +msgid "UsageQuota|Observability" +msgstr "" + msgid "UsageQuota|Pending Members" msgstr "" diff --git a/spec/controllers/groups/uploads_controller_spec.rb b/spec/controllers/groups/uploads_controller_spec.rb index 94bb9c9aa02..58d3c890f92 100644 --- a/spec/controllers/groups/uploads_controller_spec.rb +++ b/spec/controllers/groups/uploads_controller_spec.rb @@ -15,6 +15,8 @@ RSpec.describe Groups::UploadsController, feature_category: :portfolio_managemen { group_id: other_model } end + let(:legacy_version) { UploadsActions::ID_BASED_UPLOAD_PATH_VERSION - 1 } + it_behaves_like 'handle uploads' do let(:uploader_class) { NamespaceFileUploader } end @@ -36,102 +38,99 @@ RSpec.describe Groups::UploadsController, feature_category: :portfolio_managemen end describe "GET #show" do - let(:filename) { "rails_sample.jpg" } let(:user) { create(:user) } - let(:jpg) { fixture_file_upload('spec/fixtures/rails_sample.jpg', 'image/jpg') } - let(:txt) { fixture_file_upload('spec/fixtures/doc_sample.txt', 'text/plain') } - let(:uploader_class) { NamespaceFileUploader } - let(:secret) { uploader_class.generate_secret } - - let(:upload_service) do - UploadService.new(model, jpg, uploader_class).execute - end + let(:filename) { "rails_sample.jpg" } + let!(:upload) { create(:upload, :namespace_upload, :with_file, model: model, filename: filename) } let(:show_upload) do - get :show, params: params.merge(secret: secret, filename: filename) + get :show, params: params.merge(secret: upload.secret, filename: filename) end - before do - allow(uploader_class).to receive(:generate_secret).and_return(secret) + it 'responds with status 404' do + show_upload - allow_next_instance_of(uploader_class) do |instance| - allow(instance).to receive(:image?).and_return(true) - end - - upload_service + expect(response).to have_gitlab_http_status(:not_found) end - context 'when the group is public' do - before do - model.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PUBLIC) + context 'with legacy upload' do + let!(:upload) do + create(:upload, :namespace_upload, :with_file, model: model, filename: filename, version: legacy_version) end - context "when not signed in" do - it "responds with appropriate status" do - show_upload - - expect(response).to have_gitlab_http_status(:ok) + context 'when the group is public' do + before do + model.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PUBLIC) end - context 'when uploader class does not match the upload' do - let(:uploader_class) { FileUploader } - - it 'responds with status 404' do + context "when not signed in" do + it "responds with appropriate status" do show_upload - expect(response).to have_gitlab_http_status(:not_found) + expect(response).to have_gitlab_http_status(:ok) + end + + context 'when uploader class does not match the upload' do + let!(:upload) do + create(:upload, :issuable_upload, :with_file, model: model, filename: filename, version: legacy_version) + end + + it 'responds with status 404' do + show_upload + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'when filename does not match' do + let(:invalid_filename) { 'invalid_filename.jpg' } + + it 'responds with status 404' do + get :show, params: params.merge(secret: upload.secret, filename: invalid_filename) + + expect(response).to have_gitlab_http_status(:not_found) + end end end - context 'when filename does not match' do - let(:invalid_filename) { 'invalid_filename.jpg' } + context "when signed in" do + before do + sign_in(user) + end - it 'responds with status 404' do - get :show, params: params.merge(secret: secret, filename: invalid_filename) + context "when the user doesn't have access to the model" do + it "responds with status 200" do + show_upload - expect(response).to have_gitlab_http_status(:not_found) + expect(response).to have_gitlab_http_status(:ok) + end end end end - context "when signed in" do + context 'when the group is private' do before do - sign_in(user) + model.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PRIVATE) end - context "when the user doesn't have access to the model" do - it "responds with status 200" do + context "when not signed in" do + it "responds with appropriate status" do show_upload expect(response).to have_gitlab_http_status(:ok) end end - end - end - context 'when the group is private' do - before do - model.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PRIVATE) - end + context "when signed in" do + before do + sign_in(user) + end - context "when not signed in" do - it "responds with appropriate status" do - show_upload + context "when the user doesn't have access to the model" do + it "responds with status 200" do + show_upload - expect(response).to have_gitlab_http_status(:ok) - end - end - - context "when signed in" do - before do - sign_in(user) - end - - context "when the user doesn't have access to the model" do - it "responds with status 200" do - show_upload - - expect(response).to have_gitlab_http_status(:ok) + expect(response).to have_gitlab_http_status(:ok) + end end end end diff --git a/spec/controllers/projects/uploads_controller_spec.rb b/spec/controllers/projects/uploads_controller_spec.rb index 9f20856fa68..64198572899 100644 --- a/spec/controllers/projects/uploads_controller_spec.rb +++ b/spec/controllers/projects/uploads_controller_spec.rb @@ -15,6 +15,8 @@ RSpec.describe Projects::UploadsController, feature_category: :team_planning do { namespace_id: other_model.namespace.to_param, project_id: other_model } end + let(:legacy_version) { UploadsActions::ID_BASED_UPLOAD_PATH_VERSION - 1 } + it_behaves_like 'handle uploads' context 'when the URL the old style, without /-/system' do @@ -55,77 +57,40 @@ RSpec.describe Projects::UploadsController, feature_category: :team_planning do end describe "GET #show" do - let(:filename) { "rails_sample.jpg" } let(:user) { create(:user) } - let(:jpg) { fixture_file_upload('spec/fixtures/rails_sample.jpg', 'image/jpg') } - let(:txt) { fixture_file_upload('spec/fixtures/doc_sample.txt', 'text/plain') } - let(:secret) { FileUploader.generate_secret } - let(:uploader_class) { FileUploader } - - let(:upload_service) do - UploadService.new(model, jpg, uploader_class).execute - end + let(:filename) { "rails_sample.jpg" } + let!(:upload) { create(:upload, :issuable_upload, :with_file, model: model, filename: filename) } let(:show_upload) do - get :show, params: params.merge(secret: secret, filename: filename) + get :show, params: params.merge(secret: upload.secret, filename: filename) end - before do - allow(FileUploader).to receive(:generate_secret).and_return(secret) + it 'responds with status 404' do + show_upload - allow_next_instance_of(FileUploader) do |instance| - allow(instance).to receive(:image?).and_return(true) - end - - upload_service + expect(response).to have_gitlab_http_status(:not_found) end - context 'when project is private do' do - before do - model.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PRIVATE) + context 'with legacy upload' do + let!(:upload) do + create(:upload, :issuable_upload, :with_file, model: model, filename: filename, version: legacy_version) end - context "when not signed in" do - context 'when the project has setting enforce_auth_checks_on_uploads true' do - before do - model.update!(enforce_auth_checks_on_uploads: true) - end - - it "responds with status 302" do - show_upload - - expect(response).to have_gitlab_http_status(:redirect) - end - end - - context 'when the project has setting enforce_auth_checks_on_uploads false' do - before do - model.update!(enforce_auth_checks_on_uploads: false) - end - - it "responds with status 200" do - show_upload - - expect(response).to have_gitlab_http_status(:ok) - end - end - end - - context "when signed in" do + context 'when project is private do' do before do - sign_in(user) + model.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PRIVATE) end - context "when the user doesn't have access to the model" do + context "when not signed in" do context 'when the project has setting enforce_auth_checks_on_uploads true' do before do model.update!(enforce_auth_checks_on_uploads: true) end - it "responds with status 404" do + it "responds with status 302" do show_upload - expect(response).to have_gitlab_http_status(:not_found) + expect(response).to have_gitlab_http_status(:redirect) end end @@ -141,46 +106,46 @@ RSpec.describe Projects::UploadsController, feature_category: :team_planning do end end end - end - end - context 'when project is public' do - before do - model.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PUBLIC) - end - - context "when not signed in" do - context 'when the project has setting enforce_auth_checks_on_uploads true' do + context "when signed in" do before do - model.update!(enforce_auth_checks_on_uploads: true) + sign_in(user) end - it "responds with status 200" do - show_upload + context "when the user doesn't have access to the model" do + context 'when the project has setting enforce_auth_checks_on_uploads true' do + before do + model.update!(enforce_auth_checks_on_uploads: true) + end - expect(response).to have_gitlab_http_status(:ok) - end - end + it "responds with status 404" do + show_upload - context 'when the project has setting enforce_auth_checks_on_uploads false' do - before do - model.update!(enforce_auth_checks_on_uploads: false) - end + expect(response).to have_gitlab_http_status(:not_found) + end + end - it "responds with status 200" do - show_upload + context 'when the project has setting enforce_auth_checks_on_uploads false' do + before do + model.update!(enforce_auth_checks_on_uploads: false) + end - expect(response).to have_gitlab_http_status(:ok) + it "responds with status 200" do + show_upload + + expect(response).to have_gitlab_http_status(:ok) + end + end end end end - context "when signed in" do + context 'when project is public' do before do - sign_in(user) + model.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PUBLIC) end - context "when the user doesn't have access to the model" do + context "when not signed in" do context 'when the project has setting enforce_auth_checks_on_uploads true' do before do model.update!(enforce_auth_checks_on_uploads: true) @@ -205,6 +170,38 @@ RSpec.describe Projects::UploadsController, feature_category: :team_planning do end end end + + context "when signed in" do + before do + sign_in(user) + end + + context "when the user doesn't have access to the model" do + context 'when the project has setting enforce_auth_checks_on_uploads true' do + before do + model.update!(enforce_auth_checks_on_uploads: true) + end + + it "responds with status 200" do + show_upload + + expect(response).to have_gitlab_http_status(:ok) + end + end + + context 'when the project has setting enforce_auth_checks_on_uploads false' do + before do + model.update!(enforce_auth_checks_on_uploads: false) + end + + it "responds with status 200" do + show_upload + + expect(response).to have_gitlab_http_status(:ok) + end + end + end + end end end end diff --git a/spec/factories/uploads.rb b/spec/factories/uploads.rb index 31ae7354a43..abe2ec0d5b1 100644 --- a/spec/factories/uploads.rb +++ b/spec/factories/uploads.rb @@ -8,6 +8,7 @@ FactoryBot.define do mount_point { :avatar } secret { nil } store { ObjectStorage::Store::LOCAL } + version { RecordsUploads::Concern::VERSION } # we should build a mount agnostic upload by default transient do diff --git a/spec/lib/bitbucket/representation/pull_request_spec.rb b/spec/lib/bitbucket/representation/pull_request_spec.rb index 7050d1858dd..a3a675dec51 100644 --- a/spec/lib/bitbucket/representation/pull_request_spec.rb +++ b/spec/lib/bitbucket/representation/pull_request_spec.rb @@ -108,6 +108,7 @@ RSpec.describe Bitbucket::Representation::PullRequest, feature_category: :import source_branch_sha: 'source-commit-hash', merge_commit_sha: 'merge-commit-hash', state: 'merged', + closed_by: nil, target_branch_name: 'destination-branch-name', target_branch_sha: 'destination-commit-hash', title: 'title', diff --git a/spec/lib/gitlab/background_migration/backfill_packages_conan_metadata_project_id_spec.rb b/spec/lib/gitlab/background_migration/backfill_packages_conan_metadata_project_id_spec.rb new file mode 100644 index 00000000000..bd8becd24a3 --- /dev/null +++ b/spec/lib/gitlab/background_migration/backfill_packages_conan_metadata_project_id_spec.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::BackgroundMigration::BackfillPackagesConanMetadataProjectId, + feature_category: :package_registry, + schema: 20240613071711 do + include_examples 'desired sharding key backfill job' do + let(:batch_table) { :packages_conan_metadata } + let(:backfill_column) { :project_id } + let(:backfill_via_table) { :packages_packages } + let(:backfill_via_column) { :project_id } + let(:backfill_via_foreign_key) { :package_id } + end +end diff --git a/spec/lib/gitlab/bitbucket_import/importers/pull_request_importer_spec.rb b/spec/lib/gitlab/bitbucket_import/importers/pull_request_importer_spec.rb index f888f8dba8a..ca0a310f203 100644 --- a/spec/lib/gitlab/bitbucket_import/importers/pull_request_importer_spec.rb +++ b/spec/lib/gitlab/bitbucket_import/importers/pull_request_importer_spec.rb @@ -9,8 +9,10 @@ RSpec.describe Gitlab::BitbucketImport::Importers::PullRequestImporter, :clean_g let_it_be(:bitbucket_user) { create(:user) } let_it_be(:user_2) { create(:user) } let_it_be(:user_3) { create(:user) } + let_it_be(:closed_by_user) { create(:user) } let_it_be(:identity) { create(:identity, user: bitbucket_user, extern_uid: '{123}', provider: :bitbucket) } let_it_be(:identity_2) { create(:identity, user: user_2, extern_uid: 'user_2', provider: :bitbucket) } + let_it_be(:closed_by_identity) { create(:identity, user: closed_by_user, extern_uid: '{345}', provider: :bitbucket) } let(:mentions_converter) { Gitlab::Import::MentionsConverter.new('bitbucket', project) } let(:source_branch_sha) { project.repository.commit.sha } let(:target_branch_sha) { project.repository.commit('refs/heads/master').sha } @@ -29,7 +31,8 @@ RSpec.describe Gitlab::BitbucketImport::Importers::PullRequestImporter, :clean_g target_branch_sha: target_branch_sha, title: 'title', updated_at: Date.today, - reviewers: %w[user_2 user_3] + reviewers: %w[user_2 user_3], + closed_by: '{345}' } end @@ -65,6 +68,8 @@ RSpec.describe Gitlab::BitbucketImport::Importers::PullRequestImporter, :clean_g expect(merge_request.reviewer_ids).to eq([user_2.id]) expect(merge_request.merge_request_diffs.first.base_commit_sha).to eq(source_branch_sha) expect(merge_request.merge_request_diffs.first.head_commit_sha).to eq(target_branch_sha) + expect(merge_request.metrics.merged_by_id).to eq(closed_by_user.id) + expect(merge_request.metrics.latest_closed_by_id).to be_nil end it 'converts mentions in the description' do @@ -78,6 +83,8 @@ RSpec.describe Gitlab::BitbucketImport::Importers::PullRequestImporter, :clean_g described_class.new(project, hash.merge(state: 'closed')).execute expect(project.merge_requests.first.closed?).to be_truthy + expect(project.merge_requests.first.metrics.latest_closed_by_id).to eq(closed_by_user.id) + expect(project.merge_requests.first.metrics.merged_by_id).to be_nil end end @@ -86,6 +93,8 @@ RSpec.describe Gitlab::BitbucketImport::Importers::PullRequestImporter, :clean_g described_class.new(project, hash.merge(state: 'opened')).execute expect(project.merge_requests.first.opened?).to be_truthy + expect(project.merge_requests.first.metrics.latest_closed_by_id).to be_nil + expect(project.merge_requests.first.metrics.merged_by_id).to be_nil end end @@ -131,6 +140,30 @@ RSpec.describe Gitlab::BitbucketImport::Importers::PullRequestImporter, :clean_g end end + context 'when closed by user cannot be found' do + before do + User.find(closed_by_user.id).destroy! + end + + it 'sets the merged by user to the project creator' do + importer.execute + + expect(project.merge_requests.first.metrics.merged_by_id).to eq(project.creator_id) + expect(project.merge_requests.first.metrics.latest_closed_by_id).to be_nil + end + + context 'when merge state is closed' do + let(:hash) { super().merge(state: 'closed') } + + it 'sets the closed by user to the project creator' do + importer.execute + + expect(project.merge_requests.first.metrics.latest_closed_by_id).to eq(project.creator_id) + expect(project.merge_requests.first.metrics.merged_by_id).to be_nil + end + end + end + describe 'head_commit_sha for merge request diff' do let(:diff) { project.merge_requests.first.merge_request_diffs.first } let(:min_length) { Commit::MIN_SHA_LENGTH } diff --git a/spec/migrations/20240613071715_queue_backfill_packages_conan_metadata_project_id_spec.rb b/spec/migrations/20240613071715_queue_backfill_packages_conan_metadata_project_id_spec.rb new file mode 100644 index 00000000000..8d622117bc5 --- /dev/null +++ b/spec/migrations/20240613071715_queue_backfill_packages_conan_metadata_project_id_spec.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe QueueBackfillPackagesConanMetadataProjectId, feature_category: :package_registry do + let!(:batched_migration) { described_class::MIGRATION } + + it 'schedules a new batched migration' do + reversible_migration do |migration| + migration.before -> { + expect(batched_migration).not_to have_scheduled_batched_migration + } + + migration.after -> { + expect(batched_migration).to have_scheduled_batched_migration( + table_name: :packages_conan_metadata, + column_name: :id, + interval: described_class::DELAY_INTERVAL, + batch_size: described_class::BATCH_SIZE, + sub_batch_size: described_class::SUB_BATCH_SIZE, + gitlab_schema: :gitlab_main_cell, + job_arguments: [ + :project_id, + :packages_packages, + :project_id, + :package_id + ] + ) + } + end + end +end diff --git a/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb b/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb index 772e03950da..31a1100fb06 100644 --- a/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb +++ b/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb @@ -70,24 +70,21 @@ RSpec.shared_examples 'handle uploads' do describe "GET #show" do let(:filename) { "rails_sample.jpg" } - - let(:upload_service) do - UploadService.new(model, jpg, uploader_class).execute + let(:secret) { upload.secret } + let!(:upload) do + create( + :upload, :issuable_upload, :with_file, + uploader: uploader_class.to_s, model: model, filename: filename, version: legacy_version + ) end let(:show_upload) do get :show, params: params.merge(secret: secret, filename: filename) end - before do - allow(FileUploader).to receive(:generate_secret).and_return(secret) - upload_service - end - context 'when the secret is invalid' do let(:secret) { "../../../../../../../../" } let(:filename) { "Gemfile.lock" } - let(:upload_service) { nil } it 'responds with status 404' do show_upload @@ -107,11 +104,9 @@ RSpec.shared_examples 'handle uploads' do end context 'when the upload does not have a MIME type that Rails knows' do - let(:po) { fixture_file_upload('spec/fixtures/missing_metadata.po', 'text/plain') } + let(:filename) { 'missing_metadata.po' } it 'falls back to the null type' do - UploadService.new(model, po, uploader_class).execute - get :show, params: params.merge(secret: secret, filename: 'missing_metadata.po') expect(response.headers['Content-Type']).to eq('application/octet-stream')