Subscription id handling on NACK/ACKs for STOMP 1.1 clients.

[PaulGale]

The Client class has broken implementations for these methods under STOMP 1.1. Correct implementation is provided below (with the addition of the helper method).

The gem will create and manage a subscription id header for you on subscribing when using STOMP 1.1, if you do not explicitly provide one. The id header is mandatory under STOMP 1.1.

However, Client#acknowledge() lacks handling for STOMP 1.1. It only caters for 1.0 and 1.2. Given that under STOMP 1.1 the 'msg' being ACK'ed must have an id header already, use it for the ACK! This negates the need for the caller to supply the id via the headers hash argument.

You can leave the method name acknowledge in place if you wish but the name choice is totally dumb and asymmetric with nack. Therefore introduce the ack method and deprecate acknowledge which can delegate to ack.

The signature of nack should change. It should take a message not a message_id. Please, no BS arguments about 'breaking existing' users; they can cope with such a minor change and that's not what 'breaking' an API even means.

    def ack(message, headers = {})
      txn_id = headers[:transaction]

      if txn_id
        replay_list = @replay_messages_by_txn[txn_id]

        if replay_list.nil?
          replay_list = []
          @replay_messages_by_txn[txn_id] = replay_list
        end

        replay_list << message
      end

      if block_given?
        headers['receipt'] = register_receipt_listener(lambda { |r| yield r })
      end

      context = ack_context_for message, headers
      @connection.ack context[:message_id], context[:headers]
    end

    def nack(message, headers = {})
      context = ack_context_for message, headers
      @connection.nack context[:message_id], context[:headers]
    end

    def ack_context_for(message, headers)
      id = case protocol
             when Stomp::SPL_12
               'ack'
             when Stomp::SPL_11
               headers.merge!(:subscription => message.headers['subscription'])
               'message-id'
             else
               'message-id'
           end

      {:message_id => message.headers[id], :headers => headers}
    end

[ismith]

Please submit this as a pull request, not a blockquote of code.

[PaulGale]

Gimme a break. You have everything you need.

[PaulGale]

This still seems broken for nack, no? It doesn't appear to be pre-populating the appropriate headers the way that ack is. The original implementation of nack is broken in that regard.

[gmallard]

Status: Closed Ref: 70180dd Gem Version: 1.3.2

If the issue persists please reopen this issue or create a new issue.