Skip to content

Commit b3bb4b4

Browse files
committed
feat(nats): validate queue name + add stream description on creation
- MessageBroker SPI: new default no-op validateQueueName(String) hook. - JetStreamMessageBroker overrides it to reject queue names containing '.', '*', '>' or whitespace. NATS subjects use '.' as a hierarchy separator and stream / consumer names disallow it outright, so a silent accept turned into an opaque driver-side rejection at first publish; now it fails loudly at registration with a message that points users at '-' / '_'. - BaseMessageSender.registerQueueInternal and RqueueMessageListenerContainer.initializeQueueRegistry both call validateQueueName, so both the explicit RqueueEndpointManager.registerQueue path and the @RqueueListener bootstrap path validate. - NatsProvisioner.ensureStream gains a description overload; the broker passes "rqueue queue: <name>" (and "(priority=<p>)" / "rqueue DLQ for queue: <name>" for the priority and DLQ variants) so operators can map a stream back to the queue that created it via `nats stream info`. - NatsStreamValidator passes the same description on the bootstrap path. Tests: JetStreamMessageBrokerQueueNameValidationTest pins the character-rejection rules; JetStreamMessageBrokerStreamDescriptionTest pins the description format on enqueue / onQueueRegistered / priority enqueue paths; RqueueEndpointManagerImplTest gains a regression covering broker-validation propagation through registerQueue. Assisted-By: Claude Code
1 parent ef9e690 commit b3bb4b4

9 files changed

Lines changed: 318 additions & 9 deletions

File tree

rqueue-core/src/main/java/com/github/sonus21/rqueue/core/impl/BaseMessageSender.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,7 @@ protected Object deleteAllMessages(QueueDetail queueDetail) {
204204

205205
protected void registerQueueInternal(String queueName, QueueType type, String... priorities) {
206206
validateQueue(queueName);
207+
messageBroker.validateQueueName(queueName);
207208
notNull(priorities, "priorities cannot be null");
208209
Map<String, Integer> priorityMap = new HashMap<>();
209210
priorityMap.put(DEFAULT_PRIORITY_KEY, 1);

rqueue-core/src/main/java/com/github/sonus21/rqueue/core/spi/MessageBroker.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,18 @@ default void enqueue(QueueDetail q, String priority, RqueueMessage m) {
5353
*/
5454
default void onQueueRegistered(QueueDetail q) {}
5555

56+
/**
57+
* Validate the queue name against backend-specific rules. Called from every queue-registration
58+
* path ({@code RqueueEndpointManager.registerQueue} and the {@code @RqueueListener} bootstrap)
59+
* before the queue is added to the registry, so an illegal name fails fast with a clear error
60+
* instead of surfacing later as an opaque NATS / driver-side rejection.
61+
*
62+
* <p>Default is a no-op — backends like Redis accept any non-empty name.
63+
*
64+
* @throws IllegalArgumentException if {@code queueName} is not legal for this backend
65+
*/
66+
default void validateQueueName(String queueName) {}
67+
5668
/**
5769
* Reactive variant of {@link #enqueue(QueueDetail, RqueueMessage)}. The default falls back to the
5870
* blocking implementation wrapped in {@code Mono.fromRunnable}; backends with native async

rqueue-core/src/main/java/com/github/sonus21/rqueue/listener/RqueueMessageListenerContainer.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -294,9 +294,15 @@ public void stop(Runnable callback) {
294294
private void initializeQueueRegistry() {
295295
log.info("Initializing queue registry");
296296
EndpointRegistry.delete();
297+
// Validate every @RqueueListener queue name against backend rules before registering, so
298+
// illegal names (e.g. dots when running on NATS) fail loudly at startup instead of surfacing
299+
// as opaque driver errors at first publish.
297300
for (MappingInformation mappingInformation :
298301
rqueueMessageHandler.getHandlerMethodMap().keySet()) {
299302
for (String queue : mappingInformation.getQueueNames()) {
303+
if (messageBroker != null) {
304+
messageBroker.validateQueueName(queue);
305+
}
300306
for (QueueDetail queueDetail : getQueueDetail(queue, mappingInformation)) {
301307
EndpointRegistry.register(queueDetail);
302308
}

rqueue-core/src/test/java/com/github/sonus21/rqueue/core/impl/RqueueEndpointManagerImplTest.java

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import com.github.sonus21.rqueue.core.EndpointRegistry;
3030
import com.github.sonus21.rqueue.core.RqueueEndpointManager;
3131
import com.github.sonus21.rqueue.core.RqueueMessageTemplate;
32+
import com.github.sonus21.rqueue.core.spi.MessageBroker;
3233
import com.github.sonus21.rqueue.core.spi.redis.RedisMessageBroker;
3334
import com.github.sonus21.rqueue.dao.RqueueSystemConfigDao;
3435
import com.github.sonus21.rqueue.listener.RqueueMessageHeaders;
@@ -186,4 +187,29 @@ void pauseQueueWithPriority() {
186187
.pauseUnpauseQueue(request);
187188
assertFalse(rqueueEndpointManager.pauseUnpauseQueue(queue, priorities[0], false));
188189
}
190+
191+
/**
192+
* registerQueue must consult {@link MessageBroker#validateQueueName(String)}; an exception there
193+
* must propagate so callers fail fast on illegal names (e.g. dots when running on NATS).
194+
*/
195+
@Test
196+
void registerQueue_propagatesBrokerValidationFailure() throws IllegalAccessException {
197+
MessageBroker rejectingBroker = new RedisMessageBroker(messageTemplate) {
198+
@Override
199+
public void validateQueueName(String queueName) {
200+
if (queueName.contains(".")) {
201+
throw new IllegalArgumentException("dot rejected: " + queueName);
202+
}
203+
}
204+
};
205+
RqueueEndpointManager mgr = new RqueueEndpointManagerImpl(
206+
messageTemplate, rejectingBroker, messageConverter, messageHeaders);
207+
RqueueConfig rqueueConfig = new RqueueConfig(redisConnectionFactory, null, false, 1);
208+
FieldUtils.writeField(mgr, "rqueueConfig", rqueueConfig, true);
209+
210+
IllegalArgumentException ex =
211+
assertThrows(IllegalArgumentException.class, () -> mgr.registerQueue("orders.us"));
212+
assertTrue(ex.getMessage().contains("orders.us"));
213+
assertFalse(mgr.isQueueRegistered("orders.us"));
214+
}
189215
}

rqueue-nats/src/main/java/com/github/sonus21/rqueue/nats/internal/NatsProvisioner.java

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,12 @@ public KeyValue ensureKv(String bucketName, Duration ttl)
125125
* use {@link #ensureStream(String, List, QueueType)} instead.
126126
*/
127127
public void ensureStream(String streamName, List<String> subjects) {
128-
ensureStream(streamName, subjects, QueueType.QUEUE);
128+
ensureStream(streamName, subjects, QueueType.QUEUE, null);
129+
}
130+
131+
/** See {@link #ensureStream(String, List, QueueType, String)}. */
132+
public void ensureStream(String streamName, List<String> subjects, QueueType queueType) {
133+
ensureStream(streamName, subjects, queueType, null);
129134
}
130135

131136
/**
@@ -138,11 +143,16 @@ public void ensureStream(String streamName, List<String> subjects) {
138143
* independent durable consumer group receives all messages; stream/fan-out semantics.
139144
* </ul>
140145
*
146+
* <p>{@code description} is forwarded to JetStream as the stream's description (visible via
147+
* {@code nats stream info}). Callers should pass the rqueue queue name so operators can map a
148+
* stream back to the queue that created it; pass {@code null} to skip.
149+
*
141150
* <p>Hits the NATS backend at most once per stream name per process lifetime; subsequent calls
142151
* return immediately from the in-process cache. If the stream already exists with a different
143152
* retention policy, a WARNING is logged and the existing config is left untouched.
144153
*/
145-
public void ensureStream(String streamName, List<String> subjects, QueueType queueType) {
154+
public void ensureStream(
155+
String streamName, List<String> subjects, QueueType queueType, String description) {
146156
if (streamsDone.contains(streamName)) {
147157
return;
148158
}
@@ -169,6 +179,9 @@ public void ensureStream(String streamName, List<String> subjects, QueueType que
169179
.retentionPolicy(desired)
170180
.duplicateWindow(sd.getDuplicateWindow())
171181
.compressionOption(CompressionOption.S2);
182+
if (description != null && !description.isEmpty()) {
183+
b.description(description);
184+
}
172185
if (sd.getMaxMsgs() > 0) {
173186
b.maxMessages(sd.getMaxMsgs());
174187
}

rqueue-nats/src/main/java/com/github/sonus21/rqueue/nats/js/JetStreamMessageBroker.java

Lines changed: 55 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import com.github.sonus21.rqueue.core.RqueueMessage;
1515
import com.github.sonus21.rqueue.core.spi.Capabilities;
1616
import com.github.sonus21.rqueue.core.spi.MessageBroker;
17+
import com.github.sonus21.rqueue.enums.QueueType;
1718
import com.github.sonus21.rqueue.listener.QueueDetail;
1819
import com.github.sonus21.rqueue.nats.RqueueNatsConfig;
1920
import com.github.sonus21.rqueue.nats.RqueueNatsException;
@@ -159,13 +160,29 @@ private String dlqSubjectFor(QueueDetail q) {
159160
: subjectFor(q) + config.getDlqSubjectSuffix();
160161
}
161162

163+
/** Stream description shown in {@code nats stream info} so operators can map back to rqueue. */
164+
private static String streamDescription(QueueDetail q) {
165+
return "rqueue queue: " + q.getName();
166+
}
167+
168+
/** Stream description for the priority sub-stream. */
169+
private static String streamDescription(QueueDetail q, String priority) {
170+
return priority == null || priority.isEmpty()
171+
? streamDescription(q)
172+
: "rqueue queue: " + q.getName() + " (priority=" + priority + ")";
173+
}
174+
175+
private static String dlqStreamDescription(QueueDetail q) {
176+
return "rqueue DLQ for queue: " + q.getName();
177+
}
178+
162179
// ---- MessageBroker -----------------------------------------------------
163180

164181
@Override
165182
public void enqueue(QueueDetail q, RqueueMessage m) {
166183
String subject = subjectFor(q);
167184
String stream = streamFor(q);
168-
provisioner.ensureStream(stream, List.of(subject), q.getType());
185+
provisioner.ensureStream(stream, List.of(subject), q.getType(), streamDescription(q));
169186
Headers headers = new Headers();
170187
if (m.getId() != null) {
171188
headers.add("Nats-Msg-Id", m.getId());
@@ -198,7 +215,8 @@ public void enqueue(QueueDetail q, RqueueMessage m) {
198215
public void enqueue(QueueDetail q, String priority, RqueueMessage m) {
199216
String subject = subjectFor(q, priority);
200217
String stream = streamFor(q, priority);
201-
provisioner.ensureStream(stream, List.of(subject), q.getType());
218+
provisioner.ensureStream(
219+
stream, List.of(subject), q.getType(), streamDescription(q, priority));
202220
Headers headers = new Headers();
203221
if (m.getId() != null) {
204222
headers.add("Nats-Msg-Id", m.getId());
@@ -243,7 +261,7 @@ public Mono<Void> enqueueReactive(QueueDetail q, RqueueMessage m) {
243261
String subject = subjectFor(q);
244262
String stream = streamFor(q);
245263
try {
246-
provisioner.ensureStream(stream, List.of(subject), q.getType());
264+
provisioner.ensureStream(stream, List.of(subject), q.getType(), streamDescription(q));
247265
} catch (Exception e) {
248266
return Mono.error(new RqueueNatsException(
249267
"Failed to provision stream for reactive enqueue id="
@@ -431,7 +449,8 @@ public void moveToDlq(
431449
headers.add("Nats-Msg-Id", updated.getId() + "-dlq");
432450
}
433451
try {
434-
provisioner.ensureStream(dlqStream, List.of(dlqSubject));
452+
provisioner.ensureStream(
453+
dlqStream, List.of(dlqSubject), QueueType.QUEUE, "rqueue DLQ for queue: " + targetQueue);
435454
byte[] payload = serdes.serialize(updated);
436455
js.publish(dlqSubject, headers, payload);
437456
} catch (IOException | JetStreamApiException e) {
@@ -528,7 +547,36 @@ public void publish(String channel, String payload) {
528547
public void onQueueRegistered(QueueDetail q) {
529548
String stream = streamFor(q);
530549
String subject = subjectFor(q);
531-
provisioner.ensureStream(stream, List.of(subject), q.getType());
550+
provisioner.ensureStream(stream, List.of(subject), q.getType(), streamDescription(q));
551+
}
552+
553+
/**
554+
* NATS subjects use {@code .} as a hierarchy separator and stream / consumer names disallow it
555+
* outright. A queue name like {@code "orders.us"} would (a) silently turn the publish subject
556+
* into a two-level token tree and (b) make {@code rqueue-js-orders.us} an invalid stream name —
557+
* the JetStream API would reject it with an opaque error at first publish. Reject the name at
558+
* registration so the failure is loud and local.
559+
*
560+
* <p>{@code *} and {@code &gt;} are also illegal in subject tokens (they're NATS wildcards), and
561+
* whitespace is rejected by the server; check those too while we're here.
562+
*/
563+
@Override
564+
public void validateQueueName(String queueName) {
565+
if (queueName == null || queueName.isEmpty()) {
566+
return;
567+
}
568+
for (int i = 0; i < queueName.length(); i++) {
569+
char c = queueName.charAt(i);
570+
if (c == '.' || c == '*' || c == '>' || Character.isWhitespace(c)) {
571+
throw new IllegalArgumentException(
572+
"Queue name '"
573+
+ queueName
574+
+ "' contains illegal character '"
575+
+ c
576+
+ "' for the NATS backend. Subject hierarchy ('.'), wildcards ('*', '>') and"
577+
+ " whitespace are not allowed in queue names — use '-' or '_' instead.");
578+
}
579+
}
532580
}
533581

534582
@Override
@@ -578,7 +626,8 @@ public void close() {
578626
public void provisionDlq(QueueDetail q) {
579627
// Explicit call — always provision, bypassing the autoCreateDlqStream flag.
580628
// That flag gates automatic provisioning at bootstrap; here the caller is explicitly opting in.
581-
provisioner.ensureStream(dlqStreamFor(q), List.of(dlqSubjectFor(q)));
629+
provisioner.ensureStream(
630+
dlqStreamFor(q), List.of(dlqSubjectFor(q)), QueueType.QUEUE, dlqStreamDescription(q));
582631
}
583632

584633
/**

rqueue-nats/src/main/java/com/github/sonus21/rqueue/nats/js/NatsStreamValidator.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,8 @@ private void tryEnsureConsumer(
165165

166166
private int tryEnsure(List<String> failures, String streamName, String subject, QueueDetail q) {
167167
try {
168-
provisioner.ensureStream(streamName, List.of(subject), q.getType());
168+
provisioner.ensureStream(
169+
streamName, List.of(subject), q.getType(), "rqueue queue: " + q.getName());
169170
return 1;
170171
} catch (RqueueNatsException e) {
171172
failures.add(streamName + " (subject " + subject + "): " + rootCause(e));
Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
/*
2+
* Copyright (c) 2024-2026 Sonu Kumar
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* You may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*/
10+
package com.github.sonus21.rqueue.nats;
11+
12+
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
13+
import static org.junit.jupiter.api.Assertions.assertThrows;
14+
import static org.junit.jupiter.api.Assertions.assertTrue;
15+
import static org.mockito.Mockito.mock;
16+
17+
import com.github.sonus21.rqueue.nats.js.JetStreamMessageBroker;
18+
import com.github.sonus21.rqueue.serdes.RqJacksonSerDes;
19+
import com.github.sonus21.rqueue.serdes.SerializationUtils;
20+
import io.nats.client.Connection;
21+
import io.nats.client.JetStream;
22+
import io.nats.client.JetStreamManagement;
23+
import org.junit.jupiter.api.Test;
24+
25+
/**
26+
* Pins the dot/wildcard/whitespace rejection on queue names for the NATS backend. NATS subjects
27+
* use {@code .} as a hierarchy separator and stream / consumer names disallow it outright, so
28+
* silently accepting an illegal name leads to an opaque driver-side rejection at first publish.
29+
*/
30+
@NatsUnitTest
31+
class JetStreamMessageBrokerQueueNameValidationTest {
32+
33+
private JetStreamMessageBroker newBroker() {
34+
return new JetStreamMessageBroker(
35+
mock(Connection.class),
36+
mock(JetStream.class),
37+
mock(JetStreamManagement.class),
38+
RqueueNatsConfig.defaults(),
39+
new RqJacksonSerDes(SerializationUtils.getObjectMapper()),
40+
null);
41+
}
42+
43+
@Test
44+
void rejects_queueName_with_dot() {
45+
JetStreamMessageBroker broker = newBroker();
46+
IllegalArgumentException ex =
47+
assertThrows(IllegalArgumentException.class, () -> broker.validateQueueName("orders.us"));
48+
assertTrue(ex.getMessage().contains("orders.us"));
49+
assertTrue(ex.getMessage().contains("'.'"));
50+
}
51+
52+
@Test
53+
void rejects_queueName_with_star_wildcard() {
54+
JetStreamMessageBroker broker = newBroker();
55+
assertThrows(IllegalArgumentException.class, () -> broker.validateQueueName("foo*bar"));
56+
}
57+
58+
@Test
59+
void rejects_queueName_with_gt_wildcard() {
60+
JetStreamMessageBroker broker = newBroker();
61+
assertThrows(IllegalArgumentException.class, () -> broker.validateQueueName("foo>bar"));
62+
}
63+
64+
@Test
65+
void rejects_queueName_with_whitespace() {
66+
JetStreamMessageBroker broker = newBroker();
67+
assertThrows(IllegalArgumentException.class, () -> broker.validateQueueName("foo bar"));
68+
assertThrows(IllegalArgumentException.class, () -> broker.validateQueueName("foo\tbar"));
69+
}
70+
71+
@Test
72+
void accepts_legal_queue_names() {
73+
JetStreamMessageBroker broker = newBroker();
74+
assertDoesNotThrow(() -> broker.validateQueueName("orders"));
75+
assertDoesNotThrow(() -> broker.validateQueueName("orders-us"));
76+
assertDoesNotThrow(() -> broker.validateQueueName("orders_us"));
77+
assertDoesNotThrow(() -> broker.validateQueueName("orders123"));
78+
assertDoesNotThrow(() -> broker.validateQueueName("a"));
79+
}
80+
81+
@Test
82+
void accepts_null_or_empty_so_that_callers_higher_up_the_stack_handle_those_errors() {
83+
JetStreamMessageBroker broker = newBroker();
84+
assertDoesNotThrow(() -> broker.validateQueueName(null));
85+
assertDoesNotThrow(() -> broker.validateQueueName(""));
86+
}
87+
}

0 commit comments

Comments
 (0)