From bfecd20ef421024f8370c47d05fed9b886864d13 Mon Sep 17 00:00:00 2001 From: Anton-Ivanov Date: Thu, 25 Jul 2024 17:02:35 +0300 Subject: [PATCH] allow to create network_prefixes with the same prefix but diff length of number --- app/models/system/network_prefix.rb | 20 ++++- ...ge_unique_index_on_the_network_prefixes.rb | 21 +++++ db/structure.sql | 16 ++-- spec/factories/system/network_prefixes.rb | 2 +- spec/models/system/network_prefix_spec.rb | 78 +++++++++++++++++++ 5 files changed, 126 insertions(+), 11 deletions(-) create mode 100644 db/migrate/20240725132743_change_unique_index_on_the_network_prefixes.rb create mode 100644 spec/models/system/network_prefix_spec.rb diff --git a/app/models/system/network_prefix.rb b/app/models/system/network_prefix.rb index 4d71ca839..de74d539d 100644 --- a/app/models/system/network_prefix.rb +++ b/app/models/system/network_prefix.rb @@ -14,7 +14,7 @@ # # Indexes # -# network_prefixes_prefix_key (prefix) UNIQUE +# network_prefixes_prefix_key (prefix,number_min_length,number_max_length) UNIQUE # network_prefixes_prefix_range_idx (((prefix)::prefix_range)) USING gist # network_prefixes_uuid_key (uuid) UNIQUE # @@ -51,8 +51,9 @@ class System::NetworkPrefix < ApplicationRecord if: :number_max_length } - validates :prefix, uniqueness: { allow_blank: true }, presence: true + validates :prefix, presence: true validates :network, presence: true + validate :validate_prefix_uniqueness, if: :prefix?, on: %i[create update] scope :number_contains, lambda { |prefix| where('prefix_range(sys.network_prefixes.prefix)@>prefix_range(?)', prefix.to_s) @@ -83,4 +84,19 @@ def self.ransackable_scopes(_auth_object = nil) :number_contains ] end + + private + + def validate_prefix_uniqueness + return if find_conflicting_prefixes.empty? + + errors.add(:prefix, :taken) + errors.add(:number_max_length, "length with #{prefix} prefix already taken") + errors.add(:number_min_length, "length with #{prefix} prefix already taken") + end + + def find_conflicting_prefixes + base_query = System::NetworkPrefix.where(prefix:, number_max_length:, number_min_length:) + persisted? ? base_query.where.not(id: id) : base_query + end end diff --git a/db/migrate/20240725132743_change_unique_index_on_the_network_prefixes.rb b/db/migrate/20240725132743_change_unique_index_on_the_network_prefixes.rb new file mode 100644 index 000000000..fb26d3f5e --- /dev/null +++ b/db/migrate/20240725132743_change_unique_index_on_the_network_prefixes.rb @@ -0,0 +1,21 @@ +class ChangeUniqueIndexOnTheNetworkPrefixes < ActiveRecord::Migration[7.0] + disable_ddl_transaction! + + def up + execute <<~SQL.squish + ALTER TABLE ONLY sys.network_prefixes + DROP CONSTRAINT network_prefixes_prefix_key; + + CREATE UNIQUE INDEX network_prefixes_prefix_key ON sys.network_prefixes USING btree (prefix, number_min_length, number_max_length); + SQL + end + + def down + execute <<~SQL.squish + DROP INDEX IF EXISTS network_prefixes_prefix_key; + + ALTER TABLE ONLY sys.network_prefixes + ADD CONSTRAINT network_prefixes_prefix_key UNIQUE (prefix); + SQL + end +end diff --git a/db/structure.sql b/db/structure.sql index f2e87ed10..48b1f765d 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -39032,14 +39032,6 @@ ALTER TABLE ONLY sys.network_prefixes ADD CONSTRAINT network_prefixes_pkey PRIMARY KEY (id); --- --- Name: network_prefixes network_prefixes_prefix_key; Type: CONSTRAINT; Schema: sys; Owner: - --- - -ALTER TABLE ONLY sys.network_prefixes - ADD CONSTRAINT network_prefixes_prefix_key UNIQUE (prefix); - - -- -- Name: network_prefixes network_prefixes_uuid_key; Type: CONSTRAINT; Schema: sys; Owner: - -- @@ -39846,6 +39838,13 @@ CREATE INDEX "index_sys.cdr_exports_on_customer_account_id" ON sys.cdr_exports U CREATE UNIQUE INDEX "index_sys.cdr_exports_on_uuid" ON sys.cdr_exports USING btree (uuid); +-- +-- Name: network_prefixes_prefix_key; Type: INDEX; Schema: sys; Owner: - +-- + +CREATE UNIQUE INDEX network_prefixes_prefix_key ON sys.network_prefixes USING btree (prefix, number_min_length, number_max_length); + + -- -- Name: network_prefixes_prefix_range_idx; Type: INDEX; Schema: sys; Owner: - -- @@ -41166,6 +41165,7 @@ INSERT INTO "public"."schema_migrations" (version) VALUES ('20240702142447'), ('20240704100545'), ('20240721201110'), +('20240725132743'), ('20240725135654'); diff --git a/spec/factories/system/network_prefixes.rb b/spec/factories/system/network_prefixes.rb index 5b0c77976..48c3c818e 100644 --- a/spec/factories/system/network_prefixes.rb +++ b/spec/factories/system/network_prefixes.rb @@ -14,7 +14,7 @@ # # Indexes # -# network_prefixes_prefix_key (prefix) UNIQUE +# network_prefixes_prefix_key (prefix,number_min_length,number_max_length) UNIQUE # network_prefixes_prefix_range_idx (((prefix)::prefix_range)) USING gist # network_prefixes_uuid_key (uuid) UNIQUE # diff --git a/spec/models/system/network_prefix_spec.rb b/spec/models/system/network_prefix_spec.rb new file mode 100644 index 000000000..541b9d2d1 --- /dev/null +++ b/spec/models/system/network_prefix_spec.rb @@ -0,0 +1,78 @@ +# frozen_string_literal: true + +RSpec.describe System::NetworkPrefix, type: :model do + describe '#validate_prefix_uniqueness' do + subject { network_prefix.errors.messages } + + let(:network_prefix_attrs) { { prefix: '9', number_min_length: 1, number_max_length: 1 } } + + context 'when no conflicting prefixes exist' do + let(:network_prefix) { FactoryBot.build(:network_prefix, prefix: '1', number_min_length: 12) } + + before do + FactoryBot.create(:network_prefix, network_prefix_attrs) + network_prefix.valid? + end + + it 'does not add any validation errors' do + expect(subject[:number_max_length]).to eq [] + expect(subject[:number_min_length]).to eq [] + expect(subject[:prefix]).to eq [] + end + end + + context 'when conflicting prefixes exist' do + let(:network_prefix) { FactoryBot.build(:network_prefix, network_prefix_attrs) } + + before do + FactoryBot.create(:network_prefix, network_prefix_attrs) + network_prefix.valid? + end + + it 'adds errors for prefix, number_max_length, and number_min_length' do + expect(subject[:prefix]).to contain_exactly('has already been taken') + expect(subject[:number_max_length]).to contain_exactly('length with 9 prefix already taken') + expect(subject[:number_min_length]).to contain_exactly('length with 9 prefix already taken') + end + end + + context 'when create network_prefix with "prefix" = nil' do + subject { network_prefix.valid? } + + let(:network_prefix) { FactoryBot.build(:network_prefix, prefix: nil) } + + it 'should not execute validation' do + expect(network_prefix).not_to receive(:validate_prefix_uniqueness) + subject + end + end + + context 'when update record and there is conflicting record' do + subject { record.update(prefix: '2') } + + let!(:record) { FactoryBot.create(:network_prefix, prefix: '1') } + + before { FactoryBot.create(:network_prefix, prefix: '2') } + + it 'should add error validation' do + subject + + expect(record.errors[:prefix]).to contain_exactly('has already been taken') + expect(record.errors[:number_max_length]).to contain_exactly('length with 2 prefix already taken') + expect(record.errors[:number_min_length]).to contain_exactly('length with 2 prefix already taken') + end + end + end + + describe 'network_prefixes_prefix_key' do + subject { network_prefix.save!(validate: false) } + + let(:network_prefix) { FactoryBot.build(:network_prefix, prefix: '1') } + + before { FactoryBot.create(:network_prefix, prefix: '1') } + + it 'should raise PG error with "network_prefixes_prefix_key" index' do + expect { subject }.to raise_error(ActiveRecord::RecordNotUnique).with_message(/network_prefixes_prefix_key/) + end + end +end