Refactoring: Use Rails validation callback to ensure valid Webhook endpoint and extended test coverage.

This commit is contained in:
Mantas Masalskis 2021-03-16 07:50:12 +00:00 committed by Thorsten Eckel
parent 9a9c2ed719
commit d0dcae4776
2 changed files with 36 additions and 15 deletions

View file

@ -5,19 +5,19 @@ class Webhook < ApplicationModel
include ChecksLatestChangeObserved include ChecksLatestChangeObserved
include HasCollectionUpdate include HasCollectionUpdate
before_create :validate_endpoint
before_update :validate_endpoint
before_destroy Webhook::EnsureNoRelatedObjects before_destroy Webhook::EnsureNoRelatedObjects
validates :name, presence: true validates :name, presence: true
validate :validate_endpoint
private private
def validate_endpoint def validate_endpoint
uri = URI.parse(endpoint) uri = URI.parse(endpoint)
raise Exceptions::UnprocessableEntity, 'Invalid endpoint (no http/https)!' if !uri.is_a?(URI::HTTP)
raise Exceptions::UnprocessableEntity, 'Invalid endpoint (no hostname)!' if uri.host.nil? errors.add(:endpoint, 'Invalid endpoint (no http/https)!') if !uri.is_a?(URI::HTTP)
errors.add(:endpoint, 'Invalid endpoint (no hostname)!') if uri.host.nil?
rescue URI::InvalidURIError rescue URI::InvalidURIError
raise Exceptions::UnprocessableEntity, 'Invalid endpoint!' errors.add :endpoint, 'Invalid endpoint!'
end end
end end

View file

@ -3,40 +3,61 @@ require 'rails_helper'
RSpec.describe Webhook, type: :model do RSpec.describe Webhook, type: :model do
describe 'check endpoint' do describe 'check endpoint' do
subject(:webhook) { create(:webhook, endpoint: endpoint) } subject(:webhook) { build(:webhook, endpoint: endpoint) }
let(:endpoint) { 'example.com' } before { webhook.valid? }
let(:endpoint_errors) { webhook.errors.messages[:endpoint] }
context 'with missing http type' do context 'with missing http type' do
it 'raise an error' do let(:endpoint) { 'example.com' }
expect { webhook }.to raise_error(Exceptions::UnprocessableEntity, 'Invalid endpoint (no http/https)!')
it { is_expected.not_to be_valid }
it 'has an error' do
expect(endpoint_errors).to include 'Invalid endpoint (no http/https)!'
end end
end end
context 'with spaces in invalid hostname' do context 'with spaces in invalid hostname' do
let(:endpoint) { 'http:// example.com' } let(:endpoint) { 'http:// example.com' }
it 'raise an error' do it { is_expected.not_to be_valid }
expect { webhook }.to raise_error(Exceptions::UnprocessableEntity, 'Invalid endpoint!')
it 'has an error' do
expect(endpoint_errors).to include 'Invalid endpoint!'
end end
end end
context 'with ? in hostname' do context 'with ? in hostname' do
let(:endpoint) { 'http://?example.com' } let(:endpoint) { 'http://?example.com' }
it 'raise an error' do it { is_expected.not_to be_valid }
expect { webhook }.to raise_error(Exceptions::UnprocessableEntity, 'Invalid endpoint (no hostname)!')
it 'has an error' do
expect(endpoint_errors).to include 'Invalid endpoint (no hostname)!'
end end
end end
context 'with nil in endpoint' do context 'with nil in endpoint' do
let(:endpoint) { nil } let(:endpoint) { nil }
it 'raise an error' do it { is_expected.not_to be_valid }
expect { webhook }.to raise_error(Exceptions::UnprocessableEntity, 'Invalid endpoint!')
it 'has an error' do
expect(endpoint_errors).to include 'Invalid endpoint!'
end end
end end
context 'with a valid endpoint' do
let(:endpoint) { 'https://example.com/endpoint' }
it { is_expected.to be_valid }
it 'has no errors' do
expect(endpoint_errors).to be_empty
end
end
end end
describe '#destroy' do describe '#destroy' do