Comparing changes

@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash
 
 # script/test: Run test suite for application. Optionally pass in a path to an
 #              individual test file to run a single test.
@@ -10,8 +10,12 @@ cd "$(dirname "$0")/.."
 
 [ -z "$DEBUG" ] || set -x
 
-echo [$(date "+%H:%M:%S")] "==> Running setup…"
+echo ["$(date "+%H:%M:%S")"] "==> Running setup…"
 bin/setup
 
-echo [$(date "+%H:%M:%S")] "==> Running tests…"
-bundle exec rake spec
+echo ["$(date "+%H:%M:%S")"] "==> Running tests…"
+if [[ $# -eq 0 ]]; then
+  bundle exec rake spec
+else
+  bundle exec rspec "$1"
+fi

@@ -5,6 +5,8 @@ module Scim
     module V2
       # Represents a dynamic attribute that corresponds to a SCIM type
       module Attributable
+        include Enumerable
+
         def dynamic_attributes
           @dynamic_attributes ||= {}.with_indifferent_access
         end
@@ -25,10 +27,8 @@ module Scim
           end
         end
 
-        private
-
         def attribute_for(name)
-          dynamic_attributes[name.to_s.underscore]
+          dynamic_attributes[name.to_s.underscore] || UnknownAttribute.new(name)
         end
 
         def read_attribute(name)
@@ -42,13 +42,18 @@ module Scim
           if value.is_a?(Hash)
             attribute_for(name)&.assign_attributes(value)
           else
-            attribute = attribute_for(name)
-            raise Scim::Kit::UnknownAttributeError, name unless attribute
+            attribute_for(name)._value = value
+          end
+        end
 
-            attribute._value = value
+        def each
+          dynamic_attributes.each do |_name, attribute|
+            yield attribute
           end
         end
 
+        private
+
         def create_module_for(type)
           name = type.name.to_sym
           Module.new do

@@ -14,16 +14,21 @@ module Scim
 
         validate :presence_of_value, if: proc { |x| x._type.required }
         validate :inclusion_of_value, if: proc { |x| x._type.canonical_values }
-        validate :validate_type
+        validate :validate_type, unless: proc { |x| x.complex? }
+        validate :validate_complex, if: proc { |x| x.complex? }
+        validate :multiple, if: proc { |x| x.multi_valued && !x.complex? }
+
+        delegate :complex?, :multi_valued, to: :_type
 
         def initialize(resource:, type:, value: nil)
           @_type = type
           @_value = value || type.multi_valued ? [] : nil
           @_resource = resource
+
           define_attributes_for(resource, type.attributes)
         end
 
-        def _assign(new_value, coerce: true)
+        def _assign(new_value, coerce: false)
           @_value = coerce ? _type.coerce(new_value) : new_value
         end
 
@@ -39,6 +44,10 @@ module Scim
           true
         end
 
+        def each_value(&block)
+          Array(_value).each(&block)
+        end
+
         private
 
         def server_only?
@@ -66,11 +75,28 @@ module Scim
         end
 
         def validate_type
+          return if _value.nil?
           return if _type.valid?(_value)
 
           errors.add(_type.name, I18n.t('errors.messages.invalid'))
         end
 
+        def validate_complex
+          validates_with ComplexAttributeValidator
+        end
+
+        def multiple
+          return unless _value.respond_to?(:to_a)
+
+          duped_type = _type.dup
+          duped_type.multi_valued = false
+          _value.to_a.each do |x|
+            unless duped_type.valid?(x)
+              errors.add(duped_type.name, I18n.t('errors.messages.invalid'))
+            end
+          end
+        end
+
         def read_only?
           _type.mutability == Mutability::READ_ONLY
         end

@@ -69,11 +69,9 @@ module Scim
           if multi_valued
             return value unless value.respond_to?(:to_a)
 
-            value.to_a.map do |x|
-              COERCION.fetch(type, ->(y) { y }).call(x)
-            end
+            value.to_a.map { |x| coerce_single(x) }
           else
-            COERCION.fetch(type, ->(x) { x }).call(value)
+            coerce_single(value)
           end
         end
 
@@ -89,6 +87,13 @@ module Scim
 
         private
 
+        def coerce_single(value)
+          COERCION.fetch(type, ->(x) { x }).call(value)
+        rescue StandardError => error
+          Scim::Kit.logger.error(error)
+          value
+        end
+
         def validate(value)
           complex? ? valid_complex?(value) : valid_simple?(value)
         end

@@ -0,0 +1,44 @@
+# frozen_string_literal: true
+
+module Scim
+  module Kit
+    module V2
+      # Validates a complex attribute
+      class ComplexAttributeValidator < ::ActiveModel::Validator
+        def validate(item)
+          if item._type.multi_valued
+            multi_valued_validation(item)
+          else
+            item.each do |attribute|
+              item.errors.merge!(attribute.errors) unless attribute.valid?
+            end
+          end
+        end
+
+        private
+
+        def multi_valued_validation(item)
+          item.each_value do |hash|
+            validated = hash.map do |key, value|
+              attribute = item.attribute_for(key)
+              attribute._assign(value)
+              item.errors.merge!(attribute.errors) unless attribute.valid?
+
+              key.to_sym
+            end
+            validate_missing(item, hash, validated)
+          end
+        end
+
+        def validate_missing(item, hash, validated)
+          not_validated = item.map { |x| x._type.name.to_sym } - validated
+          not_validated.each do |key|
+            attribute = item.attribute_for(key)
+            attribute._assign(hash[key])
+            item.errors.merge!(attribute.errors) unless attribute.valid?
+          end
+        end
+      end
+    end
+  end
+end

@@ -52,7 +52,7 @@ module Scim
 
         def validate_attribute(type)
           attribute = attribute_for(type.name)
-          errors.copy!(attribute.errors) unless attribute.valid?
+          errors.merge!(attribute.errors) unless attribute.valid?
         end
       end
     end

@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+
+module Scim
+  module Kit
+    module V2
+      # Represents an Unknown/Unrecognized Attribute
+      class UnknownAttribute
+        include ::ActiveModel::Validations
+        validate :unknown
+        attr_reader :name
+
+        def initialize(name)
+          @name = name
+        end
+
+        def _assign(*_args)
+          valid?
+        end
+
+        def _value=(*_args)
+          raise Scim::Kit::UnknownAttributeError, name
+        end
+
+        def unknown
+          errors.add(name, I18n.t('errors.messages.invalid'))
+        end
+      end
+    end
+  end
+end

@@ -4,6 +4,7 @@ require 'scim/kit/v2/attributable'
 require 'scim/kit/v2/attribute'
 require 'scim/kit/v2/attribute_type'
 require 'scim/kit/v2/authentication_scheme'
+require 'scim/kit/v2/complex_attribute_validator'
 require 'scim/kit/v2/configuration'
 require 'scim/kit/v2/messages'
 require 'scim/kit/v2/meta'
@@ -17,6 +18,7 @@ require 'scim/kit/v2/schemas'
 require 'scim/kit/v2/service_provider_configuration'
 require 'scim/kit/v2/supportable'
 require 'scim/kit/v2/uniqueness'
+require 'scim/kit/v2/unknown_attribute'
 
 module Scim
   module Kit

@@ -2,6 +2,6 @@
 
 module Scim
   module Kit
-    VERSION = '0.2.15'
+    VERSION = '0.2.16'
   end
 end

@@ -3,6 +3,7 @@
 require 'active_model'
 require 'active_support/core_ext/hash/indifferent_access'
 require 'json'
+require 'logger'
 require 'pathname'
 require 'tilt'
 require 'tilt/jbuilder'
@@ -14,8 +15,17 @@ require 'scim/kit/v2'
 require 'scim/kit/version'
 
 module Scim
+  # @api
   module Kit
     class Error < StandardError; end
     class UnknownAttributeError < Error; end
+
+    def self.logger
+      @logger ||= Logger.new(STDOUT)
+    end
+
+    def self.logger=(logger)
+      @logger = logger
+    end
   end
 end

@@ -23,9 +23,22 @@ RSpec.describe Scim::Kit::V2::Attribute do
       before { type.multi_valued = true }
 
       specify { expect(subject._value).to match_array([]) }
-      specify do
-        subject._value = %w[superman batman]
-        expect(subject._value).to match_array(%w[superman batman])
+
+      context 'when multiple valid values are added' do
+        before do
+          subject._value = %w[superman batman]
+        end
+
+        specify { expect(subject._value).to match_array(%w[superman batman]) }
+        specify { expect(subject).to be_valid }
+      end
+
+      context 'when multiple invalid values are added' do
+        before do
+          subject._assign(['superman', {}], coerce: false)
+        end
+
+        specify { expect(subject).not_to be_valid }
       end
     end
 
@@ -215,14 +228,16 @@ RSpec.describe Scim::Kit::V2::Attribute do
       x
     end
 
-    before { subject._value = { name: 'mo', age: 34 } }
-
-    specify { expect(subject).to be_valid }
-
-    context 'when invalid sub attribute' do
-      before { subject._value = { name: 34, age: 'wrong' } }
+    specify do
+      subject.name = 'mo'
+      subject.age = 34
+      expect(subject).to be_valid
+    end
 
-      specify { expect(subject).not_to be_valid }
+    specify do
+      subject.name = 'mo'
+      subject.age = []
+      expect(subject).not_to be_valid
     end
   end
 
@@ -230,7 +245,9 @@ RSpec.describe Scim::Kit::V2::Attribute do
     let(:type) do
       x = Scim::Kit::V2::AttributeType.new(name: 'emails', type: :complex)
       x.multi_valued = true
-      x.add_attribute(name: 'value')
+      x.add_attribute(name: 'value') do |y|
+        y.required = true
+      end
       x.add_attribute(name: 'primary', type: :boolean)
       x
     end
@@ -246,6 +263,7 @@ RSpec.describe Scim::Kit::V2::Attribute do
 
     specify { expect(subject._value).to match_array([{ value: email, primary: true }, { value: other_email, primary: false }]) }
     specify { expect(subject.as_json[:emails]).to match_array([{ value: email, primary: true }, { value: other_email, primary: false }]) }
+    specify { expect(subject).to be_valid }
 
     context 'when the hash is invalid' do
       before do
@@ -254,7 +272,8 @@ RSpec.describe Scim::Kit::V2::Attribute do
       end
 
       specify { expect(subject).not_to be_valid }
-      specify { expect(subject.errors[:emails]).to be_present }
+      specify { expect(subject.errors[:blah]).to be_present }
+      specify { expect(subject.errors[:value]).to be_present }
     end
   end
 

@@ -96,6 +96,17 @@ RSpec.describe Scim::Kit::V2::Resource do
     specify { expect(subject.send(:attribute_for, :type)._type).to be_instance_of(Scim::Kit::V2::AttributeType) }
   end
 
+  context 'with attribute named $ref' do
+    before do
+      schema.add_attribute(name: '$ref')
+      subject.write_attribute('$ref', 'User')
+    end
+
+    specify { expect(subject.read_attribute('$ref')).to eql('User') }
+    specify { expect(subject.as_json['$ref']).to eql('User') }
+    specify { expect(subject.send(:attribute_for, '$ref')._type).to be_instance_of(Scim::Kit::V2::AttributeType) }
+  end
+
   context 'with a complex attribute' do
     before do
       schema.add_attribute(name: 'name') do |x|
@@ -122,7 +133,9 @@ RSpec.describe Scim::Kit::V2::Resource do
     before do
       schema.add_attribute(name: 'emails', type: :complex) do |x|
         x.multi_valued = true
-        x.add_attribute(name: 'value')
+        x.add_attribute(name: 'value') do |y|
+          y.required = true
+        end
         x.add_attribute(name: 'primary', type: :boolean)
       end
       subject.emails = [
@@ -133,6 +146,26 @@ RSpec.describe Scim::Kit::V2::Resource do
 
     specify { expect(subject.emails).to match_array([{ value: email, primary: true }, { value: other_email, primary: false }]) }
     specify { expect(subject.as_json[:emails]).to match_array([{ value: email, primary: true }, { value: other_email, primary: false }]) }
+
+    context 'when one attribute has an invalid type' do
+      before do
+        subject.emails = [{ value: email, primary: 'q' }]
+        subject.valid?
+      end
+
+      specify { expect(subject).not_to be_valid }
+      specify { expect(subject.errors[:primary]).to be_present }
+    end
+
+    context 'when a required attribute is missing' do
+      before do
+        subject.emails = [{ primary: true }]
+        subject.valid?
+      end
+
+      specify { expect(subject).not_to be_valid }
+      specify { expect(subject.errors[:value]).to be_present }
+    end
   end
 
   context 'with multiple schemas' do
@@ -186,6 +219,63 @@ RSpec.describe Scim::Kit::V2::Resource do
       specify { expect(subject).not_to be_valid }
       specify { expect(subject.errors[:hero]).to be_present }
     end
+
+    context 'when validating a complex type' do
+      before do
+        schema.add_attribute(name: :manager, type: :complex) do |x|
+          x.multi_valued = false
+          x.required = false
+          x.mutability = :read_write
+          x.returned = :default
+          x.add_attribute(name: :value, type: :string) do |y|
+            y.multi_valued = false
+            y.required = false
+            y.case_exact = false
+            y.mutability = :read_write
+            y.returned = :default
+            y.uniqueness = :none
+          end
+          x.add_attribute(name: '$ref', type: :reference) do |y|
+            y.multi_valued = false
+            y.required = false
+            y.case_exact = false
+            y.mutability = :read_write
+            y.returned = :default
+            y.uniqueness = :none
+          end
+          x.add_attribute(name: :display_name, type: :string) do |y|
+            y.multi_valued = false
+            y.required = true
+            y.case_exact = false
+            y.mutability = :read_only
+            y.returned = :default
+            y.uniqueness = :none
+          end
+        end
+      end
+
+      context 'when valid' do
+        before do
+          subject.manager.value = SecureRandom.uuid
+          subject.manager.write_attribute('$ref', FFaker::Internet.uri('https'))
+          subject.manager.display_name = SecureRandom.uuid
+        end
+
+        specify { expect(subject).to be_valid }
+      end
+
+      context 'when invalid' do
+        before do
+          subject.manager.value = SecureRandom.uuid
+          subject.manager.write_attribute('$ref', SecureRandom.uuid)
+          subject.manager.display_name = nil
+          subject.valid?
+        end
+
+        specify { expect(subject).not_to be_valid }
+        specify { expect(subject.errors[:display_name]).to be_present }
+      end
+    end
   end
 
   context 'when building a new resource' do
@@ -401,16 +491,16 @@ RSpec.describe Scim::Kit::V2::Resource do
           x.add_attribute(name: :primary, type: :boolean)
         end
         subject.assign_attributes(schemas: schemas.map(&:id), emails: [
-                                    { value: email, primary: true },
-                                    { value: other_email, primary: false }
-                                  ])
+          { value: email, primary: true },
+          { value: other_email, primary: false }
+        ])
       end
 
       specify do
         expect(subject.emails).to match_array([
-                                                { value: email, primary: true },
-                                                { value: other_email, primary: false }
-                                              ])
+          { value: email, primary: true },
+          { value: other_email, primary: false }
+        ])
       end
 
       specify { expect(subject.emails[0][:value]).to eql(email) }

@@ -6,6 +6,8 @@ require 'ffaker'
 require 'json'
 require 'byebug'
 
+Scim::Kit.logger = Logger.new('/dev/null')
+
 RSpec.configure do |config|
   # Enable flags like --only-failures and --next-failure
   config.example_status_persistence_file_path = '.rspec_status'

@@ -9,6 +9,9 @@ AllCops:
     - 'vendor/**/*'
   TargetRubyVersion: 2.5
 
+Layout/IndentArray:
+  EnforcedStyle: consistent
+
 Metrics/BlockLength:
   Exclude:
     - '*.gemspec'
@@ -24,3 +27,6 @@ Naming/FileName:
 
 RSpec/NamedSubject:
   Enabled: false
+
+RSpec/NestedGroups:
+  Max: 4

@@ -0,0 +1,42 @@
+Version 0.2.16
+# Changelog
+All notable changes to this project will be documented in this file.
+
+The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
+and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
+
+## [Unreleased]
+### Changed
+- nil
+
+## [0.2.16] - 2019-02-03
+### Added
+- Default logger
+- Attributes now implement Enumerable
+- Attributable#attribute\_for now returns a null object instead of nil.
+- Validations for multi valued attributes
+- Validations for complex attributes
+- rescue errors from type coercion.
+
+### Changed
+- \_assign does not coerce values by default.
+- errors are merged together instead of overwritten during attribute validation.
+
+[Unreleased]: https://github.com/mokhan/scim-kit/compare/v0.2.16...HEAD
+[0.2.16]: https://github.com/mokhan/scim-kit/compare/v0.2.15...v0.2.16
+[0.2.15]: https://github.com/mokhan/scim-kit/compare/v0.2.14...v0.2.15
+[0.2.14]: https://github.com/mokhan/scim-kit/compare/v0.2.13...v0.2.14
+[0.2.13]: https://github.com/mokhan/scim-kit/compare/v0.2.12...v0.2.13
+[0.2.12]: https://github.com/mokhan/scim-kit/compare/v0.2.11...v0.2.12
+[0.2.11]: https://github.com/mokhan/scim-kit/compare/v0.2.10...v0.2.11
+[0.2.10]: https://github.com/mokhan/scim-kit/compare/v0.2.9...v0.2.10
+[0.2.9]: https://github.com/mokhan/scim-kit/compare/v0.2.8...v0.2.9
+[0.2.8]: https://github.com/mokhan/scim-kit/compare/v0.2.7...v0.2.8
+[0.2.7]: https://github.com/mokhan/scim-kit/compare/v0.2.6...v0.2.7
+[0.2.6]: https://github.com/mokhan/scim-kit/compare/v0.2.5...v0.2.6
+[0.2.5]: https://github.com/mokhan/scim-kit/compare/v0.2.4...v0.2.5
+[0.2.4]: https://github.com/mokhan/scim-kit/compare/v0.2.3...v0.2.4
+[0.2.3]: https://github.com/mokhan/scim-kit/compare/v0.2.2...v0.2.3
+[0.2.2]: https://github.com/mokhan/scim-kit/compare/v0.2.1...v0.2.2
+[0.2.1]: https://github.com/mokhan/scim-kit/compare/v0.2.0...v0.2.1
+[0.2.0]: https://github.com/mokhan/scim-kit/compare/v0.1.0...v0.2.0