diff --git a/bootic_client.gemspec b/bootic_client.gemspec index 4bb1c32..ed917bc 100644 --- a/bootic_client.gemspec +++ b/bootic_client.gemspec @@ -17,6 +17,9 @@ Gem::Specification.new do |spec| spec.test_files = spec.files.grep(%r{^(test|spec|features)/}) spec.require_paths = ["lib"] + spec.add_dependency "ostruct" + spec.add_dependency "base64" + spec.add_dependency "bigdecimal" spec.add_dependency "faraday", '~> 2.2' spec.add_dependency 'faraday-net_http_persistent', '~> 2.0' spec.add_dependency "uri_template", '~> 0.7' @@ -24,6 +27,7 @@ Gem::Specification.new do |spec| spec.add_dependency "net-http-persistent", '~> 4' spec.add_dependency "oauth2", "~> 1.4" + spec.add_development_dependency 'rexml' spec.add_development_dependency "rake" spec.add_development_dependency "rspec", "3.5.0" spec.add_development_dependency "jwt", "~> 1.5" diff --git a/lib/bootic_client.rb b/lib/bootic_client.rb index 9fd7026..8d14a91 100644 --- a/lib/bootic_client.rb +++ b/lib/bootic_client.rb @@ -1,9 +1,9 @@ require 'logger' -require "bootic_client/version" -require "bootic_client/entity" -require "bootic_client/relation" -require "bootic_client/client" -require "bootic_client/configuration" +require_relative './bootic_client/version' +require_relative './bootic_client/entity' +require_relative './bootic_client/relation' +require_relative './bootic_client/client' +require_relative './bootic_client/configuration' module BooticClient class << self @@ -13,6 +13,7 @@ def strategies def client(strategy_name, client_opts = {}, &on_new_token) return @stubber if @stubber + opts = client_opts.dup opts[:logging] = configuration.logging opts[:logger] = configuration.logger if configuration.logging diff --git a/lib/bootic_client/client.rb b/lib/bootic_client/client.rb index c2437bf..720e1ba 100644 --- a/lib/bootic_client/client.rb +++ b/lib/bootic_client/client.rb @@ -3,15 +3,14 @@ require 'base64' require 'faraday' require 'faraday-http-cache' -require "bootic_client/errors" +require 'bootic_client/errors' require 'faraday/net_http_persistent' module BooticClient class Client - - USER_AGENT = "[BooticClient v#{VERSION}] Ruby-#{RUBY_VERSION} - #{RUBY_PLATFORM}".freeze - JSON_MIME = 'application/json'.freeze + USER_AGENT = "[BooticClient v#{VERSION}] Ruby-#{RUBY_VERSION} - #{RUBY_PLATFORM}" + JSON_MIME = 'application/json' attr_reader :options @@ -22,7 +21,7 @@ def initialize(options = {}, &block) user_agent: USER_AGENT }.merge(options.dup) - @options[:cache_store] = @options[:cache_store] || Faraday::HttpCache::MemoryStore.new + @options[:cache_store] ||= BoundedMemoryStore.new conn &block if block_given? end @@ -61,9 +60,41 @@ def delete(href, _ = {}, headers = {}) end end + def close + @conn = nil + end + + # In-memory cache with a hard upper bound on entries to prevent unbounded growth. + # Evicts the oldest entry when the limit is reached. + class BoundedMemoryStore + DEFAULT_MAX_SIZE = 500 + + def initialize(max_size: DEFAULT_MAX_SIZE) + @store = {} + @max_size = max_size + end + + def read(key) + @store[key] + end + + def write(key, value) + @store.delete(@store.keys.first) if !@store.key?(key) && @store.size >= @max_size + @store[key] = value + end + + def delete(key) + @store.delete(key) + end + + def exist?(key) + @store.key?(key) + end + end + class SafeCacheSerializer - PREFIX = '__booticclient__base64__:'.freeze - PREFIX_EXP = %r{^#{PREFIX}}.freeze + PREFIX = '__booticclient__base64__:' + PREFIX_EXP = /^#{PREFIX}/.freeze def self.dump(data) data[:body] = "#{PREFIX}#{Base64.strict_encode64(data[:body])}" if data[:body].is_a?(String) @@ -81,15 +112,22 @@ def self.load(string) private - def conn(&block) - @conn ||= Faraday.new do |f| - cache_options = {serializer: SafeCacheSerializer, shared_cache: false, store: options[:cache_store]} + DEFAULT_TIMEOUT = 20.freeze # seconds + + def conn + request_opts = { + timeout: (options[:timeout] || DEFAULT_TIMEOUT).to_i, # both read/open timeout + open_timeout: (options[:open_timeout] || DEFAULT_TIMEOUT).to_i # only open timeout + } + + @conn ||= Faraday.new(request: request_opts) do |f| + cache_options = { serializer: SafeCacheSerializer, shared_cache: false, store: options[:cache_store] } cache_options[:logger] = options[:logger] if options[:logging] f.use :http_cache, **cache_options f.response :logger, options[:logger] if options[:logging] yield f if block_given? - f.adapter *Array(options[:faraday_adapter]) + f.adapter(*Array(options[:faraday_adapter])) end end @@ -102,6 +140,8 @@ def request_headers end def validated_request!(verb, href, &block) + retries ||= 0 + resp = conn.send(verb) do |req| req.url href req.headers.update request_headers @@ -110,17 +150,27 @@ def validated_request!(verb, href, &block) raise_if_invalid! resp, "#{verb.upcase} #{href}" resp + + rescue Faraday::ConnectionFailed, Faraday::TimeoutError => e + if (retries += 1) < 3 # max retries + puts "Got #{e.class} error, attempt #{retries}, retrying..." + retry + else + raise + end end def raise_if_invalid!(resp, url = nil) - raise ServerError.new("Server Error", url) if resp.status > 499 - raise NotFoundError.new("Not Found", url) if resp.status == 404 - raise UnauthorizedError.new("Unauthorized Request", url) if resp.status == 401 - raise AccessForbiddenError.new("Access Forbidden", url) if resp.status == 403 + raise ServerError.new('Server Error', url) if resp.status > 499 + raise TooManyRequestsError.new('Too Many Requests', url) if resp.status == 429 + raise NotFoundError.new('Not Found', url) if resp.status == 404 + raise UnauthorizedError.new('Unauthorized Request', url) if resp.status == 401 + raise AccessForbiddenError.new('Access Forbidden', url) if resp.status == 403 end def sanitized(payload) - return payload unless payload.kind_of?(Hash) + return payload unless payload.is_a?(Hash) + payload.each_with_object({}) do |(k, v), memo| memo[k] = if v.kind_of?(Hash) sanitized v diff --git a/lib/bootic_client/configuration.rb b/lib/bootic_client/configuration.rb index b275619..57dd272 100644 --- a/lib/bootic_client/configuration.rb +++ b/lib/bootic_client/configuration.rb @@ -52,7 +52,7 @@ def api_root end def logger - @logger || ::Logger.new(STDOUT) + @logger ||= ::Logger.new(STDOUT) end def response_handlers diff --git a/lib/bootic_client/entity.rb b/lib/bootic_client/entity.rb index e695b3e..9f93703 100644 --- a/lib/bootic_client/entity.rb +++ b/lib/bootic_client/entity.rb @@ -1,13 +1,15 @@ # frozen_string_literal: true -require "bootic_client/relation" +require 'bootic_client/relation' +require 'forwardable' +require 'weakref' module BooticClient module EnumerableEntity include Enumerable def each(&block) - self[:items].each &block + entities.get(:items).each(&block) end def full_set @@ -30,69 +32,84 @@ def full_set class Entity - CURIE_EXP = /(.+):(.+)/.freeze + CURIE_NS = 'btc' CURIES_REL = 'curies'.freeze SPECIAL_PROP_EXP = /^_.+/.freeze - attr_reader :curies, :entities + def self.wrap(obj, client: nil, top: nil) + case obj + when Hash + new(obj, client, top: top) + when Array + EntityArray.new(obj, client, top) + else + obj + end + end def initialize(attrs, client, top: self) - @attrs = attrs.kind_of?(Hash) ? attrs : {} - @client, @top = client, top - build! - self.extend EnumerableEntity if iterable? + @attrs = attrs.is_a?(Hash) ? attrs : {} + @client = client ? WeakRef.new(client) : nil + @top = top + extend EnumerableEntity if iterable? end def to_hash @attrs end + alias_method :to_h, :to_hash + + def as_json(opts = {}) + to_hash + end + def [](key) - key = key.to_sym - has_property?(key) ? properties[key] : entities[key] + has_property?(key) ? properties.get(key) : entities.get(key) end + alias_method :try, :[] + def has?(prop_name) has_property?(prop_name) || has_entity?(prop_name) || has_rel?(prop_name) end def can?(rel_name) - has_rel? rel_name + has_rel?(rel_name) end def inspect - %(#<#{self.class.name} props: [#{properties.keys.join(', ')}] rels: [#{rels.keys.join(', ')}] entities: [#{entities.keys.join(', ')}]>) + %(#<#{self.class.name} properties: [#{properties.keys.join(', ')}] relations: [#{rels.keys.join(', ')}] entities: [#{entities.keys.join(', ')}]>) end def properties - @properties ||= attrs.select{|k,v| !(k =~ SPECIAL_PROP_EXP)}.each_with_object({}) do |(k,v),memo| - memo[k.to_sym] = Entity.wrap(v, client: client, top: top) - end + @properties ||= PropertySet.new(attrs.select { |k,v| !(k =~ SPECIAL_PROP_EXP) }) end - def links - @links ||= attrs.fetch('_links', {}) + alias_method :props, :properties + + def entities + @entities ||= EntitySet.new(attrs.fetch('_embedded', {}), client, top) end - def self.wrap(obj, client: nil, top: nil) - case obj - when Hash - new(obj, client, top: top) - when Array - obj.map{|e| wrap(e, client: client, top: top)} - else - obj - end + def relations + @relations ||= RelationSet.new(attrs.fetch('_links', {}), client, top, curies) + end + + alias_method :rels, :relations + + def links + @links ||= attrs.fetch('_links', {}) end def method_missing(name, *args, &block) if !block_given? if has_property?(name) - self[name] + properties.get(name) elsif has_entity?(name) - entities[name] + entities.get(name) elsif has_rel?(name) - rels[name].run(*args) + rels.get(name).run(*args) else super end @@ -105,54 +122,222 @@ def respond_to_missing?(method_name, include_private = false) has?(method_name) end - def has_property?(prop_name) - properties.has_key? prop_name.to_sym + def has_property?(name) + properties.has?(name) end - def has_entity?(prop_name) - entities.has_key? prop_name.to_sym + def has_entity?(name) + entities.has?(name) end - def has_rel?(prop_name) - rels.has_key? prop_name.to_sym + def has_rel?(name) + rels.has?(name) end - def rels - @rels ||= ( - links = attrs.fetch('_links', {}) - links.each_with_object({}) do |(rel,rel_attrs),memo| - if rel =~ CURIE_EXP - _, curie_namespace, rel = rel.split(CURIE_EXP) - if curie = curies.find{|c| c['name'] == curie_namespace} - rel_attrs['docs'] = Relation.expand(curie['href'], rel: rel) - end - end - if rel != CURIES_REL - rel_attrs['name'] = rel - memo[rel.to_sym] = Relation.new(rel_attrs, client) - end - end - ) + private + attr_reader :top, :attrs + + def curies + @curies ||= top.links.fetch('curies', []) end - private + def client + return nil unless @client - attr_reader :client, :top, :attrs + @client.__getobj__ + rescue WeakRef::RefError + raise 'BooticClient: the client for this entity has been garbage collected. ' + + 'Hold a reference to your strategy/client for as long as you need to follow links.' + end def iterable? - has_entity?(:items) && entities[:items].respond_to?(:each) + entities.has?(:items) && entities.get(:items).is_a?(EntityArray) + end + + class EntityArray + include Enumerable + extend Forwardable + + def initialize(items, client, top) + @items = items + @client, @top = client, top + @cache = {} + end + + def_instance_delegators :@items, :count, :size, :length, :empty? + + def inspect + %(#<#{self.class.name} length: #{length}]>) + end + + # def first + # self[0] + # end + + def last + self[length-1] + end + + def [](index) + @cache[index] ||= Entity.wrap(@items[index], client: @client, top: @top) + end + + def get(index) + self[index] + end + + def each(&block) + return enum_for(:each) unless block_given? + length.times { |i| yield self[i] } + end end - def build! - @curies = top.links.fetch('curies', []) + class PropertySet + include Enumerable + + def initialize(attrs) + @attrs = stringify_keys(attrs || {}) + @cache = {} + end + + # overwrite Enumerable#count because some Entities have this prop + def count + get('count') or raise NoMethodError, "undefined method `count` for #{self.inspect}" + end + + def keys + @keys ||= @attrs.keys + end - @entities = attrs.fetch('_embedded', {}).each_with_object({}) do |(k,v),memo| - memo[k.to_sym] = if v.kind_of?(Array) - v.map{|ent_attrs| Entity.new(ent_attrs, client, top: top)} + def has?(key) + q = has_key?(key.to_s) || !!has_boolean?(key.to_s) + end + + def inspect + %(#<#{self.class.name} properties: [#{keys.join(', ')}]>) + end + + def to_hash + @attrs + end + + alias_method :to_h, :to_hash + + def as_json(opts = {}) + to_hash + end + + def dig(*keys) + @attrs.dig(*keys) + end + + def [](key) + get(key) + end + + def get(key) + if !has_key?(key.to_s) and found = has_boolean?(key.to_s) + key = found + end + + @cache[key.to_s] ||= wrap(@attrs[key.to_s]) + end + + def each(&block) + keys.each { |k| yield k, get(k) } + end + + private + + def wrap(value) + case value + when Hash + PropertySet.new(value) + when Array + value.map { |e| wrap(e) } else - Entity.new(v, client, top: top) + value + end + end + + def method_missing(name, *args, &block) + if has?(name.to_s) + get(name) + else + super + end + end + + def has_key?(key) + @attrs.has_key?(key) + end + + def has_boolean?(key) + if key[key.size-1] == '?' and key = key.chomp('?') + return key if is_boolean?(key) end end + + def is_boolean?(key) + @attrs[key].is_a?(TrueClass) || @attrs[key].is_a?(FalseClass) + end + + # def all + # keys.map { |k| get(key) } + # end + + def stringify_keys(hash) + hash.inject({}) { |memo,(k,v)| memo[k.to_s] = v; memo } + end end + + class EntitySet < PropertySet + def initialize(attrs, client, top) + super(attrs) + @client, @top = client, top + end + + def inspect + %(#<#{self.class.name} entities: [#{keys.join(', ')}]>) + end + + def get(key) + @cache[key.to_s] ||= Entity.wrap(@attrs[key.to_s], client: @client, top: @top) + end + end + + class RelationSet < EntitySet + def initialize(attrs, client, top, curies) + super(attrs, client, top) + @curies = curies + end + + def inspect + %(#<#{self.class.name} relations: [#{keys.join(', ')}]>) + end + + def has?(key) + super || @attrs.has_key?("#{CURIE_NS}:#{key}") + end + + def get(key) + return if key.to_s == CURIES_REL + + @cache[key.to_s] ||= begin + key = key.to_s + obj = @attrs[key] + + if obj.nil? and obj = @attrs["#{CURIE_NS}:#{key}"] + if curie = @curies.find { |c| c['name'] == CURIE_NS } + obj['docs'] = Relation.expand(curie['href'], rel: key) + end + end + + Relation.new(obj, @client) + end + end + + end + end end diff --git a/lib/bootic_client/errors.rb b/lib/bootic_client/errors.rb index 9bcc824..08fed0f 100644 --- a/lib/bootic_client/errors.rb +++ b/lib/bootic_client/errors.rb @@ -14,6 +14,7 @@ class NotFoundError < ServerError; end class AuthorizationError < ServerError; end class UnauthorizedError < AuthorizationError; end class AccessForbiddenError < AuthorizationError; end + class TooManyRequestsError < TransportError; end class ClientError < TransportError; end class InvalidURLError < ClientError; end end diff --git a/lib/bootic_client/relation.rb b/lib/bootic_client/relation.rb index d913f72..b1443e5 100644 --- a/lib/bootic_client/relation.rb +++ b/lib/bootic_client/relation.rb @@ -3,6 +3,7 @@ require "bootic_client/whiny_uri" require "bootic_client/entity" require 'ostruct' +require 'weakref' module BooticClient @@ -21,7 +22,8 @@ def complain_on_undeclared_params end def initialize(attrs, client, complain_on_undeclared_params: self.class.complain_on_undeclared_params) - @attrs, @client = attrs, client + @attrs = attrs + @client = client ? WeakRef.new(client) : nil @complain_on_undeclared_params = complain_on_undeclared_params end @@ -84,7 +86,16 @@ def self.expand(href, opts = {}) end protected - attr_reader :client, :attrs, :complain_on_undeclared_params + + attr_reader :attrs, :complain_on_undeclared_params + + def client + return nil unless @client + @client.__getobj__ + rescue WeakRef::RefError + raise "BooticClient: the client for this relation has been garbage collected. " \ + "Hold a reference to your strategy/client for as long as you need to follow links." + end def uri @uri ||= WhinyURI.new(href, complain_on_undeclared_params) diff --git a/lib/bootic_client/strategies/oauth2_strategy.rb b/lib/bootic_client/strategies/oauth2_strategy.rb index b21ca42..12b2203 100644 --- a/lib/bootic_client/strategies/oauth2_strategy.rb +++ b/lib/bootic_client/strategies/oauth2_strategy.rb @@ -32,21 +32,21 @@ def request_headers def retryable(&block) begin - yield + super rescue AuthorizationError => e update_token! - yield + super end end def update_token! new_token = get_token options[:access_token] = new_token - on_new_token.call new_token + on_new_token.call(new_token) if on_new_token end def get_token - raise "Implement this in subclasses" + raise 'Implement this in subclasses' end def auth diff --git a/lib/bootic_client/strategies/strategy.rb b/lib/bootic_client/strategies/strategy.rb index 920d241..62cbec4 100644 --- a/lib/bootic_client/strategies/strategy.rb +++ b/lib/bootic_client/strategies/strategy.rb @@ -7,7 +7,7 @@ class Strategy attr_reader :options def initialize(config, client_opts = {}, &on_new_token) - @config, @options, @on_new_token = config, client_opts, (on_new_token || Proc.new{}) + @config, @options, @on_new_token = config, client_opts, on_new_token raise ArgumentError, 'must include a Configuration object' unless config validate! end diff --git a/spec/bounded_memory_store_spec.rb b/spec/bounded_memory_store_spec.rb new file mode 100644 index 0000000..dea4316 --- /dev/null +++ b/spec/bounded_memory_store_spec.rb @@ -0,0 +1,91 @@ +require 'spec_helper' + +describe BooticClient::Client::BoundedMemoryStore do + subject(:store) { described_class.new(max_size: 3) } + + describe '#write and #read' do + it 'stores and retrieves values' do + store.write('a', 'alpha') + expect(store.read('a')).to eq 'alpha' + end + + it 'returns nil for missing keys' do + expect(store.read('missing')).to be_nil + end + end + + describe '#exist?' do + it 'returns true for a stored key' do + store.write('a', 'alpha') + expect(store.exist?('a')).to be true + end + + it 'returns false for an absent key' do + expect(store.exist?('nope')).to be false + end + end + + describe '#delete' do + it 'removes an entry' do + store.write('a', 'alpha') + store.delete('a') + expect(store.exist?('a')).to be false + end + end + + describe 'eviction when max_size is reached' do + it 'evicts the oldest entry when a new key is added beyond capacity' do + store.write('a', 'alpha') + store.write('b', 'beta') + store.write('c', 'gamma') + store.write('d', 'delta') + + expect(store.exist?('a')).to be false + expect(store.exist?('b')).to be true + expect(store.exist?('c')).to be true + expect(store.exist?('d')).to be true + end + + it 'evicts the second-oldest when the oldest slot has already been evicted' do + store.write('a', 'alpha') + store.write('b', 'beta') + store.write('c', 'gamma') + store.write('d', 'delta') # evicts 'a' + store.write('e', 'epsilon') # evicts 'b' + + expect(store.exist?('a')).to be false + expect(store.exist?('b')).to be false + expect(store.exist?('c')).to be true + expect(store.exist?('d')).to be true + expect(store.exist?('e')).to be true + end + + it 'does not evict when updating an existing key' do + store.write('a', 'alpha') + store.write('b', 'beta') + store.write('c', 'gamma') + store.write('a', 'updated') + + expect(store.exist?('a')).to be true + expect(store.exist?('b')).to be true + expect(store.exist?('c')).to be true + expect(store.read('a')).to eq 'updated' + end + + it 'never exceeds max_size' do + 100.times { |i| store.write("key#{i}", "val#{i}") } + count = (0...100).count { |i| store.exist?("key#{i}") } + expect(count).to eq 3 + end + end + + describe 'default max_size' do + it 'defaults to 500 entries' do + expect(described_class::DEFAULT_MAX_SIZE).to eq 500 + default_store = described_class.new + 501.times { |i| default_store.write("k#{i}", "v#{i}") } + count = (0...501).count { |i| default_store.exist?("k#{i}") } + expect(count).to eq 500 + end + end +end diff --git a/spec/client_spec.rb b/spec/client_spec.rb index 57797f7..3273e57 100644 --- a/spec/client_spec.rb +++ b/spec/client_spec.rb @@ -129,6 +129,35 @@ def assert_successful_response(response) end end + context 'timeouts' do + it 'triggers a new request' do + client = described_class.new(timeout: '60') + req = stub_request(:get, root_url) + .to_timeout + + expect do + client.get(root_url, {}, request_headers) + end.to raise_error(Faraday::TimeoutError) + + expect(req).to have_been_requested.times(3) + end + + it 'can be configured' do + expect(Faraday).to receive(:new).with(request: { + open_timeout: 20, + timeout: 60 + }).and_call_original + + client = described_class.new(timeout: '60') + + req = stub_request(:get, root_url) + .to_return(status: 200, body: JSON.dump(root_data), headers: response_headers) + + client.get(root_url, {}, request_headers) + expect(req).to have_been_requested + end + end + context 'errors' do describe '500 Server error' do before do @@ -205,10 +234,55 @@ def assert_successful_response(response) end end end + + describe '429 Too Many Requests' do + before do + stub_request(:get, root_url) + .to_return(status: 429, body: JSON.dump(message: 'Rate Limited'), headers: response_headers) + end + + it 'raises TooManyRequestsError' do + expect { + client.get(root_url) + }.to raise_error(BooticClient::TooManyRequestsError) + end + + it 'carries the request URL on the error' do + begin + client.get(root_url) + rescue BooticClient::TooManyRequestsError => e + expect(e.url).to eq("GET #{root_url}") + end + end + + it 'is not a subclass of ServerError' do + expect(BooticClient::TooManyRequestsError.ancestors).not_to include(BooticClient::ServerError) + end + end end end + describe '#close' do + before do + stub_request(:get, root_url) + .to_return(status: 200, body: JSON.dump(root_data), headers: response_headers) + end + + it 'resets the Faraday connection so it can be garbage collected' do + client.get(root_url) + conn_before = client.send(:conn) + client.close + conn_after = client.send(:conn) + expect(conn_before).not_to equal(conn_after) + end + + it 'allows making new requests after closing' do + client.close + expect { client.get(root_url) }.not_to raise_error + end + end + context 'HTTP verbs' do describe 'GET' do diff --git a/spec/configuration_spec.rb b/spec/configuration_spec.rb index f649e21..b206e3f 100644 --- a/spec/configuration_spec.rb +++ b/spec/configuration_spec.rb @@ -71,5 +71,23 @@ expect(config.response_handlers.to_a.first).not_to be_nil end end + + describe '#logger' do + it 'returns the same Logger instance on every call when no logger is configured' do + expect(config.logger).to equal(config.logger) + end + + it 'returns the configured logger when one has been set' do + custom = Logger.new(STDOUT) + config.logger = custom + expect(config.logger).to equal(custom) + end + + it 'does not allocate a new Logger on every call' do + first_call_id = config.logger.object_id + second_call_id = config.logger.object_id + expect(first_call_id).to eq(second_call_id) + end + end end end diff --git a/spec/entity_spec.rb b/spec/entity_spec.rb index 9229f51..5d34f29 100644 --- a/spec/entity_spec.rb +++ b/spec/entity_spec.rb @@ -75,11 +75,24 @@ end it 'wraps object properties as entities' do - expect(entity.an_object).to be_a described_class + expect(entity.an_object).to be_a BooticClient::Entity::PropertySet + expect(entity.an_object.name).to eql('Foobar') expect(entity.an_object.age).to eql(22) - expect(entity.an_object.another_object).to be_a described_class + expect(entity.an_object.another_object).to be_a BooticClient::Entity::PropertySet expect(entity.an_object.another_object.foo).to eq 'bar' + + expect(entity.an_object.dig('another_object', 'foo')).to eq('bar') + expect(entity.an_object.to_hash).to eq({"name"=>"Foobar", "age"=>22, "another_object"=>{"foo"=>"bar"}}) + expect(entity.an_object.to_h).to eq({"name"=>"Foobar", "age"=>22, "another_object"=>{"foo"=>"bar"}}) + end + + it 'allows enumerating over property sets' do + expect(entity.an_object.respond_to?(:each_with_object)).to eq(true) + values = entity.an_object.map { |key, val| val } + expect(values[0]).to eq("Foobar") + expect(values[1]).to eq(22) + expect(values[2]).to be_a(BooticClient::Entity::PropertySet) end it 'has a #properties object' do @@ -92,6 +105,18 @@ expect(entity.has?(:foobar)).to eql(false) end + it 'responds to #[]' do + expect(entity[:total_items]).to eql(10) + expect(entity[:items]).to be_a(BooticClient::Entity::EntityArray) + expect(entity[:foobar]).to eql(nil) + end + + it 'responds to #try (same behaviour as [])' do + expect(entity.try(:total_items)).to eql(10) + expect(entity.try(:items)).to be_a(BooticClient::Entity::EntityArray) + expect(entity.try(:foobar)).to eql(nil) + end + describe '#to_hash' do it 'returns original data' do expect(entity.to_hash).to eql(list_payload) @@ -100,13 +125,32 @@ describe 'embedded entities' do + it 'is a EntitySet' do + expect(entity.entities).to be_a(BooticClient::Entity::EntitySet) + expect(entity.entities.to_hash).to eq({ + "items"=> [{"title"=>"iPhone 4", "price"=>12345, "published"=>false, "_links"=>{"self"=>{:href=>"/products/iphone4"}, "btc:delete_product"=>{"href"=>"/products/12345"}}, "_embedded"=>{"shop"=>{"name"=>"Acme"}}}, {"title"=>"iPhone 5", "price"=>12342, "published"=>true, "_links"=>{"self"=>{:href=>"/products/iphone5"}}, "_embedded"=>{"shop"=>{"name"=>"Apple"}}}] + }) + end + it 'has a #entities object' do - expect(entity.entities[:items]).to be_a(Array) + expect(entity.entities[:items]).to be_a(BooticClient::Entity::EntityArray) expect(entity.entities[:items].first.entities[:shop]).to be_kind_of(BooticClient::Entity) end + it '#items is a enumerable object' do + items = entity.entities[:items] + expect(items).to be_a(Enumerable) + expect(items.to_a).to be_a(Array) + expect(items.any?).to eq(true) + + res = items.any? do |item| + expect(item).to be_a(BooticClient::Entity) + item.title == 'iPhone 4' + end + expect(res).to be(true) + end + it 'are exposed like normal attributes' do - expect(entity.items).to be_kind_of(Array) entity.items.first.tap do |product| expect(product).to be_kind_of(BooticClient::Entity) expect(product.title).to eql('iPhone 4') @@ -125,9 +169,17 @@ end end - it 'includes FALSE values' do + it 'includes FALSE values, and allows querying with ?' do expect(entity.items.first.published).to be false + expect(entity.items.first.published?).to be false + expect(entity.items.last.published).to be true + expect(entity.items.last.published?).to be true + + expect(entity.items.first.has?(:published?)).to be true + expect(entity.items.first.has?(:published)).to be true + expect(entity.items.last.has?(:published)).to be true + expect(entity.items.last.has?(:published?)).to be true end end #/ embedded entities @@ -189,18 +241,24 @@ end describe 'iterating' do - it 'is an enumerable if it is a list' do + it 'is an enumerable if it contains embedded items' do prods = [] - entity.each{|pr| prods << pr} - expect(prods).to match_array(entity.items) - expect(entity.map{|pr| pr}).to match_array(entity.items) + entity.each { |pr| prods << pr } + expect(prods[0]).to eq(entity.items[0]) + expect(prods[1]).to eq(entity.items[1]) + + mapped = entity.map{|pr| pr} + expect(mapped[0]).to eq(entity.items[0]) + expect(mapped[1]).to eq(entity.items[1]) + expect(entity.reduce(0){|sum,e| sum + e.price.to_i}).to eql(24687) expect(entity.each).to be_kind_of(Enumerator) end - it 'is not treated as an array if not a list' do + it 'does not respond to each if no embedded items' do ent = BooticClient::Entity.new({'foo' => 'bar'}, client) expect(ent).not_to respond_to(:each) + # expect { ent.each { |i| } }.to raise_error(NoMethodError) end end @@ -275,4 +333,46 @@ end end + context 'memory leak prevention' do + let(:linked_payload) do + { + 'title' => 'Root', + '_links' => {'next' => {'href' => '/page/2'}}, + '_embedded' => {'items' => [{'title' => 'Child'}]} + } + end + + it 'stores client as a WeakRef so entity graphs do not pin the strategy in memory' do + entity = described_class.new(linked_payload, client) + expect(entity.instance_variable_get(:@client)).to be_a(WeakRef) + end + + it 'also wraps client as a WeakRef in embedded child entities' do + entity = described_class.new(linked_payload, client) + child = entity.entities[:items].first + expect(child.instance_variable_get(:@client)).to be_a(WeakRef) + end + + it 'raises a descriptive error when the client has been garbage collected and a link is followed' do + entity = described_class.new(linked_payload, Object.new) + dead_ref = entity.instance_variable_get(:@client) + allow(dead_ref).to receive(:__getobj__).and_raise(WeakRef::RefError) + expect { entity.next }.to raise_error(RuntimeError, /garbage collected/) + end + + it 'still exposes plain properties after the client is gone, because they are memoised at build time' do + entity = described_class.new(linked_payload, Object.new) + # warm the properties cache while the client is alive + _ = entity.properties + dead_ref = entity.instance_variable_get(:@client) + allow(dead_ref).to receive(:__getobj__).and_raise(WeakRef::RefError) + # reading already-memoised properties must not need the client + expect(entity.title).to eq 'Root' + end + + it 'accepts a nil client without error' do + expect { described_class.new(linked_payload, nil) }.not_to raise_error + end + end + end diff --git a/spec/relation_spec.rb b/spec/relation_spec.rb index fcf2b42..708f29c 100644 --- a/spec/relation_spec.rb +++ b/spec/relation_spec.rb @@ -125,4 +125,22 @@ end end end + + context 'memory leak prevention' do + it 'stores the client as a WeakRef so relations do not pin the strategy in memory' do + relation = described_class.new({'href' => '/foo'}, client) + expect(relation.instance_variable_get(:@client)).to be_a(WeakRef) + end + + it 'raises a descriptive error when the client has been garbage collected' do + relation = described_class.new({'href' => '/foo'}, Object.new) + dead_ref = relation.instance_variable_get(:@client) + allow(dead_ref).to receive(:__getobj__).and_raise(WeakRef::RefError) + expect { relation.run }.to raise_error(RuntimeError, /garbage collected/) + end + + it 'accepts a nil client without error' do + expect { described_class.new({'href' => '/foo'}, nil) }.not_to raise_error + end + end end