From 660993b415d928b45ae851b8e6d09d0e8af2f7f3 Mon Sep 17 00:00:00 2001
From: Matt Jankowski <matt@jankowski.online>
Date: Fri, 28 Jul 2023 17:12:25 -0400
Subject: [PATCH] Add coverage for `URLValidator` (#25591)
---
app/validators/url_validator.rb | 23 +++++++--
spec/validators/url_validator_spec.rb | 70 +++++++++++++++++++--------
2 files changed, 70 insertions(+), 23 deletions(-)
diff --git a/app/validators/url_validator.rb b/app/validators/url_validator.rb
index a90fb6958..f88a37030 100644
--- a/app/validators/url_validator.rb
+++ b/app/validators/url_validator.rb
@@ -1,16 +1,31 @@
# frozen_string_literal: true
class URLValidator < ActiveModel::EachValidator
+ VALID_SCHEMES = %w(http https).freeze
+
def validate_each(record, attribute, value)
- record.errors.add(attribute, :invalid) unless compliant?(value)
+ @value = value
+
+ record.errors.add(attribute, :invalid) unless compliant_url?
end
private
- def compliant?(url)
- parsed_url = Addressable::URI.parse(url)
- parsed_url && %w(http https).include?(parsed_url.scheme) && parsed_url.host
+ def compliant_url?
+ parsed_url.present? && valid_url_scheme? && valid_url_host?
+ end
+
+ def parsed_url
+ Addressable::URI.parse(@value)
rescue Addressable::URI::InvalidURIError
false
end
+
+ def valid_url_scheme?
+ VALID_SCHEMES.include?(parsed_url.scheme)
+ end
+
+ def valid_url_host?
+ parsed_url.host.present?
+ end
end
diff --git a/spec/validators/url_validator_spec.rb b/spec/validators/url_validator_spec.rb
index f2220e32b..4f32b7b39 100644
--- a/spec/validators/url_validator_spec.rb
+++ b/spec/validators/url_validator_spec.rb
@@ -2,32 +2,64 @@
require 'rails_helper'
-RSpec.describe URLValidator, type: :validator do
- describe '#validate_each' do
- before do
- allow(validator).to receive(:compliant?).with(value) { compliant }
- validator.validate_each(record, attribute, value)
+describe URLValidator do
+ let(:record_class) do
+ Class.new do
+ include ActiveModel::Validations
+ attr_accessor :profile
+
+ validates :profile, url: true
end
+ end
+ let(:record) { record_class.new }
- let(:validator) { described_class.new(attributes: [attribute]) }
- let(:record) { instance_double(Webhook, errors: errors) }
- let(:errors) { instance_double(ActiveModel::Errors, add: nil) }
- let(:value) { '' }
- let(:attribute) { :foo }
+ describe '#validate_each' do
+ context 'with a nil value' do
+ it 'adds errors' do
+ record.profile = nil
- context 'when not compliant?' do
- let(:compliant) { false }
-
- it 'calls errors.add' do
- expect(errors).to have_received(:add).with(attribute, :invalid)
+ expect(record).to_not be_valid
+ expect(record.errors.first.attribute).to eq(:profile)
+ expect(record.errors.first.type).to eq(:invalid)
end
end
- context 'when compliant?' do
- let(:compliant) { true }
+ context 'with an invalid url scheme' do
+ it 'adds errors' do
+ record.profile = 'ftp://example.com/page'
- it 'not calls errors.add' do
- expect(errors).to_not have_received(:add).with(attribute, any_args)
+ expect(record).to_not be_valid
+ expect(record.errors.first.attribute).to eq(:profile)
+ expect(record.errors.first.type).to eq(:invalid)
+ end
+ end
+
+ context 'without a hostname' do
+ it 'adds errors' do
+ record.profile = 'https:///page'
+
+ expect(record).to_not be_valid
+ expect(record.errors.first.attribute).to eq(:profile)
+ expect(record.errors.first.type).to eq(:invalid)
+ end
+ end
+
+ context 'with an unparseable value' do
+ it 'adds errors' do
+ record.profile = 'https://host:port/page' # non-numeric port string causes invalid uri error
+
+ expect(record).to_not be_valid
+ expect(record.errors.first.attribute).to eq(:profile)
+ expect(record.errors.first.type).to eq(:invalid)
+ end
+ end
+
+ context 'with a valid url' do
+ it 'does not add errors' do
+ record.profile = 'https://example.com/page'
+
+ expect(record).to be_valid
+ expect(record.errors).to be_empty
end
end
end