diff --git a/db/migrations/20250225132929_add_apps_file_based_service_binding_feature_columns.rb b/db/migrations/20250225132929_add_apps_file_based_service_binding_feature_columns.rb index e82506b8e1..9064fcc05e 100644 --- a/db/migrations/20250225132929_add_apps_file_based_service_binding_feature_columns.rb +++ b/db/migrations/20250225132929_add_apps_file_based_service_binding_feature_columns.rb @@ -16,7 +16,7 @@ add_column :apps, :service_binding_k8s_enabled, :boolean, default: false, null: false unless schema(:apps).map(&:first).include?(:service_binding_k8s_enabled) add_column :apps, :file_based_vcap_services_enabled, :boolean, default: false, null: false unless schema(:apps).map(&:first).include?(:file_based_vcap_services_enabled) - unless check_constraint_exists?(self) + if check_constraint_supported?(self) && !check_constraint_exists?(self) run('ALTER TABLE apps ADD CONSTRAINT only_one_sb_feature_enabled CHECK (NOT (service_binding_k8s_enabled AND file_based_vcap_services_enabled))') end end @@ -24,7 +24,7 @@ down do alter_table :apps do - drop_constraint :only_one_sb_feature_enabled if check_constraint_exists?(@db) + drop_constraint :only_one_sb_feature_enabled if check_constraint_supported?(@db) && check_constraint_exists?(@db) drop_column :service_binding_k8s_enabled if @db.schema(:apps).map(&:first).include?(:service_binding_k8s_enabled) drop_column :file_based_vcap_services_enabled if @db.schema(:apps).map(&:first).include?(:file_based_vcap_services_enabled) end @@ -39,3 +39,9 @@ def check_constraint_exists?(database) CONSTRAINT_NAME: 'only_one_sb_feature_enabled').any? end end + +# check constraints are not available in Mysql versions < 8 +# this is also enforced on application level, so it should be fine not to create it on that version +def check_constraint_supported?(database) + database.database_type == :postgres || (database.database_type == :mysql && database.server_version >= 80_000) +end diff --git a/spec/migrations/20250225132929_add_apps_file_based_service_binding_feature_columns_spec.rb b/spec/migrations/20250225132929_add_apps_file_based_service_binding_feature_columns_spec.rb index c0616abdf5..7d3f564636 100644 --- a/spec/migrations/20250225132929_add_apps_file_based_service_binding_feature_columns_spec.rb +++ b/spec/migrations/20250225132929_add_apps_file_based_service_binding_feature_columns_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' require 'migrations/helpers/migration_shared_context' -RSpec.describe 'migration to add service_binding_k8s_enabled column to apps table', isolation: :truncation, type: :migration do +RSpec.describe 'migration to add file-based service binding feature columns to apps table', isolation: :truncation, type: :migration do include_context 'migration' do let(:migration_filename) { '20250225132929_add_apps_file_based_service_binding_feature_columns.rb' } end @@ -44,7 +44,7 @@ expect { run_migration }.not_to raise_error expect(db[:apps].columns).to include(:service_binding_k8s_enabled) expect(db[:apps].columns).to include(:file_based_vcap_services_enabled) - expect(check_constraint_exists?(db)).to be(true) + expect(check_constraint_exists?(db)).to be(true) if check_constraint_supported?(db) end end end @@ -83,38 +83,56 @@ expect { run_migration }.not_to raise_error expect(db[:apps].columns).to include(:service_binding_k8s_enabled) expect(db[:apps].columns).to include(:file_based_vcap_services_enabled) - expect(check_constraint_exists?(db)).to be(true) + expect(check_constraint_exists?(db)).to be(true) if check_constraint_supported?(db) end end end describe 'check constraint' do - it 'adds the check constraint' do - expect(check_constraint_exists?(db)).to be(false) - run_migration - expect(check_constraint_exists?(db)).to be(true) - end + context 'when supported' do + before do + skip 'check constraint not supported by db' unless check_constraint_supported?(db) + end - it 'forbids setting both features to true' do - run_migration - expect { db[:apps].insert(guid: 'some_app', file_based_vcap_services_enabled: true, service_binding_k8s_enabled: true) }.to(raise_error do |error| - expect(error.inspect).to include('only_one_sb_feature_enabled', 'violate') - end) - end + it 'adds the check constraint' do + expect(check_constraint_exists?(db)).to be(false) + run_migration + expect(check_constraint_exists?(db)).to be(true) + end - context 'when it already exists' do - before do - db.add_column :apps, :service_binding_k8s_enabled, :boolean, default: false, null: false, if_not_exists: true - db.add_column :apps, :file_based_vcap_services_enabled, :boolean, default: false, null: false, if_not_exists: true - db.alter_table :apps do - add_constraint(name: :only_one_sb_feature_enabled) do - Sequel.lit('NOT (service_binding_k8s_enabled AND file_based_vcap_services_enabled)') + it 'forbids setting both features to true' do + run_migration + expect { db[:apps].insert(guid: 'some_app', file_based_vcap_services_enabled: true, service_binding_k8s_enabled: true) }.to(raise_error do |error| + expect(error.inspect).to include('only_one_sb_feature_enabled', 'violate') + end) + end + + context 'when it already exists' do + before do + db.add_column :apps, :service_binding_k8s_enabled, :boolean, default: false, null: false, if_not_exists: true + db.add_column :apps, :file_based_vcap_services_enabled, :boolean, default: false, null: false, if_not_exists: true + db.alter_table :apps do + add_constraint(name: :only_one_sb_feature_enabled) do + Sequel.lit('NOT (service_binding_k8s_enabled AND file_based_vcap_services_enabled)') + end end end + + it 'does not fail' do + expect { run_migration }.not_to raise_error + end + end + end + + context 'when not supported' do + before do + skip 'check constraint supported by db' if check_constraint_supported?(db) end it 'does not fail' do expect { run_migration }.not_to raise_error + expect(db[:apps].columns).to include(:service_binding_k8s_enabled) + expect(db[:apps].columns).to include(:file_based_vcap_services_enabled) end end end @@ -178,25 +196,43 @@ end describe 'check constraint' do - it 'removes the check constraint' do - expect(check_constraint_exists?(db)).to be(true) - run_rollback - expect(check_constraint_exists?(db)).to be(false) + context 'when supported' do + before do + skip 'check constraint not supported by db' unless check_constraint_supported?(db) + end + + it 'removes the check constraint' do + expect(check_constraint_exists?(db)).to be(true) + run_rollback + expect(check_constraint_exists?(db)).to be(false) + end + + context 'when it does not exist' do + before do + db.alter_table :apps do + drop_constraint :only_one_sb_feature_enabled + end + end + + it 'does not fail' do + expect(check_constraint_exists?(db)).to be(false) + expect { run_rollback }.not_to raise_error + expect(db[:apps].columns).not_to include(:service_binding_k8s_enabled) + expect(db[:apps].columns).not_to include(:file_based_vcap_services_enabled) + expect(check_constraint_exists?(db)).to be(false) + end + end end - context 'when it does not exist' do + context 'when not supported' do before do - db.alter_table :apps do - drop_constraint :only_one_sb_feature_enabled - end + skip 'check constraint supported by db' if check_constraint_supported?(db) end it 'does not fail' do - expect(check_constraint_exists?(db)).to be(false) expect { run_rollback }.not_to raise_error expect(db[:apps].columns).not_to include(:service_binding_k8s_enabled) expect(db[:apps].columns).not_to include(:file_based_vcap_services_enabled) - expect(check_constraint_exists?(db)).to be(false) end end end