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

Adds cached_client method in BaseConnection #371

Merged
merged 1 commit into from
Nov 1, 2018
Merged

Adds cached_client method in BaseConnection #371

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
19 changes: 19 additions & 0 deletions lib/train/plugins/base_connection.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,25 @@ def initialize(options = nil)
end
end

# Returns cached client if caching enabled. Otherwise returns whatever is
# given in the block.
#
# @example
#
# def demo_client
# cached_client(:api_call, :demo_connection) do
# DemoClient.new(args)
# end
# end
#
# @param [symbol] type one of [:api_call, :file, :command]
# @param [symbol] key for your cached connection
def cached_client(type, key)
return yield unless cache_enabled?(type)

@cache[type][key] ||= yield
Copy link

Choose a reason for hiding this comment

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

@cache.dig(type, key) ||= yield?

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 wasn't sure if you could do assignment when using dig.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That doesn't work. You aren't allowed to do assignment from the return of dig.

train/lib/train/plugins/base_connection.rb:56: syntax error, unexpected tOP_ASGN,
expecting keyword_end (SyntaxError)
      @cache.dig(type, key) ||= yield

Copy link

Choose a reason for hiding this comment

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

Yeah, you're right. I wasn't thinking about the assignment. That's a shame--it should work that way!

end

def cache_enabled?(type)
@cache_enabled[type.to_sym]
end
Expand Down
48 changes: 48 additions & 0 deletions test/unit/plugins/connection_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,54 @@
.method(:logger).call.must_equal(l)
end

describe 'cached_client helper' do
class DemoConnection < Train::Plugins::Transport::BaseConnection
def initialize(options = {})
super(options)
@cache_enabled[:api_call] = true
@cache[:api_call] = {}
end

def demo_client
cached_client(:api_call, :demo_client) do
DemoClient.new
end
end

class DemoClient
end
end

it 'returns a new connection when cached disabled' do
conn = DemoConnection.new
conn.disable_cache(:api_call)

client1 = conn.demo_client
client2 = conn.demo_client

client1.wont_be_same_as client2
end

it 'returns a new connection when cache enabled and not hydrated' do
conn = DemoConnection.new
conn.enable_cache(:api_call)

client1 = conn.demo_client
Copy link

@TrevorBramble TrevorBramble Oct 22, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

It wouldn't be useful in the other tests, because they don't look anything like real world code, but this one made me wonder if we could just return self from enable_cache() and disable_cache(). I quick search shows that the implicit return of a boolean is never treated as significant.

def enable_cache(type)
  fail Train::UnknownCacheType, "#{type} is not a valid cache type" unless @cache_enabled.keys.include?(type.to_sym)

  @cache_enabled[type.to_sym] = true

  self
end
it 'returns a new connection when cache enabled and not hydrated' do
  conn = DemoConnection.new.enable_cache(:api_call)

  client1 = conn.demo_client
  client1.must_be_instance_of DemoConnection::DemoClient
end

# or even...

it 'returns a new connection when cache enabled and not hydrated' do
  client1 = DemoConnection.new.enable_cache(:api_call).demo_client

  client1.must_be_instance_of DemoConnection::DemoClient
end


client1.must_be_instance_of DemoConnection::DemoClient
end

it 'returns a cached connection when cache enabled and hydrated' do
conn = DemoConnection.new
conn.enable_cache(:api_call)

client1 = conn.demo_client
client2 = conn.demo_client

client1.must_be_same_as client2
end
end

describe 'create cache connection' do
it 'default connection cache settings' do
connection.cache_enabled?(:file).must_equal true
Expand Down