Permalink
Browse files

Add better nullability support.

json-schema supports nullability through a type union.
However, it can be cumbersome to have to use type unions
everywhere. This commit adds better support for nullability:

* Any property can declare its nullability with a simple
  `nullable: true` declaration.
* Add a new `scalars_nullable_by_default` config setting.
  • Loading branch information...
1 parent 93e631a commit a5476283afd33bd25d2ff4e7c58dec1508a8a5af @myronmarston myronmarston committed Apr 27, 2013
View
9 lib/interpol/configuration.rb
@@ -1,3 +1,4 @@
+require 'interpol'
require 'interpol/endpoint'
require 'interpol/errors'
require 'yaml'
@@ -156,6 +157,14 @@ def example_response_for(endpoint_def, env)
selector.call(endpoint_def, env)
end
+ def scalars_nullable_by_default=(value)
+ @scalars_nullable_by_default = value
+ end
+
+ def scalars_nullable_by_default?
+ @scalars_nullable_by_default
+ end
+
def self.default
@default ||= Configuration.new
end
View
44 lib/interpol/endpoint.rb
@@ -1,6 +1,7 @@
require 'json-schema'
require 'interpol/errors'
require 'forwardable'
+require 'set'
module JSON
# The JSON-schema namespace
@@ -65,13 +66,14 @@ def fetch_from(hash, key)
# based on the endpoint definitions in the YAML files.
class Endpoint
include HashFetcher
- attr_reader :name, :route, :method, :custom_metadata
+ attr_reader :name, :route, :method, :custom_metadata, :configuration
- def initialize(endpoint_hash)
+ def initialize(endpoint_hash, configuration = Interpol.default_configuration)
@name = fetch_from(endpoint_hash, 'name')
@route = fetch_from(endpoint_hash, 'route')
@method = fetch_from(endpoint_hash, 'method').downcase.to_sym
+ @configuration = configuration
@custom_metadata = endpoint_hash.fetch('meta') { {} }
@definitions_hash, @all_definitions = extract_definitions_from(endpoint_hash)
@@ -181,7 +183,7 @@ class EndpointDefinition
attr_reader :endpoint, :message_type, :version, :schema,
:path_params, :query_params, :examples, :custom_metadata
extend Forwardable
- def_delegators :endpoint, :route
+ def_delegators :endpoint, :route, :configuration
DEFAULT_PARAM_HASH = { 'type' => 'object', 'properties' => {} }
@@ -244,12 +246,14 @@ def make_schema_strict!(raw_schema, modify_object=true)
end
end
- def make_schema_hash_strict!(raw_schema, modify_object=true)
+ def make_schema_hash_strict!(raw_schema, make_this_schema_strict=true)
+ conditionally_make_nullable(raw_schema)
+
raw_schema.each do |key, value|
make_schema_strict!(value, key != 'properties')
end
- return unless modify_object
+ return unless make_this_schema_strict
if raw_schema.has_key?('properties')
raw_schema['additionalProperties'] ||= false
@@ -258,9 +262,35 @@ def make_schema_hash_strict!(raw_schema, modify_object=true)
raw_schema['required'] = !raw_schema.delete('optional')
end
- def make_schema_array_strict!(raw_schema, modify_object=true)
+ def make_schema_array_strict!(raw_schema, make_nested_schemas_strict=true)
raw_schema.each do |entry|
- make_schema_strict!(entry, modify_object)
+ make_schema_strict!(entry, make_nested_schemas_strict)
+ end
+ end
+
+ def conditionally_make_nullable(raw_schema)
+ return unless should_be_nullable?(raw_schema)
+
+ types = Array(raw_schema['type'])
+ return unless types.any?
+
+ types << "null" unless types.include?('null')
+
+ raw_schema['type'] = types
+ end
+
+ def should_be_nullable?(raw_schema)
+ raw_schema.fetch('nullable') do
+ configuration.scalars_nullable_by_default? && scalar?(raw_schema)
+ end
+ end
+
+ NON_SCALAR_TYPES = %w[ object array ]
+ def scalar?(raw_schema)
+ types = Array(raw_schema['type']).to_set
+
+ NON_SCALAR_TYPES.none? do |non_scalar|
+ types.include?(non_scalar)
end
end
View
11 spec/unit/interpol/configuration_spec.rb
@@ -291,6 +291,17 @@ def assert_expected_endpoint
end
end
+ describe "#scalars_nullable_by_default?" do
+ it 'defaults to false' do
+ expect(config.scalars_nullable_by_default?).to be_false
+ end
+
+ it 'can be set to true' do
+ config.scalars_nullable_by_default = true
+ expect(config.scalars_nullable_by_default?).to be_true
+ end
+ end
+
describe "#api_version" do
before { config.stub(:warn) }
View
133 spec/unit/interpol/endpoint_spec.rb
@@ -1,4 +1,5 @@
require 'fast_spec_helper'
+require 'interpol'
require 'interpol/endpoint'
module Interpol
@@ -178,6 +179,19 @@ def endpoint(route)
expect(endpoint('/foo/bar').route_matches?('/foo/bar/bazz')).to be_false
end
end
+
+ describe "#configuration" do
+ it 'defaults to the Interpol default config' do
+ endpoint = Endpoint.new(build_hash)
+ expect(endpoint.configuration).to be(Interpol.default_configuration)
+ end
+
+ it 'allows a config instance to be passed' do
+ config = Configuration.new
+ endpoint = Endpoint.new(build_hash, config)
+ expect(endpoint.configuration).to be(config)
+ end
+ end
end
describe EndpointDefinition do
@@ -189,7 +203,11 @@ def build_hash(hash = {})
end
let(:version) { '1.0' }
- let(:endpoint) { fire_double("Interpol::Endpoint", :name => 'my-endpoint').as_null_object }
+ let(:config) { Configuration.new }
+ let(:endpoint) do
+ fire_double("Interpol::Endpoint", :name => 'my-endpoint',
+ :configuration => config).as_null_object
+ end
it 'initializes the endpoint' do
endpoint_def = EndpointDefinition.new(endpoint, version, 'response', build_hash)
@@ -360,6 +378,105 @@ def new_with(hash)
}.to raise_error(ValidationError)
end
+ context 'when scalars_nullable_by_default is set to true' do
+ before { config.scalars_nullable_by_default = true }
+
+ it 'allows nulls even when the property does not explicitly allow it' do
+ expect {
+ subject.validate_data!('foo' => nil)
+ }.not_to raise_error
+ end
+
+ it 'works with existing union types' do
+ schema['properties']['foo']['type'] = %w[ integer string ]
+
+ expect { subject.validate_data!('foo' => 123) }.not_to raise_error
+ expect { subject.validate_data!('foo' => 'a') }.not_to raise_error
+ expect { subject.validate_data!('foo' => nil) }.not_to raise_error
+ end
+
+ it 'does not add an extra `null` entry to an existing nullable type' do
+ schema['properties']['foo']['type'] = %w[ integer null ]
+
+ ::JSON::Validator.should_receive(:fully_validate_schema) do |schema|
+ expect(schema['properties']['foo']['type']).to match_array(%w[ integer null ])
+ [] # no errors
+ end
+
+ subject.validate_data!('foo' => nil)
+ end
+
+ it 'does not allow nulls when the property has `nullable: false`' do
+ schema['properties']['foo']['nullable'] = false
+
+ expect {
+ subject.validate_data!('foo' => nil)
+ }.to raise_error(ValidationError)
+ end
+
+ it 'does not automatically make arrays nullable' do
+ schema['properties']['foo']['type'] = 'array'
+
+ expect {
+ subject.validate_data!('foo' => [1])
+ }.not_to raise_error
+
+ expect {
+ subject.validate_data!('foo' => nil)
+ }.to raise_error(ValidationError)
+ end
+
+ it 'does not automatically make objects nullable' do
+ schema['properties']['foo']['type'] = 'object'
+
+ expect {
+ subject.validate_data!('foo' => { 'a' => 3 })
+ }.not_to raise_error
+
+ expect {
+ subject.validate_data!('foo' => nil)
+ }.to raise_error(ValidationError)
+ end
+
+ it 'does not make unioned types that include non-scalars nullable' do
+ schema['properties']['foo']['type'] = %w[ object array integer ]
+
+ expect {
+ subject.validate_data!('foo' => { 'a' => 3 })
+ }.not_to raise_error
+
+ expect {
+ subject.validate_data!('foo' => 3)
+ }.not_to raise_error
+
+ expect {
+ subject.validate_data!('foo' => [])
+ }.not_to raise_error
+
+ expect {
+ subject.validate_data!('foo' => nil)
+ }.to raise_error(ValidationError)
+ end
+ end
+
+ context 'when scalars_nullable_by_default is set to false' do
+ before { config.scalars_nullable_by_default = false }
+
+ it 'does not allow nulls by default' do
+ expect {
+ subject.validate_data!('foo' => nil)
+ }.to raise_error(ValidationError)
+ end
+
+ it 'allow nulls when the property has `nullable: true`' do
+ schema['properties']['foo']['nullable'] = true
+
+ expect {
+ subject.validate_data!('foo' => nil)
+ }.not_to raise_error
+ end
+ end
+
context 'a schema with nested objects' do
before do
schema['properties']['foo'] = {
@@ -368,6 +485,20 @@ def new_with(hash)
}
end
+ it 'allows sub-properties to be nullable' do
+ schema['properties']['foo']['properties']['name']['nullable'] = true
+
+ expect {
+ subject.validate_data!('foo' => { 'name' => nil })
+ }.not_to raise_error
+ end
+
+ it 'does not make sub-properties nullable by default' do
+ expect {
+ subject.validate_data!('foo' => { 'name' => nil })
+ }.to raise_error(ValidationError)
+ end
+
it 'requires all properties on nested objects' do
expect {
subject.validate_data!('foo' => {})

0 comments on commit a547628

Please sign in to comment.