From bbf1baf3b26708c6e611c8ca870fd04d388d2363 Mon Sep 17 00:00:00 2001 From: Clebert Suconic Date: Tue, 31 Jan 2023 06:16:43 -0500 Subject: [PATCH] ARTEMIS-3178 Fixing validation on PagingStoreImpl for parameters set --- .../core/paging/impl/PagingStoreImpl.java | 7 +- .../config/impl/ConfigurationImplTest.java | 84 ++++++++++++++++--- 2 files changed, 76 insertions(+), 15 deletions(-) diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/impl/PagingStoreImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/impl/PagingStoreImpl.java index 137d18a21b7..675b85ee9d9 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/impl/PagingStoreImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/impl/PagingStoreImpl.java @@ -254,14 +254,15 @@ public void applySetting(final AddressSettings addressSettings) { pageLimitMessages = null; } - if (pageLimitBytes == null && pageLimitMessages == null && pageFullMessagePolicy != null) { ActiveMQServerLogger.LOGGER.noPageLimitsSet(address, pageFullMessagePolicy); this.pageFullMessagePolicy = null; } - if (pageLimitBytes != null && pageLimitMessages != null && pageFullMessagePolicy == null) { - ActiveMQServerLogger.LOGGER.noPagefullPolicySet(address, pageLimitBytes, pageLimitMessages); + if (pageFullMessagePolicy == null) { + if (pageLimitBytes != null || pageLimitMessages != null) { + ActiveMQServerLogger.LOGGER.noPagefullPolicySet(address, pageLimitBytes, pageLimitMessages); + } this.pageFullMessagePolicy = null; this.pageLimitMessages = null; this.pageLimitBytes = null; diff --git a/artemis-server/src/test/java/org/apache/activemq/artemis/core/config/impl/ConfigurationImplTest.java b/artemis-server/src/test/java/org/apache/activemq/artemis/core/config/impl/ConfigurationImplTest.java index 1f846cc1eb3..1ba4381c20b 100644 --- a/artemis-server/src/test/java/org/apache/activemq/artemis/core/config/impl/ConfigurationImplTest.java +++ b/artemis-server/src/test/java/org/apache/activemq/artemis/core/config/impl/ConfigurationImplTest.java @@ -1051,27 +1051,28 @@ public void testAddressSettingsPageLimitInvalidConfiguration1() throws Throwable String randomString = RandomUtil.randomString(); + // not setting pageFullMessagePolicy properties.put("addressSettings.#.expiryAddress", randomString); properties.put("addressSettings.#.pageLimitMessages", "300"); properties.put("addressSettings.#.pageLimitBytes", "300000"); - //properties.put("addressSettings.#.pageFullMessagePolicy", "DROP"); // removing the pageFull on purpose configuration.parsePrefixedProperties(properties, null); Assert.assertEquals(1, configuration.getAddressSettings().size()); Assert.assertEquals(SimpleString.toSimpleString(randomString), configuration.getAddressSettings().get("#").getExpiryAddress()); - Assert.assertEquals(300L, configuration.getAddressSettings().get("#").getPageLimitMessages().longValue()); - Assert.assertEquals(300000L, configuration.getAddressSettings().get("#").getPageLimitBytes().longValue()); + Assert.assertEquals((Long)300L, configuration.getAddressSettings().get("#").getPageLimitMessages()); + Assert.assertEquals((Long)300000L, configuration.getAddressSettings().get("#").getPageLimitBytes()); Assert.assertEquals(null, configuration.getAddressSettings().get("#").getPageFullMessagePolicy()); PagingStore storeImpl = new PagingStoreImpl(new SimpleString("Test"), (ScheduledExecutorService) null, 100L, Mockito.mock(PagingManager.class), Mockito.mock(StorageManager.class), Mockito.mock(SequentialFileFactory.class), Mockito.mock(PagingStoreFactory.class), new SimpleString("Test"), configuration.getAddressSettings().get("#"), null, null, true); - Assert.assertTrue(AssertionLoggerHandler.findText("AMQ224125")); Assert.assertEquals(null, storeImpl.getPageLimitMessages()); Assert.assertEquals(null, storeImpl.getPageLimitBytes()); Assert.assertEquals(null, storeImpl.getPageFullMessagePolicy()); + Assert.assertTrue(AssertionLoggerHandler.findText("AMQ224125")); } + @Test public void testAddressSettingsPageLimitInvalidConfiguration2() throws Throwable { AssertionLoggerHandler.startCapture(); @@ -1082,10 +1083,69 @@ public void testAddressSettingsPageLimitInvalidConfiguration2() throws Throwable String randomString = RandomUtil.randomString(); + // pageLimitBytes and pageFullMessagePolicy not set on purpose + properties.put("addressSettings.#.expiryAddress", randomString); + properties.put("addressSettings.#.pageLimitMessages", "300"); + + configuration.parsePrefixedProperties(properties, null); + + Assert.assertEquals(1, configuration.getAddressSettings().size()); + Assert.assertEquals(SimpleString.toSimpleString(randomString), configuration.getAddressSettings().get("#").getExpiryAddress()); + Assert.assertEquals((Long)300L, configuration.getAddressSettings().get("#").getPageLimitMessages()); + Assert.assertEquals(null, configuration.getAddressSettings().get("#").getPageLimitBytes()); + Assert.assertEquals(null, configuration.getAddressSettings().get("#").getPageFullMessagePolicy()); + + PagingStore storeImpl = new PagingStoreImpl(new SimpleString("Test"), (ScheduledExecutorService) null, 100L, Mockito.mock(PagingManager.class), Mockito.mock(StorageManager.class), Mockito.mock(SequentialFileFactory.class), Mockito.mock(PagingStoreFactory.class), new SimpleString("Test"), configuration.getAddressSettings().get("#"), null, null, true); + + Assert.assertEquals(null, storeImpl.getPageLimitMessages()); + Assert.assertEquals(null, storeImpl.getPageLimitBytes()); + Assert.assertEquals(null, storeImpl.getPageFullMessagePolicy()); + Assert.assertTrue(AssertionLoggerHandler.findText("AMQ224125")); + } + + @Test + public void testAddressSettingsPageLimitInvalidConfiguration3() throws Throwable { + AssertionLoggerHandler.startCapture(); + runAfter(AssertionLoggerHandler::stopCapture); + ConfigurationImpl configuration = new ConfigurationImpl(); + + Properties properties = new Properties(); + + String randomString = RandomUtil.randomString(); + + // pageLimitMessages and pageFullMessagePolicy not set on purpose + properties.put("addressSettings.#.expiryAddress", randomString); + properties.put("addressSettings.#.pageLimitBytes", "300000"); // removing this on purpose + + configuration.parsePrefixedProperties(properties, null); + + Assert.assertEquals(1, configuration.getAddressSettings().size()); + Assert.assertEquals(SimpleString.toSimpleString(randomString), configuration.getAddressSettings().get("#").getExpiryAddress()); + Assert.assertEquals(null, configuration.getAddressSettings().get("#").getPageLimitMessages()); + Assert.assertEquals((Long)300000L, configuration.getAddressSettings().get("#").getPageLimitBytes()); + Assert.assertEquals(null, configuration.getAddressSettings().get("#").getPageFullMessagePolicy()); + + PagingStore storeImpl = new PagingStoreImpl(new SimpleString("Test"), (ScheduledExecutorService) null, 100L, Mockito.mock(PagingManager.class), Mockito.mock(StorageManager.class), Mockito.mock(SequentialFileFactory.class), Mockito.mock(PagingStoreFactory.class), new SimpleString("Test"), configuration.getAddressSettings().get("#"), null, null, true); + + Assert.assertEquals(null, storeImpl.getPageLimitMessages()); + Assert.assertEquals(null, storeImpl.getPageLimitBytes()); + Assert.assertEquals(null, storeImpl.getPageFullMessagePolicy()); + Assert.assertTrue(AssertionLoggerHandler.findText("AMQ224125")); + } + + @Test + public void testAddressSettingsPageLimitInvalidConfiguration4() throws Throwable { + AssertionLoggerHandler.startCapture(); + runAfter(AssertionLoggerHandler::stopCapture); + ConfigurationImpl configuration = new ConfigurationImpl(); + + Properties properties = new Properties(); + + String randomString = RandomUtil.randomString(); + + // leaving out pageLimitMessages and pageLimitBytes properties.put("addressSettings.#.expiryAddress", randomString); - //properties.put("addressSettings.#.pageLimitMessages", "300"); // removing this on purpose - //properties.put("addressSettings.#.pageLimitBytes", "300000"); // removing this on purpose - properties.put("addressSettings.#.pageFullMessagePolicy", "DROP"); // keeping this on purpose + properties.put("addressSettings.#.pageFullMessagePolicy", "DROP"); configuration.parsePrefixedProperties(properties, null); @@ -1096,16 +1156,16 @@ public void testAddressSettingsPageLimitInvalidConfiguration2() throws Throwable Assert.assertEquals("DROP", configuration.getAddressSettings().get("#").getPageFullMessagePolicy().toString()); PagingStore storeImpl = new PagingStoreImpl(new SimpleString("Test"), (ScheduledExecutorService) null, 100L, Mockito.mock(PagingManager.class), Mockito.mock(StorageManager.class), Mockito.mock(SequentialFileFactory.class), Mockito.mock(PagingStoreFactory.class), new SimpleString("Test"), configuration.getAddressSettings().get("#"), null, null, true); - Assert.assertTrue(AssertionLoggerHandler.findText("AMQ224124")); Assert.assertEquals(null, storeImpl.getPageLimitMessages()); Assert.assertEquals(null, storeImpl.getPageLimitBytes()); Assert.assertEquals(null, storeImpl.getPageFullMessagePolicy()); + Assert.assertTrue(AssertionLoggerHandler.findText("AMQ224124")); } @Test - public void testAddressSettingsPageLimitInvalidConfiguration3() throws Throwable { + public void testAddressSettingsPageLimitInvalidConfiguration5() throws Throwable { ConfigurationImpl configuration = new ConfigurationImpl(); Properties properties = new Properties(); @@ -1113,9 +1173,9 @@ public void testAddressSettingsPageLimitInvalidConfiguration3() throws Throwable String randomString = RandomUtil.randomString(); properties.put("addressSettings.#.expiryAddress", randomString); - properties.put("addressSettings.#.pageLimitMessages", "-1"); // -1 on purpose, to make it null on final parsing - properties.put("addressSettings.#.pageLimitBytes", "-1"); // -1 on purpose, to make it null on final parsing - properties.put("addressSettings.#.pageFullMessagePolicy", "DROP"); // keeping this on purpose + properties.put("addressSettings.#.pageLimitMessages", "-1"); // this should make the PagingStore to parse it as null + properties.put("addressSettings.#.pageLimitBytes", "-1"); // this should make the PagingStore to parse it as null + properties.put("addressSettings.#.pageFullMessagePolicy", "DROP"); configuration.parsePrefixedProperties(properties, null);