jmeter-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Emilian Bold <emilian.b...@gmail.com>
Subject Re: Checking values of properties more thoroughly
Date Sun, 16 Jul 2017 13:44:03 GMT
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.

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));

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.

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
).


--emi


On Sun, Jul 16, 2017 at 4:13 PM, Felix Schumacher
<felix.schumacher@internetallee.de> 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
>

Mime
View raw message