Skip to content

refactor connection establishment #180

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.

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

Merged
merged 1 commit into from
Dec 24, 2014
Merged
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
1 change: 1 addition & 0 deletions Contributors.rdoc
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,4 @@ Contributions since:
* Erik Hetzner (egh)
* nowhereman
* David J. Lee (DavidJLee)
* Cody Cutrer (ccutrer)
138 changes: 44 additions & 94 deletions lib/net/ldap.rb
Original file line number Diff line number Diff line change
Expand Up @@ -670,12 +670,7 @@ def open

instrument "open.net_ldap" do |payload|
begin
@open_connection =
Net::LDAP::Connection.new \
:host => @host,
:port => @port,
:encryption => @encryption,
:instrumentation_service => @instrumentation_service
@open_connection = new_connection
payload[:connection] = @open_connection
payload[:bind] = @open_connection.bind(@auth)
yield self
Expand Down Expand Up @@ -745,27 +740,11 @@ def search(args = {})
result_set = return_result_set ? [] : nil

instrument "search.net_ldap", args do |payload|
if @open_connection
@result = @open_connection.search(args) { |entry|
@result = use_connection(args) do |conn|
conn.search(args) { |entry|
result_set << entry if result_set
yield entry if block_given?
}
else
begin
conn = Net::LDAP::Connection.new \
:host => @host,
:port => @port,
:encryption => @encryption,
:instrumentation_service => @instrumentation_service
if (@result = conn.bind(args[:auth] || @auth)).result_code == Net::LDAP::ResultCodeSuccess
@result = conn.search(args) { |entry|
result_set << entry if result_set
yield entry if block_given?
}
end
ensure
conn.close if conn
end
end

if return_result_set
Expand Down Expand Up @@ -844,11 +823,7 @@ def bind(auth = @auth)
payload[:bind] = @result = @open_connection.bind(auth)
else
begin
conn = Connection.new \
:host => @host,
:port => @port,
:encryption => @encryption,
:instrumentation_service => @instrumentation_service
conn = new_connection
payload[:connection] = conn
payload[:bind] = @result = conn.bind(auth)
ensure
Expand Down Expand Up @@ -946,22 +921,8 @@ def bind_as(args = {})
# end
def add(args)
instrument "add.net_ldap", args do |payload|
if @open_connection
@result = @open_connection.add(args)
else
@result = 0
begin
conn = Connection.new \
:host => @host,
:port => @port,
:encryption => @encryption,
:instrumentation_service => @instrumentation_service
if (@result = conn.bind(args[:auth] || @auth)).result_code == Net::LDAP::ResultCodeSuccess
@result = conn.add(args)
end
ensure
conn.close if conn
end
@result = use_connection(args) do |conn|
conn.add(args)
end
@result.success?
end
Expand Down Expand Up @@ -1050,24 +1011,9 @@ def add(args)
# does _not_ imply transactional atomicity, which LDAP does not provide.
def modify(args)
instrument "modify.net_ldap", args do |payload|
if @open_connection
@result = @open_connection.modify(args)
else
@result = 0
begin
conn = Connection.new \
:host => @host,
:port => @port,
:encryption => @encryption,
:instrumentation_service => @instrumentation_service
if (@result = conn.bind(args[:auth] || @auth)).result_code == Net::LDAP::ResultCodeSuccess
@result = conn.modify(args)
end
ensure
conn.close if conn
end
@result = use_connection(args) do |conn|
conn.modify(args)
end

@result.success?
end
end
Expand Down Expand Up @@ -1127,22 +1073,8 @@ def delete_attribute(dn, attribute)
# _Documentation_ _stub_
def rename(args)
instrument "rename.net_ldap", args do |payload|
if @open_connection
@result = @open_connection.rename(args)
else
@result = 0
begin
conn = Connection.new \
:host => @host,
:port => @port,
:encryption => @encryption,
:instrumentation_service => @instrumentation_service
if (@result = conn.bind(args[:auth] || @auth)).result_code == Net::LDAP::ResultCodeSuccess
@result = conn.rename(args)
end
ensure
conn.close if conn
end
@result = use_connection(args) do |conn|
conn.rename(args)
end
@result.success?
end
Expand All @@ -1160,22 +1092,8 @@ def rename(args)
# ldap.delete :dn => dn
def delete(args)
instrument "delete.net_ldap", args do |payload|
if @open_connection
@result = @open_connection.delete(args)
else
@result = 0
begin
conn = Connection.new \
:host => @host,
:port => @port,
:encryption => @encryption,
:instrumentation_service => @instrumentation_service
if (@result = conn.bind(args[:auth] || @auth)).result_code == Net::LDAP::ResultCodeSuccess
@result = conn.delete(args)
end
ensure
conn.close
end
@result = use_connection(args) do |conn|
conn.delete(args)
end
@result.success?
end
Expand Down Expand Up @@ -1277,4 +1195,36 @@ def paged_searches_supported?
@server_caps ||= search_root_dse
@server_caps[:supportedcontrol].include?(Net::LDAP::LDAPControls::PAGED_RESULTS)
end

private

# Yields an open connection if there is one, otherwise establishes a new
# connection, binds, and yields it. If binding fails, it will return the
# result from that, and :use_connection: will not yield at all. If not
# the return value is whatever is returned from the block.
Copy link
Member

Choose a reason for hiding this comment

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

Was thinking it'd be good to document that the result of the block is passed through, since that's relevant to the @result = assignment we do in the caller. Not a blocker but would be good to communicate that.

Copy link
Member

Choose a reason for hiding this comment

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

Derp, didn't read well enough, that's exactly what's documented here.

def use_connection(args)
if @open_connection
yield @open_connection
else
begin
conn = new_connection
if (result = conn.bind(args[:auth] || @auth)).result_code == Net::LDAP::ResultCodeSuccess
yield conn
else
return result
end
ensure
conn.close if conn
end
Copy link
Member

Choose a reason for hiding this comment

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

This method feels very similar to Net::LDAP#open.

end
end

# Establish a new connection to the LDAP server
def new_connection
Copy link
Member

Choose a reason for hiding this comment

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

Needs minimal documentation.

Net::LDAP::Connection.new \
:host => @host,
:port => @port,
:encryption => @encryption,
:instrumentation_service => @instrumentation_service
end
end # class LDAP