Add group_id column to the services table

In order to support group-level integrations
https://gitlab.com/groups/gitlab-org/-/epics/2543, we need to add a new
column group_id to the services table.

app/models/service.rb

@@ -48,9 +48,11 @@ class Service < ApplicationRecord
   belongs_to :project, inverse_of: :services
   has_one :service_hook
 
-  validates :project_id, presence: true, unless: -> { template? || instance? }
-  validates :project_id, absence: true, if: -> { template? || instance? }
-  validates :type, uniqueness: { scope: :project_id }, unless: -> { template? || instance? }, on: :create
+  validates :project_id, presence: true, unless: -> { template? || instance? || group_id }
+  validates :group_id, presence: true, unless: -> { template? || instance? || project_id }
+  validates :project_id, :group_id, absence: true, if: -> { template? || instance? }
+  validates :type, uniqueness: { scope: :project_id }, unless: -> { template? || instance? || group_id }, on: :create
+  validates :type, uniqueness: { scope: :group_id }, unless: -> { template? || instance? || project_id }
   validates :type, presence: true
   validates :template, uniqueness: { scope: :type }, if: -> { template? }
   validates :instance, uniqueness: { scope: :type }, if: -> { instance? }

changelogs/unreleased/209824-add-group-id-to-services.yml

+---
+title: Add group_id column to the services table
+merge_request: 38499
+author:
+type: other

db/migrate/20200803111512_add_group_id_to_services.rb

+# frozen_string_literal: true
+
+class AddGroupIdToServices < ActiveRecord::Migration[6.0]
+  DOWNTIME = false
+
+  def change
+    add_column :services, :group_id, :bigint
+  end
+end

db/migrate/20200803112806_add_index_group_id_to_services.rb

+# frozen_string_literal: true
+
+class AddIndexGroupIdToServices < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  INDEX_NAME = 'index_services_on_unique_group_id_and_type'
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :services, [:group_id, :type], unique: true, name: INDEX_NAME
+
+    add_concurrent_foreign_key :services, :namespaces, column: :group_id
+  end
+
+  def down
+    remove_foreign_key_if_exists :services, column: :group_id
+
+    remove_concurrent_index_by_name :services, INDEX_NAME
+  end
+end

db/schema_migrations/20200803111512

+d0ea5e75c74d8405014fe5be6906e21b009d9eddb8eaefea939a67e01ee146e7
\ No newline at end of file

db/schema_migrations/20200803112806

+8e6ffd7b1d44ac846b79cf37798abc57eee169fbde0de096ea49df590af864b5
\ No newline at end of file

db/structure.sql

@@ -15333,7 +15333,8 @@ CREATE TABLE public.services (
     instance boolean DEFAULT false NOT NULL,
     comment_detail smallint,
     inherit_from_id bigint,
-    alert_events boolean
+    alert_events boolean,
+    group_id bigint
 );
 
 CREATE SEQUENCE public.services_id_seq
@@ -20567,6 +20568,8 @@ CREATE UNIQUE INDEX index_services_on_type_and_template_partial ON public.servic
 
 CREATE INDEX index_services_on_type_id_when_active_not_instance_not_template ON public.services USING btree (type, id) WHERE ((active = true) AND (instance = false) AND (template = false));
 
+CREATE UNIQUE INDEX index_services_on_unique_group_id_and_type ON public.services USING btree (group_id, type);
+
 CREATE UNIQUE INDEX index_shards_on_name ON public.shards USING btree (name);
 
 CREATE INDEX index_slack_integrations_on_service_id ON public.slack_integrations USING btree (service_id);
@@ -21774,6 +21777,9 @@ ALTER TABLE ONLY public.application_settings
 ALTER TABLE ONLY public.ci_triggers
     ADD CONSTRAINT fk_e8e10d1964 FOREIGN KEY (owner_id) REFERENCES public.users(id) ON DELETE CASCADE;
 
+ALTER TABLE ONLY public.services
+    ADD CONSTRAINT fk_e8fe908a34 FOREIGN KEY (group_id) REFERENCES public.namespaces(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY public.pages_domains
     ADD CONSTRAINT fk_ea2f6dfc6f FOREIGN KEY (project_id) REFERENCES public.projects(id) ON DELETE CASCADE;
 

spec/models/service_spec.rb

@@ -11,28 +11,43 @@ RSpec.describe Service do
   end
 
   describe 'validations' do
+    let(:group) { create(:group) }
+    let(:project) { create(:project) }
+
     it { is_expected.to validate_presence_of(:type) }
 
-    it 'validates presence of project_id if not template', :aggregate_failures do
+    it 'validates presence of project_id if not template, instance or group_id', :aggregate_failures do
       expect(build(:service, project_id: nil, template: true)).to be_valid
-      expect(build(:service, project_id: nil, template: false)).to be_invalid
-    end
-
-    it 'validates presence of project_id if not instance', :aggregate_failures do
       expect(build(:service, project_id: nil, instance: true)).to be_valid
+      expect(build(:service, project_id: nil, group_id: group.id)).to be_valid
+      expect(build(:service, project_id: nil, template: false)).to be_invalid
       expect(build(:service, project_id: nil, instance: false)).to be_invalid
+      expect(build(:service, project_id: nil, group_id: nil)).to be_invalid
     end
 
-    it 'validates absence of project_id if instance', :aggregate_failures do
-      expect(build(:service, project_id: nil, instance: true)).to be_valid
-      expect(build(:service, instance: true)).to be_invalid
+    it 'validates presence of group_id if not template, instance or project_id', :aggregate_failures do
+      expect(build(:service, group_id: nil, project_id: nil, template: true)).to be_valid
+      expect(build(:service, group_id: nil, project_id: nil, instance: true)).to be_valid
+      expect(build(:service, group_id: nil, project_id: project.id)).to be_valid
+      expect(build(:service, group_id: nil, project_id: nil, template: false)).to be_invalid
+      expect(build(:service, group_id: nil, project_id: nil, instance: false)).to be_invalid
+      expect(build(:service, group_id: nil, project_id: nil)).to be_invalid
     end
 
-    it 'validates absence of project_id if template', :aggregate_failures do
+    it 'validates absence of project_id if instance or template', :aggregate_failures do
+      expect(build(:service, instance: true)).to validate_absence_of(:project_id)
       expect(build(:service, template: true)).to validate_absence_of(:project_id)
+      expect(build(:service, instance: false)).not_to validate_absence_of(:project_id)
       expect(build(:service, template: false)).not_to validate_absence_of(:project_id)
     end
 
+    it 'validates absence of group_id if instance or template', :aggregate_failures do
+      expect(build(:service, instance: true)).to validate_absence_of(:group_id)
+      expect(build(:service, template: true)).to validate_absence_of(:group_id)
+      expect(build(:service, instance: false)).not_to validate_absence_of(:group_id)
+      expect(build(:service, template: false)).not_to validate_absence_of(:group_id)
+    end
+
     it 'validates service is template or instance' do
       expect(build(:service, project_id: nil, template: true, instance: true)).to be_invalid
     end
@@ -58,12 +73,15 @@ RSpec.describe Service do
     end
 
     it 'validates uniqueness of type and project_id on create' do
-      project = create(:project)
-
       expect(create(:service, project: project, type: 'Service')).to be_valid
       expect(build(:service, project: project, type: 'Service').valid?(:create)).to eq(false)
       expect(build(:service, project: project, type: 'Service').valid?(:update)).to eq(true)
     end
+
+    it 'validates uniqueness of type and group_id' do
+      expect(create(:service, group_id: group.id, project_id: nil, type: 'Service')).to be_valid
+      expect(build(:service, group_id: group.id, project_id: nil, type: 'Service')).to be_invalid
+    end
   end
 
   describe 'Scopes' do