Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Add root argument #32

Merged
merged 18 commits into from
Oct 17, 2017
Merged

Add root argument #32

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
6a68a03
Add root argument: Add specs and basic argument carrying
chrisatanasian Oct 13, 2017
b5c35c8
Add root argument: Refactor Surrealist::Copier#deep_copy
chrisatanasian Oct 13, 2017
e539eac
Add root argument: Add logic and get tests to pass
chrisatanasian Oct 13, 2017
d31054c
Add root argument: Add more thorough tests
chrisatanasian Oct 14, 2017
02ccef2
Add root argument: Update README and comments
chrisatanasian Oct 14, 2017
189e9ce
Merge branch 'master' into add_root_argument
chrisatanasian Oct 14, 2017
cd63060
Add root argument: Remove unused parameter
chrisatanasian Oct 14, 2017
9aee27a
Add root argument: Undo README whitespace changes
chrisatanasian Oct 14, 2017
e316b77
Add root argument: Fix rubocop errors
chrisatanasian Oct 14, 2017
f96e9b0
Add root argument: Clean up README
chrisatanasian Oct 14, 2017
cb3ecd9
Add root argument: Add missing param name and DRY methods
chrisatanasian Oct 15, 2017
7fe664e
Add root argument: Raise if root is not nil, non-empty string, or sym…
chrisatanasian Oct 15, 2017
f5986c1
Merge branch 'master' into add_root_argument
chrisatanasian Oct 15, 2017
b481a88
Add root argument: Use struct to clean up specs
chrisatanasian Oct 16, 2017
7c61abc
Add root argument: Make root override include_root and include_namesp…
chrisatanasian Oct 16, 2017
eedb1df
Add root argument: Strip root of whitespaces
chrisatanasian Oct 17, 2017
da1054a
Merge branch 'add_root_argument' of https://github.com/chrisatanasian…
chrisatanasian Oct 17, 2017
78375c7
Add root argument: Update README.md to match logic
chrisatanasian Oct 17, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ to serialize nested objects and structures. [Introductory blogpost.](https://med
* [Include root](#include-root)
* [Include namespaces](#include-namespaces)
* [Collection Surrealization](#collection-surrealization)
* [Root](#root)
* [Bool and Any](#bool-and-any)
* [Type errors](#type-errors)
* [Undefined methods in schema](#undefined-methods-in-schema)
Expand Down Expand Up @@ -354,6 +355,35 @@ features (like associations, inheritance etc) are supported and covered. Other O
issues as well, tests are in progress. All optional arguments (`camelize`, `include_root` etc) are also supported.
Guides on where to use `#surrealize_collection` vs `#surrealize` for all ORMs are coming.

### Root
If you want to wrap the resulting JSON into a specified root key, you can pass optional `root` argument
to `#surrealize` or `#build_schema`.
``` ruby
class Cat
include Surrealist

json_schema do
{ weight: String }
end

def weight
'3 kilos'
end
end

Cat.new.surrealize(root: :kitten)
# => '{ "kitten": { "weight": "3 kilos" } }'
Cat.new.surrealize(root: 'kitten')
# => '{ "kitten": { "weight": "3 kilos" } }'
```
This works with the `include_root` and `include_namespaces` arguments too.
``` ruby
Animal::Cat.new.surrealize(include_root: true, root: :kitten)
# => '{ "kitten": { "cat": { "weight": "3 kilos" } } }'
Animal::Cat.new.surrealize(include_namespaces: true, root: 'kitten')
# => '{ "kitten": { "animal": { "cat": { "weight": "3 kilos" } } } }'
```

### Bool and Any
If you have a parameter that is of boolean type, or if you don't care about the type, you
can use `Bool` and `Any` respectively.
Expand Down
4 changes: 3 additions & 1 deletion lib/surrealist.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ def included(base)
# @param [Boolean] camelize optional argument for converting hash to camelBack.
# @param [Boolean] include_root optional argument for having the root key of the resulting hash
# as instance's class name.
# @param [String] root optional argument for using a specified root key for the resulting hash
#
# @return [Object] the Collection#map with elements being json-formatted string corresponding
# to the schema provided in the object's class. Values will be taken from the return values
Expand All @@ -44,7 +45,7 @@ def included(base)
# Surrealist.surrealize_collection(User.all)
# # => "[{\"name\":\"Nikita\",\"age\":23}, {\"name\":\"Alessandro\",\"age\":24}]"
# # For more examples see README
def surrealize_collection(collection, camelize: false, include_root: false, include_namespaces: false, namespaces_nesting_level: DEFAULT_NESTING_LEVEL) # rubocop:disable Metrics/LineLength
def surrealize_collection(collection, camelize: false, include_root: false, include_namespaces: false, root: nil, namespaces_nesting_level: DEFAULT_NESTING_LEVEL) # rubocop:disable Metrics/LineLength
unless collection.respond_to?(:each)
raise Surrealist::ExceptionRaiser.raise_invalid_collection!
end
Expand All @@ -54,6 +55,7 @@ def surrealize_collection(collection, camelize: false, include_root: false, incl
camelize: camelize,
include_root: include_root,
include_namespaces: include_namespaces,
root: root,
namespaces_nesting_level: namespaces_nesting_level,
)
end)
Expand Down
19 changes: 15 additions & 4 deletions lib/surrealist/carrier.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,21 +10,23 @@ class Carrier
# as instance's class name.
# @param [Boolean] include_namespaces optional argument for having root key as a nested hash of
# instance's namespaces. Animal::Cat.new.surrealize -> (animal: { cat: { weight: '3 kilos' } })
# @param [String] root optional argument for using a specified root key for the resulting hash
# @param [Integer] namespaces_nesting_level level of namespaces nesting.
#
# @raise ArgumentError if types of arguments are wrong.
#
# @return [Carrier] self if type checks were passed.
def self.call(camelize:, include_root:, include_namespaces:, namespaces_nesting_level:)
new(camelize, include_root, include_namespaces, namespaces_nesting_level).sanitize!
def self.call(camelize:, include_root:, include_namespaces:, root:, namespaces_nesting_level:)
new(camelize, include_root, include_namespaces, root, namespaces_nesting_level).sanitize!
end

attr_reader :camelize, :include_root, :include_namespaces, :namespaces_nesting_level
attr_reader :camelize, :include_root, :include_namespaces, :root, :namespaces_nesting_level

def initialize(camelize, include_root, include_namespaces, namespaces_nesting_level)
def initialize(camelize, include_root, include_namespaces, root, namespaces_nesting_level)
@camelize = camelize
@include_root = include_root
@include_namespaces = include_namespaces
@root = root
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to add sanitization of root parameter to #sanitize! method and specs for it (spec/carrier_spec.rb). Right now there are specs only for strings and nil. It should work with symbols as well (Something.surrealize(root: :wrapper)). I suggest that we raise ArgumentError if root is anything except non-empty string or a symbol (Class, Integer and so on).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about nil? Seems like the way the code path is setup, Surrealist#call is always expecting something for all of its arguments. In the case of include_root, for example, this is fine because it just defaults to false, which does nothing. But if only non-empty string and a symbol are allowed for root, then there is no way to prevent the schema from being wrapped into root.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, good point. There are two ways here:

  1. You can default root argument to '' instead of nil and then validate it on not being anything except string or symbol.
  2. You can leave default root argument as it is (nil), and then validate it on being either nil, non-empty string or a symbol.

I think I prefer the second option, it feels more naturally to have nil if we don't want to do anything. What do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think '2' is better and less arbitrary than empty string. I think the validation/sanitation needs to considers blank strings/symbols. We need to disallow things like " " of :" ", and maybe even consider stripping white space from these args: " hello " => "hello".

This may be something that we should also go back and check on for other params potentially @nesaulov ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, nil seems more natural to me.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AlessandroMinali yes, you are right, we need to strip string argument. This is not relevant for other arguments, because they are either booleans or integers. @chrisatanasian could you please add a method in Carrier.sanitize! that will strip root argument?
And also add specs for this case:
Animal.surrealize(root: ' wat ') #=> '{ "wat" => {...} }'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@namespaces_nesting_level = namespaces_nesting_level
end

Expand All @@ -34,6 +36,7 @@ def initialize(camelize, include_root, include_namespaces, namespaces_nesting_le
def sanitize!
check_booleans!
check_namespaces_nesting!
check_root!
self
end

Expand Down Expand Up @@ -65,5 +68,13 @@ def check_namespaces_nesting!
Surrealist::ExceptionRaiser.raise_invalid_nesting!(namespaces_nesting_level)
end
end

# Checks if root is not nil, a non-empty string, or symbol
# @raise ArgumentError
def check_root!
unless root.nil? || (root.is_a?(String) && root.present?) || root.is_a?(Symbol)
Surrealist::ExceptionRaiser.raise_invalid_root!(root)
end
end
end
end
40 changes: 30 additions & 10 deletions lib/surrealist/copier.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,40 @@ class << self
def deep_copy(hash:, klass: false, carrier:)
namespaces_condition = carrier.include_namespaces || carrier.namespaces_nesting_level != DEFAULT_NESTING_LEVEL # rubocop:disable Metrics/LineLength

return copy_hash(hash) unless carrier.include_root || namespaces_condition
if !klass && (carrier.include_root || namespaces_condition)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice decision! 👍

Surrealist::ExceptionRaiser.raise_unknown_root!
end

copy_before_root = copied_and_possibly_wrapped_hash(hash, klass, carrier, namespaces_condition)

if carrier.root
wrap_schema_into_root(schema: copy_before_root, carrier: carrier, root: carrier.root.to_s)
else
copy_before_root
end
end

Surrealist::ExceptionRaiser.raise_unknown_root! unless klass
private

# Deeply copies the schema hash and wraps it if there is a need to.
#
# @param [Object] hash object to be copied.
# @param [String] klass instance's class name.
# @param [Object] carrier instance of Carrier class that carries arguments passed to +surrealize+
# @param [Bool] namespaces_condition whether to wrap into namespace.
#
# @return [Hash] deeply copied hash, possibly wrapped.
def copied_and_possibly_wrapped_hash(hash, klass, carrier, namespaces_condition)
if namespaces_condition
wrap_schema_into_namespace(schema: hash, klass: klass, carrier: carrier)
elsif carrier.include_root
wrap_schema_into_root(schema: hash, klass: klass, carrier: carrier)
actual_class = Surrealist::StringUtils.extract_class(klass)
wrap_schema_into_root(schema: hash, carrier: carrier, root: actual_class)
else
copy_hash(hash)
end
end

private

# Goes through the hash recursively and deeply copies it.
#
# @param [Hash] hash the hash to be copied.
Expand All @@ -42,16 +63,15 @@ def copy_hash(hash, wrapper: {})
# Wraps schema into a root key if `include_root` is passed to Surrealist.
#
# @param [Hash] schema schema hash.
# @param [String] klass name of the class where schema is defined.
# @param [Object] carrier instance of Carrier class that carries arguments passed to +surrealize+
# @param [String] root what the schema will be wrapped into
#
# @return [Hash] a hash with schema wrapped inside a root key.
def wrap_schema_into_root(schema:, klass:, carrier:)
actual_class = Surrealist::StringUtils.extract_class(klass)
def wrap_schema_into_root(schema:, carrier:, root:)
root_key = if carrier.camelize
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks very similar to #wrap_schema_into_root. Maybe we can DRY it out a bit? 🤔

Surrealist::StringUtils.camelize(actual_class, false).to_sym
Surrealist::StringUtils.camelize(root, false).to_sym
else
Surrealist::StringUtils.underscore(actual_class).to_sym
Surrealist::StringUtils.underscore(root).to_sym
end
result = Hash[root_key => {}]
copy_hash(schema, wrapper: result[root_key])
Expand Down
8 changes: 8 additions & 0 deletions lib/surrealist/exception_raiser.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,14 @@ def raise_invalid_nesting!(value)
raise ArgumentError,
"Expected `namespaces_nesting_level` to be a positive integer, got: #{value}"
end

# Raises ArgumentError if root is not nil, a non-empty stringk or symbol.
#
# @raise ArgumentError
def raise_invalid_root!(value)
raise ArgumentError,
"Expected `root` to be nil, a non-empty string, or symbol, got: #{value}"
end
end
end
end
7 changes: 5 additions & 2 deletions lib/surrealist/instance_methods.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ module InstanceMethods
# as instance's class name.
# @param [Boolean] include_namespaces optional argument for having root key as a nested hash of
# instance's namespaces. Animal::Cat.new.surrealize -> (animal: { cat: { weight: '3 kilos' } })
# @param [String] root optional argument for using a specified root key for the hash
# @param [Integer] namespaces_nesting_level level of namespaces nesting.
#
# @return [String] a json-formatted string corresponding to the schema
Expand Down Expand Up @@ -47,23 +48,25 @@ module InstanceMethods
# User.new.surrealize
# # => "{\"name\":\"Nikita\",\"age\":23}"
# # For more examples see README
def surrealize(camelize: false, include_root: false, include_namespaces: false, namespaces_nesting_level: DEFAULT_NESTING_LEVEL) # rubocop:disable Metrics/LineLength
def surrealize(camelize: false, include_root: false, include_namespaces: false, root: nil, namespaces_nesting_level: DEFAULT_NESTING_LEVEL) # rubocop:disable Metrics/LineLength
JSON.dump(
build_schema(
camelize: camelize,
include_root: include_root,
include_namespaces: include_namespaces,
root: root,
namespaces_nesting_level: namespaces_nesting_level,
),
)
end

# Invokes +Surrealist+'s class method +build_schema+
def build_schema(camelize: false, include_root: false, include_namespaces: false, namespaces_nesting_level: DEFAULT_NESTING_LEVEL) # rubocop:disable Metrics/LineLength
def build_schema(camelize: false, include_root: false, include_namespaces: false, root: nil, namespaces_nesting_level: DEFAULT_NESTING_LEVEL) # rubocop:disable Metrics/LineLength
carrier = Surrealist::Carrier.call(
camelize: camelize,
include_namespaces: include_namespaces,
include_root: include_root,
root: root,
namespaces_nesting_level: namespaces_nesting_level,
)

Expand Down
2 changes: 1 addition & 1 deletion spec/carrier_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
context 'valid arguments' do
VALID_PARAMS.each do |hash|
result = described_class.call(hash)
%i[camelize include_namespaces include_root namespaces_nesting_level].each do |method|
%i[camelize include_namespaces include_root namespaces_nesting_level root].each do |method|
it "stores #{method} in Carrier and returns self for #{hash}" do
expect(result).to be_a(Surrealist::Carrier)
expect(result.send(method)).to eq(hash[method])
Expand Down
65 changes: 46 additions & 19 deletions spec/carriers/params.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,24 +1,51 @@
VALID_PARAMS = [
{ camelize: true, include_namespaces: true, include_root: true, namespaces_nesting_level: 3 },
{ camelize: false, include_namespaces: true, include_root: true, namespaces_nesting_level: 3 },
{ camelize: false, include_namespaces: false, include_root: true, namespaces_nesting_level: 3 },
{ camelize: false, include_namespaces: false, include_root: false, namespaces_nesting_level: 3 },
{ camelize: true, include_namespaces: false, include_root: false, namespaces_nesting_level: 3 },
{ camelize: true, include_namespaces: true, include_root: false, namespaces_nesting_level: 3 },
{ camelize: true, include_namespaces: false, include_root: true, namespaces_nesting_level: 3 },
{ camelize: true, include_namespaces: false, include_root: true, namespaces_nesting_level: 435 },
{ camelize: true, include_namespaces: false, include_root: true, namespaces_nesting_level: 666 },
{ camelize: true, include_namespaces: true, include_root: true,
root: 'root', namespaces_nesting_level: 3 },
{ camelize: false, include_namespaces: true, include_root: true,
root: :root, namespaces_nesting_level: 3 },
{ camelize: false, include_namespaces: false, include_root: true,
root: nil, namespaces_nesting_level: 3 },
{ camelize: false, include_namespaces: false, include_root: false,
root: :root, namespaces_nesting_level: 3 },
{ camelize: true, include_namespaces: false, include_root: false,
root: 'root', namespaces_nesting_level: 3 },
{ camelize: true, include_namespaces: true, include_root: false,
root: nil, namespaces_nesting_level: 3 },
{ camelize: true, include_namespaces: false, include_root: true,
root: 'root', namespaces_nesting_level: 3 },
{ camelize: true, include_namespaces: false, include_root: true,
root: :root, namespaces_nesting_level: 435 },
{ camelize: true, include_namespaces: false, include_root: true,
root: nil, namespaces_nesting_level: 666 },
].freeze

INVALID_PARAMS = [
{ camelize: 'NO', include_namespaces: false, include_root: true, namespaces_nesting_level: 3 },
{ camelize: true, include_namespaces: 'false', include_root: true, namespaces_nesting_level: 3 },
{ camelize: true, include_namespaces: false, include_root: true, namespaces_nesting_level: 0 },
{ camelize: true, include_namespaces: false, include_root: false, namespaces_nesting_level: -3 },
{ camelize: true, include_namespaces: false, include_root: 'yep', namespaces_nesting_level: 3 },
{ camelize: 'NO', include_namespaces: false, include_root: true, namespaces_nesting_level: '3' },
{ camelize: 'NO', include_namespaces: false, include_root: true, namespaces_nesting_level: 3.14 },
{ camelize: Integer, include_namespaces: false, include_root: true, namespaces_nesting_level: 3 },
{ camelize: 'NO', include_namespaces: 'no', include_root: true, namespaces_nesting_level: '3.4' },
{ camelize: 'f', include_namespaces: false, include_root: 't', namespaces_nesting_level: true },
{ camelize: 'NO', include_namespaces: false, include_root: true,
root: 'root', namespaces_nesting_level: 3 },
{ camelize: true, include_namespaces: 'false', include_root: true,
root: :root, namespaces_nesting_level: 3 },
{ camelize: true, include_namespaces: false, include_root: true,
root: 'root', namespaces_nesting_level: 0 },
{ camelize: true, include_namespaces: false, include_root: false,
root: :root, namespaces_nesting_level: -3 },
{ camelize: true, include_namespaces: false, include_root: 'yep',
root: 'root', namespaces_nesting_level: 3 },
{ camelize: 'NO', include_namespaces: false, include_root: true,
root: :root, namespaces_nesting_level: '3' },
{ camelize: 'NO', include_namespaces: false, include_root: true,
root: 'root', namespaces_nesting_level: 3.14 },
{ camelize: Integer, include_namespaces: false, include_root: true,
root: :root, namespaces_nesting_level: 3 },
{ camelize: 'NO', include_namespaces: 'no', include_root: true,
root: 'root', namespaces_nesting_level: '3.4' },
{ camelize: 'f', include_namespaces: false, include_root: 't',
root: :root, namespaces_nesting_level: true },
{ camelize: true, include_namespaces: false, include_root: true,
root: '', namespaces_nesting_level: 666 },
{ camelize: true, include_namespaces: false, include_root: true,
root: 3, namespaces_nesting_level: 666 },
{ camelize: true, include_namespaces: false, include_root: true,
root: -3, namespaces_nesting_level: 666 },
{ camelize: true, include_namespaces: false, include_root: true,
root: 3.14, namespaces_nesting_level: 666 },
].freeze
Loading