-
Notifications
You must be signed in to change notification settings - Fork 28
Add hasResponded check to Message #84
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,4 +22,8 @@ public interface Message { | |
|
|
||
| void forceFlush(); | ||
|
|
||
| default boolean hasResponded() { | ||
| return false; | ||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| package com.sproutsocial.nsq; | ||
|
|
||
| import java.io.IOException; | ||
| import java.util.concurrent.atomic.AtomicBoolean; | ||
|
|
||
| class NSQMessage implements Message { | ||
|
|
||
|
|
@@ -10,6 +11,7 @@ class NSQMessage implements Message { | |
| private final byte[] data; | ||
| private final String topic; | ||
| private final SubConnection connection; | ||
| private final AtomicBoolean responded = new AtomicBoolean(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if we want more granular information about the final state of the message like was it finished, requeued, etc.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @chrisparrinello this is mostly for bringing our library at-par (with respect to this feature) with other ones (go/python) |
||
|
|
||
| NSQMessage(long timestamp, int attempts, String id, byte[] data, String topic, SubConnection connection) { | ||
| this.timestamp = timestamp; | ||
|
|
@@ -47,21 +49,28 @@ public String getTopic() { | |
|
|
||
| @Override | ||
| public void finish() { | ||
| connection.finish(id); | ||
| if (responded.compareAndSet(false, true)) { | ||
| connection.finish(id); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public void requeue() { | ||
| connection.requeue(id); | ||
| requeue(0); | ||
| } | ||
|
|
||
| @Override | ||
| public void requeue(int delayMillis) { | ||
| connection.requeue(id, delayMillis); | ||
| if (responded.compareAndSet(false, true)) { | ||
| connection.requeue(id, delayMillis); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public void touch() { | ||
| if (responded.get()) { | ||
| return; | ||
| } | ||
| connection.touch(id); | ||
| } | ||
|
|
||
|
|
@@ -74,6 +83,11 @@ public void forceFlush() { | |
| } | ||
| } | ||
|
|
||
| @Override | ||
| public boolean hasResponded() { | ||
| return responded.get(); | ||
| } | ||
|
|
||
| SubConnection getConnection() { | ||
| return connection; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| package com.sproutsocial.nsq; | ||
|
|
||
| import org.junit.Before; | ||
| import org.junit.Test; | ||
| import org.mockito.Mockito; | ||
|
|
||
| import static org.junit.Assert.assertFalse; | ||
| import static org.junit.Assert.assertTrue; | ||
|
|
||
| public class NSQMessageTest { | ||
|
|
||
| private final SubConnection connection = Mockito.mock(SubConnection.class); | ||
|
|
||
| @Before | ||
| public void resetTest() { | ||
| Mockito.reset(connection); | ||
| } | ||
|
|
||
| @Test | ||
| public void shouldMarkRespondedOnFinish() { | ||
| final NSQMessage message = new NSQMessage(0L, 0, "id", null, "topic", connection); | ||
| assertFalse(message.hasResponded()); | ||
| message.finish(); | ||
| assertTrue(message.hasResponded()); | ||
| Mockito.verify(connection).finish("id"); | ||
| Mockito.verifyNoMoreInteractions(connection); | ||
| } | ||
|
|
||
| @Test | ||
| public void shouldMarkRespondedOnRequeue() { | ||
| final NSQMessage message = new NSQMessage(0L, 0, "id", null, "topic", connection); | ||
| assertFalse(message.hasResponded()); | ||
| message.requeue(); | ||
| assertTrue(message.hasResponded()); | ||
| Mockito.verify(connection).requeue("id", 0); | ||
| Mockito.verifyNoMoreInteractions(connection); | ||
| } | ||
|
|
||
| @Test | ||
| public void shouldNotTouchWhenResponded() { | ||
| final NSQMessage message = new NSQMessage(0L, 0, "id", null, "topic", connection); | ||
| assertFalse(message.hasResponded()); | ||
| message.touch(); | ||
| Mockito.verify(connection).touch("id"); | ||
| message.requeue(); | ||
| assertTrue(message.hasResponded()); | ||
| // reset mock | ||
| Mockito.reset(connection); | ||
| message.touch(); | ||
| Mockito.verifyZeroInteractions(connection); | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think I like a default for this.