From 87d1ec0dfbb2e1ef85523ae53a93699070d194fb Mon Sep 17 00:00:00 2001 From: Karthik Sivadas Date: Wed, 17 Apr 2024 21:20:35 +0530 Subject: [PATCH 1/2] fix: Add greater than zero validation for sync_interval --- server/app/models/sync.rb | 2 +- server/spec/models/sync_spec.rb | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/server/app/models/sync.rb b/server/app/models/sync.rb index 34adbcc9..56eddbe4 100644 --- a/server/app/models/sync.rb +++ b/server/app/models/sync.rb @@ -28,7 +28,7 @@ class Sync < ApplicationRecord validates :model_id, presence: true validates :configuration, presence: true validates :schedule_type, presence: true - validates :sync_interval, presence: true + validates :sync_interval, presence: true, numericality: { greater_than: 0 } validates :sync_interval_unit, presence: true validates :stream_name, presence: true validates :status, presence: true diff --git a/server/spec/models/sync_spec.rb b/server/spec/models/sync_spec.rb index 02063013..7fd68bce 100644 --- a/server/spec/models/sync_spec.rb +++ b/server/spec/models/sync_spec.rb @@ -314,4 +314,23 @@ hash_including(options: hash_including(workflow_id: a_string_starting_with("terminate-")))) end end + + describe "validations" do + let(:source) do + create(:connector, connector_type: "source", connector_name: "Snowflake") + end + let(:destination) { create(:connector, connector_type: "destination") } + let!(:catalog) { create(:catalog, connector: destination) } + let(:sync) { build(:sync, sync_interval: 3, sync_interval_unit: "hours", source:, destination:) } + + it "validates that sync_interval is greater than 0" do + sync.sync_interval = 0 + expect(sync).not_to be_valid + expect(sync.errors[:sync_interval]).to include("must be greater than 0") + + sync.sync_interval = -1 + expect(sync).not_to be_valid + expect(sync.errors[:sync_interval]).to include("must be greater than 0") + end + end end From 3213774fdf9e1316d1c81640b361c90a84c26732 Mon Sep 17 00:00:00 2001 From: Karthik Sivadas Date: Wed, 17 Apr 2024 21:59:07 +0530 Subject: [PATCH 2/2] Add validation in sync contract --- server/app/contracts/sync_contracts.rb | 8 ++++ server/spec/contracts/sync_contracts_spec.rb | 44 ++++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/server/app/contracts/sync_contracts.rb b/server/app/contracts/sync_contracts.rb index 2c355389..485a3a6d 100644 --- a/server/app/contracts/sync_contracts.rb +++ b/server/app/contracts/sync_contracts.rb @@ -42,6 +42,10 @@ class Create < Dry::Validation::Contract rule(sync: :sync_interval_unit) do key.failure("invalid connector type") unless Sync.sync_interval_units.keys.include?(value.downcase) end + + rule("sync.sync_interval") do + key.failure("must be greater than 0") if value <= 0 + end end class Update < Dry::Validation::Contract @@ -73,6 +77,10 @@ class Update < Dry::Validation::Contract rule(sync: :sync_interval_unit) do key.failure("invalid connector type") if key? && !Sync.sync_interval_units.keys.include?(value.downcase) end + + rule("sync.sync_interval") do + key.failure("must be greater than 0") if value <= 0 + end end class Destroy < Dry::Validation::Contract diff --git a/server/spec/contracts/sync_contracts_spec.rb b/server/spec/contracts/sync_contracts_spec.rb index 1d59d1df..5ac66e41 100644 --- a/server/spec/contracts/sync_contracts_spec.rb +++ b/server/spec/contracts/sync_contracts_spec.rb @@ -65,6 +65,50 @@ expect(result.errors[:sync][:sync_mode]).to include("invalid sync mode") end end + + context "with non-positive sync_interval" do + let(:invalid_inputs) { { sync: valid_inputs[:sync].merge(sync_interval: 0) } } + + it "fails validation" do + result = contract.call(invalid_inputs) + expect(result.errors[:sync][:sync_interval]).to include("must be greater than 0") + end + end + end + + describe SyncContracts::Update do + subject(:contract) { described_class.new } + + let(:valid_inputs) do + { + id: 1, + sync: { + source_id: 1, + model_id: 2, + destination_id: 3, + schedule_type: "automated", + sync_interval: 15, + sync_interval_unit: "hours", + sync_mode: "incremental", + stream_name: "updated_stream" + } + } + end + + context "with valid inputs" do + it "passes validation" do + expect(contract.call(valid_inputs)).to be_success + end + end + + context "with non-positive sync_interval" do + let(:invalid_inputs) { { id: 1, sync: valid_inputs[:sync].merge(sync_interval: -1) } } + + it "fails validation" do + result = contract.call(invalid_inputs) + expect(result.errors[:sync][:sync_interval]).to include("must be greater than 0") + end + end end describe SyncContracts::Destroy do