From 7543c7cc98b1d9ada2dfb259a2a7ad4a9fd40dc0 Mon Sep 17 00:00:00 2001 From: Olivier El Mekki Date: Thu, 28 Sep 2023 14:46:10 +0200 Subject: [PATCH 01/16] ADD model related code for ActivityPub subscription Those changes add everything we need on the database part to implement the subscription of an external user to an internal resource. We're implementing base architecture for all ActivityPub features through a first used case, being the releases actor. After discussions on [the blueprint](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/131548#note_1560597374), the models were migrated from using STI to using an abstract model, so we have here this abstract model (`ActivityPub::Subscription`) from which all other models will inherit, using their own database table - here `activity_pub_releases_subscriptions`. The subscription happens in several steps, which is why there is those `status` and `payload` fields: 1. the record is created with the `requested` status, we save the request JSON payload of the Follow activity (we're going to need it later) and store the `subscriber_url`, which is the profile URL for the external subscriber 2. the background job retrieve the subscriber inbox URL by fetching the profile page, and stores it in the record 3. the background job sends out an `Accept` activity, which must contains a copy of the original `Follow` activity 4. the record status is changed to `accepted`. --- app/models/activity_pub.rb | 7 +++ .../activity_pub/releases_subscription.rb | 8 +++ app/models/activity_pub/subscription.rb | 18 +++++++ .../activity_pub_follow_payload.json | 53 +++++++++++++++++++ .../activity_pub_releases_subscriptions.yml | 9 ++++ ...ate_activity_pub_releases_subscriptions.rb | 39 ++++++++++++++ db/schema_migrations/20230919143316 | 1 + db/structure.sql | 34 ++++++++++++ .../activity_pub/releases_subscriptions.rb | 22 ++++++++ .../releases_subscription_spec.rb | 51 ++++++++++++++++++ 10 files changed, 242 insertions(+) create mode 100644 app/models/activity_pub.rb create mode 100644 app/models/activity_pub/releases_subscription.rb create mode 100644 app/models/activity_pub/subscription.rb create mode 100644 app/validators/json_schemas/activity_pub_follow_payload.json create mode 100644 db/docs/activity_pub_releases_subscriptions.yml create mode 100644 db/migrate/20230919143316_create_activity_pub_releases_subscriptions.rb create mode 100644 db/schema_migrations/20230919143316 create mode 100644 spec/factories/activity_pub/releases_subscriptions.rb create mode 100644 spec/models/activity_pub/releases_subscription_spec.rb diff --git a/app/models/activity_pub.rb b/app/models/activity_pub.rb new file mode 100644 index 00000000000000..9131d8be776705 --- /dev/null +++ b/app/models/activity_pub.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +module ActivityPub + def self.table_name_prefix + "activity_pub_" + end +end diff --git a/app/models/activity_pub/releases_subscription.rb b/app/models/activity_pub/releases_subscription.rb new file mode 100644 index 00000000000000..2e075698f1c57d --- /dev/null +++ b/app/models/activity_pub/releases_subscription.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +module ActivityPub + class ReleasesSubscription < Subscription + belongs_to :project + validates :project, presence: true + end +end diff --git a/app/models/activity_pub/subscription.rb b/app/models/activity_pub/subscription.rb new file mode 100644 index 00000000000000..e5ae13a4baace5 --- /dev/null +++ b/app/models/activity_pub/subscription.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module ActivityPub + class Subscription < ApplicationRecord + self.abstract_class = true + + enum :status, [:denied, :requested, :accepted], default: :requested + + attribute :payload, Gitlab::Database::Type::JsonPgSafe.new + + validates :payload, json_schema: { filename: 'activity_pub_follow_payload' }, allow_blank: true + validates :subscriber_url, presence: true + + def self.find_by_subscriber_url(subscriber_url) + find_by(subscriber_url: subscriber_url) + end + end +end diff --git a/app/validators/json_schemas/activity_pub_follow_payload.json b/app/validators/json_schemas/activity_pub_follow_payload.json new file mode 100644 index 00000000000000..1f453ce840f44a --- /dev/null +++ b/app/validators/json_schemas/activity_pub_follow_payload.json @@ -0,0 +1,53 @@ +{ + "description": "ActivityPub Follow activity payload", + "type": "object", + "required": [ + "@context", + "id", + "type", + "actor", + "object" + ], + "properties": { + "@context": { + "oneOf": [ + { + "type": "string" + }, + { + "type": "array" + } + ] + }, + "id": { + "type": "string" + }, + "type": { + "type": "string" + }, + "actor": { + "oneOf": [ + { + "type": "string" + }, + { + "type": "object", + "required": [ + "id" + ], + "id": { + "type": "string" + }, + "inbox": { + "type": "string" + }, + "additionalProperties": true + } + ] + }, + "object": { + "type": "string" + }, + "additionalProperties": true + } +} diff --git a/db/docs/activity_pub_releases_subscriptions.yml b/db/docs/activity_pub_releases_subscriptions.yml new file mode 100644 index 00000000000000..059bd1670cf158 --- /dev/null +++ b/db/docs/activity_pub_releases_subscriptions.yml @@ -0,0 +1,9 @@ +--- +table_name: activity_pub_releases_subscriptions +classes: +- ActivityPub::ReleasesSubscription +feature_categories: +- release_orchestration +description: Stores subscriptions from external users through ActivityPub for project releases +#introduced_by_url: fill when MR is created +gitlab_schema: gitlab_main diff --git a/db/migrate/20230919143316_create_activity_pub_releases_subscriptions.rb b/db/migrate/20230919143316_create_activity_pub_releases_subscriptions.rb new file mode 100644 index 00000000000000..358499f4169ece --- /dev/null +++ b/db/migrate/20230919143316_create_activity_pub_releases_subscriptions.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +# See https://docs.gitlab.com/ee/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class CreateActivityPubReleasesSubscriptions < Gitlab::Database::Migration[2.1] + # When using the methods "add_concurrent_index" or "remove_concurrent_index" + # you must disable the use of transactions + # as these methods can not run in an existing transaction. + # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure + # that either of them is the _only_ method called in the migration, + # any other changes should go in a separate migration. + # This ensures that upon failure _only_ the index creation or removing fails + # and can be retried or reverted easily. + # + # To disable transactions uncomment the following line and remove these + # comments: + # disable_ddl_transaction! + + enable_lock_retries! + + def up + create_table :activity_pub_releases_subscriptions do |t| + t.text :subscriber_inbox_url, limit: 1024 + t.text :subscriber_url, limit: 1024, null: false + t.integer :status, null: false, limit: 2, default: 1 + t.references :project, null: false, foreign_key: { on_delete: :cascade } + t.jsonb :payload, null: true + + t.timestamps_with_timezone null: false + end + add_index :activity_pub_releases_subscriptions, :subscriber_url, + name: 'index_activity_pub_releases_subscriptions_on_subscriber_url' + end + + def down + drop_table :activity_pub_releases_subscriptions + end +end diff --git a/db/schema_migrations/20230919143316 b/db/schema_migrations/20230919143316 new file mode 100644 index 00000000000000..bd5c5920a55e20 --- /dev/null +++ b/db/schema_migrations/20230919143316 @@ -0,0 +1 @@ +b93179cd8ad232fe3cfa6fc062ee2d183eaf0e961ce75ec92547991f91d6a4d2 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index d7d5d469d9eb12..150dcb8fc165e8 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -10880,6 +10880,28 @@ CREATE SEQUENCE achievements_id_seq ALTER SEQUENCE achievements_id_seq OWNED BY achievements.id; +CREATE TABLE activity_pub_releases_subscriptions ( + id bigint NOT NULL, + subscriber_inbox_url text, + subscriber_url text NOT NULL, + status smallint DEFAULT 1 NOT NULL, + project_id bigint NOT NULL, + payload jsonb, + created_at timestamp with time zone NOT NULL, + updated_at timestamp with time zone NOT NULL, + CONSTRAINT check_0ebf38bcaa CHECK ((char_length(subscriber_inbox_url) <= 1024)), + CONSTRAINT check_2afd35ba17 CHECK ((char_length(subscriber_url) <= 1024)) +); + +CREATE SEQUENCE activity_pub_releases_subscriptions_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE activity_pub_releases_subscriptions_id_seq OWNED BY activity_pub_releases_subscriptions.id; + CREATE TABLE agent_activity_events ( id bigint NOT NULL, agent_id bigint NOT NULL, @@ -25864,6 +25886,8 @@ ALTER TABLE ONLY abuse_trust_scores ALTER COLUMN id SET DEFAULT nextval('abuse_t ALTER TABLE ONLY achievements ALTER COLUMN id SET DEFAULT nextval('achievements_id_seq'::regclass); +ALTER TABLE ONLY activity_pub_releases_subscriptions ALTER COLUMN id SET DEFAULT nextval('activity_pub_releases_subscriptions_id_seq'::regclass); + ALTER TABLE ONLY agent_activity_events ALTER COLUMN id SET DEFAULT nextval('agent_activity_events_id_seq'::regclass); ALTER TABLE ONLY agent_group_authorizations ALTER COLUMN id SET DEFAULT nextval('agent_group_authorizations_id_seq'::regclass); @@ -27632,6 +27656,9 @@ ALTER TABLE ONLY abuse_trust_scores ALTER TABLE ONLY achievements ADD CONSTRAINT achievements_pkey PRIMARY KEY (id); +ALTER TABLE ONLY activity_pub_releases_subscriptions + ADD CONSTRAINT activity_pub_releases_subscriptions_pkey PRIMARY KEY (id); + ALTER TABLE ONLY agent_activity_events ADD CONSTRAINT agent_activity_events_pkey PRIMARY KEY (id); @@ -31229,6 +31256,10 @@ CREATE INDEX index_abuse_trust_scores_on_user_id_and_source_and_created_at ON ab CREATE UNIQUE INDEX "index_achievements_on_namespace_id_LOWER_name" ON achievements USING btree (namespace_id, lower(name)); +CREATE INDEX index_activity_pub_releases_subscriptions_on_project_id ON activity_pub_releases_subscriptions USING btree (project_id); + +CREATE INDEX index_activity_pub_releases_subscriptions_on_subscriber_url ON activity_pub_releases_subscriptions USING btree (subscriber_url); + CREATE INDEX index_agent_activity_events_on_agent_id_and_recorded_at_and_id ON agent_activity_events USING btree (agent_id, recorded_at, id); CREATE INDEX index_agent_activity_events_on_agent_token_id ON agent_activity_events USING btree (agent_token_id) WHERE (agent_token_id IS NOT NULL); @@ -38272,6 +38303,9 @@ ALTER TABLE ONLY operations_strategies_user_lists ALTER TABLE ONLY analytics_cycle_analytics_value_stream_settings ADD CONSTRAINT fk_rails_4360d37256 FOREIGN KEY (value_stream_id) REFERENCES analytics_cycle_analytics_group_value_streams(id) ON DELETE CASCADE; +ALTER TABLE ONLY activity_pub_releases_subscriptions + ADD CONSTRAINT fk_rails_4337598314 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; + ALTER TABLE ONLY merge_request_assignment_events ADD CONSTRAINT fk_rails_4378a2e8d7 FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE SET NULL; diff --git a/spec/factories/activity_pub/releases_subscriptions.rb b/spec/factories/activity_pub/releases_subscriptions.rb new file mode 100644 index 00000000000000..060f955470d72b --- /dev/null +++ b/spec/factories/activity_pub/releases_subscriptions.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :activity_pub_releases_subscription, class: 'ActivityPub::ReleasesSubscription' do + project + subscriber_url { 'https://example.com/actor' } + status { :requested } + payload do + { + '@context': 'https://www.w3.org/ns/activitystreams', + id: 'https://example.com/actor#follow/1', + type: 'Follow', + actor: 'https://example.com/actor', + object: 'http://localhost/user/project/-/releases' + } + end + + trait :inbox do + subscriber_inbox_url { 'https://example.com/actor/inbox' } + end + end +end diff --git a/spec/models/activity_pub/releases_subscription_spec.rb b/spec/models/activity_pub/releases_subscription_spec.rb new file mode 100644 index 00000000000000..57bfb191c28319 --- /dev/null +++ b/spec/models/activity_pub/releases_subscription_spec.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ActivityPub::ReleasesSubscription, type: :model, feature_category: :release_orchestration do + describe 'factory' do + subject { build(:activity_pub_releases_subscription) } + + it { is_expected.to be_valid } + end + + describe 'associations' do + it { is_expected.to belong_to(:project) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:project) } + it { is_expected.to validate_presence_of(:subscriber_url) } + + describe 'payload' do + it { is_expected.not_to allow_value("string").for(:payload) } + it { is_expected.not_to allow_value(1.0).for(:payload) } + + it do + is_expected.to allow_value({ + '@context': 'https://www.w3.org/ns/activitystreams', + id: 'https://example.com/actor#follow/1', + type: 'Follow', + actor: 'https://example.com/actor', + object: 'http://localhost/user/project/-/releases' + }).for(:payload) + end + end + end + + describe '.find_by_subscriber_url' do + let_it_be(:subscription) { create(:activity_pub_releases_subscription) } + + it 'returns a record if arguments match' do + result = described_class.find_by_subscriber_url(subscription.subscriber_url) + + expect(result).to eq(subscription) + end + + it 'returns nil if project does not match' do + result = described_class.find_by_subscriber_url('I really should not exist') + + expect(result).to be(nil) + end + end +end -- GitLab From 3fbc5f5c06ad586395283d375a47b892bb83b01a Mon Sep 17 00:00:00 2001 From: Olivier El Mekki Date: Mon, 2 Oct 2023 17:04:44 +0200 Subject: [PATCH 02/16] ADD uniqueness and public_urlvalidations --- app/models/activity_pub/releases_subscription.rb | 4 ++++ .../activity_pub/releases_subscription_spec.rb | 16 ++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/app/models/activity_pub/releases_subscription.rb b/app/models/activity_pub/releases_subscription.rb index 2e075698f1c57d..f3a9b41fc7cfcd 100644 --- a/app/models/activity_pub/releases_subscription.rb +++ b/app/models/activity_pub/releases_subscription.rb @@ -4,5 +4,9 @@ module ActivityPub class ReleasesSubscription < Subscription belongs_to :project validates :project, presence: true + validates :subscriber_url, presence: true, uniqueness: { case_sensitive: false, scope: :project_id }, + public_url: true + validates :subscriber_inbox_url, uniqueness: { case_sensitive: false, scope: :project_id }, + public_url: { allow_nil: true } end end diff --git a/spec/models/activity_pub/releases_subscription_spec.rb b/spec/models/activity_pub/releases_subscription_spec.rb index 57bfb191c28319..7a2cc44f322876 100644 --- a/spec/models/activity_pub/releases_subscription_spec.rb +++ b/spec/models/activity_pub/releases_subscription_spec.rb @@ -17,6 +17,22 @@ it { is_expected.to validate_presence_of(:project) } it { is_expected.to validate_presence_of(:subscriber_url) } + describe 'subscriber_url' do + subject { build(:activity_pub_releases_subscription) } + + it { is_expected.to validate_uniqueness_of(:subscriber_url).case_insensitive.scoped_to([:project_id]) } + it { is_expected.to allow_value("http://example.com/actor").for(:subscriber_url) } + it { is_expected.not_to allow_values("I'm definitely not a URL").for(:subscriber_url) } + end + + describe 'subscriber_inbox_url' do + subject { build(:activity_pub_releases_subscription) } + + it { is_expected.to validate_uniqueness_of(:subscriber_inbox_url).case_insensitive.scoped_to([:project_id]) } + it { is_expected.to allow_value("http://example.com/actor").for(:subscriber_inbox_url) } + it { is_expected.not_to allow_values("I'm definitely not a URL").for(:subscriber_inbox_url) } + end + describe 'payload' do it { is_expected.not_to allow_value("string").for(:payload) } it { is_expected.not_to allow_value(1.0).for(:payload) } -- GitLab From da6cd768f4349e294a92ed4d9c9130c4616f6ad1 Mon Sep 17 00:00:00 2001 From: Doug Stull Date: Tue, 3 Oct 2023 08:01:48 +0000 Subject: [PATCH 03/16] cleanup migration file --- ...reate_activity_pub_releases_subscriptions.rb | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/db/migrate/20230919143316_create_activity_pub_releases_subscriptions.rb b/db/migrate/20230919143316_create_activity_pub_releases_subscriptions.rb index 358499f4169ece..3fa5fea3f58744 100644 --- a/db/migrate/20230919143316_create_activity_pub_releases_subscriptions.rb +++ b/db/migrate/20230919143316_create_activity_pub_releases_subscriptions.rb @@ -1,22 +1,6 @@ # frozen_string_literal: true -# See https://docs.gitlab.com/ee/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - class CreateActivityPubReleasesSubscriptions < Gitlab::Database::Migration[2.1] - # When using the methods "add_concurrent_index" or "remove_concurrent_index" - # you must disable the use of transactions - # as these methods can not run in an existing transaction. - # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure - # that either of them is the _only_ method called in the migration, - # any other changes should go in a separate migration. - # This ensures that upon failure _only_ the index creation or removing fails - # and can be retried or reverted easily. - # - # To disable transactions uncomment the following line and remove these - # comments: - # disable_ddl_transaction! - enable_lock_retries! def up @@ -29,6 +13,7 @@ def up t.timestamps_with_timezone null: false end + add_index :activity_pub_releases_subscriptions, :subscriber_url, name: 'index_activity_pub_releases_subscriptions_on_subscriber_url' end -- GitLab From 9b323cc9e704d4b084e853ed09b6d66a93b50fc3 Mon Sep 17 00:00:00 2001 From: Olivier El Mekki Date: Wed, 4 Oct 2023 16:51:05 +0200 Subject: [PATCH 04/16] ADD unique constraints in database --- ...19_create_activity_pub_releases_subscriptions.rb} | 8 ++++++++ db/schema_migrations/20230919143316 | 1 - db/schema_migrations/20231004142019 | 1 + db/structure.sql | 12 +++++++++--- 4 files changed, 18 insertions(+), 4 deletions(-) rename db/migrate/{20230919143316_create_activity_pub_releases_subscriptions.rb => 20231004142019_create_activity_pub_releases_subscriptions.rb} (67%) delete mode 100644 db/schema_migrations/20230919143316 create mode 100644 db/schema_migrations/20231004142019 diff --git a/db/migrate/20230919143316_create_activity_pub_releases_subscriptions.rb b/db/migrate/20231004142019_create_activity_pub_releases_subscriptions.rb similarity index 67% rename from db/migrate/20230919143316_create_activity_pub_releases_subscriptions.rb rename to db/migrate/20231004142019_create_activity_pub_releases_subscriptions.rb index 3fa5fea3f58744..d448bbc23eb9b9 100644 --- a/db/migrate/20230919143316_create_activity_pub_releases_subscriptions.rb +++ b/db/migrate/20231004142019_create_activity_pub_releases_subscriptions.rb @@ -16,6 +16,14 @@ def up add_index :activity_pub_releases_subscriptions, :subscriber_url, name: 'index_activity_pub_releases_subscriptions_on_subscriber_url' + + execute <<-SQL + ALTER TABLE activity_pub_releases_subscriptions + ADD CONSTRAINT uniq_apub_releases_subs_on_sub_url_and_proj_id UNIQUE (project_id, subscriber_url); + + ALTER TABLE activity_pub_releases_subscriptions + ADD CONSTRAINT uniq_apub_releases_subs_on_sub_inbox_url_and_proj_id UNIQUE (project_id, subscriber_inbox_url); + SQL end def down diff --git a/db/schema_migrations/20230919143316 b/db/schema_migrations/20230919143316 deleted file mode 100644 index bd5c5920a55e20..00000000000000 --- a/db/schema_migrations/20230919143316 +++ /dev/null @@ -1 +0,0 @@ -b93179cd8ad232fe3cfa6fc062ee2d183eaf0e961ce75ec92547991f91d6a4d2 \ No newline at end of file diff --git a/db/schema_migrations/20231004142019 b/db/schema_migrations/20231004142019 new file mode 100644 index 00000000000000..086ecb605bf7b3 --- /dev/null +++ b/db/schema_migrations/20231004142019 @@ -0,0 +1 @@ +43110f171980d27a70021d18340ec1c04d6de3fd9d9847345f263d766803e2c9 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 150dcb8fc165e8..277f86c959b9e3 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -29501,6 +29501,12 @@ ALTER TABLE ONLY topics ALTER TABLE ONLY trending_projects ADD CONSTRAINT trending_projects_pkey PRIMARY KEY (id); +ALTER TABLE ONLY activity_pub_releases_subscriptions + ADD CONSTRAINT uniq_apub_releases_subs_on_sub_inbox_url_and_proj_id UNIQUE (project_id, subscriber_inbox_url); + +ALTER TABLE ONLY activity_pub_releases_subscriptions + ADD CONSTRAINT uniq_apub_releases_subs_on_sub_url_and_proj_id UNIQUE (project_id, subscriber_url); + ALTER TABLE ONLY upcoming_reconciliations ADD CONSTRAINT upcoming_reconciliations_pkey PRIMARY KEY (id); @@ -38300,12 +38306,12 @@ ALTER TABLE ONLY batched_background_migration_jobs ALTER TABLE ONLY operations_strategies_user_lists ADD CONSTRAINT fk_rails_43241e8d29 FOREIGN KEY (strategy_id) REFERENCES operations_strategies(id) ON DELETE CASCADE; -ALTER TABLE ONLY analytics_cycle_analytics_value_stream_settings - ADD CONSTRAINT fk_rails_4360d37256 FOREIGN KEY (value_stream_id) REFERENCES analytics_cycle_analytics_group_value_streams(id) ON DELETE CASCADE; - ALTER TABLE ONLY activity_pub_releases_subscriptions ADD CONSTRAINT fk_rails_4337598314 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; +ALTER TABLE ONLY analytics_cycle_analytics_value_stream_settings + ADD CONSTRAINT fk_rails_4360d37256 FOREIGN KEY (value_stream_id) REFERENCES analytics_cycle_analytics_group_value_streams(id) ON DELETE CASCADE; + ALTER TABLE ONLY merge_request_assignment_events ADD CONSTRAINT fk_rails_4378a2e8d7 FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE SET NULL; -- GitLab From 03eff7c80dadefbdd4f2c8df9dba73d49490cc01 Mon Sep 17 00:00:00 2001 From: Olivier El Mekki Date: Wed, 4 Oct 2023 17:05:02 +0200 Subject: [PATCH 05/16] REMOVE :denied status --- app/models/activity_pub/subscription.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/activity_pub/subscription.rb b/app/models/activity_pub/subscription.rb index e5ae13a4baace5..83720a3d40b9c8 100644 --- a/app/models/activity_pub/subscription.rb +++ b/app/models/activity_pub/subscription.rb @@ -4,7 +4,7 @@ module ActivityPub class Subscription < ApplicationRecord self.abstract_class = true - enum :status, [:denied, :requested, :accepted], default: :requested + enum :status, [:requested, :accepted], default: :requested attribute :payload, Gitlab::Database::Type::JsonPgSafe.new -- GitLab From c6d7d4a47404f3030e9f13697c25626141e2dc75 Mon Sep 17 00:00:00 2001 From: Olivier El Mekki Date: Wed, 4 Oct 2023 18:44:08 +0200 Subject: [PATCH 06/16] REMOVE project_id foreign key --- ...31004142019_create_activity_pub_releases_subscriptions.rb | 2 +- db/structure.sql | 5 ----- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/db/migrate/20231004142019_create_activity_pub_releases_subscriptions.rb b/db/migrate/20231004142019_create_activity_pub_releases_subscriptions.rb index d448bbc23eb9b9..cdb71d4bd4ac0b 100644 --- a/db/migrate/20231004142019_create_activity_pub_releases_subscriptions.rb +++ b/db/migrate/20231004142019_create_activity_pub_releases_subscriptions.rb @@ -8,7 +8,7 @@ def up t.text :subscriber_inbox_url, limit: 1024 t.text :subscriber_url, limit: 1024, null: false t.integer :status, null: false, limit: 2, default: 1 - t.references :project, null: false, foreign_key: { on_delete: :cascade } + t.bigint :project_id, null: false t.jsonb :payload, null: true t.timestamps_with_timezone null: false diff --git a/db/structure.sql b/db/structure.sql index 277f86c959b9e3..aa93800b9a03a8 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -31262,8 +31262,6 @@ CREATE INDEX index_abuse_trust_scores_on_user_id_and_source_and_created_at ON ab CREATE UNIQUE INDEX "index_achievements_on_namespace_id_LOWER_name" ON achievements USING btree (namespace_id, lower(name)); -CREATE INDEX index_activity_pub_releases_subscriptions_on_project_id ON activity_pub_releases_subscriptions USING btree (project_id); - CREATE INDEX index_activity_pub_releases_subscriptions_on_subscriber_url ON activity_pub_releases_subscriptions USING btree (subscriber_url); CREATE INDEX index_agent_activity_events_on_agent_id_and_recorded_at_and_id ON agent_activity_events USING btree (agent_id, recorded_at, id); @@ -38306,9 +38304,6 @@ ALTER TABLE ONLY batched_background_migration_jobs ALTER TABLE ONLY operations_strategies_user_lists ADD CONSTRAINT fk_rails_43241e8d29 FOREIGN KEY (strategy_id) REFERENCES operations_strategies(id) ON DELETE CASCADE; -ALTER TABLE ONLY activity_pub_releases_subscriptions - ADD CONSTRAINT fk_rails_4337598314 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; - ALTER TABLE ONLY analytics_cycle_analytics_value_stream_settings ADD CONSTRAINT fk_rails_4360d37256 FOREIGN KEY (value_stream_id) REFERENCES analytics_cycle_analytics_group_value_streams(id) ON DELETE CASCADE; -- GitLab From ea383d0cec019f4e748fa0b7b2c85609eb248530 Mon Sep 17 00:00:00 2001 From: Olivier El Mekki Date: Thu, 5 Oct 2023 13:27:29 +0200 Subject: [PATCH 07/16] FIX spec complaining about duplicate indexes --- ...reate_activity_pub_releases_subscriptions.rb | 17 +++++------------ db/structure.sql | 13 +++++++------ 2 files changed, 12 insertions(+), 18 deletions(-) diff --git a/db/migrate/20231004142019_create_activity_pub_releases_subscriptions.rb b/db/migrate/20231004142019_create_activity_pub_releases_subscriptions.rb index cdb71d4bd4ac0b..2f00652c6c6d2b 100644 --- a/db/migrate/20231004142019_create_activity_pub_releases_subscriptions.rb +++ b/db/migrate/20231004142019_create_activity_pub_releases_subscriptions.rb @@ -8,22 +8,15 @@ def up t.text :subscriber_inbox_url, limit: 1024 t.text :subscriber_url, limit: 1024, null: false t.integer :status, null: false, limit: 2, default: 1 - t.bigint :project_id, null: false + t.references :project, index: false, foreign_key: { on_delete: :cascade }, null: false t.jsonb :payload, null: true t.timestamps_with_timezone null: false + t.index [:subscriber_url], name: :index_activity_pub_releases_subscriptions_on_subscriber_url + t.index [:project_id, :subscriber_url], name: :index_activity_pub_releases_sub_on_project_id_sub_url, unique: true + t.index [:project_id, :subscriber_inbox_url], name: :index_activity_pub_releases_sub_on_project_id_inbox_url, + unique: true end - - add_index :activity_pub_releases_subscriptions, :subscriber_url, - name: 'index_activity_pub_releases_subscriptions_on_subscriber_url' - - execute <<-SQL - ALTER TABLE activity_pub_releases_subscriptions - ADD CONSTRAINT uniq_apub_releases_subs_on_sub_url_and_proj_id UNIQUE (project_id, subscriber_url); - - ALTER TABLE activity_pub_releases_subscriptions - ADD CONSTRAINT uniq_apub_releases_subs_on_sub_inbox_url_and_proj_id UNIQUE (project_id, subscriber_inbox_url); - SQL end def down diff --git a/db/structure.sql b/db/structure.sql index aa93800b9a03a8..2bf521e81756d3 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -29501,12 +29501,6 @@ ALTER TABLE ONLY topics ALTER TABLE ONLY trending_projects ADD CONSTRAINT trending_projects_pkey PRIMARY KEY (id); -ALTER TABLE ONLY activity_pub_releases_subscriptions - ADD CONSTRAINT uniq_apub_releases_subs_on_sub_inbox_url_and_proj_id UNIQUE (project_id, subscriber_inbox_url); - -ALTER TABLE ONLY activity_pub_releases_subscriptions - ADD CONSTRAINT uniq_apub_releases_subs_on_sub_url_and_proj_id UNIQUE (project_id, subscriber_url); - ALTER TABLE ONLY upcoming_reconciliations ADD CONSTRAINT upcoming_reconciliations_pkey PRIMARY KEY (id); @@ -31262,6 +31256,10 @@ CREATE INDEX index_abuse_trust_scores_on_user_id_and_source_and_created_at ON ab CREATE UNIQUE INDEX "index_achievements_on_namespace_id_LOWER_name" ON achievements USING btree (namespace_id, lower(name)); +CREATE UNIQUE INDEX index_activity_pub_releases_sub_on_project_id_inbox_url ON activity_pub_releases_subscriptions USING btree (project_id, subscriber_inbox_url); + +CREATE UNIQUE INDEX index_activity_pub_releases_sub_on_project_id_sub_url ON activity_pub_releases_subscriptions USING btree (project_id, subscriber_url); + CREATE INDEX index_activity_pub_releases_subscriptions_on_subscriber_url ON activity_pub_releases_subscriptions USING btree (subscriber_url); CREATE INDEX index_agent_activity_events_on_agent_id_and_recorded_at_and_id ON agent_activity_events USING btree (agent_id, recorded_at, id); @@ -38304,6 +38302,9 @@ ALTER TABLE ONLY batched_background_migration_jobs ALTER TABLE ONLY operations_strategies_user_lists ADD CONSTRAINT fk_rails_43241e8d29 FOREIGN KEY (strategy_id) REFERENCES operations_strategies(id) ON DELETE CASCADE; +ALTER TABLE ONLY activity_pub_releases_subscriptions + ADD CONSTRAINT fk_rails_4337598314 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; + ALTER TABLE ONLY analytics_cycle_analytics_value_stream_settings ADD CONSTRAINT fk_rails_4360d37256 FOREIGN KEY (value_stream_id) REFERENCES analytics_cycle_analytics_group_value_streams(id) ON DELETE CASCADE; -- GitLab From a525f83b65519816dfa7ed8829f240ab03cfca9b Mon Sep 17 00:00:00 2001 From: Olivier El Mekki Date: Mon, 9 Oct 2023 15:34:28 +0200 Subject: [PATCH 08/16] ADD shared_inbox_url attribute --- app/models/activity_pub/releases_subscription.rb | 4 ---- app/models/activity_pub/subscription.rb | 6 +++++- ...004142019_create_activity_pub_releases_subscriptions.rb | 1 + db/structure.sql | 4 +++- spec/factories/activity_pub/releases_subscriptions.rb | 4 ++++ spec/models/activity_pub/releases_subscription_spec.rb | 7 +++++++ 6 files changed, 20 insertions(+), 6 deletions(-) diff --git a/app/models/activity_pub/releases_subscription.rb b/app/models/activity_pub/releases_subscription.rb index f3a9b41fc7cfcd..2e075698f1c57d 100644 --- a/app/models/activity_pub/releases_subscription.rb +++ b/app/models/activity_pub/releases_subscription.rb @@ -4,9 +4,5 @@ module ActivityPub class ReleasesSubscription < Subscription belongs_to :project validates :project, presence: true - validates :subscriber_url, presence: true, uniqueness: { case_sensitive: false, scope: :project_id }, - public_url: true - validates :subscriber_inbox_url, uniqueness: { case_sensitive: false, scope: :project_id }, - public_url: { allow_nil: true } end end diff --git a/app/models/activity_pub/subscription.rb b/app/models/activity_pub/subscription.rb index 83720a3d40b9c8..22a1c1d00b483f 100644 --- a/app/models/activity_pub/subscription.rb +++ b/app/models/activity_pub/subscription.rb @@ -9,7 +9,11 @@ class Subscription < ApplicationRecord attribute :payload, Gitlab::Database::Type::JsonPgSafe.new validates :payload, json_schema: { filename: 'activity_pub_follow_payload' }, allow_blank: true - validates :subscriber_url, presence: true + validates :subscriber_url, presence: true, uniqueness: { case_sensitive: false, scope: :project_id }, + public_url: true + validates :subscriber_inbox_url, uniqueness: { case_sensitive: false, scope: :project_id }, + public_url: { allow_nil: true } + validates :shared_inbox_url, public_url: { allow_nil: true } def self.find_by_subscriber_url(subscriber_url) find_by(subscriber_url: subscriber_url) diff --git a/db/migrate/20231004142019_create_activity_pub_releases_subscriptions.rb b/db/migrate/20231004142019_create_activity_pub_releases_subscriptions.rb index 2f00652c6c6d2b..40cec137606bed 100644 --- a/db/migrate/20231004142019_create_activity_pub_releases_subscriptions.rb +++ b/db/migrate/20231004142019_create_activity_pub_releases_subscriptions.rb @@ -5,6 +5,7 @@ class CreateActivityPubReleasesSubscriptions < Gitlab::Database::Migration[2.1] def up create_table :activity_pub_releases_subscriptions do |t| + t.text :shared_inbox_url, limit: 1024 t.text :subscriber_inbox_url, limit: 1024 t.text :subscriber_url, limit: 1024, null: false t.integer :status, null: false, limit: 2, default: 1 diff --git a/db/structure.sql b/db/structure.sql index 2bf521e81756d3..30c7e8010e2e5f 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -10882,6 +10882,7 @@ ALTER SEQUENCE achievements_id_seq OWNED BY achievements.id; CREATE TABLE activity_pub_releases_subscriptions ( id bigint NOT NULL, + shared_inbox_url text, subscriber_inbox_url text, subscriber_url text NOT NULL, status smallint DEFAULT 1 NOT NULL, @@ -10890,7 +10891,8 @@ CREATE TABLE activity_pub_releases_subscriptions ( created_at timestamp with time zone NOT NULL, updated_at timestamp with time zone NOT NULL, CONSTRAINT check_0ebf38bcaa CHECK ((char_length(subscriber_inbox_url) <= 1024)), - CONSTRAINT check_2afd35ba17 CHECK ((char_length(subscriber_url) <= 1024)) + CONSTRAINT check_2afd35ba17 CHECK ((char_length(subscriber_url) <= 1024)), + CONSTRAINT check_61b77ced49 CHECK ((char_length(shared_inbox_url) <= 1024)) ); CREATE SEQUENCE activity_pub_releases_subscriptions_id_seq diff --git a/spec/factories/activity_pub/releases_subscriptions.rb b/spec/factories/activity_pub/releases_subscriptions.rb index 060f955470d72b..b789188528a671 100644 --- a/spec/factories/activity_pub/releases_subscriptions.rb +++ b/spec/factories/activity_pub/releases_subscriptions.rb @@ -18,5 +18,9 @@ trait :inbox do subscriber_inbox_url { 'https://example.com/actor/inbox' } end + + trait :shared_inbox do + shared_inbox_url { 'https://example.com/shared-inbox' } + end end end diff --git a/spec/models/activity_pub/releases_subscription_spec.rb b/spec/models/activity_pub/releases_subscription_spec.rb index 7a2cc44f322876..028f9acc3ef796 100644 --- a/spec/models/activity_pub/releases_subscription_spec.rb +++ b/spec/models/activity_pub/releases_subscription_spec.rb @@ -33,6 +33,13 @@ it { is_expected.not_to allow_values("I'm definitely not a URL").for(:subscriber_inbox_url) } end + describe 'shared_inbox_url' do + subject { build(:activity_pub_releases_subscription) } + + it { is_expected.to allow_value("http://example.com/actor").for(:shared_inbox_url) } + it { is_expected.not_to allow_values("I'm definitely not a URL").for(:shared_inbox_url) } + end + describe 'payload' do it { is_expected.not_to allow_value("string").for(:payload) } it { is_expected.not_to allow_value(1.0).for(:payload) } -- GitLab From e7dc29b9408a3139e68c48febaa32a809f5a5328 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Tue, 10 Oct 2023 07:22:54 +0000 Subject: [PATCH 09/16] ADD MR URL in documentation --- db/docs/activity_pub_releases_subscriptions.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/docs/activity_pub_releases_subscriptions.yml b/db/docs/activity_pub_releases_subscriptions.yml index 059bd1670cf158..d4a25b18bcd558 100644 --- a/db/docs/activity_pub_releases_subscriptions.yml +++ b/db/docs/activity_pub_releases_subscriptions.yml @@ -5,5 +5,5 @@ classes: feature_categories: - release_orchestration description: Stores subscriptions from external users through ActivityPub for project releases -#introduced_by_url: fill when MR is created +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/132889 gitlab_schema: gitlab_main -- GitLab From 2dfa590ab09c0987744a7014dda61dd8c8a3128f Mon Sep 17 00:00:00 2001 From: Olivier El Mekki Date: Thu, 12 Oct 2023 10:05:43 +0200 Subject: [PATCH 10/16] FIX columns ordering and removing useless index --- ...42019_create_activity_pub_releases_subscriptions.rb | 7 +++---- db/structure.sql | 10 ++++------ 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/db/migrate/20231004142019_create_activity_pub_releases_subscriptions.rb b/db/migrate/20231004142019_create_activity_pub_releases_subscriptions.rb index 40cec137606bed..e7937c29fabb2c 100644 --- a/db/migrate/20231004142019_create_activity_pub_releases_subscriptions.rb +++ b/db/migrate/20231004142019_create_activity_pub_releases_subscriptions.rb @@ -5,15 +5,14 @@ class CreateActivityPubReleasesSubscriptions < Gitlab::Database::Migration[2.1] def up create_table :activity_pub_releases_subscriptions do |t| + t.references :project, index: false, foreign_key: { on_delete: :cascade }, null: false + t.timestamps_with_timezone null: false + t.integer :status, null: false, limit: 2, default: 1 t.text :shared_inbox_url, limit: 1024 t.text :subscriber_inbox_url, limit: 1024 t.text :subscriber_url, limit: 1024, null: false - t.integer :status, null: false, limit: 2, default: 1 - t.references :project, index: false, foreign_key: { on_delete: :cascade }, null: false t.jsonb :payload, null: true - t.timestamps_with_timezone null: false - t.index [:subscriber_url], name: :index_activity_pub_releases_subscriptions_on_subscriber_url t.index [:project_id, :subscriber_url], name: :index_activity_pub_releases_sub_on_project_id_sub_url, unique: true t.index [:project_id, :subscriber_inbox_url], name: :index_activity_pub_releases_sub_on_project_id_inbox_url, unique: true diff --git a/db/structure.sql b/db/structure.sql index 30c7e8010e2e5f..d2ed3e3cf585b8 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -10882,14 +10882,14 @@ ALTER SEQUENCE achievements_id_seq OWNED BY achievements.id; CREATE TABLE activity_pub_releases_subscriptions ( id bigint NOT NULL, + project_id bigint NOT NULL, + created_at timestamp with time zone NOT NULL, + updated_at timestamp with time zone NOT NULL, + status smallint DEFAULT 1 NOT NULL, shared_inbox_url text, subscriber_inbox_url text, subscriber_url text NOT NULL, - status smallint DEFAULT 1 NOT NULL, - project_id bigint NOT NULL, payload jsonb, - created_at timestamp with time zone NOT NULL, - updated_at timestamp with time zone NOT NULL, CONSTRAINT check_0ebf38bcaa CHECK ((char_length(subscriber_inbox_url) <= 1024)), CONSTRAINT check_2afd35ba17 CHECK ((char_length(subscriber_url) <= 1024)), CONSTRAINT check_61b77ced49 CHECK ((char_length(shared_inbox_url) <= 1024)) @@ -31262,8 +31262,6 @@ CREATE UNIQUE INDEX index_activity_pub_releases_sub_on_project_id_inbox_url ON a CREATE UNIQUE INDEX index_activity_pub_releases_sub_on_project_id_sub_url ON activity_pub_releases_subscriptions USING btree (project_id, subscriber_url); -CREATE INDEX index_activity_pub_releases_subscriptions_on_subscriber_url ON activity_pub_releases_subscriptions USING btree (subscriber_url); - CREATE INDEX index_agent_activity_events_on_agent_id_and_recorded_at_and_id ON agent_activity_events USING btree (agent_id, recorded_at, id); CREATE INDEX index_agent_activity_events_on_agent_token_id ON agent_activity_events USING btree (agent_token_id) WHERE (agent_token_id IS NOT NULL); -- GitLab From 9c0fbfe29983e6be206a11ed0abb9b6153bea8c9 Mon Sep 17 00:00:00 2001 From: Doug Stull Date: Fri, 13 Oct 2023 08:24:24 +0000 Subject: [PATCH 11/16] USE belongs_to optional instead of belongs_to + validates presence --- app/models/activity_pub/releases_subscription.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/models/activity_pub/releases_subscription.rb b/app/models/activity_pub/releases_subscription.rb index 2e075698f1c57d..f208a9897892d9 100644 --- a/app/models/activity_pub/releases_subscription.rb +++ b/app/models/activity_pub/releases_subscription.rb @@ -2,7 +2,6 @@ module ActivityPub class ReleasesSubscription < Subscription - belongs_to :project - validates :project, presence: true + belongs_to :project, optional: false end end -- GitLab From 13225792f6b35582175ed0dcd968ca5c30b2b965 Mon Sep 17 00:00:00 2001 From: Olivier El Mekki Date: Fri, 13 Oct 2023 11:29:27 +0200 Subject: [PATCH 12/16] FIX specs following validation/belongs_to change --- spec/models/activity_pub/releases_subscription_spec.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spec/models/activity_pub/releases_subscription_spec.rb b/spec/models/activity_pub/releases_subscription_spec.rb index 028f9acc3ef796..c164b00983d898 100644 --- a/spec/models/activity_pub/releases_subscription_spec.rb +++ b/spec/models/activity_pub/releases_subscription_spec.rb @@ -10,11 +10,10 @@ end describe 'associations' do - it { is_expected.to belong_to(:project) } + it { is_expected.to belong_to(:project).optional(false) } end describe 'validations' do - it { is_expected.to validate_presence_of(:project) } it { is_expected.to validate_presence_of(:subscriber_url) } describe 'subscriber_url' do -- GitLab From 40db977562c6e652583ac3de9066c90f89656a62 Mon Sep 17 00:00:00 2001 From: Olivier El Mekki Date: Fri, 13 Oct 2023 11:40:15 +0200 Subject: [PATCH 13/16] REFACTOR don't use an abstract model just yet Let's remove the parent abstract model in the ActivityPub namespace since there is only one model using it for now. --- .../activity_pub/releases_subscription.rb | 17 +++++++++++++- app/models/activity_pub/subscription.rb | 22 ------------------- 2 files changed, 16 insertions(+), 23 deletions(-) delete mode 100644 app/models/activity_pub/subscription.rb diff --git a/app/models/activity_pub/releases_subscription.rb b/app/models/activity_pub/releases_subscription.rb index f208a9897892d9..03c1be1c89fe5a 100644 --- a/app/models/activity_pub/releases_subscription.rb +++ b/app/models/activity_pub/releases_subscription.rb @@ -1,7 +1,22 @@ # frozen_string_literal: true module ActivityPub - class ReleasesSubscription < Subscription + class ReleasesSubscription < ApplicationRecord belongs_to :project, optional: false + + enum :status, [:requested, :accepted], default: :requested + + attribute :payload, Gitlab::Database::Type::JsonPgSafe.new + + validates :payload, json_schema: { filename: 'activity_pub_follow_payload' }, allow_blank: true + validates :subscriber_url, presence: true, uniqueness: { case_sensitive: false, scope: :project_id }, + public_url: true + validates :subscriber_inbox_url, uniqueness: { case_sensitive: false, scope: :project_id }, + public_url: { allow_nil: true } + validates :shared_inbox_url, public_url: { allow_nil: true } + + def self.find_by_subscriber_url(subscriber_url) + find_by(subscriber_url: subscriber_url) + end end end diff --git a/app/models/activity_pub/subscription.rb b/app/models/activity_pub/subscription.rb deleted file mode 100644 index 22a1c1d00b483f..00000000000000 --- a/app/models/activity_pub/subscription.rb +++ /dev/null @@ -1,22 +0,0 @@ -# frozen_string_literal: true - -module ActivityPub - class Subscription < ApplicationRecord - self.abstract_class = true - - enum :status, [:requested, :accepted], default: :requested - - attribute :payload, Gitlab::Database::Type::JsonPgSafe.new - - validates :payload, json_schema: { filename: 'activity_pub_follow_payload' }, allow_blank: true - validates :subscriber_url, presence: true, uniqueness: { case_sensitive: false, scope: :project_id }, - public_url: true - validates :subscriber_inbox_url, uniqueness: { case_sensitive: false, scope: :project_id }, - public_url: { allow_nil: true } - validates :shared_inbox_url, public_url: { allow_nil: true } - - def self.find_by_subscriber_url(subscriber_url) - find_by(subscriber_url: subscriber_url) - end - end -end -- GitLab From 0d5059916b50f08d51640a7178053c48f71e52ea Mon Sep 17 00:00:00 2001 From: Olivier El Mekki Date: Mon, 16 Oct 2023 14:35:26 +0200 Subject: [PATCH 14/16] FIX subscriber_url index not being leveraged --- app/models/activity_pub/releases_subscription.rb | 2 +- ...9_create_activity_pub_releases_subscriptions.rb | 14 ++++++++++---- db/structure.sql | 4 ++-- .../activity_pub/releases_subscription_spec.rb | 6 ++++++ 4 files changed, 19 insertions(+), 7 deletions(-) diff --git a/app/models/activity_pub/releases_subscription.rb b/app/models/activity_pub/releases_subscription.rb index 03c1be1c89fe5a..a6304f1fc3544b 100644 --- a/app/models/activity_pub/releases_subscription.rb +++ b/app/models/activity_pub/releases_subscription.rb @@ -16,7 +16,7 @@ class ReleasesSubscription < ApplicationRecord validates :shared_inbox_url, public_url: { allow_nil: true } def self.find_by_subscriber_url(subscriber_url) - find_by(subscriber_url: subscriber_url) + find_by('LOWER(subscriber_url) = ?', subscriber_url.downcase) end end end diff --git a/db/migrate/20231004142019_create_activity_pub_releases_subscriptions.rb b/db/migrate/20231004142019_create_activity_pub_releases_subscriptions.rb index e7937c29fabb2c..16251d01076e35 100644 --- a/db/migrate/20231004142019_create_activity_pub_releases_subscriptions.rb +++ b/db/migrate/20231004142019_create_activity_pub_releases_subscriptions.rb @@ -12,11 +12,17 @@ def up t.text :subscriber_inbox_url, limit: 1024 t.text :subscriber_url, limit: 1024, null: false t.jsonb :payload, null: true - - t.index [:project_id, :subscriber_url], name: :index_activity_pub_releases_sub_on_project_id_sub_url, unique: true - t.index [:project_id, :subscriber_inbox_url], name: :index_activity_pub_releases_sub_on_project_id_inbox_url, - unique: true end + + execute <<~SQL + CREATE UNIQUE INDEX index_activity_pub_releases_sub_on_project_id_sub_url + ON activity_pub_releases_subscriptions + USING btree (project_id, LOWER(subscriber_url)); + + CREATE UNIQUE INDEX index_activity_pub_releases_sub_on_project_id_inbox_url + ON activity_pub_releases_subscriptions + USING btree (project_id, LOWER(subscriber_inbox_url)); + SQL end def down diff --git a/db/structure.sql b/db/structure.sql index d2ed3e3cf585b8..99141edd313ecf 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -31258,9 +31258,9 @@ CREATE INDEX index_abuse_trust_scores_on_user_id_and_source_and_created_at ON ab CREATE UNIQUE INDEX "index_achievements_on_namespace_id_LOWER_name" ON achievements USING btree (namespace_id, lower(name)); -CREATE UNIQUE INDEX index_activity_pub_releases_sub_on_project_id_inbox_url ON activity_pub_releases_subscriptions USING btree (project_id, subscriber_inbox_url); +CREATE UNIQUE INDEX index_activity_pub_releases_sub_on_project_id_inbox_url ON activity_pub_releases_subscriptions USING btree (project_id, lower(subscriber_inbox_url)); -CREATE UNIQUE INDEX index_activity_pub_releases_sub_on_project_id_sub_url ON activity_pub_releases_subscriptions USING btree (project_id, subscriber_url); +CREATE UNIQUE INDEX index_activity_pub_releases_sub_on_project_id_sub_url ON activity_pub_releases_subscriptions USING btree (project_id, lower(subscriber_url)); CREATE INDEX index_agent_activity_events_on_agent_id_and_recorded_at_and_id ON agent_activity_events USING btree (agent_id, recorded_at, id); diff --git a/spec/models/activity_pub/releases_subscription_spec.rb b/spec/models/activity_pub/releases_subscription_spec.rb index c164b00983d898..0c873a5c18ae1d 100644 --- a/spec/models/activity_pub/releases_subscription_spec.rb +++ b/spec/models/activity_pub/releases_subscription_spec.rb @@ -64,6 +64,12 @@ expect(result).to eq(subscription) end + it 'returns a record if arguments match case insensitively' do + result = described_class.find_by_subscriber_url(subscription.subscriber_url.upcase) + + expect(result).to eq(subscription) + end + it 'returns nil if project does not match' do result = described_class.find_by_subscriber_url('I really should not exist') -- GitLab From 43600e2f316877c981af1f5e3af0bdb057edbc92 Mon Sep 17 00:00:00 2001 From: Olivier El Mekki Date: Mon, 16 Oct 2023 18:05:16 +0200 Subject: [PATCH 15/16] FIX use rails DSL for indexes --- ...9_create_activity_pub_releases_subscriptions.rb | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/db/migrate/20231004142019_create_activity_pub_releases_subscriptions.rb b/db/migrate/20231004142019_create_activity_pub_releases_subscriptions.rb index 16251d01076e35..19693c29a33c0a 100644 --- a/db/migrate/20231004142019_create_activity_pub_releases_subscriptions.rb +++ b/db/migrate/20231004142019_create_activity_pub_releases_subscriptions.rb @@ -12,17 +12,11 @@ def up t.text :subscriber_inbox_url, limit: 1024 t.text :subscriber_url, limit: 1024, null: false t.jsonb :payload, null: true + t.index 'project_id, LOWER(subscriber_url)', name: :index_activity_pub_releases_sub_on_project_id_sub_url, + unique: true + t.index 'project_id, LOWER(subscriber_inbox_url)', + name: :index_activity_pub_releases_sub_on_project_id_inbox_url, unique: true end - - execute <<~SQL - CREATE UNIQUE INDEX index_activity_pub_releases_sub_on_project_id_sub_url - ON activity_pub_releases_subscriptions - USING btree (project_id, LOWER(subscriber_url)); - - CREATE UNIQUE INDEX index_activity_pub_releases_sub_on_project_id_inbox_url - ON activity_pub_releases_subscriptions - USING btree (project_id, LOWER(subscriber_inbox_url)); - SQL end def down -- GitLab From 9b6713103f18a9d1d7e339418d15655652ad3e9e Mon Sep 17 00:00:00 2001 From: Olivier El Mekki Date: Tue, 17 Oct 2023 12:03:43 +0200 Subject: [PATCH 16/16] FIX put migration on top of the pile The test suite is giving perplexing seemingly random fails, but never fails to fail. Before this started, there were warning in the suite that rolling back migrations was not giving the same schema as before running the migration. That's a long shot, but maybe we have a problem with the migration in this MR not being the last one in the migrations added since rebase? Trying to put it on top of the pile to see if it helps. --- db/docs/activity_pub_releases_subscriptions.yml | 3 ++- ...231017095738_create_activity_pub_releases_subscriptions.rb} | 0 db/schema_migrations/20231004142019 | 1 - db/schema_migrations/20231017095738 | 1 + 4 files changed, 3 insertions(+), 2 deletions(-) rename db/migrate/{20231004142019_create_activity_pub_releases_subscriptions.rb => 20231017095738_create_activity_pub_releases_subscriptions.rb} (100%) delete mode 100644 db/schema_migrations/20231004142019 create mode 100644 db/schema_migrations/20231017095738 diff --git a/db/docs/activity_pub_releases_subscriptions.yml b/db/docs/activity_pub_releases_subscriptions.yml index d4a25b18bcd558..8a27a51f9f3f0b 100644 --- a/db/docs/activity_pub_releases_subscriptions.yml +++ b/db/docs/activity_pub_releases_subscriptions.yml @@ -4,6 +4,7 @@ classes: - ActivityPub::ReleasesSubscription feature_categories: - release_orchestration -description: Stores subscriptions from external users through ActivityPub for project releases +description: Stores subscriptions from external users through ActivityPub for project + releases introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/132889 gitlab_schema: gitlab_main diff --git a/db/migrate/20231004142019_create_activity_pub_releases_subscriptions.rb b/db/migrate/20231017095738_create_activity_pub_releases_subscriptions.rb similarity index 100% rename from db/migrate/20231004142019_create_activity_pub_releases_subscriptions.rb rename to db/migrate/20231017095738_create_activity_pub_releases_subscriptions.rb diff --git a/db/schema_migrations/20231004142019 b/db/schema_migrations/20231004142019 deleted file mode 100644 index 086ecb605bf7b3..00000000000000 --- a/db/schema_migrations/20231004142019 +++ /dev/null @@ -1 +0,0 @@ -43110f171980d27a70021d18340ec1c04d6de3fd9d9847345f263d766803e2c9 \ No newline at end of file diff --git a/db/schema_migrations/20231017095738 b/db/schema_migrations/20231017095738 new file mode 100644 index 00000000000000..20feb63b199841 --- /dev/null +++ b/db/schema_migrations/20231017095738 @@ -0,0 +1 @@ +730b861c660b96556969054402a7776f622d42ed98055b0f7099c940ecf03c32 \ No newline at end of file -- GitLab