Am 16.07.2017 um 15:44 schrieb Emilian Bold:
I don't know the codebase well, but I wouldn't add a validator to
getPropDefault.
The reason being that I expect getPropDefault to already return a
valid value or *my* default which I also know it's valid.
The getPropDefault looks for values inside text files and tries to find
one matching the name of the first argument. If it can't find one, it
gives back the default value (which is the second argument).
So, validation should happen at a layer bellow getPropDefault.
Also, having the validator in there is not very good if you don't have
a constant. For example, I see in JMeter the call:
RemoteJMeterEngineImpl.startServer(JMeterUtils.getPropDefault("server_port",
0));
Than this is more an argument against this specific usage of
getPropDefault, isn't it?
If you have the same key accessed multiple times with getPropDefault
they each would have to register a potentially different validator.
A solution would be to validate the whole configuration file at start up.
Where would you register/configure the validators? What happens with
third party modules, that would want to be checked, too?
In order to have the validators you could use an annotation processor
to gather some validation annotations from the codebase. Eg.
@PositiveOrZero
private static int TIMEOUT =
JMeterUtils.getPropDefault("some.timeout", 5 * 1000);
Where @ PositiveOrZero is something I took from BeanValidation but we
don't have to use that (
http://beanvalidation.org/latest-draft/spec/#builtinconstraints-positiveorzero
).
Right a annotation would have been my first choice.
But it would basically have the same issues like my proposed first
version and we don't use annotations in jmeter (that is up to now).
How would you integrate these annotations into our codebase?
Felix
--emi
On Sun, Jul 16, 2017 at 4:13 PM, Felix Schumacher
<[email protected]> wrote:
While looking for easy enhancements and bugs I stumbled upon:
https://bz.apache.org/bugzilla/show_bug.cgi?id=56862
I have to admit, that the error messages for wrong values are not always
that helpful. Especially, when those values are used very late.
To help things, we could either add a validate function (lambda?) to the
JMeterUtils#getPropDefault family of functions, or surround those calls by a
validator.
I was thinking of something like
private static int TIMEOUT = JMeterUtils.getPropDefault("some.timeout", 5 *
1000, i -> i >= 0);
private static int TIMEOUT = JMeterUtils.getPropDefault("some.timeout", 5 *
1000, Validator::notNegative);
or
private static int TIMEOUT =
Validator.notNegative(JMeterUtils.getPropDefault("some.timeout", 5 * 1000),
"some.timeout must not be negative");
At the moment I prefer the first variants.
What do you think?
Felix
