From 3f1f8e8b691e36578db60ed9bcb258dfa8879359 Mon Sep 17 00:00:00 2001 From: LeFnord Date: Tue, 15 Nov 2016 22:42:07 +0100 Subject: [PATCH] updates dependencies - adds changelog entry --- CHANGELOG.md | 1 + Gemfile | 1 - grape-entity.gemspec | 1 + lib/grape_entity/condition/base.rb | 4 +- lib/grape_entity/entity.rb | 18 ++--- lib/grape_entity/exposure/base.rb | 4 +- lib/grape_entity/exposure/nesting_exposure.rb | 6 +- .../nesting_exposure/output_builder.rb | 16 ++--- lib/grape_entity/options.rb | 66 +++++++++---------- spec/grape_entity/entity_spec.rb | 33 +++++----- 10 files changed, 71 insertions(+), 79 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e356b0e9..3d5caf49 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ #### Features +* [#247](https://github.com/ruby-grape/grape-entity/pull/247): Updates dependencies; refactores to make specs green - [@LeFnord](https://github.com/LeFnord). * Your contribution here. #### Fixes diff --git a/Gemfile b/Gemfile index 04329d40..50f4e357 100644 --- a/Gemfile +++ b/Gemfile @@ -10,7 +10,6 @@ end gem 'json', '< 2', group: [:development, :test] group :development, :test do - gem 'rubocop', '0.31.0' gem 'ruby-grape-danger', '~> 0.1.0', require: false end diff --git a/grape-entity.gemspec b/grape-entity.gemspec index 27f486aa..10adfcee 100644 --- a/grape-entity.gemspec +++ b/grape-entity.gemspec @@ -19,6 +19,7 @@ Gem::Specification.new do |s| s.add_development_dependency 'bundler' s.add_development_dependency 'rake' + s.add_development_dependency 'rubocop', '~> 0.40' s.add_development_dependency 'rspec', '~> 3.0' s.add_development_dependency 'rack-test' s.add_development_dependency 'maruku' diff --git a/lib/grape_entity/condition/base.rb b/lib/grape_entity/condition/base.rb index d5b7998a..0142a22c 100644 --- a/lib/grape_entity/condition/base.rb +++ b/lib/grape_entity/condition/base.rb @@ -11,7 +11,7 @@ def initialize(inverse = false) end def ==(other) - (self.class == other.class) && (self.inversed? == other.inversed?) + (self.class == other.class) && (inversed? == other.inversed?) end def inversed? @@ -23,7 +23,7 @@ def met?(entity, options) end def if_value(_entity, _options) - fail NotImplementedError + raise NotImplementedError end def unless_value(entity, options) diff --git a/lib/grape_entity/entity.rb b/lib/grape_entity/entity.rb index 1a4ee1ed..d74c7020 100644 --- a/lib/grape_entity/entity.rb +++ b/lib/grape_entity/entity.rb @@ -151,11 +151,11 @@ def self.expose(*args, &block) options = merge_options(args.last.is_a?(Hash) ? args.pop : {}) if args.size > 1 - fail ArgumentError, 'You may not use the :as option on multi-attribute exposures.' if options[:as] - fail ArgumentError, 'You may not use block-setting on multi-attribute exposures.' if block_given? + raise ArgumentError, 'You may not use the :as option on multi-attribute exposures.' if options[:as] + raise ArgumentError, 'You may not use block-setting on multi-attribute exposures.' if block_given? end - fail ArgumentError, 'You may not use block-setting when also using format_with' if block_given? && options[:format_with].respond_to?(:call) + raise ArgumentError, 'You may not use block-setting when also using format_with' if block_given? && options[:format_with].respond_to?(:call) if block_given? if block.parameters.any? @@ -214,7 +214,7 @@ def self.can_unexpose? end def self.cannot_unexpose! - fail "You cannot call 'unexpose` inside of nesting exposure!" + raise "You cannot call 'unexpose` inside of nesting exposure!" end # Set options that will be applied to any exposures declared inside the block. @@ -270,7 +270,7 @@ def self.documentation # end # def self.format_with(name, &block) - fail ArgumentError, 'You must pass a block for formatters' unless block_given? + raise ArgumentError, 'You must pass a block for formatters' unless block_given? formatters[name.to_sym] = block end @@ -392,8 +392,8 @@ def self.present_collection(present_collection = false, collection_name = :items # @option options :only [Array] all the fields that should be returned # @option options :except [Array] all the fields that should not be returned def self.represent(objects, options = {}) - if objects.respond_to?(:to_ary) && ! @present_collection - root_element = root_element(:collection_root) + if objects.respond_to?(:to_ary) && !@present_collection + root_element = root_element(:collection_root) inner = objects.to_ary.map { |object| new(object, options.reverse_merge(collection: true)).presented } else objects = { @collection_name => objects } if @present_collection @@ -485,7 +485,7 @@ def delegate_attribute(attribute) end end - alias_method :as_json, :serializable_hash + alias as_json serializable_hash def to_json(options = {}) options = options.to_h if options && options.respond_to?(:to_h) @@ -536,7 +536,7 @@ def self.merge_options(options) # @param options [Hash] Exposure options. def self.valid_options(options) options.keys.each do |key| - fail ArgumentError, "#{key.inspect} is not a valid option." unless OPTIONS.include?(key) + raise ArgumentError, "#{key.inspect} is not a valid option." unless OPTIONS.include?(key) end options[:using] = options.delete(:with) if options.key?(:with) diff --git a/lib/grape_entity/exposure/base.rb b/lib/grape_entity/exposure/base.rb index d86a0f6e..656c077e 100644 --- a/lib/grape_entity/exposure/base.rb +++ b/lib/grape_entity/exposure/base.rb @@ -51,12 +51,12 @@ def valid?(entity) if @is_safe is_delegatable else - is_delegatable || fail(NoMethodError, "#{entity.class.name} missing attribute `#{@attribute}' on #{entity.object}") + is_delegatable || raise(NoMethodError, "#{entity.class.name} missing attribute `#{@attribute}' on #{entity.object}") end end def value(_entity, _options) - fail NotImplementedError + raise NotImplementedError end def serializable_value(entity, options) diff --git a/lib/grape_entity/exposure/nesting_exposure.rb b/lib/grape_entity/exposure/nesting_exposure.rb index 95da4075..0c0f9776 100644 --- a/lib/grape_entity/exposure/nesting_exposure.rb +++ b/lib/grape_entity/exposure/nesting_exposure.rb @@ -107,11 +107,7 @@ def normalized_exposures(entity, options) # For the given key if the last candidates for exposing are nesting then combine them. nesting_tail = [] exposures.reverse_each do |exposure| - if exposure.nesting? - nesting_tail.unshift exposure - else - break - end + nesting_tail.unshift exposure if exposure.nesting? end new_nested_exposures = nesting_tail.flat_map(&:nested_exposures) NestingExposure.new(key, {}, [], new_nested_exposures).tap do |new_exposure| diff --git a/lib/grape_entity/exposure/nesting_exposure/output_builder.rb b/lib/grape_entity/exposure/nesting_exposure/output_builder.rb index 66f77a43..9dc1d726 100644 --- a/lib/grape_entity/exposure/nesting_exposure/output_builder.rb +++ b/lib/grape_entity/exposure/nesting_exposure/output_builder.rb @@ -12,24 +12,20 @@ def add(exposure, result) # Save a result array in collections' array if it should be merged if result.is_a?(Array) && exposure.for_merge @output_collection << result - else - + elsif exposure.for_merge # If we have an array which should not be merged - save it with a key as a hash # If we have hash which should be merged - save it without a key (merge) - if exposure.for_merge - return unless result - @output_hash.merge! result, &merge_strategy(exposure.for_merge) - else - @output_hash[exposure.key] = result - end - + return unless result + @output_hash.merge! result, &merge_strategy(exposure.for_merge) + else + @output_hash[exposure.key] = result end end def kind_of?(klass) klass == output.class || super end - alias_method :is_a?, :kind_of? + alias is_a? kind_of? def __getobj__ output diff --git a/lib/grape_entity/options.rb b/lib/grape_entity/options.rb index 7d916efa..65ab974f 100644 --- a/lib/grape_entity/options.rb +++ b/lib/grape_entity/options.rb @@ -54,11 +54,11 @@ def empty? end def ==(other) - if other.is_a? Options - @opts_hash == other.opts_hash - else - @opts_hash == other - end + @opts_hash == if other.is_a? Options + other.opts_hash + else + other + end end def should_return_key?(key) @@ -79,42 +79,20 @@ def only_fields(for_key = nil) return nil unless @has_only @only_fields ||= @opts_hash[:only].each_with_object({}) do |attribute, allowed_fields| - if attribute.is_a?(Hash) - attribute.each do |attr, nested_attrs| - allowed_fields[attr] ||= [] - allowed_fields[attr] += nested_attrs - end - else - allowed_fields[attribute] = true - end - end.symbolize_keys - - if for_key && @only_fields[for_key].is_a?(Array) - @only_fields[for_key] - elsif for_key.nil? - @only_fields + build_symbolized_hash(attribute, allowed_fields) end + + only_for_given(for_key, @only_fields) end def except_fields(for_key = nil) return nil unless @has_except @except_fields ||= @opts_hash[:except].each_with_object({}) do |attribute, allowed_fields| - if attribute.is_a?(Hash) - attribute.each do |attr, nested_attrs| - allowed_fields[attr] ||= [] - allowed_fields[attr] += nested_attrs - end - else - allowed_fields[attribute] = true - end - end.symbolize_keys - - if for_key && @except_fields[for_key].is_a?(Array) - @except_fields[for_key] - elsif for_key.nil? - @except_fields + build_symbolized_hash(attribute, allowed_fields) end + + only_for_given(for_key, @except_fields) end def with_attr_path(part) @@ -141,6 +119,28 @@ def build_for_nesting(key) Options.new(new_opts_hash) end + + def build_symbolized_hash(attribute, hash) + if attribute.is_a?(Hash) + attribute.each do |attr, nested_attrs| + hash[attr.to_sym] = build_symbolized_hash(nested_attrs, {}) + end + elsif attribute.is_a?(Array) + return attribute.each { |x| build_symbolized_hash(x, {}) } + else + hash[attribute.to_sym] = true + end + + hash + end + + def only_for_given(key, fields) + if key && fields[key].is_a?(Array) + fields[key] + elsif key.nil? + fields + end + end end end end diff --git a/spec/grape_entity/entity_spec.rb b/spec/grape_entity/entity_spec.rb index d9f5e80f..4221b8fd 100644 --- a/spec/grape_entity/entity_spec.rb +++ b/spec/grape_entity/entity_spec.rb @@ -289,8 +289,7 @@ class Parent < Person end additional_hash = { users: [{ id: 1, name: 'John' }, { id: 2, name: 'Jay' }], - admins: [{ id: 3, name: 'Jack' }, { id: 4, name: 'James' }] - } + admins: [{ id: 3, name: 'Jack' }, { id: 4, name: 'James' }] } expect(subject.represent(additional_hash).serializable_hash).to eq( profiles: additional_hash[:users] + additional_hash[:admins], awesome: { just_a_key: 'value', just_another_key: 'value' } @@ -354,20 +353,20 @@ class Parent < Person subject.expose :birthday, format_with: :timestamp - model = { birthday: Time.gm(2012, 2, 27) } + model = { birthday: Time.gm(2012, 2, 27) } expect(subject.new(double(model)).as_json[:birthday]).to eq '02/27/2012' end it 'formats an exposure with a :format_with lambda that returns a value from the entity instance' do object = {} - subject.expose(:size, format_with: ->(_value) { self.object.class.to_s }) + subject.expose(:size, format_with: ->(_value) { object.class.to_s }) expect(subject.represent(object).value_for(:size)).to eq object.class.to_s end it 'formats an exposure with a :format_with symbol that returns a value from the entity instance' do subject.format_with :size_formatter do |_date| - self.object.class.to_s + object.class.to_s end object = {} @@ -378,7 +377,7 @@ class Parent < Person it 'works global on Grape::Entity' do Grape::Entity.format_with :size_formatter do |_date| - self.object.class.to_s + object.class.to_s end object = {} @@ -609,14 +608,14 @@ class Parent < Person end it 'returns multiple entities if called with a collection' do - representation = subject.represent(4.times.map { Object.new }) + representation = subject.represent(Array.new(4) { Object.new }) expect(representation).to be_kind_of Array expect(representation.size).to eq(4) expect(representation.reject { |r| r.is_a?(subject) }).to be_empty end it 'adds the collection: true option if called with a collection' do - representation = subject.represent(4.times.map { Object.new }) + representation = subject.represent(Array.new(4) { Object.new }) representation.each { |r| expect(r.options[:collection]).to be true } end @@ -628,7 +627,7 @@ class Parent < Person it 'returns a serialized array of hashes of multiple objects if serializable: true' do subject.expose(:awesome) { |_| true } - representation = subject.represent(2.times.map { Object.new }, serializable: true) + representation = subject.represent(Array.new(2) { Object.new }, serializable: true) expect(representation).to eq([{ awesome: true }, { awesome: true }]) end @@ -903,7 +902,7 @@ class Parent < Person subject.present_collection true subject.expose :items - representation = subject.represent(4.times.map { Object.new }) + representation = subject.represent(Array.new(4) { Object.new }) expect(representation).to be_kind_of(subject) expect(representation.object).to be_kind_of(Hash) expect(representation.object).to have_key :items @@ -915,7 +914,7 @@ class Parent < Person subject.present_collection true, :my_items subject.expose :my_items - representation = subject.represent(4.times.map { Object.new }, serializable: true) + representation = subject.represent(Array.new(4) { Object.new }, serializable: true) expect(representation).to be_kind_of(Grape::Entity::Exposure::NestingExposure::OutputBuilder) expect(representation).to be_kind_of(Hash) expect(representation).to have_key :my_items @@ -941,7 +940,7 @@ class Parent < Person context 'with an array of objects' do it 'allows a root element name to be specified' do - representation = subject.represent(4.times.map { Object.new }) + representation = subject.represent(Array.new(4) { Object.new }) expect(representation).to be_kind_of Hash expect(representation).to have_key 'things' expect(representation['things']).to be_kind_of Array @@ -952,13 +951,13 @@ class Parent < Person context 'it can be overridden' do it 'can be disabled' do - representation = subject.represent(4.times.map { Object.new }, root: false) + representation = subject.represent(Array.new(4) { Object.new }, root: false) expect(representation).to be_kind_of Array expect(representation.size).to eq 4 expect(representation.reject { |r| r.is_a?(subject) }).to be_empty end it 'can use a different name' do - representation = subject.represent(4.times.map { Object.new }, root: 'others') + representation = subject.represent(Array.new(4) { Object.new }, root: 'others') expect(representation).to be_kind_of Hash expect(representation).to have_key 'others' expect(representation['others']).to be_kind_of Array @@ -984,7 +983,7 @@ class Parent < Person context 'with an array of objects' do it 'allows a root element name to be specified' do - representation = subject.represent(4.times.map { Object.new }) + representation = subject.represent(Array.new(4) { Object.new }) expect(representation).to be_kind_of Array expect(representation.size).to eq 4 expect(representation.reject { |r| r.is_a?(subject) }).to be_empty @@ -1005,7 +1004,7 @@ class Parent < Person context 'with an array of objects' do it 'allows a root element name to be specified' do - representation = subject.represent(4.times.map { Object.new }) + representation = subject.represent(Array.new(4) { Object.new }) expect(representation).to be_kind_of Hash expect(representation).to have_key('things') expect(representation['things']).to be_kind_of Array @@ -1030,7 +1029,7 @@ class Parent < Person it 'inherits array root root' do child_class = Class.new(subject) - representation = child_class.represent(4.times.map { Object.new }) + representation = child_class.represent(Array.new(4) { Object.new }) expect(representation).to be_kind_of Hash expect(representation).to have_key('things') expect(representation['things']).to be_kind_of Array