gemmellr commented on code in PR #4966:
URL: https://github.com/apache/activemq-artemis/pull/4966#discussion_r1701646452
##########
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/RandomUtil.java:
##########
@@ -60,6 +60,10 @@ public static int randomPositiveInt() {
return Math.abs(RandomUtil.randomInt());
}
+ public static int randomPowerOfTwo() {
+ return (int) Math.pow(2, RandomUtil.randomInterval(1, 10));
+ }
+
Review Comment:
This seems to be restricted to not returning anything above 1024 (or 1,
though thats more understandable for most uses of such a method), leaving out a
fair number of values. I can see why that might be desirable in some tests
given the aimed use case, but I dont think it makes sense to introduce the
method with that unspecified behaviour this class (which I'd like to
move/disappear long term given its main uses). It should either be made more
general or I would omit adding the method here and just do whats desired in the
test.
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/config/impl/Validators.java:
##########
@@ -107,6 +107,14 @@ public interface Validator<T> {
}
};
+ public static final Validator<Number> POSITIVE_POWER_OF_TWO = (name, value)
-> {
+ if ((value.longValue() & (value.longValue() - 1)) == 0 &&
value.longValue() > 0) {
+ return value;
+ } else {
+ throw ActiveMQMessageBundle.BUNDLE.powerOfTwo(name, value);
Review Comment:
if the validator is being called POSITIVE_POWER_OF_TWO would it make sense
for the exception bundle method to be specific as well?
##########
artemis-server/src/test/java/org/apache/activemq/artemis/core/config/impl/ValidatorsTest.java:
##########
@@ -152,4 +152,14 @@ public void testTWO_CHARACTERS() {
ValidatorsTest.success(Validators.NULL_OR_TWO_CHARACTERS, null);
}
+ @Test
+ public void testPOSITIVE_POWER_OF_TWO() {
+ ValidatorsTest.failure(Validators.POSITIVE_POWER_OF_TWO, 0);
+ ValidatorsTest.failure(Validators.POSITIVE_POWER_OF_TWO, -10);
+ ValidatorsTest.failure(Validators.POSITIVE_POWER_OF_TWO, 127);
+
+ ValidatorsTest.success(Validators.POSITIVE_POWER_OF_TWO, 2);
+ ValidatorsTest.success(Validators.POSITIVE_POWER_OF_TWO, 64);
Review Comment:
Would check the high extreme values that are expected to fail and work
respectively.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information, visit: https://activemq.apache.org/contact
