[pip][design] PIP-280: Refactor CLI for measurement units (time, byte)

pip/pip-280.md

@@ -0,0 +1,119 @@
+# Title: Refactor CLI Argument Parsing Logic for Measurement Units using JCommander's custom converter
+
+## Motivation
+
+In the current Pulsar codebase, the logic to parse CLI arguments for measurement units like time and bytes is
+scattered across various CLI classes. Each value read has its distinct parsing implementation, leading to a lack of code
+reuse.
+
+## Goals
+
+This PIP is to refactor the argument parsing logic to leverage the `@Parameter.converter`
+functionality provided by JCommander [link 3]. This will isolate the measurement-specific parsing logic and increase
+code
+reusability.
+
+### In Scope
+
+- Refactor all `Cmd` classes to utilize the converter functionality of JCommander. This will streamline the parsing
+  logic and simplify the codebase.
+- Refer to bottom section "Concrete Example", before "Links"
+- Or on-going PR with small use case in https://github.com/apache/pulsar/pull/20663
+
+### Out of Scope
+
+- Creation of a "util" module is out of the scope of this PIP.
+
+## Design & Implementation Details
+
+- The refactoring will be carried out on a class-by-class basis or per inner-class basis.
+- Target command classes for this refactoring include
+    - `CmdNamespaces.java`
+    - `CmdTopics.java`,
+    - `CmdTopicPolicies.java`.
+
+## Note
+
+- Additional classes may be included as the refactoring progresses.
+- Respective PRs will be added here also.
+- The refactoring should not introduce any breaking change
+- New parameters should be covered by unit test (at least by existing and preferably new)
+
+## Concrete Example
+
+Consider the code snippet
+from [CmdNamespaces.java](https://github.com/apache/pulsar/blob/200fb562dd4437857ccaba3850bd64b0a9a50b3c/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdNamespaces.java#L2352-L2359)
+for example. The existing code uses a local variable `maxBlockSizeStr` to temporarily store the value
+of `--maxBlockSize` or `-mbs`. This is then parsed and validated in a separate section of the code.
+
+### BEFORE
+
+```java
+  @Parameter(
+        names = {"--maxBlockSize", "-mbs"},
+        description = "Max block size (eg: 32M, 64M), default is 64MB s3 and google-cloud-storage requires this parameter",
+        required = false)
+private String maxBlockSizeStr;
+```
+
+parsing like below ....
+
+```java
+    // parsing like....
+    int maxBlockSizeInBytes=OffloadPoliciesImpl.DEFAULT_MAX_BLOCK_SIZE_IN_BYTES;
+    if(StringUtils.isNotEmpty(maxBlockSizeStr)){
+        long maxBlockSize=validateSizeString(maxBlockSizeStr);
+        if(positiveCheck("MaxBlockSize",maxBlockSize)
+            &&maxValueCheck("MaxBlockSize",maxBlockSize,Integer.MAX_VALUE)) {
+            maxBlockSizeInBytes=Long.valueOf(maxBlockSize).intValue();
+        }
+    }
+```
+
+### AFTER
+
+```java
+    @Parameter(
+        names = {"--maxBlockSize", "-mbs"},
+        description = "Max block size (eg: 32M, 64M), default is 64MB s3 and google-cloud-storage requires this parameter",
+        required = false, converter = MemoryUnitToByteConverter.class) // <--- parsing logic "inline" easy to follow
+private long maxBlockSizeStr=DEFAULT_MAX_BLOCK_SIZE_IN_BYTES; // <---- default value in line
+```
+
+... and actual parsing in isolation, ready for reuse like...
+
+```java
+class MemoryUnitToByteConverter implements IStringConverter<Long> {
+
+    private static Set<Character> sizeUnit = Sets.newHashSet('k', 'K', 'm', 'M', 'g', 'G', 't', 'T');
+    private final long defaultValue;
+
+    public MemoryUnitToByteConverter(long defaultValue) {
+        this.defaultValue = defaultValue;
+    }
+
+    @Override
+    public Long convert(String memoryLimitArgument) {
+        parseBytes(memoryLimitArgument);
+    }
+
+    long parseBytes(String memoryLimitArgument) {
+        if (StringUtils.isNotEmpty(memoryLimitArgument)) {
+            long memoryLimitArg = validateSizeString(memoryLimitArgument);
+            if (positiveCheckStatic("memory-limit", memoryLimitArg)) {
+                return memoryLimitArg;
+            }
+        }
+        return defaultValue;
+    }
+    ...
+    more internal
+    helper methods
+}
+```
+
+## Links
+
+- Mailing List discussion thread: https://lists.apache.org/thread/b77bfnjlt62w7zywcs8tqklvyokpykok
+- Mailing List voting thread: https://lists.apache.org/thread/0r3bh0h7f86g2x9odvrd1fp2gwddq904
+- [3] https://jcommander.org/#_custom_types_converters_and_splitters