Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix optional aware validator #97

Merged
merged 3 commits into from
Feb 17, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/diaspora_federation/discovery/h_card.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class HCard < Entity
# @!attribute [r] nickname
# The first part of the diaspora* ID
# @return [String] nickname
property :nickname, :string, default: nil
property :nickname, :string, optional: true

# @!attribute [r] full_name
# @return [String] display name of the user
Expand All @@ -60,7 +60,7 @@ class HCard < Entity
# installations).
#
# @return [String] link to the pod
property :url, :string, default: nil
property :url, :string, optional: true

# @!attribute [r] public_key
# When a user is created on the pod, the pod MUST generate a pgp keypair
Expand Down
8 changes: 4 additions & 4 deletions lib/diaspora_federation/discovery/web_finger.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class WebFinger < Entity

# @!attribute [r] profile_url
# @return [String] link to the users profile
property :profile_url, :string, default: nil
property :profile_url, :string, optional: true

# @!attribute [r] atom_url
# This atom feed is an Activity Stream of the user's public posts. diaspora*
Expand All @@ -55,18 +55,18 @@ class WebFinger < Entity
# Note that this feed MAY also be made available through the PubSubHubbub
# mechanism by supplying a <link rel="hub"> in the atom feed itself.
# @return [String] atom feed url
property :atom_url, :string, default: nil
property :atom_url, :string, optional: true

# @!attribute [r] salmon_url
# @note could be nil
# @return [String] salmon endpoint url
# @see https://cdn.rawgit.com/salmon-protocol/salmon-protocol/master/draft-panzer-salmon-00.html#SMLR
# Panzer draft for Salmon, paragraph 3.3
property :salmon_url, :string, default: nil
property :salmon_url, :string, optional: true

# @!attribute [r] subscribe_url
# This url is used to find another user on the home-pod of the user in the webfinger.
property :subscribe_url, :string, default: nil
property :subscribe_url, :string, optional: true

# +hcard_url+ link relation
REL_HCARD = "http://microformats.org/profile/hcard".freeze
Expand Down
2 changes: 1 addition & 1 deletion lib/diaspora_federation/entities/account_migration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def old_identity
# Returns diaspora* ID of the new person identity.
# @return [String] diaspora* ID of the new person identity
def new_identity
profile.author
profile.author if profile
end

# @return [String] string representation of this object
Expand Down
2 changes: 1 addition & 1 deletion lib/diaspora_federation/test/factories.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ module Factories

Fabricator(:account_migration_entity, class_name: DiasporaFederation::Entities::AccountMigration) do
author { Fabricate.sequence(:diaspora_id) }
profile { Fabricate(:profile_entity) }
profile {|attrs| Fabricate(:profile_entity, author: attrs[:author]) }
old_identity { Fabricate.sequence(:diaspora_id) }
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class ConversationValidator < OptionalAwareValidator

rule :subject, [:not_empty, length: {maximum: 255}]

rule :participants, diaspora_id_list: {minimum: 2}
rule :participants, [:not_empty, diaspora_id_list: {minimum: 2}]
rule :messages, :not_nil
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ class EventParticipationValidator < OptionalAwareValidator

include RelayableValidator

rule :status, regular_expression: {regex: /\A(accepted|declined|tentative)\z/}
rule :status, [:not_empty, regular_expression: {regex: /\A(accepted|declined|tentative)\z/}]
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,8 @@ def rules
private

def optional_props
entity_name = self.class.name.split("::").last.sub("Validator", "")
return [] unless Entities.const_defined?(entity_name)

Entities.const_get(entity_name).optional_props
return [] unless @obj.class.respond_to?(:optional_props)
@obj.class.optional_props
end
end
end
Expand Down
Original file line number Diff line number Diff line change
@@ -1,47 +1,46 @@
module DiasporaFederation
describe Validators::OptionalAwareValidator do
let(:entity_data) {
{test1: "abc", test2: true, test7: "abc", multi: []}
}
def entity_stub(additional_data={})
allow_any_instance_of(Entities::TestComplexEntity).to receive(:freeze)
allow_any_instance_of(Entities::TestComplexEntity).to receive(:validate)
entity_data = {test1: "abc", test2: true, test3: nil, test4: nil, test5: nil, test6: nil, test7: "abc", multi: []}
Entities::TestComplexEntity.new(entity_data.merge(additional_data))
end

it "validates a valid object" do
validator = Validators::TestComplexEntityValidator.new(OpenStruct.new(entity_data))
validator = Validators::TestComplexEntityValidator.new(entity_stub)
expect(validator).to be_valid
expect(validator.errors).to be_empty
end

it "fails when a mandatory property is invalid" do
["ab", nil].each do |val|
entity = OpenStruct.new(entity_data.merge(test1: val))
validator = Validators::TestComplexEntityValidator.new(entity)
validator = Validators::TestComplexEntityValidator.new(entity_stub(test1: val))
expect(validator).not_to be_valid
expect(validator.errors).to include(:test1)
end
end

it "fails when an optional property is invalid" do
entity = OpenStruct.new(entity_data.merge(test7: "ab"))
validator = Validators::TestComplexEntityValidator.new(entity)
validator = Validators::TestComplexEntityValidator.new(entity_stub(test7: "ab"))
expect(validator).not_to be_valid
expect(validator.errors).to include(:test7)
end

it "allows an optional property to be nil" do
entity = OpenStruct.new(entity_data.merge(test7: nil))
validator = Validators::TestComplexEntityValidator.new(entity)
validator = Validators::TestComplexEntityValidator.new(entity_stub(test7: nil))
expect(validator).to be_valid
expect(validator.errors).to be_empty
end

it "doesn't ignore 'not_nil' rules for an optional property" do
entity = OpenStruct.new(entity_data.merge(multi: nil))
validator = Validators::TestComplexEntityValidator.new(entity)
validator = Validators::TestComplexEntityValidator.new(entity_stub(multi: nil))
expect(validator).not_to be_valid
expect(validator.errors).to include(:multi)
end

it "doesn't fail when there is no entity for this validator" do
entity = OpenStruct.new(entity_data.merge(test1: nil))
it "doesn't fail when the entity doesn't have optional props" do
entity = OpenStruct.new(test1: nil)
validator = Validators::TestUnknownEntityValidator.new(entity)
expect(validator).not_to be_valid
expect(validator.errors).to include(:test1)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ module DiasporaFederation
describe "##{prop}" do
it_behaves_like "a property with a value validation/restriction" do
let(:property) { prop }
let(:wrong_values) { ["", "https://asdf$%.com", "example.com"] }
let(:wrong_values) { %w[https://asdf$.com example.com] }
let(:correct_values) { [nil] }
end

Expand Down
6 changes: 4 additions & 2 deletions spec/support/shared_validator_specs.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
def entity_stub(entity, data={})
OpenStruct.new(Fabricate.schematic(entity).options[:class_name].default_values
.merge(Fabricate.attributes_for(entity)).merge(data))
entity_class = Fabricate.schematic(entity).options[:class_name]
allow_any_instance_of(entity_class).to receive(:freeze)
allow_any_instance_of(entity_class).to receive(:validate)
entity_class.new(Fabricate.attributes_for(entity).merge(data))
end

ALPHANUMERIC_RANGE = [*"0".."9", *"A".."Z", *"a".."z"].freeze
Expand Down