From 51cdc19070a287b55d5d22d9edae57d5c325c771 Mon Sep 17 00:00:00 2001 From: Paul Thornthwaite Date: Tue, 22 Jan 2013 14:25:05 +0000 Subject: [PATCH 1/7] [aws|compute] Fixes schema in image tests The schema for the AWS ImagesSet declared blockMappingDevice as an Array with no values inside. Mocks are generating an image with a device present which no longer fits the schema. This is a premptive fix because corecting the #formats method caused this issue to surface. See #1477 --- tests/aws/requests/compute/image_tests.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/aws/requests/compute/image_tests.rb b/tests/aws/requests/compute/image_tests.rb index 1543151ca..1fc6998d9 100644 --- a/tests/aws/requests/compute/image_tests.rb +++ b/tests/aws/requests/compute/image_tests.rb @@ -2,7 +2,7 @@ Shindo.tests('Fog::Compute[:aws] | image requests', ['aws']) do @describe_images_format = { 'imagesSet' => [{ 'architecture' => String, - 'blockDeviceMapping' => [], + 'blockDeviceMapping' => [Fog::Nullable::Hash], 'description' => Fog::Nullable::String, 'hypervisor' => String, 'imageId' => String, @@ -135,4 +135,4 @@ Shindo.tests('Fog::Compute[:aws] | image requests', ['aws']) do Fog::Compute[:aws].modify_image_attribute('ami-00000000', { 'Add.UserId' => ['123456789012'] }).body end end -end \ No newline at end of file +end From 8ed3a056c73275d2502121b36d7875fd1d16590c Mon Sep 17 00:00:00 2001 From: Paul Thornthwaite Date: Tue, 22 Jan 2013 14:38:54 +0000 Subject: [PATCH 2/7] [openstack|identity] Marks test as pending Issue #1477 conceals a bug in the #formats helper that allowed this to pass such that the response of #delete_user_role does not match the declared schema. Response when mocked is an empty String which does not match the declared schema of a Hash. --- tests/openstack/requests/identity/role_tests.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/openstack/requests/identity/role_tests.rb b/tests/openstack/requests/identity/role_tests.rb index 8124c7e44..6bf25ab2d 100644 --- a/tests/openstack/requests/identity/role_tests.rb +++ b/tests/openstack/requests/identity/role_tests.rb @@ -27,7 +27,13 @@ Shindo.tests('Fog::Identity[:openstack] | role requests', ['openstack']) do Fog::Identity[:openstack].list_roles_for_user_on_tenant(@tenant['id'], @user['id']).body['roles'] end + tests("#delete_user_role with tenant").returns("") do + Fog::Identity[:openstack].delete_user_role(@tenant['id'], @user['id'], @role['id']).body + end + tests("#delete_user_role with tenant").formats(@role_format) do + # FIXME - Response (under mocks) is empty String which does not match schema + pending Fog::Identity[:openstack].delete_user_role(@tenant['id'], @user['id'], @role['id']).body end From 593a3e529c5809bce8af9ed8dd5c0511697fd80a Mon Sep 17 00:00:00 2001 From: Paul Thornthwaite Date: Tue, 22 Jan 2013 16:25:49 +0000 Subject: [PATCH 3/7] [aws] Fixes security group template Security group ipRanges schema was declared as an empty Hash so only matches empty hashes. Running with mocks gets unrecognised values which when #formats is working correctly fails this test. This is a premptive fix because corecting the #formats method caused this issue to surface. See #1477 --- tests/aws/requests/compute/security_group_tests.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/aws/requests/compute/security_group_tests.rb b/tests/aws/requests/compute/security_group_tests.rb index 40d062e91..ceea99e36 100644 --- a/tests/aws/requests/compute/security_group_tests.rb +++ b/tests/aws/requests/compute/security_group_tests.rb @@ -15,7 +15,7 @@ Shindo.tests('Fog::Compute[:aws] | security group requests', ['aws']) do 'fromPort' => Fog::Nullable::Integer, 'groups' => [{ 'groupName' => Fog::Nullable::String, 'userId' => String, 'groupId' => String }], 'ipProtocol' => String, - 'ipRanges' => [], + 'ipRanges' => [Fog::Nullable::Hash], 'toPort' => Fog::Nullable::Integer, }], 'ipPermissionsEgress' => [], From 3863cd38c8a3d964e588a9acd9d4394f91907d86 Mon Sep 17 00:00:00 2001 From: Paul Thornthwaite Date: Tue, 22 Jan 2013 10:45:23 +0000 Subject: [PATCH 4/7] [tests] Updates format tests Tests for #formats (the public interface) have been added however there are no fail tests due to the way Shindo operates. Adds a few more test cases and rewords the descriptions in preparation to refactor the #format_kernels method. Changes the tests and schema keys to be String based since decoding the values rarely return Symbols. --- tests/helpers/formats_helper_tests.rb | 131 +++++++++++++++++++++----- 1 file changed, 109 insertions(+), 22 deletions(-) diff --git a/tests/helpers/formats_helper_tests.rb b/tests/helpers/formats_helper_tests.rb index 8bd857c73..9553a76c3 100644 --- a/tests/helpers/formats_helper_tests.rb +++ b/tests/helpers/formats_helper_tests.rb @@ -1,56 +1,143 @@ Shindo.tests('test_helper', 'meta') do + tests('#formats backwards compatible changes') do + + tests('when value matches schema expectation') do + formats({"key" => String}) { {"key" => "Value"} } + end + + tests('when values within an array all match schema expectation') do + formats({"key" => [Integer]}) { {"key" => [1, 2]} } + end + + tests('when nested values match schema expectation') do + formats({"key" => {:nested_key => String}}) { {"key" => {:nested_key => "Value"}} } + end + + tests('when collection of values all match schema expectation') do + formats([{"key" => String}]) { [{"key" => "Value"}, {"key" => "Value"}] } + end + + tests('when collection is empty although schema covers optional members') do + formats([{"key" => String}]) { [] } + end + + tests('when additional keys are passed and not strict') do + formats({"key" => String}, false) { {"key" => "Value", :extra => "Bonus"} } + end + + tests('when value is nil and schema expects NilClass') do + formats({"key" => NilClass}) { {"key" => nil} } + end + + tests('when value and schema match as hashes') do + formats({}) { {} } + end + + tests('when value and schema match as arrays') do + formats([]) { [] } + end + + tests('when value is a Time') do + formats({"time" => Time}) { {"time" => Time.now} } + end + + tests('when key is missing but value should be NilClass (#1477)') do + formats({"key" => NilClass}) { {} } + end + + tests('when key is missing but value is nullable (#1477)') do + formats({"key" => Fog::Nullable::String}) { {} } + end + + end + tests('#formats_kernel') do tests('returns true') do - test('when format of value matches') do - formats_kernel({:a => :b}, {:a => Symbol}) + returns(true, 'when value matches schema expectation') do + formats_kernel({"key" => "Value"}, {"key" => String}) end - test('when format of nested array elements matches') do - formats_kernel({:a => [:b, :c]}, {:a => [Symbol]}) + returns(true, 'when values within an array all match schema expectation') do + formats_kernel({"key" => [1, 2]}, {"key" => [Integer]}) end - test('when format of nested hash matches') do - formats_kernel({:a => {:b => :c}}, {:a => {:b => Symbol}}) + returns(true, 'when nested values match schema expectation') do + formats_kernel({"key" => {:nested_key => "Value"}}, {"key" => {:nested_key => String}}) end - test('when format of an array') do - formats_kernel([{:a => :b}], [{:a => Symbol}]) + returns(true, 'when collection of values all match schema expectation') do + formats_kernel([{"key" => "Value"}, {"key" => "Value"}], [{"key" => String}]) end - test('non strict extra data') do - formats_kernel({:a => :b, :b => :c}, {:a => Symbol}, true, false) + returns(true, 'when collection is empty although schema covers optional members') do + formats_kernel([], [{"key" => String}]) + end + + returns(true, 'when additional keys are passed and not strict') do + formats_kernel({"key" => "Value", :extra => "Bonus"}, {"key" => String}, true, false) + end + + returns(true, 'when value is nil and schema expects NilClass') do + formats_kernel({"key" => nil}, {"key" => NilClass}) + end + + returns(true, 'when value and schema match as hashes') do + formats_kernel({}, {}) + end + + returns(true, 'when value and schema match as arrays') do + formats_kernel([], []) + end + + returns(true, 'when value is a Time') do + formats_kernel({"time" => Time.now}, {"time" => Time}) + end + + returns(true, 'when key is missing but value should be NilClass (#1477)') do + formats_kernel({}, {"key" => NilClass}) + end + + returns(true, 'when key is missing but value is nullable (#1477)') do + formats_kernel({}, {"key" => Fog::Nullable::String}) end end tests('returns false') do - test('when format of value does not match') do - !formats_kernel({:a => :b}, {:a => String}) + returns(false, 'when value does not match schema expectation') do + formats_kernel({"key" => nil}, {"key" => String}) end - test('when not all keys are checked') do - !formats_kernel({:a => :b}, {}) + returns(false, 'when key formats do not match') do + formats_kernel({"key" => "Value"}, {:key => String}) end - test('when some keys do not appear') do - !formats_kernel({}, {:a => String}) + returns(false, 'when additional keys are passed and strict') do + formats_kernel({"key" => "Missing"}, {}) end - test('when an array is expected but another data type is found') do - !formats_kernel({:a => 'not an array'}, {:a => []}) + returns(false, 'when some keys do not appear') do + formats_kernel({}, {"key" => String}) end - test('when a hash is expected but another data type is found') do - !formats_kernel({:a => 'not a hash'}, {:a => {}}, true, false) + returns(false, 'when collection contains a member that does not match schema') do + formats_kernel([{"key" => "Value"}, {"key" => 5}], [{"key" => String}]) end + returns(false, 'when hash and array are compared') do + formats_kernel({}, []) + end - test('non strict extra data') do - !formats_kernel({:a => :b, :b => :c}, {:z => Symbol}, true, false) + returns(false, 'when array and hash are compared') do + formats_kernel([], {}) + end + + returns(false, 'when a hash is expected but another data type is found') do + formats_kernel({"key" => {:nested_key => []}}, {"key" => {:nested_key => {}}}) end end From 0793a42a641daaaa6f220ab2345fcac10066a5b8 Mon Sep 17 00:00:00 2001 From: Paul Thornthwaite Date: Wed, 16 Jan 2013 15:44:56 +0000 Subject: [PATCH 5/7] [tests] Changes to format testing helper * Adds and documents Shindo::Tests#data_matches_schema * Deprecates Shindo::Tests#format method * Strict mode is replace by expandable options * Uses Fog::Logger instead of p for debug output * Shindo::Tests#formats_kernel removed (was private) * Shindo::Tests#confirm_data_matches_schema added * #confirm_data_matches_schema uses yield Related to issue #1477 Shindo::Tests#formats incorrectly treated missing keys as valid if their value is allowed to be "nullable" or NilClass. These keys are not optional. They are required but can be the value nil or the Nullable class. Unfortunately due to lack of documentation and a bug in the code the current Nullable classes can be interpreted as optional and work as such. This replaces the old format checker with a new version that supports options to control the matching behaviour related to extra keys in either the data or the schema. This corrects the behaviour so extra keys in either the schema or the supplied data fail the check unless non strict has been used. A legacy version of the #formats test is in place that passes the options so it behaves as the old version does. Premptive patches have been added to fix those tests that were already broken but passed the original checks. --- tests/helpers/formats_helper.rb | 174 +++++++++++++++++++------- tests/helpers/formats_helper_tests.rb | 55 ++++---- 2 files changed, 162 insertions(+), 67 deletions(-) diff --git a/tests/helpers/formats_helper.rb b/tests/helpers/formats_helper.rb index 00c3067bf..59a2bd30c 100644 --- a/tests/helpers/formats_helper.rb +++ b/tests/helpers/formats_helper.rb @@ -25,62 +25,144 @@ end module Shindo class Tests - def formats(format, strict=true) - raise ArgumentError, 'format is nil' unless format + # Generates a Shindo test that compares a hash schema to the result + # of the passed in block returning true if they match. + # + # The schema that is passed in is a Hash or Array of hashes that + # have Classes in place of values. When checking the schema the + # value should match the Class. + # + # Strict mode will fail if the data has additional keys. Setting + # +strict+ to +false+ will allow additional keys to appear. + # + # @param [Hash] schmea A Hash schema + # @param [Hash] options Options to change validation rules + # @option options [Boolean] :allow_extra_keys + # If +true+ deoes not fail when keys are in the data that are + # not specified in the schema. This allows new values to + # appear in API output without breaking the check. + # @option options [Boolean] :allow_optional_rules + # If +true+ does not fail if extra keys are in the schema + # that do not match the data. Not recommended! + # @yield Data to check with schema + # + # @example Using in a test + # Shindo.tests("data matches schema") do + # data = {:string => "Hello" } + # data_matches_schema(:string => String) { data } + # end + # + # data matches schema + # + has proper format + # + # @example Example schema + # { + # "id" => String, + # "ram" => Integer, + # "disks" => [ + # "size" => Float + # ], + # "dns_name" => Fog::Nullable::String, + # "active" => Fog::Boolean, + # "created" => DateTime + # } + # + # @return [Boolean] + def data_matches_schema(schema, options = {}) + test('data matches schema') do + confirm_data_matches_schema(yield, schema, options) + end + end + # @deprecation #formats is deprecated. Use #data_matches_schema instead + def formats(format, strict = true) test('has proper format') do - formats_kernel(instance_eval(&Proc.new), format, true, strict) + if strict + options = {:allow_extra_keys => false, :allow_optional_rules => true} + else + options = {:allow_extra_keys => true, :allow_optional_rules => true} + end + confirm_data_matches_schema(yield, format, options) end end private - def formats_kernel(original_data, original_format, original = true, strict = true) - valid = true - data = original_data.dup - format = original_format.dup - if format.is_a?(Array) - data = {:element => data} - format = {:element => format} - end - for key, value in format - datum = data.delete(key) - format.delete(key) - case value - when Array - p("#{key.inspect} not Array: #{datum.inspect}") unless datum.is_a?(Array) - valid &&= datum.is_a?(Array) - if datum.is_a?(Array) && !value.empty? - for element in datum - type = value.first - if type.is_a?(Hash) - valid &&= formats_kernel({:element => element}, {:element => type}, false, strict) - else - valid &&= element.is_a?(type) - end - end - end - when Hash - p("#{key.inspect} not Hash: #{datum.inspect}") unless datum.is_a?(Hash) - valid &&= datum.is_a?(Hash) - valid &&= formats_kernel(datum, value, false, strict) - else - p "#{key.inspect} not #{value.inspect}: #{datum.inspect}" unless datum.is_a?(value) - valid &&= datum.is_a?(value) - end - end - p data unless data.empty? - p format unless format.empty? - if strict - valid &&= data.empty? && format.empty? - else - valid &&= format.empty? - end - if !valid && original - @message = "#{original_data.inspect} does not match #{original_format.inspect}" + # Checks if the data structure matches the schema passed in and + # returns true if it fits. + # + # @param [Object] data Hash or Array to check + # @param [Object] schema Schema pattern to check against + # @param [Boolean] options + # @option options [Boolean] :allow_extra_keys + # If +true+ does not fail if extra keys are in the data + # that are not in the schema. Allows + # @option options [Boolean] :allow_optional_rules + # If +true+ does not fail if extra keys are in the schema + # that do not match the data. Not recommended! + # + # @return [Boolean] Did the data fit the schema? + def confirm_data_matches_schema(data, schema, options = {}) + # Clear message passed to the Shindo tests + @message = nil + + valid = validate_value(schema, data, options) + + unless valid + @message = "#{data.inspect} does not match #{schema.inspect}" end valid end + # This contains a slightly modified version of the Hashidator gem + # but unfortunately the gem does not cope with Array schemas. + # + # @see https://github.com/vangberg/hashidator/blob/master/lib/hashidator.rb + # + def validate_value(validator, value, options) + Fog::Logger.write :debug, "[yellow][DEBUG] #{value.inspect} against #{validator.inspect}[/]\n" + + # When being strict values not specified in the schema are fails + unless options[:allow_extra_keys] + if validator.respond_to?(:empty?) && value.respond_to?(:empty?) + # Validator is empty but values are not + return false if !value.empty? && validator.empty? + end + end + + unless options[:allow_optional_rules] + if validator.respond_to?(:empty?) && value.respond_to?(:empty?) + # Validator has rules left but no more values + return false if value.empty? && !validator.empty? + end + end + + case validator + when Array + return false if value.is_a?(Hash) + value.respond_to?(:all?) && value.all? {|x| validate_value(validator[0], x, options)} + when Symbol + value.respond_to? validator + when Hash + return false if value.is_a?(Array) + validator.all? do |key, sub_validator| + Fog::Logger.write :debug, "[blue][DEBUG] #{key.inspect} against #{sub_validator.inspect}[/]\n" + validate_value(sub_validator, value[key], options) + end + else + result = validator == value + result = validator === value unless result + # Repeat unless we have a Boolean already + unless (result.is_a?(TrueClass) || result.is_a?(FalseClass)) + result = validate_value(result, value, options) + end + if result + Fog::Logger.write :debug, "[green][DEBUG] Validation passed: #{value.inspect} against #{validator.inspect}[/]\n" + else + Fog::Logger.write :debug, "[red][DEBUG] Validation failed: #{value.inspect} against #{validator.inspect}[/]\n" + end + result + end + end end end diff --git a/tests/helpers/formats_helper_tests.rb b/tests/helpers/formats_helper_tests.rb index 9553a76c3..6248b251d 100644 --- a/tests/helpers/formats_helper_tests.rb +++ b/tests/helpers/formats_helper_tests.rb @@ -52,56 +52,61 @@ Shindo.tests('test_helper', 'meta') do end - tests('#formats_kernel') do + tests('data matches schema') do + data = {:welcome => "Hello" } + data_matches_schema(:welcome => String) { data } + end + + tests('#confirm_data_matches_schema') do tests('returns true') do returns(true, 'when value matches schema expectation') do - formats_kernel({"key" => "Value"}, {"key" => String}) + confirm_data_matches_schema({"key" => "Value"}, {"key" => String}) end returns(true, 'when values within an array all match schema expectation') do - formats_kernel({"key" => [1, 2]}, {"key" => [Integer]}) + confirm_data_matches_schema({"key" => [1, 2]}, {"key" => [Integer]}) end returns(true, 'when nested values match schema expectation') do - formats_kernel({"key" => {:nested_key => "Value"}}, {"key" => {:nested_key => String}}) + confirm_data_matches_schema({"key" => {:nested_key => "Value"}}, {"key" => {:nested_key => String}}) end returns(true, 'when collection of values all match schema expectation') do - formats_kernel([{"key" => "Value"}, {"key" => "Value"}], [{"key" => String}]) + confirm_data_matches_schema([{"key" => "Value"}, {"key" => "Value"}], [{"key" => String}]) end returns(true, 'when collection is empty although schema covers optional members') do - formats_kernel([], [{"key" => String}]) + confirm_data_matches_schema([], [{"key" => String}], {:allow_optional_rules => true}) end returns(true, 'when additional keys are passed and not strict') do - formats_kernel({"key" => "Value", :extra => "Bonus"}, {"key" => String}, true, false) + confirm_data_matches_schema({"key" => "Value", :extra => "Bonus"}, {"key" => String}, {:allow_extra_keys => true}) end returns(true, 'when value is nil and schema expects NilClass') do - formats_kernel({"key" => nil}, {"key" => NilClass}) + confirm_data_matches_schema({"key" => nil}, {"key" => NilClass}) end returns(true, 'when value and schema match as hashes') do - formats_kernel({}, {}) + confirm_data_matches_schema({}, {}) end returns(true, 'when value and schema match as arrays') do - formats_kernel([], []) + confirm_data_matches_schema([], []) end returns(true, 'when value is a Time') do - formats_kernel({"time" => Time.now}, {"time" => Time}) + confirm_data_matches_schema({"time" => Time.now}, {"time" => Time}) end returns(true, 'when key is missing but value should be NilClass (#1477)') do - formats_kernel({}, {"key" => NilClass}) + confirm_data_matches_schema({}, {"key" => NilClass}, {:allow_optional_rules => true}) end returns(true, 'when key is missing but value is nullable (#1477)') do - formats_kernel({}, {"key" => Fog::Nullable::String}) + confirm_data_matches_schema({}, {"key" => Fog::Nullable::String}, {:allow_optional_rules => true}) end end @@ -109,35 +114,43 @@ Shindo.tests('test_helper', 'meta') do tests('returns false') do returns(false, 'when value does not match schema expectation') do - formats_kernel({"key" => nil}, {"key" => String}) + confirm_data_matches_schema({"key" => nil}, {"key" => String}) end returns(false, 'when key formats do not match') do - formats_kernel({"key" => "Value"}, {:key => String}) + confirm_data_matches_schema({"key" => "Value"}, {:key => String}) end returns(false, 'when additional keys are passed and strict') do - formats_kernel({"key" => "Missing"}, {}) + confirm_data_matches_schema({"key" => "Missing"}, {}) end returns(false, 'when some keys do not appear') do - formats_kernel({}, {"key" => String}) + confirm_data_matches_schema({}, {"key" => String}) end returns(false, 'when collection contains a member that does not match schema') do - formats_kernel([{"key" => "Value"}, {"key" => 5}], [{"key" => String}]) + confirm_data_matches_schema([{"key" => "Value"}, {"key" => 5}], [{"key" => String}]) end returns(false, 'when hash and array are compared') do - formats_kernel({}, []) + confirm_data_matches_schema({}, []) end returns(false, 'when array and hash are compared') do - formats_kernel([], {}) + confirm_data_matches_schema([], {}) end returns(false, 'when a hash is expected but another data type is found') do - formats_kernel({"key" => {:nested_key => []}}, {"key" => {:nested_key => {}}}) + confirm_data_matches_schema({"key" => {:nested_key => []}}, {"key" => {:nested_key => {}}}) + end + + returns(false, 'when key is missing but value should be NilClass (#1477)') do + confirm_data_matches_schema({}, {"key" => NilClass}, {:allow_optional_rules => false}) + end + + returns(false, 'when key is missing but value is nullable (#1477)') do + confirm_data_matches_schema({}, {"key" => Fog::Nullable::String}, {:allow_optional_rules => false}) end end From 5f63112640bd24dd3e839e9795c5187d4d2f6c11 Mon Sep 17 00:00:00 2001 From: Paul Thornthwaite Date: Wed, 23 Jan 2013 11:27:14 +0000 Subject: [PATCH 6/7] [tests] Extracts schema validator to class Created Fog::Schema::DataValidator to contain the matching logic for the data based schemas used by the tests. This is tested in isolation and removes dependency on Shindo. Also exposes a #message call to get the last message set by the validator to avoid coupling the test output to the internals. Tests for the #data_matches_schema helper are added as well. --- lib/fog/schema/data_validator.rb | 153 +++++++++++++++++++++++ tests/helpers/formats_helper.rb | 107 +++------------- tests/helpers/formats_helper_tests.rb | 159 ++++++++---------------- tests/helpers/schema_validator_tests.rb | 103 +++++++++++++++ 4 files changed, 329 insertions(+), 193 deletions(-) create mode 100644 lib/fog/schema/data_validator.rb create mode 100644 tests/helpers/schema_validator_tests.rb diff --git a/lib/fog/schema/data_validator.rb b/lib/fog/schema/data_validator.rb new file mode 100644 index 000000000..108f23972 --- /dev/null +++ b/lib/fog/schema/data_validator.rb @@ -0,0 +1,153 @@ +module Fog + module Schema + # This validates a data object against a Ruby based schema to see + # if they match. + # + # * An object matches the schema if +==+ or +===+ returns +true+ + # * Hashes match if all the key's values match the classes given + # in the schema as well. This can be configured in the options + # * Arrays match when every element in the data matches the case + # given in the schema. + # + # The schema and validation are very simple and probably not + # suitable for some cases. + # + # The following classes can be used to check for special behaviour + # + # * Fog::Boolean - value may be +true+ or +false+ + # * Fog::Nullable::Boolean - value may be +true+, +false+ or +nil+ + # * Fog::Nullable::Integer - value may be an Integer or +nil+ + # * Fog::Nullable::String + # * Fog::Nullable::Time + # * Fog::Nullable::Float + # * Fog::Nullable::Hash + # * Fog::Nullable::Array + # + # All the "nullable" objects will pass if the value is of the class + # or if it is +nil+. This allows you to match APIs that may include + # keys when the value is not available in some cases but will + # always be a String. Such as an password that is only displayed + # on the reset action. + # + # The keys for "nullable" resources should always be present but + # original matcher had a bug that allowed these to also appear to + # work as optional keys/values. + # + # If you need the original behaviour, data with a missing key is + # still valid, then you may pass the +:allow_optional_rules+ + # option to the #validate method. + # + # That is not recommended because you are describing a schema + # with optional keys in a format that does not support it. + # + # Setting +:allow_extra_keys+ as +true+ allows the data to + # contain keys not declared by the schema and still pass. This + # is useful if new attributes appear in the API in a backwards + # compatible manner and can be ignored. + # + # This is the behaviour you would have seen with +strict+ being + # +false+ in the original test helper. + # + # @example Schema example + # { + # "id" => String, + # "ram" => Integer, + # "disks" => [ + # "size" => Float + # ], + # "dns_name" => Fog::Nullable::String, + # "active" => Fog::Boolean, + # "created" => DateTime + # } + # + class DataValidator + + def initialize + @message = nil + end + + # Checks if the data structure matches the schema passed in and + # returns +true+ if it fits. + # + # @param [Object] data Hash or Array to check + # @param [Object] schema Schema pattern to check against + # @param [Boolean] options + # @option options [Boolean] :allow_extra_keys + # If +true+ does not fail if extra keys are in the data + # that are not in the schema. + # @option options [Boolean] :allow_optional_rules + # If +true+ does not fail if extra keys are in the schema + # that do not match the data. Not recommended! + # + # @return [Boolean] Did the data fit the schema? + def validate(data, schema, options = {}) + valid = validate_value(schema, data, options) + + unless valid + @message = "#{data.inspect} does not match #{schema.inspect}" + end + valid + end + + # This returns the last message set by the validator + # + # @return [String] + def message + @message + end + + private + + # This contains a slightly modified version of the Hashidator gem + # but unfortunately the gem does not cope with Array schemas. + # + # @see https://github.com/vangberg/hashidator/blob/master/lib/hashidator.rb + # + def validate_value(validator, value, options) + Fog::Logger.write :debug, "[yellow][DEBUG] #{value.inspect} against #{validator.inspect}[/]\n" + + # When being strict values not specified in the schema are fails + unless options[:allow_extra_keys] + if validator.respond_to?(:empty?) && value.respond_to?(:empty?) + # Validator is empty but values are not + return false if !value.empty? && validator.empty? + end + end + + unless options[:allow_optional_rules] + if validator.respond_to?(:empty?) && value.respond_to?(:empty?) + # Validator has rules left but no more values + return false if value.empty? && !validator.empty? + end + end + + case validator + when Array + return false if value.is_a?(Hash) + value.respond_to?(:all?) && value.all? {|x| validate_value(validator[0], x, options)} + when Symbol + value.respond_to? validator + when Hash + return false if value.is_a?(Array) + validator.all? do |key, sub_validator| + Fog::Logger.write :debug, "[blue][DEBUG] #{key.inspect} against #{sub_validator.inspect}[/]\n" + validate_value(sub_validator, value[key], options) + end + else + result = validator == value + result = validator === value unless result + # Repeat unless we have a Boolean already + unless (result.is_a?(TrueClass) || result.is_a?(FalseClass)) + result = validate_value(result, value, options) + end + if result + Fog::Logger.write :debug, "[green][DEBUG] Validation passed: #{value.inspect} against #{validator.inspect}[/]\n" + else + Fog::Logger.write :debug, "[red][DEBUG] Validation failed: #{value.inspect} against #{validator.inspect}[/]\n" + end + result + end + end + end + end +end diff --git a/tests/helpers/formats_helper.rb b/tests/helpers/formats_helper.rb index 59a2bd30c..00caa1a85 100644 --- a/tests/helpers/formats_helper.rb +++ b/tests/helpers/formats_helper.rb @@ -1,3 +1,5 @@ +require "fog/schema/data_validator" + # format related hackery # allows both true.is_a?(Fog::Boolean) and false.is_a?(Fog::Boolean) # allows both nil.is_a?(Fog::Nullable::String) and ''.is_a?(Fog::Nullable::String) @@ -35,10 +37,10 @@ module Shindo # Strict mode will fail if the data has additional keys. Setting # +strict+ to +false+ will allow additional keys to appear. # - # @param [Hash] schmea A Hash schema + # @param [Hash] schema A Hash schema # @param [Hash] options Options to change validation rules # @option options [Boolean] :allow_extra_keys - # If +true+ deoes not fail when keys are in the data that are + # If +true+ does not fail when keys are in the data that are # not specified in the schema. This allows new values to # appear in API output without breaking the check. # @option options [Boolean] :allow_optional_rules @@ -47,13 +49,13 @@ module Shindo # @yield Data to check with schema # # @example Using in a test - # Shindo.tests("data matches schema") do - # data = {:string => "Hello" } - # data_matches_schema(:string => String) { data } + # Shindo.tests("comparing welcome data against schema") do + # data = {:welcome => "Hello" } + # data_matches_schema(:welcome => String) { data } # end # - # data matches schema - # + has proper format + # comparing welcome data against schema + # + data matches schema # # @example Example schema # { @@ -70,11 +72,14 @@ module Shindo # @return [Boolean] def data_matches_schema(schema, options = {}) test('data matches schema') do - confirm_data_matches_schema(yield, schema, options) + validator = Fog::Schema::DataValidator.new + valid = validator.validate(yield, schema, options) + @message = validator.message unless valid + valid end end - # @deprecation #formats is deprecated. Use #data_matches_schema instead + # @deprecated #formats is deprecated. Use #data_matches_schema instead def formats(format, strict = true) test('has proper format') do if strict @@ -82,86 +87,10 @@ module Shindo else options = {:allow_extra_keys => true, :allow_optional_rules => true} end - confirm_data_matches_schema(yield, format, options) - end - end - - private - - # Checks if the data structure matches the schema passed in and - # returns true if it fits. - # - # @param [Object] data Hash or Array to check - # @param [Object] schema Schema pattern to check against - # @param [Boolean] options - # @option options [Boolean] :allow_extra_keys - # If +true+ does not fail if extra keys are in the data - # that are not in the schema. Allows - # @option options [Boolean] :allow_optional_rules - # If +true+ does not fail if extra keys are in the schema - # that do not match the data. Not recommended! - # - # @return [Boolean] Did the data fit the schema? - def confirm_data_matches_schema(data, schema, options = {}) - # Clear message passed to the Shindo tests - @message = nil - - valid = validate_value(schema, data, options) - - unless valid - @message = "#{data.inspect} does not match #{schema.inspect}" - end - valid - end - - # This contains a slightly modified version of the Hashidator gem - # but unfortunately the gem does not cope with Array schemas. - # - # @see https://github.com/vangberg/hashidator/blob/master/lib/hashidator.rb - # - def validate_value(validator, value, options) - Fog::Logger.write :debug, "[yellow][DEBUG] #{value.inspect} against #{validator.inspect}[/]\n" - - # When being strict values not specified in the schema are fails - unless options[:allow_extra_keys] - if validator.respond_to?(:empty?) && value.respond_to?(:empty?) - # Validator is empty but values are not - return false if !value.empty? && validator.empty? - end - end - - unless options[:allow_optional_rules] - if validator.respond_to?(:empty?) && value.respond_to?(:empty?) - # Validator has rules left but no more values - return false if value.empty? && !validator.empty? - end - end - - case validator - when Array - return false if value.is_a?(Hash) - value.respond_to?(:all?) && value.all? {|x| validate_value(validator[0], x, options)} - when Symbol - value.respond_to? validator - when Hash - return false if value.is_a?(Array) - validator.all? do |key, sub_validator| - Fog::Logger.write :debug, "[blue][DEBUG] #{key.inspect} against #{sub_validator.inspect}[/]\n" - validate_value(sub_validator, value[key], options) - end - else - result = validator == value - result = validator === value unless result - # Repeat unless we have a Boolean already - unless (result.is_a?(TrueClass) || result.is_a?(FalseClass)) - result = validate_value(result, value, options) - end - if result - Fog::Logger.write :debug, "[green][DEBUG] Validation passed: #{value.inspect} against #{validator.inspect}[/]\n" - else - Fog::Logger.write :debug, "[red][DEBUG] Validation failed: #{value.inspect} against #{validator.inspect}[/]\n" - end - result + validator = Fog::Schema::DataValidator.new + valid = validator.validate(yield, format, options) + @message = validator.message unless valid + valid end end end diff --git a/tests/helpers/formats_helper_tests.rb b/tests/helpers/formats_helper_tests.rb index 6248b251d..f62c79ed4 100644 --- a/tests/helpers/formats_helper_tests.rb +++ b/tests/helpers/formats_helper_tests.rb @@ -1,5 +1,60 @@ Shindo.tests('test_helper', 'meta') do + tests('comparing welcome data against schema') do + data = {:welcome => "Hello" } + data_matches_schema(:welcome => String) { data } + end + + tests('#data_matches_schema') do + tests('when value matches schema expectation') do + data_matches_schema({"key" => String}) { {"key" => "Value"} } + end + + tests('when values within an array all match schema expectation') do + data_matches_schema({"key" => [Integer]}) { {"key" => [1, 2]} } + end + + tests('when nested values match schema expectation') do + data_matches_schema({"key" => {:nested_key => String}}) { {"key" => {:nested_key => "Value"}} } + end + + tests('when collection of values all match schema expectation') do + data_matches_schema([{"key" => String}]) { [{"key" => "Value"}, {"key" => "Value"}] } + end + + tests('when collection is empty although schema covers optional members') do + data_matches_schema([{"key" => String}], {:allow_optional_rules => true}) { [] } + end + + tests('when additional keys are passed and not strict') do + data_matches_schema({"key" => String}, {:allow_extra_keys => true}) { {"key" => "Value", :extra => "Bonus"} } + end + + tests('when value is nil and schema expects NilClass') do + data_matches_schema({"key" => NilClass}) { {"key" => nil} } + end + + tests('when value and schema match as hashes') do + data_matches_schema({}) { {} } + end + + tests('when value and schema match as arrays') do + data_matches_schema([]) { [] } + end + + tests('when value is a Time') do + data_matches_schema({"time" => Time}) { {"time" => Time.now} } + end + + tests('when key is missing but value should be NilClass (#1477)') do + data_matches_schema({"key" => NilClass}, {:allow_optional_rules => true}) { {} } + end + + tests('when key is missing but value is nullable (#1477)') do + data_matches_schema({"key" => Fog::Nullable::String}, {:allow_optional_rules => true}) { {} } + end + end + tests('#formats backwards compatible changes') do tests('when value matches schema expectation') do @@ -52,109 +107,5 @@ Shindo.tests('test_helper', 'meta') do end - tests('data matches schema') do - data = {:welcome => "Hello" } - data_matches_schema(:welcome => String) { data } - end - - tests('#confirm_data_matches_schema') do - - tests('returns true') do - - returns(true, 'when value matches schema expectation') do - confirm_data_matches_schema({"key" => "Value"}, {"key" => String}) - end - - returns(true, 'when values within an array all match schema expectation') do - confirm_data_matches_schema({"key" => [1, 2]}, {"key" => [Integer]}) - end - - returns(true, 'when nested values match schema expectation') do - confirm_data_matches_schema({"key" => {:nested_key => "Value"}}, {"key" => {:nested_key => String}}) - end - - returns(true, 'when collection of values all match schema expectation') do - confirm_data_matches_schema([{"key" => "Value"}, {"key" => "Value"}], [{"key" => String}]) - end - - returns(true, 'when collection is empty although schema covers optional members') do - confirm_data_matches_schema([], [{"key" => String}], {:allow_optional_rules => true}) - end - - returns(true, 'when additional keys are passed and not strict') do - confirm_data_matches_schema({"key" => "Value", :extra => "Bonus"}, {"key" => String}, {:allow_extra_keys => true}) - end - - returns(true, 'when value is nil and schema expects NilClass') do - confirm_data_matches_schema({"key" => nil}, {"key" => NilClass}) - end - - returns(true, 'when value and schema match as hashes') do - confirm_data_matches_schema({}, {}) - end - - returns(true, 'when value and schema match as arrays') do - confirm_data_matches_schema([], []) - end - - returns(true, 'when value is a Time') do - confirm_data_matches_schema({"time" => Time.now}, {"time" => Time}) - end - - returns(true, 'when key is missing but value should be NilClass (#1477)') do - confirm_data_matches_schema({}, {"key" => NilClass}, {:allow_optional_rules => true}) - end - - returns(true, 'when key is missing but value is nullable (#1477)') do - confirm_data_matches_schema({}, {"key" => Fog::Nullable::String}, {:allow_optional_rules => true}) - end - - end - - tests('returns false') do - - returns(false, 'when value does not match schema expectation') do - confirm_data_matches_schema({"key" => nil}, {"key" => String}) - end - - returns(false, 'when key formats do not match') do - confirm_data_matches_schema({"key" => "Value"}, {:key => String}) - end - - returns(false, 'when additional keys are passed and strict') do - confirm_data_matches_schema({"key" => "Missing"}, {}) - end - - returns(false, 'when some keys do not appear') do - confirm_data_matches_schema({}, {"key" => String}) - end - - returns(false, 'when collection contains a member that does not match schema') do - confirm_data_matches_schema([{"key" => "Value"}, {"key" => 5}], [{"key" => String}]) - end - - returns(false, 'when hash and array are compared') do - confirm_data_matches_schema({}, []) - end - - returns(false, 'when array and hash are compared') do - confirm_data_matches_schema([], {}) - end - - returns(false, 'when a hash is expected but another data type is found') do - confirm_data_matches_schema({"key" => {:nested_key => []}}, {"key" => {:nested_key => {}}}) - end - - returns(false, 'when key is missing but value should be NilClass (#1477)') do - confirm_data_matches_schema({}, {"key" => NilClass}, {:allow_optional_rules => false}) - end - - returns(false, 'when key is missing but value is nullable (#1477)') do - confirm_data_matches_schema({}, {"key" => Fog::Nullable::String}, {:allow_optional_rules => false}) - end - - end - - end end diff --git a/tests/helpers/schema_validator_tests.rb b/tests/helpers/schema_validator_tests.rb new file mode 100644 index 000000000..d19e69fb9 --- /dev/null +++ b/tests/helpers/schema_validator_tests.rb @@ -0,0 +1,103 @@ +Shindo.tests('Fog::Schema::DataValidator', 'meta') do + + validator = Fog::Schema::DataValidator.new + + tests('#validate') do + + tests('returns true') do + + returns(true, 'when value matches schema expectation') do + validator.validate({"key" => "Value"}, {"key" => String}) + end + + returns(true, 'when values within an array all match schema expectation') do + validator.validate({"key" => [1, 2]}, {"key" => [Integer]}) + end + + returns(true, 'when nested values match schema expectation') do + validator.validate({"key" => {:nested_key => "Value"}}, {"key" => {:nested_key => String}}) + end + + returns(true, 'when collection of values all match schema expectation') do + validator.validate([{"key" => "Value"}, {"key" => "Value"}], [{"key" => String}]) + end + + returns(true, 'when collection is empty although schema covers optional members') do + validator.validate([], [{"key" => String}], {:allow_optional_rules => true}) + end + + returns(true, 'when additional keys are passed and not strict') do + validator.validate({"key" => "Value", :extra => "Bonus"}, {"key" => String}, {:allow_extra_keys => true}) + end + + returns(true, 'when value is nil and schema expects NilClass') do + validator.validate({"key" => nil}, {"key" => NilClass}) + end + + returns(true, 'when value and schema match as hashes') do + validator.validate({}, {}) + end + + returns(true, 'when value and schema match as arrays') do + validator.validate([], []) + end + + returns(true, 'when value is a Time') do + validator.validate({"time" => Time.now}, {"time" => Time}) + end + + returns(true, 'when key is missing but value should be NilClass (#1477)') do + validator.validate({}, {"key" => NilClass}, {:allow_optional_rules => true}) + end + + returns(true, 'when key is missing but value is nullable (#1477)') do + validator.validate({}, {"key" => Fog::Nullable::String}, {:allow_optional_rules => true}) + end + + end + + tests('returns false') do + + returns(false, 'when value does not match schema expectation') do + validator.validate({"key" => nil}, {"key" => String}) + end + + returns(false, 'when key formats do not match') do + validator.validate({"key" => "Value"}, {:key => String}) + end + + returns(false, 'when additional keys are passed and strict') do + validator.validate({"key" => "Missing"}, {}) + end + + returns(false, 'when some keys do not appear') do + validator.validate({}, {"key" => String}) + end + + returns(false, 'when collection contains a member that does not match schema') do + validator.validate([{"key" => "Value"}, {"key" => 5}], [{"key" => String}]) + end + + returns(false, 'when hash and array are compared') do + validator.validate({}, []) + end + + returns(false, 'when array and hash are compared') do + validator.validate([], {}) + end + + returns(false, 'when a hash is expected but another data type is found') do + validator.validate({"key" => {:nested_key => []}}, {"key" => {:nested_key => {}}}) + end + + returns(false, 'when key is missing but value should be NilClass (#1477)') do + validator.validate({}, {"key" => NilClass}, {:allow_optional_rules => false}) + end + + returns(false, 'when key is missing but value is nullable (#1477)') do + validator.validate({}, {"key" => Fog::Nullable::String}, {:allow_optional_rules => false}) + end + + end + end +end From cdae6f38f4be36ac8ff4094392ebf624e241667a Mon Sep 17 00:00:00 2001 From: Paul Thornthwaite Date: Wed, 23 Jan 2013 14:54:47 +0000 Subject: [PATCH 7/7] [tests] Fixes schema validator for arrays Arrays should not fail if empty. They should only fail if any member of the array does not match the schema. --- lib/fog/schema/data_validator.rb | 31 +++++++++++++------------ tests/helpers/formats_helper.rb | 4 +++- tests/helpers/schema_validator_tests.rb | 6 ++++- 3 files changed, 24 insertions(+), 17 deletions(-) diff --git a/lib/fog/schema/data_validator.rb b/lib/fog/schema/data_validator.rb index 108f23972..68a7d7a77 100644 --- a/lib/fog/schema/data_validator.rb +++ b/lib/fog/schema/data_validator.rb @@ -106,21 +106,6 @@ module Fog def validate_value(validator, value, options) Fog::Logger.write :debug, "[yellow][DEBUG] #{value.inspect} against #{validator.inspect}[/]\n" - # When being strict values not specified in the schema are fails - unless options[:allow_extra_keys] - if validator.respond_to?(:empty?) && value.respond_to?(:empty?) - # Validator is empty but values are not - return false if !value.empty? && validator.empty? - end - end - - unless options[:allow_optional_rules] - if validator.respond_to?(:empty?) && value.respond_to?(:empty?) - # Validator has rules left but no more values - return false if value.empty? && !validator.empty? - end - end - case validator when Array return false if value.is_a?(Hash) @@ -129,6 +114,22 @@ module Fog value.respond_to? validator when Hash return false if value.is_a?(Array) + + # When being strict values not specified in the schema are fails + unless options[:allow_extra_keys] + if value.respond_to?(:empty?) + # Validator is empty but values are not + return false if !value.empty? && validator.empty? + end + end + + unless options[:allow_optional_rules] + if value.respond_to?(:empty?) + # Validator has rules left but no more values + return false if value.empty? && !validator.empty? + end + end + validator.all? do |key, sub_validator| Fog::Logger.write :debug, "[blue][DEBUG] #{key.inspect} against #{sub_validator.inspect}[/]\n" validate_value(sub_validator, value[key], options) diff --git a/tests/helpers/formats_helper.rb b/tests/helpers/formats_helper.rb index 00caa1a85..3d5d39826 100644 --- a/tests/helpers/formats_helper.rb +++ b/tests/helpers/formats_helper.rb @@ -62,7 +62,9 @@ module Shindo # "id" => String, # "ram" => Integer, # "disks" => [ - # "size" => Float + # { + # "size" => Float + # } # ], # "dns_name" => Fog::Nullable::String, # "active" => Fog::Boolean, diff --git a/tests/helpers/schema_validator_tests.rb b/tests/helpers/schema_validator_tests.rb index d19e69fb9..8715af391 100644 --- a/tests/helpers/schema_validator_tests.rb +++ b/tests/helpers/schema_validator_tests.rb @@ -23,7 +23,7 @@ Shindo.tests('Fog::Schema::DataValidator', 'meta') do end returns(true, 'when collection is empty although schema covers optional members') do - validator.validate([], [{"key" => String}], {:allow_optional_rules => true}) + validator.validate([], [{"key" => String}]) end returns(true, 'when additional keys are passed and not strict') do @@ -78,6 +78,10 @@ Shindo.tests('Fog::Schema::DataValidator', 'meta') do validator.validate([{"key" => "Value"}, {"key" => 5}], [{"key" => String}]) end + returns(false, 'when collection has multiple schema patterns') do + validator.validate([{"key" => "Value"}], [{"key" => Integer}, {"key" => String}]) + end + returns(false, 'when hash and array are compared') do validator.validate({}, []) end