Merge pull request #76 from SuperTux88/photo-height-width-optional

Make height and width optional for photos

docs/_entities/photo.md

@@ -14,8 +14,6 @@ This entity represents a photo. It can be standalone or nested in a [StatusMessa
 | `created_at`        | [Timestamp][timestamp]       | The create timestamp of the photo.                               |
 | `remote_photo_path` | [URL][url]                   | The URL to the photo, without filename (see `remote_photo_name`) |
 | `remote_photo_name` | [String][string] (255)       | The filename of the photo.                                       |
-| `height`            | [Integer][integer]           | The height of the photo in pixels.                               |
-| `width`             | [Integer][integer]           | The width of the photo in pixels.                                |
 
 ## Optional Properties
 
@@ -23,6 +21,8 @@ This entity represents a photo. It can be standalone or nested in a [StatusMessa
 | --------------------- | ------------------------ | ------------------------------------------------------------------------------- |
 | `text`                | [String][string] (65535) | Description text for the photo.                                                 |
 | `status_message_guid` | [GUID][guid]             | The GUID of the [StatusMessage][status_message] to which the photo is attached. |
+| `height`              | [Integer][integer]       | The height of the photo in pixels.                                              |
+| `width`               | [Integer][integer]       | The width of the photo in pixels.                                               |
 
 ## Example
 

lib/diaspora_federation/entities/photo.rb

@@ -48,12 +48,12 @@ module DiasporaFederation
       # @!attribute [r] height
       #   Photo height
       #   @return [Integer] height
-      property :height, :integer
+      property :height, :integer, optional: true
 
       # @!attribute [r] width
       #   Photo width
       #   @return [Integer] width
-      property :width, :integer
+      property :width, :integer, optional: true
     end
   end
 end

lib/diaspora_federation/entities/reshare.rb

@@ -16,7 +16,7 @@ module DiasporaFederation
       #   Guid of the original post
       #   @see StatusMessage#guid
       #   @return [String] root guid
-      property :root_guid, :string
+      property :root_guid, :string, optional: true
 
       # @!attribute [r] public
       #   Has no meaning at the moment

lib/diaspora_federation/validators.rb

@@ -37,6 +37,7 @@ end
 require "diaspora_federation/validators/related_entity_validator"
 
 # abstract types
+require "diaspora_federation/validators/optional_aware_validator"
 require "diaspora_federation/validators/relayable_validator"
 
 # types

lib/diaspora_federation/validators/account_deletion_validator.rb

@@ -1,7 +1,7 @@
 module DiasporaFederation
   module Validators
     # This validates a {Entities::AccountDeletion}.
-    class AccountDeletionValidator < Validation::Validator
+    class AccountDeletionValidator < OptionalAwareValidator
       include Validation
 
       rule :author, %i[not_empty diaspora_id]

lib/diaspora_federation/validators/account_migration_validator.rb

@@ -1,7 +1,7 @@
 module DiasporaFederation
   module Validators
     # This validates a {Entities::AccountMigration}.
-    class AccountMigrationValidator < Validation::Validator
+    class AccountMigrationValidator < OptionalAwareValidator
       include Validation
 
       rule :author, %i[not_empty diaspora_id]

lib/diaspora_federation/validators/comment_validator.rb

@@ -1,7 +1,7 @@
 module DiasporaFederation
   module Validators
     # This validates a {Entities::Comment}.
-    class CommentValidator < Validation::Validator
+    class CommentValidator < OptionalAwareValidator
       include Validation
 
       include RelayableValidator

lib/diaspora_federation/validators/contact_validator.rb

@@ -1,7 +1,7 @@
 module DiasporaFederation
   module Validators
     # This validates a {Entities::Contact}.
-    class ContactValidator < Validation::Validator
+    class ContactValidator < OptionalAwareValidator
       include Validation
 
       rule :author, %i[not_empty diaspora_id]

lib/diaspora_federation/validators/conversation_validator.rb

@@ -1,7 +1,7 @@
 module DiasporaFederation
   module Validators
     # This validates a {Entities::Conversation}.
-    class ConversationValidator < Validation::Validator
+    class ConversationValidator < OptionalAwareValidator
       include Validation
 
       rule :author, %i[not_empty diaspora_id]

lib/diaspora_federation/validators/event_participation_validator.rb

@@ -1,7 +1,7 @@
 module DiasporaFederation
   module Validators
     # This validates a {Entities::EventParticipation}.
-    class EventParticipationValidator < Validation::Validator
+    class EventParticipationValidator < OptionalAwareValidator
       include Validation
 
       include RelayableValidator

lib/diaspora_federation/validators/event_validator.rb

@@ -1,7 +1,7 @@
 module DiasporaFederation
   module Validators
     # This validates a {Entities::Event}.
-    class EventValidator < Validation::Validator
+    class EventValidator < OptionalAwareValidator
       include Validation
 
       rule :author, %i[not_empty diaspora_id]

lib/diaspora_federation/validators/h_card_validator.rb

@@ -3,7 +3,7 @@ module DiasporaFederation
     # This validates a {Discovery::HCard}.
     #
     # @note
-    class HCardValidator < Validation::Validator
+    class HCardValidator < OptionalAwareValidator
       include Validation
 
       rule :guid, :guid

lib/diaspora_federation/validators/like_validator.rb

@@ -1,7 +1,7 @@
 module DiasporaFederation
   module Validators
     # This validates a {Entities::Like}.
-    class LikeValidator < Validation::Validator
+    class LikeValidator < OptionalAwareValidator
       include Validation
 
       include RelayableValidator

lib/diaspora_federation/validators/location_validator.rb

@@ -1,7 +1,7 @@
 module DiasporaFederation
   module Validators
     # This validates a {Entities::Location}.
-    class LocationValidator < Validation::Validator
+    class LocationValidator < OptionalAwareValidator
       include Validation
 
       rule :lat, :not_empty

lib/diaspora_federation/validators/message_validator.rb

@@ -1,7 +1,7 @@
 module DiasporaFederation
   module Validators
     # This validates a {Entities::Message}.
-    class MessageValidator < Validation::Validator
+    class MessageValidator < OptionalAwareValidator
       include Validation
 
       rule :author, %i[not_empty diaspora_id]

lib/diaspora_federation/validators/optional_aware_validator.rb

@@ -0,0 +1,23 @@
+module DiasporaFederation
+  module Validators
+    # Abstract validator which only validates optional fields when they are not nil.
+    class OptionalAwareValidator < Validation::Validator
+      def rules
+        super.reject do |field, rules|
+          @obj.public_send(field).nil? &&
+            !rules.map(&:class).include?(Validation::Rule::NotNil) &&
+            optional_props.include?(field)
+        end
+      end
+
+      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
+      end
+    end
+  end
+end

lib/diaspora_federation/validators/participation_validator.rb

@@ -1,7 +1,7 @@
 module DiasporaFederation
   module Validators
     # This validates a {Entities::Participation}.
-    class ParticipationValidator < Validation::Validator
+    class ParticipationValidator < OptionalAwareValidator
       include Validation
 
       rule :author, %i[not_empty diaspora_id]

lib/diaspora_federation/validators/person_validator.rb

@@ -1,7 +1,7 @@
 module DiasporaFederation
   module Validators
     # This validates a {Entities::Person}.
-    class PersonValidator < Validation::Validator
+    class PersonValidator < OptionalAwareValidator
       include Validation
 
       rule :guid, :guid

lib/diaspora_federation/validators/photo_validator.rb

@@ -1,7 +1,7 @@
 module DiasporaFederation
   module Validators
     # This validates a {Entities::Photo}.
-    class PhotoValidator < Validation::Validator
+    class PhotoValidator < OptionalAwareValidator
       include Validation
 
       rule :guid, :guid
@@ -14,7 +14,7 @@ module DiasporaFederation
 
       rule :remote_photo_name, :not_empty
 
-      rule :status_message_guid, guid: {nilable: true}
+      rule :status_message_guid, :guid
 
       rule :text, length: {maximum: 65_535}
 

lib/diaspora_federation/validators/poll_answer_validator.rb

@@ -1,7 +1,7 @@
 module DiasporaFederation
   module Validators
     # This validates a {Entities::PollAnswer}.
-    class PollAnswerValidator < Validation::Validator
+    class PollAnswerValidator < OptionalAwareValidator
       include Validation
 
       rule :guid, :guid

lib/diaspora_federation/validators/poll_participation_validator.rb

@@ -1,7 +1,7 @@
 module DiasporaFederation
   module Validators
     # This validates a {Entities::PollParticipation}.
-    class PollParticipationValidator < Validation::Validator
+    class PollParticipationValidator < OptionalAwareValidator
       include Validation
 
       include RelayableValidator

lib/diaspora_federation/validators/poll_validator.rb

@@ -1,7 +1,7 @@
 module DiasporaFederation
   module Validators
     # This validates a {Entities::Poll}.
-    class PollValidator < Validation::Validator
+    class PollValidator < OptionalAwareValidator
       include Validation
 
       rule :guid, :guid

lib/diaspora_federation/validators/profile_validator.rb

@@ -1,7 +1,7 @@
 module DiasporaFederation
   module Validators
     # This validates a {Entities::Profile}.
-    class ProfileValidator < Validation::Validator
+    class ProfileValidator < OptionalAwareValidator
       include Validation
 
       rule :author, :diaspora_id

lib/diaspora_federation/validators/reshare_validator.rb

@@ -1,12 +1,12 @@
 module DiasporaFederation
   module Validators
     # This validates a {Entities::Reshare}.
-    class ReshareValidator < Validation::Validator
+    class ReshareValidator < OptionalAwareValidator
       include Validation
 
       rule :root_author, :diaspora_id
 
-      rule :root_guid, guid: {nilable: true}
+      rule :root_guid, :guid
 
       rule :author, %i[not_empty diaspora_id]
 

lib/diaspora_federation/validators/retraction_validator.rb

@@ -1,7 +1,7 @@
 module DiasporaFederation
   module Validators
     # This validates a {Entities::Retraction}.
-    class RetractionValidator < Validation::Validator
+    class RetractionValidator < OptionalAwareValidator
       include Validation
 
       rule :author, %i[not_empty diaspora_id]

lib/diaspora_federation/validators/rules/guid.rb

@@ -7,21 +7,6 @@ module Validation
     # * Numbers: 0-9
     # * Special chars: '-', '_', '@', '.' and ':'
     class Guid
-      # This rule can have a +nilable+ param.
-      # @return [Hash] params
-      attr_reader :params
-
-      # Creates a new rule for guid validation
-      # @param [Hash] params
-      # @option params [Boolean] :nilable guid allowed to be nil
-      def initialize(params={})
-        if params.include?(:nilable) && !params[:nilable].is_a?(TrueClass) && !params[:nilable].is_a?(FalseClass)
-          raise ArgumentError, ":nilable needs to be a boolean"
-        end
-
-        @params = params
-      end
-
       # The error key for this rule
       # @return [Symbol] error key
       def error_key
@@ -30,7 +15,13 @@ module Validation
 
       # Determines if value is a valid +GUID+
       def valid_value?(value)
-        params[:nilable] && value.nil? || value.is_a?(String) && value.downcase =~ /\A[0-9a-z\-_@.:]{16,255}\z/
+        value.is_a?(String) && value.downcase =~ /\A[0-9a-z\-_@.:]{16,255}\z/
+      end
+
+      # This rule has no params.
+      # @return [Hash] params
+      def params
+        {}
       end
     end
   end

lib/diaspora_federation/validators/status_message_validator.rb

@@ -1,7 +1,7 @@
 module DiasporaFederation
   module Validators
     # This validates a {Entities::StatusMessage}.
-    class StatusMessageValidator < Validation::Validator
+    class StatusMessageValidator < OptionalAwareValidator
       include Validation
 
       rule :author, %i[not_empty diaspora_id]

lib/diaspora_federation/validators/web_finger_validator.rb

@@ -4,7 +4,7 @@ module DiasporaFederation
     #
     # @note It does not validate the guid and public key, because it will be
     #   removed in the webfinger.
-    class WebFingerValidator < Validation::Validator
+    class WebFingerValidator < OptionalAwareValidator
       include Validation
 
       rule :acct_uri, :not_empty

spec/entities.rb

@@ -60,7 +60,7 @@ module DiasporaFederation
       property :test5, :timestamp
       entity :test6, TestEntity
       property :test7, :string, optional: true
-      entity :multi, [OtherEntity]
+      entity :multi, [OtherEntity], optional: true
     end
 
     class TestEntityWithAuthorAndGuid < DiasporaFederation::Entity
@@ -94,5 +94,20 @@ module DiasporaFederation
       rule :author, %i[not_empty diaspora_id]
       rule :guid, :guid
     end
+
+    class TestComplexEntityValidator < OptionalAwareValidator
+      include Validation
+
+      rule :test1, length: {minimum: 3}
+      rule :test2, :boolean
+      rule :test7, length: {minimum: 3}
+      rule :multi, :not_nil
+    end
+
+    class TestUnknownEntityValidator < OptionalAwareValidator
+      include Validation
+
+      rule :test1, length: {minimum: 3}
+    end
   end
 end

spec/lib/diaspora_federation/validators/optional_aware_validator_spec.rb

@@ -0,0 +1,50 @@
+module DiasporaFederation
+  describe Validators::OptionalAwareValidator do
+    let(:entity_data) {
+      {test1: "abc", test2: true, test7: "abc", multi: []}
+    }
+
+    it "validates a valid object" do
+      validator = Validators::TestComplexEntityValidator.new(OpenStruct.new(entity_data))
+      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)
+        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)
+      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)
+      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)
+      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))
+      validator = Validators::TestUnknownEntityValidator.new(entity)
+      expect(validator).not_to be_valid
+      expect(validator.errors).to include(:test1)
+    end
+  end
+end

spec/lib/diaspora_federation/validators/photo_validator_spec.rb

@@ -38,7 +38,7 @@ module DiasporaFederation
         it_behaves_like "a property with a value validation/restriction" do
           let(:property) { prop }
           let(:wrong_values) { [true, :num, "asdf"] }
-          let(:correct_values) { [123, "123"] }
+          let(:correct_values) { [123, "123", nil] }
         end
       end
     end

spec/lib/diaspora_federation/validators/rules/guid_spec.rb

@@ -1,25 +1,9 @@
 describe Validation::Rule::Guid do
-  it "allows a nilable parameter" do
+  it "will not accept parameters" do
     validator = Validation::Validator.new({})
     expect {
-      validator.rule(:guid, guid: {nilable: false})
-    }.not_to raise_error
-  end
-
-  it "doesn't require a nilable parameter" do
-    validator = Validation::Validator.new({})
-    expect {
-      validator.rule(:guid, :guid)
-    }.not_to raise_error
-  end
-
-  it "requires a boolean as parameter" do
-    validator = Validation::Validator.new({})
-    [nil, "", 42, 5.5].each do |val|
-      expect {
-        validator.rule(:guid, guid: {nilable: val})
-      }.to raise_error ArgumentError, ":nilable needs to be a boolean"
-    end
+      validator.rule(:guid, guid: {param: true})
+    }.to raise_error ArgumentError
   end
 
   it "has an error key" do
@@ -45,14 +29,6 @@ describe Validation::Rule::Guid do
       expect(validator.errors).to be_empty
     end
 
-    it "validates a nil guid if nilable is true" do
-      validator = Validation::Validator.new(OpenStruct.new(guid: nil))
-      validator.rule(:guid, guid: {nilable: true})
-
-      expect(validator).to be_valid
-      expect(validator.errors).to be_empty
-    end
-
     it "fails if the string is too short" do
       validator = Validation::Validator.new(OpenStruct.new(guid: "012345"))
       validator.rule(:guid, :guid)
@@ -77,14 +53,6 @@ describe Validation::Rule::Guid do
       expect(validator.errors).to include(:guid)
     end
 
-    it "fails if the string contains invalid chars if nilable is true" do
-      validator = Validation::Validator.new(OpenStruct.new(guid: "ghijklmnopqrstuvwxyz++"))
-      validator.rule(:guid, guid: {nilable: true})
-
-      expect(validator).not_to be_valid
-      expect(validator.errors).to include(:guid)
-    end
-
     it "fails if the string is empty" do
       [nil, ""].each do |val|
         validator = Validation::Validator.new(OpenStruct.new(guid: val))

spec/lib/diaspora_federation/validators/rules/not_nil_spec.rb

@@ -11,7 +11,7 @@ describe Validation::Rule::NotNil do
   end
 
   context "validation" do
-    it "validates a string " do
+    it "validates a string" do
       validator = Validation::Validator.new(OpenStruct.new(not_nil: "abcd"))
       validator.rule(:not_nil, :not_nil)
 
@@ -19,7 +19,7 @@ describe Validation::Rule::NotNil do
       expect(validator.errors).to be_empty
     end
 
-    it "validates a object " do
+    it "validates a object" do
       validator = Validation::Validator.new(OpenStruct.new(not_nil: Object.new))
       validator.rule(:not_nil, :not_nil)