mrstampy / Stampy

Java implementation of the STOMP 1.2 specification
67 stars 22 forks source link

Possible incorrect message ID being added to message list for acknolwedgement tracking #17

Open rh101 opened 4 years ago

rh101 commented 4 years ago

In AbstractAcknowledgementListenerAndInterceptor.java, interceptMessage, there is this line:

String ack = msg.getHeader().getAck();

That getAck() value is added to the queue as the identifier of the message, but shouldn't it be getId() instead? getAck returns auto, client or client-individual.

Here's the current implementation:

  public void interceptMessage(StampyMessage<?> message, HostPort hostPort) throws InterceptException {
    MessageMessage msg = (MessageMessage) message;

    String ack = msg.getHeader().getAck();

    Queue<String> queue = messages.get(hostPort);
    if (queue == null) {
      queue = new ConcurrentLinkedQueue<String>();
      messages.put(hostPort, queue);
    }

    queue.add(ack);
    startTimerTask(hostPort, ack);
  }

Now, when a method like evaluateAck uses the id header to look up the message in the queue, yet it will never find the message:

  private void evaluateAck(AckHeader header, HostPort hostPort) throws Exception {
    String id = header.getId();
    if (hasMessageAck(id, hostPort)) {
      clearMessageAck(id, hostPort);
      getHandler().ackReceived(id, header.getReceipt(), header.getTransaction());
    } else {
      throw new UnexpectedAcknowledgementException("No ACK message expected, yet received id " + id + " from "
          + hostPort);
    }
  }

  private boolean hasMessageAck(String messageId, HostPort hostPort) {
    Queue<String> ids = messages.get(hostPort);
    if (ids == null || ids.isEmpty()) return false;

    return ids.contains(messageId);
  }