You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@activemq.apache.org by ro...@apache.org on 2024/07/17 13:11:27 UTC
(activemq-artemis) branch main updated: ARTEMIS-4910 fix divert config encoding
This is an automated email from the ASF dual-hosted git repository.
robbie pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/activemq-artemis.git
The following commit(s) were added to refs/heads/main by this push:
new 1a83220711 ARTEMIS-4910 fix divert config encoding
1a83220711 is described below
commit 1a8322071168e6d33053cc47dd8f657280c0ed64
Author: Justin Bertram <jb...@apache.org>
AuthorDate: Tue Jul 9 16:28:44 2024 -0500
ARTEMIS-4910 fix divert config encoding
This commit fixes 4 distinct issues with divert configuration encoding:
- The encoding size was calculated incorrectly in three ways:
- Using 0 instead of `DataConstants.SIZE_NULL` when no transformer is
defined resulting in a calculated size discrepancy of -1.
- Using `DataConstants.INT` instead of `DataConstants.SIZE_INT` for
the number of transformer properties resulting in a calculated size
discrepancy of +2 when using a configuration with a transformer.
- Using `BufferHelper.sizeOfNullableBoolean` instead of
`DataConstants.SIZE_BOOLEAN` for the `exclusive` property resulting
in a calculated discrepancy of +1.
- Encoding was using `writeString` instead of `writeNullableString` for
the name of the transformer class resulting in an actual buffer size
discrepancy of -1.
Aside from these fixes this commit also:
- Updates the divert storage test to force a reload from disk which
will also trigger this problem.
- Adds new tests specifically for encoding & decoding include a test to
verify known bytes during encoding.
Lastly, it's worth noting that this *won't fix* bad data that was
already stored to disk by an older broker or bad data that comes over
the wire from an older broker (e.g. if a older primary broker paired
with a newer backup).
---
.../artemis/core/config/DivertConfiguration.java | 16 +-
.../config/DivertConfigurationEncodingTest.java | 217 +++++++++++++++++++++
.../DivertConfigurationStorageTest.java | 33 +---
.../persistence/StorageManagerTestBase.java | 11 +-
4 files changed, 242 insertions(+), 35 deletions(-)
diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/DivertConfiguration.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/DivertConfiguration.java
index a10e97774f..64127082c9 100644
--- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/DivertConfiguration.java
+++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/DivertConfiguration.java
@@ -217,21 +217,22 @@ public class DivertConfiguration implements Serializable, EncodingSupport {
@Override
public int getEncodeSize() {
- int transformerSize = 0;
+ int transformerSize;
if (transformerConfiguration != null) {
- transformerSize += BufferHelper.sizeOfNullableString(transformerConfiguration.getClassName());
- transformerSize += DataConstants.INT;
- Map<String, String> properties = transformerConfiguration.getProperties();
- for (Map.Entry<String, String> entry : properties.entrySet()) {
+ transformerSize = BufferHelper.sizeOfNullableString(transformerConfiguration.getClassName());
+ transformerSize += DataConstants.SIZE_INT;
+ for (Map.Entry<String, String> entry : transformerConfiguration.getProperties().entrySet()) {
transformerSize += BufferHelper.sizeOfNullableString(entry.getKey());
transformerSize += BufferHelper.sizeOfNullableString(entry.getValue());
}
+ } else {
+ transformerSize = DataConstants.SIZE_NULL;
}
int size = BufferHelper.sizeOfNullableString(name) +
BufferHelper.sizeOfNullableString(address) +
BufferHelper.sizeOfNullableString(forwardingAddress) +
BufferHelper.sizeOfNullableString(routingName) +
- BufferHelper.sizeOfNullableBoolean(exclusive) +
+ DataConstants.SIZE_BOOLEAN +
BufferHelper.sizeOfNullableString(filterString) +
DataConstants.SIZE_BYTE + transformerSize;
return size;
@@ -247,7 +248,7 @@ public class DivertConfiguration implements Serializable, EncodingSupport {
buffer.writeNullableString(filterString);
buffer.writeByte(routingType != null ? routingType.getType() : ComponentConfigurationRoutingType.valueOf(ActiveMQDefaultConfiguration.getDefaultDivertRoutingType()).getType());
if (transformerConfiguration != null) {
- buffer.writeString(transformerConfiguration.getClassName());
+ buffer.writeNullableString(transformerConfiguration.getClassName());
Map<String, String> properties = transformerConfiguration.getProperties();
buffer.writeInt(properties.size());
for (Map.Entry<String, String> entry : properties.entrySet()) {
@@ -280,7 +281,6 @@ public class DivertConfiguration implements Serializable, EncodingSupport {
for (int i = 0; i < propsSize; i++) {
transformerConfiguration.getProperties().put(buffer.readNullableString(), buffer.readNullableString());
}
-
}
}
}
diff --git a/artemis-server/src/test/java/org/apache/activemq/artemis/core/config/DivertConfigurationEncodingTest.java b/artemis-server/src/test/java/org/apache/activemq/artemis/core/config/DivertConfigurationEncodingTest.java
new file mode 100644
index 0000000000..d7e58fceb2
--- /dev/null
+++ b/artemis-server/src/test/java/org/apache/activemq/artemis/core/config/DivertConfigurationEncodingTest.java
@@ -0,0 +1,217 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.core.config;
+
+import java.util.Map;
+
+import org.apache.activemq.artemis.api.core.ActiveMQBuffer;
+import org.apache.activemq.artemis.api.core.ActiveMQBuffers;
+import org.apache.activemq.artemis.core.server.ComponentConfigurationRoutingType;
+import org.apache.activemq.artemis.utils.DataConstants;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+
+public class DivertConfigurationEncodingTest {
+
+ @Test
+ public void testEncodedBytes() {
+ final String name = "aa";
+ final String address = "bb";
+ final String forwardingAddress = "cc";
+ final String routingName = "dd";
+ final boolean exclusive = true;
+ final String filterString = "ee";
+ final ComponentConfigurationRoutingType routingType = ComponentConfigurationRoutingType.ANYCAST;
+ final String transformerClass = "ff";
+ final String transformerKey = "gg";
+ final String transformerValue = "hh";
+ final TransformerConfiguration myDivertTransformer = new TransformerConfiguration(transformerClass);
+
+ myDivertTransformer.getProperties().put(transformerKey, transformerValue);
+
+ DivertConfiguration configuration = new DivertConfiguration()
+ .setName(name)
+ .setAddress(address)
+ .setForwardingAddress(forwardingAddress)
+ .setFilterString(filterString)
+ .setRoutingName(routingName)
+ .setExclusive(exclusive)
+ .setRoutingType(routingType)
+ .setTransformerConfiguration(myDivertTransformer);
+
+ int encodeSize = configuration.getEncodeSize();
+ ActiveMQBuffer data = ActiveMQBuffers.fixedBuffer(encodeSize);
+ configuration.encode(data);
+ assertEquals(encodeSize, data.writerIndex());
+
+ // name
+ byte[] read = new byte[9];
+ data.readBytes(read, 0, 9);
+ assertArrayEquals(new byte[] {DataConstants.NOT_NULL, 0, 0, 0, 2, 0, 97, 0, 97}, read);
+
+ // address
+ read = new byte[9];
+ data.readBytes(read, 0, 9);
+ assertArrayEquals(new byte[] {DataConstants.NOT_NULL, 0, 0, 0, 2, 0, 98, 0, 98}, read);
+
+ // forwardingAddress
+ read = new byte[9];
+ data.readBytes(read, 0, 9);
+ assertArrayEquals(new byte[] {DataConstants.NOT_NULL, 0, 0, 0, 2, 0, 99, 0, 99}, read);
+
+ // routingName
+ read = new byte[9];
+ data.readBytes(read, 0, 9);
+ assertArrayEquals(new byte[] {DataConstants.NOT_NULL, 0, 0, 0, 2, 0, 100, 0, 100}, read);
+
+ // exclusive
+ assertEquals((byte) -1, data.readByte());
+
+ // filterString
+ read = new byte[9];
+ data.readBytes(read, 0, 9);
+ assertArrayEquals(new byte[] {DataConstants.NOT_NULL, 0, 0, 0, 2, 0, 101, 0, 101}, read);
+
+ // routingType
+ assertEquals(ComponentConfigurationRoutingType.ANYCAST.getType(), data.readByte());
+
+ // transformerClassName
+ read = new byte[9];
+ data.readBytes(read, 0, 9);
+ assertArrayEquals(new byte[] {DataConstants.NOT_NULL, 0, 0, 0, 2, 0, 102, 0, 102}, read);
+
+ // number of transformer properties
+ read = new byte[4];
+ data.readBytes(read, 0, 4);
+ assertArrayEquals(new byte[] {0, 0, 0, 1}, read);
+
+ // transformer key name
+ read = new byte[9];
+ data.readBytes(read, 0, 9);
+ assertArrayEquals(new byte[] {DataConstants.NOT_NULL, 0, 0, 0, 2, 0, 103, 0, 103}, read);
+
+ // transformer key value
+ read = new byte[9];
+ data.readBytes(read, 0, 9);
+ assertArrayEquals(new byte[] {DataConstants.NOT_NULL, 0, 0, 0, 2, 0, 104, 0, 104}, read);
+
+ assertEquals(0, data.readableBytes());
+ }
+
+ @Test
+ public void testEncodeDecodeWithTransformer() {
+ TransformerConfiguration mytransformer = new TransformerConfiguration("myDivertTransformer");
+ mytransformer.getProperties().put("key1", "prop1");
+ mytransformer.getProperties().put("key2", "prop2");
+ mytransformer.getProperties().put("key3", "prop3");
+
+ DivertConfiguration configuration = new DivertConfiguration()
+ .setName("name")
+ .setAddress("address")
+ .setForwardingAddress("forward")
+ .setFilterString("foo='foo'")
+ .setRoutingName("myDivertRoutingName")
+ .setExclusive(true)
+ .setRoutingType(ComponentConfigurationRoutingType.STRIP)
+ .setTransformerConfiguration(mytransformer);
+
+ int encodeSize = configuration.getEncodeSize();
+ ActiveMQBuffer data = ActiveMQBuffers.fixedBuffer(encodeSize);
+ configuration.encode(data);
+ DivertConfiguration persistedDivertConfiguration = new DivertConfiguration();
+ persistedDivertConfiguration.decode(data);
+
+ assertEquals(configuration.getName(), persistedDivertConfiguration.getName());
+ assertEquals(configuration.getAddress(), persistedDivertConfiguration.getAddress());
+ assertEquals(configuration.getForwardingAddress(), persistedDivertConfiguration.getForwardingAddress());
+ assertEquals(configuration.getFilterString(), persistedDivertConfiguration.getFilterString());
+ assertEquals(configuration.getRoutingName(), persistedDivertConfiguration.getRoutingName());
+ assertEquals(configuration.isExclusive(), persistedDivertConfiguration.isExclusive());
+ assertEquals(configuration.getRoutingType(), persistedDivertConfiguration.getRoutingType());
+ assertNotNull(persistedDivertConfiguration.getTransformerConfiguration());
+ assertEquals("myDivertTransformer", persistedDivertConfiguration.getTransformerConfiguration().getClassName());
+ Map<String, String> properties = persistedDivertConfiguration.getTransformerConfiguration().getProperties();
+ assertEquals(3, properties.size());
+ assertEquals("prop1", properties.get("key1"));
+ assertEquals("prop2", properties.get("key2"));
+ assertEquals("prop3", properties.get("key3"));
+ }
+
+ @Test
+ public void testEncodeDecodeWithTransformerWithNoProperties() {
+ TransformerConfiguration mytransformer = new TransformerConfiguration("myDivertTransformer");
+
+ DivertConfiguration configuration = new DivertConfiguration()
+ .setName("name")
+ .setAddress("address")
+ .setForwardingAddress("forward")
+ .setFilterString("foo='foo'")
+ .setRoutingName("myDivertRoutingName")
+ .setExclusive(true)
+ .setRoutingType(ComponentConfigurationRoutingType.MULTICAST)
+ .setTransformerConfiguration(mytransformer);
+
+ int encodeSize = configuration.getEncodeSize();
+ ActiveMQBuffer data = ActiveMQBuffers.fixedBuffer(encodeSize);
+ configuration.encode(data);
+ DivertConfiguration persistedDivertConfiguration = new DivertConfiguration();
+ persistedDivertConfiguration.decode(data);
+
+ assertEquals(configuration.getName(), persistedDivertConfiguration.getName());
+ assertEquals(configuration.getAddress(), persistedDivertConfiguration.getAddress());
+ assertEquals(configuration.getForwardingAddress(), persistedDivertConfiguration.getForwardingAddress());
+ assertEquals(configuration.getFilterString(), persistedDivertConfiguration.getFilterString());
+ assertEquals(configuration.getRoutingName(), persistedDivertConfiguration.getRoutingName());
+ assertEquals(configuration.isExclusive(), persistedDivertConfiguration.isExclusive());
+ assertEquals(configuration.getRoutingType(), persistedDivertConfiguration.getRoutingType());
+ assertNotNull(persistedDivertConfiguration.getTransformerConfiguration());
+ assertEquals("myDivertTransformer", persistedDivertConfiguration.getTransformerConfiguration().getClassName());
+ Map<String, String> properties = persistedDivertConfiguration.getTransformerConfiguration().getProperties();
+ assertEquals(0, properties.size());
+ }
+
+ @Test
+ public void testEncodeDecodeWithoutTransformer() {
+ DivertConfiguration configuration = new DivertConfiguration()
+ .setName("name")
+ .setAddress("address")
+ .setForwardingAddress("forward")
+ .setFilterString("foo='foo'")
+ .setExclusive(true)
+ .setRoutingName("myDivertRoutingName")
+ .setRoutingType(ComponentConfigurationRoutingType.PASS);
+
+ int encodeSize = configuration.getEncodeSize();
+ ActiveMQBuffer data = ActiveMQBuffers.fixedBuffer(encodeSize);
+ configuration.encode(data);
+ DivertConfiguration decodedDivertConfiguration = new DivertConfiguration();
+ decodedDivertConfiguration.decode(data);
+
+ assertEquals(configuration.getName(), decodedDivertConfiguration.getName());
+ assertEquals(configuration.getAddress(), decodedDivertConfiguration.getAddress());
+ assertEquals(configuration.getForwardingAddress(), decodedDivertConfiguration.getForwardingAddress());
+ assertEquals(configuration.getFilterString(), decodedDivertConfiguration.getFilterString());
+ assertEquals(configuration.getRoutingName(), decodedDivertConfiguration.getRoutingName());
+ assertEquals(configuration.isExclusive(), decodedDivertConfiguration.isExclusive());
+ assertEquals(configuration.getRoutingType(), decodedDivertConfiguration.getRoutingType());
+ assertNull(decodedDivertConfiguration.getTransformerConfiguration());
+ }
+}
diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/DivertConfigurationStorageTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/DivertConfigurationStorageTest.java
index 6cc697a9b6..0c4d751e8e 100644
--- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/DivertConfigurationStorageTest.java
+++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/DivertConfigurationStorageTest.java
@@ -16,21 +16,20 @@
*/
package org.apache.activemq.artemis.tests.integration.persistence;
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertNotNull;
-import static org.junit.jupiter.api.Assertions.assertNull;
+import java.util.List;
+import java.util.Map;
import org.apache.activemq.artemis.core.config.DivertConfiguration;
import org.apache.activemq.artemis.core.config.StoreConfiguration;
import org.apache.activemq.artemis.core.config.TransformerConfiguration;
import org.apache.activemq.artemis.core.persistence.config.PersistedDivertConfiguration;
import org.apache.activemq.artemis.tests.extensions.parameterized.ParameterizedTestExtension;
-import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.TestTemplate;
import org.junit.jupiter.api.extension.ExtendWith;
-import java.util.List;
-import java.util.Map;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
//Parameters set in super class
@ExtendWith(ParameterizedTestExtension.class)
@@ -40,12 +39,6 @@ public class DivertConfigurationStorageTest extends StorageManagerTestBase {
super(storeType);
}
- @Override
- @BeforeEach
- public void setUp() throws Exception {
- super.setUp();
- }
-
@TestTemplate
public void testStoreDivertConfiguration() throws Exception {
createStorage();
@@ -64,9 +57,7 @@ public class DivertConfigurationStorageTest extends StorageManagerTestBase {
journal.storeDivertConfiguration(new PersistedDivertConfiguration(configuration));
- journal.stop();
-
- journal.start();
+ rebootStorage();
List<PersistedDivertConfiguration> divertConfigurations = journal.recoverDivertConfigurations();
@@ -85,10 +76,6 @@ public class DivertConfigurationStorageTest extends StorageManagerTestBase {
assertEquals("prop1", properties.get("key1"));
assertEquals("prop2", properties.get("key2"));
assertEquals("prop3", properties.get("key3"));
- journal.stop();
-
- journal = null;
-
}
@TestTemplate
@@ -104,9 +91,7 @@ public class DivertConfigurationStorageTest extends StorageManagerTestBase {
journal.storeDivertConfiguration(new PersistedDivertConfiguration(configuration));
- journal.stop();
-
- journal.start();
+ rebootStorage();
List<PersistedDivertConfiguration> divertConfigurations = journal.recoverDivertConfigurations();
@@ -119,9 +104,5 @@ public class DivertConfigurationStorageTest extends StorageManagerTestBase {
assertEquals(configuration.getForwardingAddress(), persistedDivertConfiguration.getDivertConfiguration().getForwardingAddress());
assertEquals(configuration.getRoutingName(), persistedDivertConfiguration.getDivertConfiguration().getRoutingName());
assertNull(persistedDivertConfiguration.getDivertConfiguration().getTransformerConfiguration());
- journal.stop();
-
- journal = null;
-
}
}
diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/StorageManagerTestBase.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/StorageManagerTestBase.java
index a30f378109..a781fcaeca 100644
--- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/StorageManagerTestBase.java
+++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/StorageManagerTestBase.java
@@ -112,6 +112,16 @@ public abstract class StorageManagerTestBase extends ActiveMQTestBase {
journal.loadMessageJournal(new FakePostOffice(), null, null, null, null, null, null, null, new FakeJournalLoader());
}
+ /**
+ * This forces a reload of the journal from disk
+ *
+ * @throws Exception
+ */
+ protected void rebootStorage() throws Exception {
+ journal.stop();
+ createStorage();
+ }
+
/**
* @param configuration
*/
@@ -129,5 +139,4 @@ public abstract class StorageManagerTestBase extends ActiveMQTestBase {
addActiveMQComponent(jsm);
return jsm;
}
-
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@activemq.apache.org
For additional commands, e-mail: commits-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact