Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 5 additions & 6 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# This configuration was generated by
# `rubocop --auto-gen-config`
# on 2020-07-12 00:41:11 -0400 using RuboCop version 0.49.1.
# on 2020-07-22 01:34:13 -0400 using RuboCop version 0.49.1.
# The point is for the user to remove these configuration records
# one by one as the offenses are removed from the code base.
# Note that changes in the inspected code, or installation of new
Expand Down Expand Up @@ -196,13 +196,12 @@ Lint/UselessAccessModifier:
Exclude:
- 'lib/net/ldap/connection.rb'

# Offense count: 6
# Offense count: 4
Lint/UselessAssignment:
Exclude:
- 'test/integration/test_add.rb'
- 'test/test_ldap_connection.rb'
- 'test/test_search.rb'
- 'test/test_snmp.rb'

# Offense count: 48
Metrics/AbcSize:
Expand All @@ -221,13 +220,13 @@ Metrics/BlockNesting:
# Offense count: 11
# Configuration parameters: CountComments.
Metrics/ClassLength:
Max: 429
Max: 437

# Offense count: 23
Metrics/CyclomaticComplexity:
Max: 41

# Offense count: 216
# Offense count: 219
# Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns.
# URISchemes: http, https
Metrics/LineLength:
Expand Down Expand Up @@ -648,7 +647,7 @@ Style/SpecialGlobalVars:
- 'net-ldap.gemspec'
- 'testserver/ldapserver.rb'

# Offense count: 656
# Offense count: 658
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle, SupportedStyles, ConsistentQuotesInMultiline.
# SupportedStyles: single_quotes, double_quotes
Expand Down
12 changes: 11 additions & 1 deletion lib/net/ldap.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ class LDAP
require_relative 'ldap/auth_adapter/simple'
require_relative 'ldap/auth_adapter/sasl'

require 'connection_pool'

Net::LDAP::AuthAdapter.register([:simple, :anon, :anonymous], Net::LDAP::AuthAdapter::Simple)
Net::LDAP::AuthAdapter.register(:sasl, Net::LDAP::AuthAdapter::Sasl)

Expand Down Expand Up @@ -553,6 +555,10 @@ def initialize(args = {})
@force_no_page = args[:force_no_page] || DefaultForceNoPage
@encryption = normalize_encryption(args[:encryption]) # may be nil
@connect_timeout = args[:connect_timeout]
if args[:connection_pool].is_a? Hash
options = { size: 5, timeout: (@connection_timeout || 5) }.merge args[:connection_pool]
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure if we should default the timeout here in this way. The time to wait for a connection to free up is not quite the same as waiting for a single connection to be made. Perhaps we should default so some higher value here like 30 seconds.

@connection_pool = ConnectionPool.new(options) { new_connection }
end

if pr = @auth[:password] and pr.respond_to?(:call)
@auth[:password] = pr.call
Expand Down Expand Up @@ -1293,7 +1299,11 @@ def connection=(connection)
# result from that, and :use_connection: will not yield at all. If not
# the return value is whatever is returned from the block.
def use_connection(args)
if @open_connection
if @connection_pool
@connection_pool.with do |conn|
yield conn
end
elsif @open_connection
yield @open_connection
else
begin
Expand Down
9 changes: 9 additions & 0 deletions lib/net/ldap/connection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ def initialize(server = {})
# Allows tests to parameterize what socket class to use
@socket_class = server.fetch(:socket_class, DefaultSocket)

ObjectSpace.define_finalizer self, finalize

yield self if block_given?
end

Expand Down Expand Up @@ -186,6 +188,13 @@ def close
@conn = nil
end

# Ensure the connection closes when this instance is destroyed.
def finalize
proc do |_obj_id|
close
end
end

# Internal: Reads messages by ID from a queue, falling back to reading from
# the connected socket until a message matching the ID is read. Any messages
# with mismatched IDs gets queued for subsequent reads by the origin of that
Expand Down
2 changes: 2 additions & 0 deletions net-ldap.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ the most recent LDAP RFCs (4510-4519, plutions of 4520-4532).}
s.required_ruby_version = ">= 2.0.0"
s.summary = %q{Net::LDAP for Ruby (also called net-ldap) implements client access for the Lightweight Directory Access Protocol (LDAP), an IETF standard protocol for accessing distributed directory services}

s.add_runtime_dependency("connection_pool", "~> 2.2")
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure if we want to make this a requirement or allow users to opt in by adding the gem in their Gemfile. If we do that, then we have to handle when the library is missing.


s.add_development_dependency("flexmock", "~> 1.3")
s.add_development_dependency("rake", "~> 12.3.3")
s.add_development_dependency("rubocop", "~> 0.49.0")
Expand Down