This is an automated email from the ASF dual-hosted git repository. lhotari pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/pulsar.git
The following commit(s) were added to refs/heads/master by this push: new fa8aa9ef705 [fix][cli] Fix set-retention with >2GB size value for topic policy (#23689) fa8aa9ef705 is described below commit fa8aa9ef7056756bcb4cd04d140b5c93a28a045f Author: Lari Hotari <lhot...@users.noreply.github.com> AuthorDate: Sat Dec 7 13:11:11 2024 +0200 [fix][cli] Fix set-retention with >2GB size value for topic policy (#23689) --- .../apache/pulsar/admin/cli/CmdTopicPolicies.java | 9 +++--- .../org/apache/pulsar/admin/cli/CmdTopics.java | 8 ++--- ...mdNamespaces.java => CmdTopicPoliciesTest.java} | 35 ++++++++++------------ .../apache/pulsar/admin/cli/TestCmdNamespaces.java | 5 ++-- .../org/apache/pulsar/admin/cli/TestCmdTopics.java | 7 +++++ 5 files changed, 33 insertions(+), 31 deletions(-) diff --git a/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopicPolicies.java b/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopicPolicies.java index 10850d107ed..9a5714cb58c 100644 --- a/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopicPolicies.java +++ b/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopicPolicies.java @@ -29,7 +29,6 @@ import java.util.concurrent.TimeUnit; import java.util.function.Supplier; import java.util.stream.Collectors; import org.apache.commons.lang3.StringUtils; -import org.apache.pulsar.cli.converters.picocli.ByteUnitToIntegerConverter; import org.apache.pulsar.cli.converters.picocli.ByteUnitToLongConverter; import org.apache.pulsar.cli.converters.picocli.TimeUnitToMillisConverter; import org.apache.pulsar.cli.converters.picocli.TimeUnitToSecondsConverter; @@ -546,8 +545,8 @@ public class CmdTopicPolicies extends CmdBase { + "For example, 4096, 10M, 16G, 3T. The size unit suffix character can be k/K, m/M, g/G, or t/T. " + "If the size unit suffix is not specified, the default unit is bytes. " + "0 or less than 1MB means no retention and -1 means infinite size retention", required = true, - converter = ByteUnitToIntegerConverter.class) - private Integer sizeLimit; + converter = ByteUnitToLongConverter.class) + private Long sizeLimit; @Option(names = { "--global", "-g" }, description = "Whether to set this policy globally. " + "If set to true, the policy is replicated to other clusters asynchronously, " @@ -560,8 +559,8 @@ public class CmdTopicPolicies extends CmdBase { final int retentionTimeInMin = retentionTimeInSec != -1 ? (int) TimeUnit.SECONDS.toMinutes(retentionTimeInSec) : retentionTimeInSec.intValue(); - final int retentionSizeInMB = sizeLimit != -1 - ? (int) (sizeLimit / (1024 * 1024)) + final long retentionSizeInMB = sizeLimit != -1 + ? (sizeLimit / (1024 * 1024)) : sizeLimit; getTopicPolicies(isGlobal).setRetention(persistentTopic, new RetentionPolicies(retentionTimeInMin, retentionSizeInMB)); diff --git a/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopics.java b/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopics.java index 8dd6b664462..22073b1a89d 100644 --- a/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopics.java +++ b/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopics.java @@ -1858,8 +1858,8 @@ public class CmdTopics extends CmdBase { + "For example, 4096, 10M, 16G, 3T. The size unit suffix character can be k/K, m/M, g/G, or t/T. " + "If the size unit suffix is not specified, the default unit is bytes. " + "0 or less than 1MB means no retention and -1 means infinite size retention", required = true, - converter = ByteUnitToIntegerConverter.class) - private Integer sizeLimit; + converter = ByteUnitToLongConverter.class) + private Long sizeLimit; @Override void run() throws PulsarAdminException { @@ -1867,8 +1867,8 @@ public class CmdTopics extends CmdBase { final int retentionTimeInMin = retentionTimeInSec != -1 ? (int) TimeUnit.SECONDS.toMinutes(retentionTimeInSec) : retentionTimeInSec.intValue(); - final int retentionSizeInMB = sizeLimit != -1 - ? (int) (sizeLimit / (1024 * 1024)) + final long retentionSizeInMB = sizeLimit != -1 + ? (sizeLimit / (1024 * 1024)) : sizeLimit; getTopics().setRetention(persistentTopic, new RetentionPolicies(retentionTimeInMin, retentionSizeInMB)); } diff --git a/pulsar-client-tools/src/test/java/org/apache/pulsar/admin/cli/TestCmdNamespaces.java b/pulsar-client-tools/src/test/java/org/apache/pulsar/admin/cli/CmdTopicPoliciesTest.java similarity index 67% copy from pulsar-client-tools/src/test/java/org/apache/pulsar/admin/cli/TestCmdNamespaces.java copy to pulsar-client-tools/src/test/java/org/apache/pulsar/admin/cli/CmdTopicPoliciesTest.java index f9ce84411c6..59d3be39019 100644 --- a/pulsar-client-tools/src/test/java/org/apache/pulsar/admin/cli/TestCmdNamespaces.java +++ b/pulsar-client-tools/src/test/java/org/apache/pulsar/admin/cli/CmdTopicPoliciesTest.java @@ -18,35 +18,30 @@ */ package org.apache.pulsar.admin.cli; -import org.apache.pulsar.client.admin.Namespaces; -import org.apache.pulsar.client.admin.PulsarAdmin; -import org.apache.pulsar.common.policies.data.RetentionPolicies; -import org.testng.annotations.AfterMethod; -import org.testng.annotations.Test; -import java.io.IOException; +import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import org.apache.pulsar.client.admin.PulsarAdmin; +import org.apache.pulsar.client.admin.TopicPolicies; +import org.apache.pulsar.common.policies.data.RetentionPolicies; +import org.testng.annotations.Test; -public class TestCmdNamespaces { - - @AfterMethod(alwaysRun = true) - public void cleanup() throws IOException { - //NOTHING FOR NOW - } - +public class CmdTopicPoliciesTest { @Test public void testSetRetentionCmd() throws Exception { - Namespaces namespaces = mock(Namespaces.class); + TopicPolicies topicPolicies = mock(TopicPolicies.class); PulsarAdmin admin = mock(PulsarAdmin.class); - when(admin.namespaces()).thenReturn(namespaces); + when(admin.topicPolicies(anyBoolean())).thenReturn(topicPolicies); - CmdNamespaces cmd = new CmdNamespaces(() -> admin); + CmdTopicPolicies cmd = new CmdTopicPolicies(() -> admin); - cmd.run("set-retention public/default -s 2T -t 2h".split("\\s+")); - verify(namespaces, times(1)).setRetention("public/default", new RetentionPolicies(120, 2 * 1024 * 1024)); - } -} + cmd.run("set-retention public/default/topic -s 2T -t 200d".split("\\s+")); + + verify(topicPolicies, times(1)).setRetention("persistent://public/default/topic", + new RetentionPolicies(200 * 24 * 60, 2 * 1024 * 1024)); + } +} \ No newline at end of file diff --git a/pulsar-client-tools/src/test/java/org/apache/pulsar/admin/cli/TestCmdNamespaces.java b/pulsar-client-tools/src/test/java/org/apache/pulsar/admin/cli/TestCmdNamespaces.java index f9ce84411c6..4ed0880d29e 100644 --- a/pulsar-client-tools/src/test/java/org/apache/pulsar/admin/cli/TestCmdNamespaces.java +++ b/pulsar-client-tools/src/test/java/org/apache/pulsar/admin/cli/TestCmdNamespaces.java @@ -46,7 +46,8 @@ public class TestCmdNamespaces { CmdNamespaces cmd = new CmdNamespaces(() -> admin); - cmd.run("set-retention public/default -s 2T -t 2h".split("\\s+")); - verify(namespaces, times(1)).setRetention("public/default", new RetentionPolicies(120, 2 * 1024 * 1024)); + cmd.run("set-retention public/default -s 2T -t 200d".split("\\s+")); + verify(namespaces, times(1)).setRetention("public/default", + new RetentionPolicies(200 * 24 * 60, 2 * 1024 * 1024)); } } diff --git a/pulsar-client-tools/src/test/java/org/apache/pulsar/admin/cli/TestCmdTopics.java b/pulsar-client-tools/src/test/java/org/apache/pulsar/admin/cli/TestCmdTopics.java index fc98b14392c..bd926edc5a8 100644 --- a/pulsar-client-tools/src/test/java/org/apache/pulsar/admin/cli/TestCmdTopics.java +++ b/pulsar-client-tools/src/test/java/org/apache/pulsar/admin/cli/TestCmdTopics.java @@ -50,6 +50,7 @@ import org.apache.pulsar.client.admin.Topics; import org.apache.pulsar.client.impl.MessageIdImpl; import org.apache.pulsar.common.naming.TopicDomain; import org.apache.pulsar.common.policies.data.ManagedLedgerInternalStats.LedgerInfo; +import org.apache.pulsar.common.policies.data.RetentionPolicies; import org.mockito.Mockito; import org.testng.Assert; import org.testng.annotations.AfterMethod; @@ -260,4 +261,10 @@ public class TestCmdTopics { mockTopics = mock(Topics.class); } + @Test + public void testSetRetentionCmd() throws Exception { + cmdTopics.run("set-retention public/default/topic -s 2T -t 200d".split("\\s+")); + verify(mockTopics, times(1)).setRetention("persistent://public/default/topic", + new RetentionPolicies(200 * 24 * 60, 2 * 1024 * 1024)); + } }