From 70cb1f9696415fc7a299958ad661c6d165eefe46 Mon Sep 17 00:00:00 2001 From: Matt Todd Date: Sun, 19 Oct 2014 03:00:11 -0700 Subject: [PATCH 1/7] Accept message_id as param to write --- lib/net/ldap/connection.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/net/ldap/connection.rb b/lib/net/ldap/connection.rb index f0e5519d..b82f18c6 100644 --- a/lib/net/ldap/connection.rb +++ b/lib/net/ldap/connection.rb @@ -146,9 +146,9 @@ def read(syntax = Net::LDAP::AsnSyntax) # # Returns the return value from writing to the connection, which in some # cases is the Integer number of bytes written to the socket. - def write(request, controls = nil) + def write(request, controls = nil, message_id = next_msgid) instrument "write.net_ldap_connection" do |payload| - packet = [next_msgid.to_ber, request, controls].compact.to_ber_sequence + packet = [message_id.to_ber, request, controls].compact.to_ber_sequence payload[:content_length] = @conn.write(packet) end end From 8f4745d4a451cc45d2762fba5268332f2e8e4a0b Mon Sep 17 00:00:00 2001 From: Matt Todd Date: Sun, 19 Oct 2014 03:01:03 -0700 Subject: [PATCH 2/7] Pull queued messages first, queue messages unless ID matches --- lib/net/ldap/connection.rb | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/lib/net/ldap/connection.rb b/lib/net/ldap/connection.rb index b82f18c6..96166a64 100644 --- a/lib/net/ldap/connection.rb +++ b/lib/net/ldap/connection.rb @@ -356,6 +356,10 @@ def search(args = {}) result_pdu = nil n_results = 0 + @queue ||= {} + message_id = next_msgid + @queue[message_id] ||= [] + instrument "search.net_ldap_connection", :filter => search_filter, :base => search_base, @@ -403,12 +407,17 @@ def search(args = {}) controls << sort_control if sort_control controls = controls.empty? ? nil : controls.to_ber_contextspecific(0) - write(request, controls) + write(request, controls, message_id) result_pdu = nil controls = [] - while pdu = read + while pdu = (@queue[message_id].shift || read) + if pdu.message_id != message_id + @queue[pdu.message_id].push pdu + next + end + case pdu.app_tag when Net::LDAP::PDU::SearchReturnedData n_results += 1 From 91618d679d6c644e49c7e62aa7490788ee6d0ce4 Mon Sep 17 00:00:00 2001 From: Matt Todd Date: Sun, 19 Oct 2014 03:01:24 -0700 Subject: [PATCH 3/7] Include message ID in event payload --- lib/net/ldap/connection.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/net/ldap/connection.rb b/lib/net/ldap/connection.rb index 96166a64..c11f4c3a 100644 --- a/lib/net/ldap/connection.rb +++ b/lib/net/ldap/connection.rb @@ -361,6 +361,7 @@ def search(args = {}) @queue[message_id] ||= [] instrument "search.net_ldap_connection", + :message_id => message_id, :filter => search_filter, :base => search_base, :scope => scope, From 9c4b45b59e9ea55842de215d8f7e7c28d417a662 Mon Sep 17 00:00:00 2001 From: Matt Todd Date: Sun, 19 Oct 2014 03:12:02 -0700 Subject: [PATCH 4/7] Fix message_id in test --- test/test_ldap_connection.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/test_ldap_connection.rb b/test/test_ldap_connection.rb index 0c3c5f34..7b489e1f 100644 --- a/test/test_ldap_connection.rb +++ b/test/test_ldap_connection.rb @@ -185,16 +185,16 @@ def test_bind_net_ldap_connection_event def test_search_net_ldap_connection_event # search data - search_data_ber = Net::BER::BerIdentifiedArray.new([2, [ + search_data_ber = Net::BER::BerIdentifiedArray.new([1, [ "uid=user1,ou=OrgUnit2,ou=OrgUnitTop,dc=openldap,dc=ghe,dc=local", [ ["uid", ["user1"]] ] ]]) search_data_ber.ber_identifier = Net::LDAP::PDU::SearchReturnedData - search_data = [2, search_data_ber] + search_data = [1, search_data_ber] # search result (end of results) search_result_ber = Net::BER::BerIdentifiedArray.new([0, "", ""]) search_result_ber.ber_identifier = Net::LDAP::PDU::SearchResult - search_result = [2, search_result_ber] + search_result = [1, search_result_ber] @tcp_socket.should_receive(:read_ber).and_return(search_data). and_return(search_result) From acd676e3568a04676e0bf208188a6457070d7bb2 Mon Sep 17 00:00:00 2001 From: Matt Todd Date: Sun, 19 Oct 2014 03:27:18 -0700 Subject: [PATCH 5/7] Extract queued_read(message_id) method --- lib/net/ldap/connection.rb | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/lib/net/ldap/connection.rb b/lib/net/ldap/connection.rb index c11f4c3a..72c60578 100644 --- a/lib/net/ldap/connection.rb +++ b/lib/net/ldap/connection.rb @@ -111,6 +111,30 @@ def close @conn = nil 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 + # message ID. + # + # Returns a Net::LDAP::PDU object or nil. + def queued_read(message_id) + if pdu = (@queue[message_id] || []).shift + return pdu + end + + while pdu = read + if pdu.message_id == message_id + return pdu + else + @queue[pdu.message_id].push pdu + + next + end + end + + pdu + end + # Internal: Reads and parses data from the configured connection. # # - syntax: the BER syntax to use to parse the read data with @@ -413,12 +437,7 @@ def search(args = {}) result_pdu = nil controls = [] - while pdu = (@queue[message_id].shift || read) - if pdu.message_id != message_id - @queue[pdu.message_id].push pdu - next - end - + while pdu = queued_read(message_id) case pdu.app_tag when Net::LDAP::PDU::SearchReturnedData n_results += 1 From 6cbb814db63874912858d309275be23e0a32a52f Mon Sep 17 00:00:00 2001 From: Matt Todd Date: Sun, 19 Oct 2014 15:42:03 -0700 Subject: [PATCH 6/7] Extract message_queue accessor, documentation, cleanup --- lib/net/ldap/connection.rb | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/lib/net/ldap/connection.rb b/lib/net/ldap/connection.rb index 72c60578..758fe31c 100644 --- a/lib/net/ldap/connection.rb +++ b/lib/net/ldap/connection.rb @@ -118,16 +118,16 @@ def close # # Returns a Net::LDAP::PDU object or nil. def queued_read(message_id) - if pdu = (@queue[message_id] || []).shift + if pdu = message_queue[message_id].shift return pdu end + # read messages until we have a match for the given message_id while pdu = read if pdu.message_id == message_id return pdu else - @queue[pdu.message_id].push pdu - + message_queue[pdu.message_id].push pdu next end end @@ -135,6 +135,21 @@ def queued_read(message_id) pdu end + # Internal: The internal queue of messages, read from the socket, grouped by + # message ID. + # + # Used by `queued_read` to return messages sent by the server with the given + # ID. If no messages are queued for that ID, `queued_read` will `read` from + # the socket and queue messages that don't match the given ID for other + # readers. + # + # Returns the message queue Hash. + def message_queue + @message_queue ||= Hash.new do |hash, key| + hash[key] = [] + end + end + # Internal: Reads and parses data from the configured connection. # # - syntax: the BER syntax to use to parse the read data with @@ -380,9 +395,7 @@ def search(args = {}) result_pdu = nil n_results = 0 - @queue ||= {} message_id = next_msgid - @queue[message_id] ||= [] instrument "search.net_ldap_connection", :message_id => message_id, From 1e5916980f563f22c6af793fc9ec9f0882ce2c46 Mon Sep 17 00:00:00 2001 From: Matt Todd Date: Sun, 19 Oct 2014 15:49:16 -0700 Subject: [PATCH 7/7] Clear search's message queue, instrument unread messages Shouldn't have unread messages but provide a way to measure it anyway. --- lib/net/ldap/connection.rb | 8 ++++++++ test/test_ldap_connection.rb | 4 ++++ 2 files changed, 12 insertions(+) diff --git a/lib/net/ldap/connection.rb b/lib/net/ldap/connection.rb index 758fe31c..ac47e6e0 100644 --- a/lib/net/ldap/connection.rb +++ b/lib/net/ldap/connection.rb @@ -518,6 +518,14 @@ def search(args = {}) result_pdu || OpenStruct.new(:status => :failure, :result_code => 1, :message => "Invalid search") end # instrument + ensure + # clean up message queue for this search + messages = message_queue.delete(message_id) + + unless messages.empty? + instrument "search_messages_unread.net_ldap_connection", + message_id: message_id, messages: messages + end end MODIFY_OPERATIONS = { #:nodoc: diff --git a/test/test_ldap_connection.rb b/test/test_ldap_connection.rb index 7b489e1f..7ed75113 100644 --- a/test/test_ldap_connection.rb +++ b/test/test_ldap_connection.rb @@ -199,6 +199,7 @@ def test_search_net_ldap_connection_event and_return(search_result) events = @service.subscribe "search.net_ldap_connection" + unread = @service.subscribe "search_messages_unread.net_ldap_connection" result = @connection.search(filter: "(uid=user1)") assert result.success?, "should be success" @@ -209,5 +210,8 @@ def test_search_net_ldap_connection_event assert payload.has_key?(:filter) assert_equal "(uid=user1)", payload[:filter].to_s assert result + + # ensure no unread + assert unread.empty?, "should not have any leftover unread messages" end end