From 458292c083ddca2afd64e33d24c51279c0ff6bbc Mon Sep 17 00:00:00 2001 From: Justin Bertram Date: Fri, 14 Feb 2025 10:41:48 -0600 Subject: [PATCH] ARTEMIS-5302 extra fixes for using QueueConfiguration more --- .../artemis/api/core/QueueConfiguration.java | 23 +++++++++++-- .../artemis/utils/SimpleStringTest.java | 32 ++++++++++++------- .../codec/PersistentQueueBindingEncoding.java | 3 +- .../core/server/impl/LastValueQueue.java | 17 ---------- .../artemis/core/server/impl/QueueImpl.java | 10 +++--- 5 files changed, 47 insertions(+), 38 deletions(-) diff --git a/artemis-commons/src/main/java/org/apache/activemq/artemis/api/core/QueueConfiguration.java b/artemis-commons/src/main/java/org/apache/activemq/artemis/api/core/QueueConfiguration.java index 02264aa0411..7f4ce79d7c3 100644 --- a/artemis-commons/src/main/java/org/apache/activemq/artemis/api/core/QueueConfiguration.java +++ b/artemis-commons/src/main/java/org/apache/activemq/artemis/api/core/QueueConfiguration.java @@ -140,6 +140,11 @@ public static QueueConfiguration of(final QueueConfiguration queueConfiguration) return new QueueConfiguration(queueConfiguration); } + /** + * @deprecated + * Use {@link #of(String)} instead. + */ + @Deprecated(forRemoval = true) public QueueConfiguration() { } @@ -410,15 +415,27 @@ public SimpleString getFilterString() { return filterString; } + /** + * This sets the {@code SimpleString} value that will be used to create a {@code Filter} for the {@code Queue} + * implementation on the broker. The filter's syntax is not validated here. + * @param filterString the filter to use; an empty value or a value filled with whitespace is equivalent to passing + * {@code null} + * @return this {@code QueueConfiguration} + */ public QueueConfiguration setFilterString(SimpleString filterString) { - if (filterString != null && !filterString.isEmpty() && !filterString.isBlank()) { - this.filterString = filterString; - } else if (filterString == null) { + if (filterString == null || filterString.isEmpty() || filterString.isBlank()) { + this.filterString = null; + } else { this.filterString = filterString; } return this; } + /** + * Converts the {@code String} parameter to {@code SimpleString} and invokes + * {@link #setFilterString(SimpleString)} + * @see #setFilterString(SimpleString) + */ public QueueConfiguration setFilterString(String filterString) { return setFilterString(SimpleString.of(filterString)); } diff --git a/artemis-commons/src/test/java/org/apache/activemq/artemis/utils/SimpleStringTest.java b/artemis-commons/src/test/java/org/apache/activemq/artemis/utils/SimpleStringTest.java index ee71a8e0e86..181961179f7 100644 --- a/artemis-commons/src/test/java/org/apache/activemq/artemis/utils/SimpleStringTest.java +++ b/artemis-commons/src/test/java/org/apache/activemq/artemis/utils/SimpleStringTest.java @@ -16,6 +16,9 @@ */ package org.apache.activemq.artemis.utils; +import java.util.Arrays; +import java.util.List; + import io.netty.buffer.ByteBuf; import io.netty.buffer.ByteBufAllocator; import org.apache.activemq.artemis.api.core.SimpleString; @@ -43,20 +46,27 @@ public void testOutOfBoundsThrownOnMalformedString() { @Test public void testBlank() { - for (int i = 0; i <= 10; i++) { - assertTrue(SimpleString.of(" ".repeat(i)).isBlank()); - } - for (int i = 0; i <= 10; i++) { - assertTrue(SimpleString.of("\t".repeat(i)).isBlank()); - } - for (int i = 0; i <= 10; i++) { - assertTrue(SimpleString.of("\n".repeat(i)).isBlank()); - } - for (int i = 0; i <= 10; i++) { - assertTrue(SimpleString.of("\r".repeat(i)).isBlank()); + List whitespace = Arrays.asList(" ", "\t", "\n", "\r"); + + // check empty and pure whitespace + for (String s : whitespace) { + for (int i = 0; i <= 10; i++) { + assertTrue(SimpleString.of(s.repeat(i)).isBlank()); + } } + + // check pure non-whitespace for (int i = 1; i <= 10; i++) { assertFalse(SimpleString.of("x".repeat(i)).isBlank()); } + + // check a mix of both whitespace and non-whitepsace + for (String s : whitespace) { + for (int i = 1; i <= 10; i++) { + assertFalse(SimpleString.of(s + "x".repeat(i)).isBlank()); + assertFalse(SimpleString.of("x".repeat(i) + s).isBlank()); + assertFalse(SimpleString.of(s + "x".repeat(i) + s).isBlank()); + } + } } } diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/codec/PersistentQueueBindingEncoding.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/codec/PersistentQueueBindingEncoding.java index 5e44631f844..e43b7b2197e 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/codec/PersistentQueueBindingEncoding.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/codec/PersistentQueueBindingEncoding.java @@ -35,7 +35,6 @@ public class PersistentQueueBindingEncoding implements EncodingSupport, QueueBin private List queueStatusEncodings; public PersistentQueueBindingEncoding() { - config = new QueueConfiguration(); } @Override @@ -67,7 +66,7 @@ public List getQueueStatusEncodings() { @Override public void decode(final ActiveMQBuffer buffer) { - config.setName(buffer.readSimpleString()); + config = QueueConfiguration.of(buffer.readSimpleString()); config.setAddress(buffer.readSimpleString()); config.setFilterString(buffer.readNullableSimpleString()); diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/LastValueQueue.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/LastValueQueue.java index dac696bad48..2373a67c8aa 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/LastValueQueue.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/LastValueQueue.java @@ -52,7 +52,6 @@ public class LastValueQueue extends QueueImpl { private final Map map = new ConcurrentHashMap<>(); - private final SimpleString lastValueKey; public LastValueQueue(final QueueConfiguration queueConfiguration, final Filter filter, @@ -66,7 +65,6 @@ public LastValueQueue(final QueueConfiguration queueConfiguration, final ActiveMQServer server, final QueueFactory factory) { super(queueConfiguration, filter, pagingStore, pageSubscription, scheduledExecutor, postOffice, storageManager, addressSettingsRepository, executor, server, factory); - this.lastValueKey = queueConfiguration.getLastValueKey(); } @Override @@ -121,11 +119,6 @@ public boolean allowsReferenceCallback() { return false; } - @Override - public QueueConfiguration getQueueConfiguration() { - return super.getQueueConfiguration().setLastValue(true).setLastValueKey(lastValueKey); - } - @Override protected void pruneLastValues() { // called with synchronized(this) from super.deliver() @@ -214,16 +207,6 @@ public boolean actMessage(Transaction tx, MessageReference ref) throws Exception }; } - @Override - public boolean isLastValue() { - return true; - } - - @Override - public SimpleString getLastValueKey() { - return lastValueKey; - } - public synchronized Set getLastValueKeys() { return Collections.unmodifiableSet(map.keySet()); } diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java index 0e2468bd093..064dd136b17 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java @@ -318,7 +318,7 @@ private void checkIDSupplier(NodeStoreFactory nodeStoreFactory private final int initialQueueBufferSize; - private final QueueConfiguration queueConfiguration; + protected final QueueConfiguration queueConfiguration; @Override public boolean isSwept() { @@ -385,7 +385,7 @@ public QueueImpl(final QueueConfiguration queueConfiguration, this.createdTimestamp = System.currentTimeMillis(); - this.queueConfiguration = queueConfiguration; + this.queueConfiguration = QueueConfiguration.of(queueConfiguration); QueueConfigurationUtils.applyStaticDefaults(this.queueConfiguration); this.refCountForConsumers = this.queueConfiguration.isTransient() ? new TransientQueueManagerImpl(server, this.queueConfiguration.getName()) : new QueueManagerImpl(server, this.queueConfiguration.getName()); @@ -537,12 +537,12 @@ public synchronized void setDispatching(boolean dispatching) { @Override public boolean isLastValue() { - return false; + return queueConfiguration.isLastValue(); } @Override public SimpleString getLastValueKey() { - return null; + return queueConfiguration.getLastValueKey(); } @Override @@ -4081,7 +4081,7 @@ public static MessageGroups groupMap(int groupBuckets) { @Override public QueueConfiguration getQueueConfiguration() { - return queueConfiguration; + return QueueConfiguration.of(queueConfiguration); } protected static class ConsumerHolder implements PriorityAware {