Make possible to run a custom method at the end of the constructor generated by @AllArgsConstructor annotation

[Eli]

Hi guys,

My name is Elemer, and I work on a SpringBoot project. I use lombok to avoid boilerplate code.

The company I work for did setup SonarQube with default rules on the build server.

I have the following constraints:

- I may not modify any of the Sonar rules on the Jenkins build server

- I have a business object with 12 instance fields for which I have a 12-args constructor

- At the end of the 12-args constructor I need to call a private method.

- The 12-args constructor is called implicitly by a JPA framework and not manually!

- Sonar does not accept more then 7 arguments on a constructor

So, I decided to use lombok's @AllArgsConstructor to avoid having a constructor with too many arguments.

But my problem is that I also need to call that private method at the end of the constructor, before it returns.

Do you have any idea how can I solve this issue?

Thanks a lot,

Elemer.

[Reinier Zwitserloot]

Our primary problem here is that there are a number of ways to do this, and the simple solution ('just' call a method at the end) is just one of many solutions, and whilst it is the simplest, it doesn't really feel like the obvious elegant solution.

This really boils down to three things:

1. validation (the ability to throw exceptions on invalid input)

2. normalization (the ability to modify values)

3. processing (the ability to do stuff, without modifying anything, such as log the fact that the object was created).

In many ways, doing it per-field is simpler and cleaner: In a hypothetical 'Person' class with, say, 12 fields including 'length' and 'dateOfBirth' have a method 'validateLength' which for example throws if length is negative.

For the validation concern, it is useful for the object to never exist in an invalid state. The 'solution' of calling a single catch-all validation method at the end fails this rule, sort of: The object DOES exist at that point, although of course if you throw an exception it won't. This becomes more crucial with builders: It's just better if the builder itself exits the process with an exception prior to even calling the constructor, if possible. Still, just for validation, 'call one method at the end' seems fair enough. It gets more complicated with setters and withers: Everytime the entire object is re-validated, which may not be desired. Note also that there are a ton of validation frameworks out there, many of them annotation-based (you'd put something like @MinInt(0) or some such on the field to indicate it must not be negative) – how would lombok interact with these?

Normalization: MUUUCH worse. Your solution just won't work, at all, unless you have non-final fields, which isn't always a luxury you can afford, or even if it is, having to make a field non-final solely because you want to use lombok's 'validate/normalize/process' system and it REQUIRES that you make them non-final, sucks. It would be much nicer if lombok did something like: "this.length = normalizeLength(paramLength)", letting you supply the normalizeLength method. The solution of 'call one no-args private method at the end' is just not suitable here.

processing: generally, the one-no-args-method solution is fine for this use case.

You can go on a per-field basis which tends to lead to more efficient and more understandable, modularized code ('this method JUST validates the length field, nothing more'), but it is basically impossible to validate/normalize/process a crosscut of multiple values (let's say this class modules some wine selling store, and a Person object should be deemed invalid if the combination of dateOfBirth and residenceCountry combine to indicate that person is below legal drinking age for said country. You can't validate that unless you have both dateOfBirth and residenceCountry).

For some use-cases, per-field is great, for others, all-at-once is great. So, what do we do? Pick one side and leave the use cases we don't cover out in the cold? Do both and make it a really convoluted feature?

There are also more convoluted, creative solutions available. What if the 'validate' method gets all constructor parameters injected, thus letting you validate BEFORE construction without introducing a ton of boilerplate? What if you can mark any method you please as a validator, and lombok dynamically scans the method for all uses of fields, and will automatically detect from there that you are a single-field validator and thus lombok will inject a call to it immediately after calling the 'builder-setter' for it? Or if a certain field is NOT touched by your validator and you call .setThatField(), that lombok does NOT call this validator? Except if your validator passes 'this' around to code we can't see, there's no way for us to know what really happens, so now lombok kind of has to warn that your validator is 'too complex' to analyse, and this all becomes an incredibly complicated feature.

Long story short: It feels like there's a right way to do this properly, but we haven't yet stumbled across it. "Call one no-args private instance method at the very end of the generated constructor" isn't it, though. It might fit your use case, but it fails to address many more.

[Marco Servetto]

Hi Reinier.
This mail is long but is very important.
I think we should embrace the 'processing' case, use the builder for
the normalization case and
DO NOT ATTEMPT validation.
If a user want to use processing for 'validation after the
constructor' is their own choice, and it can be reasonable if they
know what they are doing or they are playing with building deeply
immutable objects from deeply immutable parameters. But I do not think
we should 'claim' to support validation.
Allowing the call of the method at the end of the constructor removes
a lot of boilerplate and allows user to attempt validating in the way
they want, using the logic they may think is reasonable for their
application.
More than this and Lombok would trying to get out of its role of
'boilerplate buster' and would start trying to tell users how to code.
That is ok for equals and hashcode, but not for validation, as
discussed below.

Validation as in 'every instance of the class will always respect
predicate p', where p is encoded as Java code is a very hard problem
to solve correctly.
Especially if you want to get the clean 'you can never see a broken
object' semantic.
I'm actively researching on those topics in the last 2 years, and
I would argue that in Java it simply is not practical to do it soundly:
-testing if a boolean method p work in a certain moment give you little info:
*the method can be non deterministic (can you just ask the users to
take care of this?)
*aliasing can be used to change the inner objects without using
get/set directly
*even if the object has just (non final) int fields, you can
set-break the object, capture whatever validation exception is throw,
and now the object is visible in a broken state (you had an idea in
the past about reverting the failed setter, this has problems if the
object was being modified as part of a larger logical 'transaction',
where we expected multiple fields to be mutated atomically to keep the
overall semantic ok)

-----------Long text following-------------------
Let's make an example:
I have a Rectangle class with two Point fields: topLeft and bottomRight.
we would like topLeft to actually be more top and more left that the
bottomRight point.
In the following, ACTION is what Lombok can do, CONTRACT is what the
user of lombok has to respect.

simple case: both Point and Rectangle are final classes with only final fields.
ACTION: just check at the end of the constructor.
CONTRACT: the predicate method need to be deterministic, and need to
not leak this out. The contract method is now the only method that can
receive a broken 'this'.

challenging: as above but Rectangle have setters for the two points
ACTION: check the predicate at the end of constructor and after setters.
CONTRACT: as above, plus capturing the exception from validation
failure may expose broken Rectangles
or
ACTION: check the predicate at the end of constructor and after
setters, try-catch setters to revert the update
CONTRACT: as above, plus capturing the exception from validation
failure may expose (correct) Rectangles that have been only partially
modified. This is bad but is a general problem with exception (lack of
Strong exception safety)

hard: the Point class has non final fields
ACTION: check the predicate at the end of constructor and after setters??
CONTRACT: as above, plus as for keys in hash maps: while an object is
inside of a Rectangle, the object can not mutate in a way that
influence the result of the predicate. This is an unreasonable request
to the user, but Java request this already for keys in has maps.

hardest: the Point class is not final, so we may have EvilPoint extends Point
ACTION: check the predicate at the end of constructor and after setters??
CONTRACT: as above, but now is really impossible for the poor user to
respect the contract.
Basically, when a new Point is set, there is absolutely no way we can
guarantee the actual type of Point,
thus any predicate using methods on such point can be non
deterministic or mutate the Point unexpectedly

In addition to all the above, if you ever try to validate a circular
object graph, then you have 'random' invalid objects that can be
reachable (at least from the validate method itself)

Keep in mind that every object with a List<T> field has a possibly
mutable, possibly circular, object graph, whose behavior can possibly
be non deterministic.

Concluding: Every solution in plain java REQUIRES the user/programmer
to make sure that certain properties hold:
those properties are intrinsically non modular, hard to maintain,
impossible to verify.
Thus, in plain java, validation is, and will stay, a case-by case
beast where programmers try to make it work for the specific case at
hand, without hope for generality.

Marco.

[Elemér Zágoni]

Hi Reinier,

Thanks a lot for your quick answer.

Maybe I was not clear enough in explaining my primary goal.

My main goal here is to prevent SonarCube on signaling me the error of "Constructor has too many arguments: 12, expected no more than 7"

Normally I'd achieve my goal by entering SonarCube's config page and simpli disabling that rule, or maybe raising the limit of 7 to 12.

BUT: I have no access rights to Sonar's config page on the build server.

So I'm struggling to find any alternative solution(s).

My first idea has been to add @Builder annotation to the class, and use instead of constructor the fluent way of MyClass.builder().fld1().fld2(). ... fld12().build.

But neither this is a solution for me because the 12- args constructor needs to be called by JPA internally when returning the results of a 12 field @Query.

So, I must have a 12-args constructor ...

An here popped in the idea to put Lombok to silently generate the 12-args constructor for me in the background, so this way (since the codebase's source does not contain the constructor ==> Sonar could not complain.)

But here I have a Proje specific need that each time when creating a new instance of My Class through that constructor, I also must calculate some inner business key for it with a private computeKey() method without arguments.

That's why I thought that maybe Lombok has some more flexibility while creating a constructor it nay have a parameter in which I could also specify some additional call to be executed.

Now you know the whole background so maybe you've got any idea on how can I make Sonar Happy ... 

P.S.

//NOSONAR does not work either.

Thanks a lot again:

Elemér.

--
You received this message because you are subscribed to the Google Groups "Project Lombok" group.
To unsubscribe from this group and stop receiving emails from it, send an email to project-lombo...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/project-lombok/577cb08c-cc64-494d-a81d-7d6b6271ab57%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Mat Jaggard]

Is @SuppressWarnings("squid:S00107") on a manually generated constructor good enough for you?

To view this discussion on the web visit https://groups.google.com/d/msgid/project-lombok/CAEXP%3DLnNUPPEONqJtyLccFPQQPpq38ca4ZX9PQTUfCTnAE8%2Bwg%40mail.gmail.com.

[Elemér Zágoni]

Thanks a lot Mat, it works!

Have a great day. :)

I'm happy now :)

Elemér.

To view this discussion on the web visit https://groups.google.com/d/msgid/project-lombok/CAD0R3sAeZjG-tJi-GVN-1OkcrOTQUFsAbp6_81hOpX48c2_G6g%40mail.gmail.com.

[Reinier Zwitserloot]

@Eli: Here's lombok's standing policy vs. linters:

Lombok will generate various @PleaseIgnoreThisCodeForYourLinterAnalysis annotations. You can use the lombok.config system to generate even more.

Tell your linter to respect this annotation. Alternatively, turn it off. We will accept no feature requests or pull requests to avoid linter errors, because linter rules are mutually exclusive (one person says that all method params must be final, another says that they must never be. This state of affairs is possible because there are many linting tools out there, and most are configurable).

This particular sonarcube 'error' (12 params on a constructor) is also either idiotic or true: If the constructor is private, sonarcube is idiotic (it's private API, why make an API complaint? That makes no sense) or it is true: If this constructor is public, the fact that lombok made it makes no difference: Your API is now, at least according to your linter's rules, itself overly convoluted. "But, lombok generated it!" is no excuse.

So, in this case, you should tell sonarcube or the manager of your sonarcube config that they've done something silly. Lombok as a project obviously cannot cater to intentionally idiotic configurations.

To unsubscribe from this group and stop receiving emails from it, send an email to project-lombok+unsubscribe@googlegroups.com.

[Reinier Zwitserloot]

Good points, Marco, but the problem is: YOU get that, and we can certainly write blogposts and documentation to explain: Yeah yeah validation seems easy but trust us it is an intractible problem and it was stupid of you to think lombok worked that way, but...

people would think lombok worked that way.

The problem that really irks me with the 'one post-processing method' is that obviously this validator/postprocessor method ought to run after every setter, except the simple solution of running one no-args method means the change in value already went through and THEN the postprocessor can pipe up and go: Rut roh, not valid.

So, we have to go out of our way to say: This one method we invoke at the very end is JUST FOR PROCESSING and nothing else!

Except like 90%+ of those who want this feature want it with the idea that they can throw an exception there: They want it for validation. So, obviously, they WOULD use it for that.

This is the crucial dilemma:

If we spec this feature to be solely for post processing, then it is both confusing and only covers ~10% of what it could.

If we spec this feature to be for validation, then its interaction with setters is daft (the invalid set goes through and THEN an exception occurs), or the feature is very complicated. But it would cover it all.

Neither is good enough.

I can see something crazy where we inject every field as parameter into a validation method, and scan the method for any attempt to set these parameters, and somehow relay all that back to the 'source' methods (the constructor, setters, builder's build() method, yadayada). But actually trying to figure out what lombok is supposed to desugar quickly turns into quite the beast.

[Marco Servetto]

> So, we have to go out of our way to say: This one method we invoke at the very end is JUST FOR PROCESSING and nothing else!

Just using it on the constructor, it can work for validation of deeply
immutable values.

If the user want the setter to revert the old value in case of failure
( I really do not think is a good idea)
then you may set at the end of setters a 'different method', that can
do the revert if the user want.
So, to be more precise:

Feature:
the user can write @Post("methName") and annotate
- a class (meaning on all the autogenerated constructor)
- a method
-a field, meaning the generated getter.
the user can write @PostSet("methName") and annotate
- a field, meaning generated setters

Behavious:
The code is desugared so that the method call is injected at the end
of the method, in a try-finally block.
For PostSet, the method may have one argument, that is going to
receive the old value of the setter, or zero argument, to not receive
it.


Usage to support validation:

Case of deeply immutable class

@Post("check")@Value class Foo{String baz; void check(){if
(this.baz.size()<10)throw Error("....");} }

Case of Data class:

@Post("check")@Data@AllArgsConstructor class Foo{
@PostSet("check") String baz;
void check(){if (this.baz.size()<10)throw Error("....");}
}

Case of Data class with reversion of setter effect:

@Post("check")@Data@AllArgsConstructor class Foo{
@PostSet("checkBaz") String baz;
void check(){if (this.baz.size()<10)throw Error("....");}
void checkBaz(String baz){ try{check():}finally{this.baz=baz;} }
}

This design is interesting since it makes possible for the user to
write personalised checks for each field, knowing both the new and the
old value.
It makes clear that the user must 'take care' or injecting checks when
is needed, and that the system does not offer any guarantee that the
object is going to be valid in any moment.

> If we spec this feature to be for validation, then its interaction with setters is daft (the invalid set goes through and THEN an exception occurs), or the feature is very complicated. But it would cover it all.

I hope my proposal is not not too complicated.
I think it covers all the realistic use cases, and is 'as sound as you
can' in miserable Java...
We could generate a warning if the user do not annotate with @PostSet
all setters, where @PostSet() would just silent the annotation.
There are so many more warnings we could design and discuss, but since
you can not do it 'right' anyway.. more warnings would just make the
user that prevent them more 'confident' that is 'guaranteed
correct'....

Ideas?

Marco.

[Reinier Zwitserloot]

Putting the name of the validator method in string quotes is icky. I'd much prefer a method be annotated with a no-args '@Postprocess' or '@Validator', with lombok figuring out what to do based on the arguments and staticness. I guess '@Validator private static validateLegalDrinkingAge(LocalDate birthDate, Locale country) {}' where crucially this method is inside a class def that has field 'LocalDate birthDate' and field 'Locale country' could work; lombok would know to invoke that inside all generated constructors (and possibly append it to all explicit ones), passing birthDate and country in, and lombok can even inject these in the setters, BEFORE the fieldupdate goes through (the setBirthDate method would then invoke vLDA with the param as first parameter and the existing value for the country field as second. Your belief that validation is hard especially for mutables is proven again because you really don't want to validate until after you've updated BOTH fields, if there's a validator that crosschecks both of them like this.

We can advise not to use validators like this for mutables, as long as lombok generates code that folks expect, even if that code is less useful than you might initially think, I'm okay with it.

Throw in a second annotation which updates the so annotated method to include ALLLLL the fields, in order, as parameters, and we might have something.

[Marco Servetto]

>with a no-args '@Postprocess' or '@Validator',

Ok, in this way the validator would be some sort of @preprocess, being
called before methods

>with a no-args '@Postprocess' or '@Validator',

validator that crosschecks both of them like this.
>
> We can advise not to use validators like this for mutables, as long as lombok generates code that folks expect, even if that code is less useful than you might initially think, I'm okay with it.

Ok, as discussed, if the validation is too complex, can not work anyway

> Throw in a second annotation which updates the so annotated method to include ALLLLL the fields,
in order, as parameters, and we might have something.

Or, much simpler,
if the @Validator annotation is on top of a no args method, then all
the fields are introduced as parameters.
Since a static @Validator 'needs' some parameters to make sense

Marco!

[Martin Grajcar]

It feels like there's a right way to do this properly, but we haven't yet stumbled across it.

I'm afraid, there's none. I'd bet, that any nearly right way is too complicated for most users and probably also too complicated for inclusion in Lombok as it may need tons of code. Different users may have different ideas about validation and satisfying them all may need tons of parameters, which is obviously bad.

"Call one no-args private instance method at the very end of the generated constructor" isn't it, though. It might fit your use case, but it fails to address many more.

I've just finished a small project where I could use exactly this kind of validation (more exactly: a primitive sanity check) and this several times.

In one place, I had to write a static factory method calling my private generated constructor (which meant adding about the same amount of boilerplate as lombok saved).

In another place, I could make the Builder private and prepend my check before the build() call.

Elsewhere, I did some simple normalization just before the build() call.

Actually, I expect neither validation nor normalization nor processing from Lombok. All I want is a chance to do it myself, while using Lombok as usual. Just some hooks to modify what it usually does. A nice example is @ToString.Include, which allows me e.g., to process a single field differently, while letting Lombok process the remaining ones.

Similarly, "Call one no-args private instance method at the very end of the generated constructor" would be such a hook, failing to address many use cases but succeeding in quite a few.

Compare http://openjdk.java.net/jeps/8222777, which doesn't offer much, but allows to execute some custom code in the constructor.

For the validation concern, it is useful for the object to never exist in an invalid state.

Usually, yes, but sometimes it's nice to have it, so you can use its methods during the validation.

With deserialization or tools like Hibernate, you can't do anything to prevent invalid object creation.

That's only to say that there are different notions of validation and no perfect solution.

Normalization: MUUUCH worse....

Sure, but "Call one no-args private instance method of the Builder just before build" would do in my case. It'd only work when the Builder is used, but that's my problem.

For some use-cases, per-field is great, for others, all-at-once is great. So, what do we do? Pick one side and leave the use cases we don't cover out in the cold? Do both and make it a really convoluted feature?

Here, I'd suggest to offer the whole-object variant as it's much much simpler - simpler to implement and to use. The per-field variant may come later and both may be used together. I guess, you can typically process each field separately, except for one or two conditions spanning multiple fields. These are two different features on the language level (I wouldn't call them "validators" as they may be used otherwise).

For me, the general answer is: Provide multiple simple features. Do not hope, they can satisfy everyone, do not hope, they can solve all kind of problems. I don't think there's a perfect solution, and I think, this shouldn't prevent Lombok from offering some partial very useful solutions. I'm not calling for a feature creep; every feature must be as good as possible given the constrains including time - note that SuperBuilder came five years after Builder, so waiting for the perfect solution is no solution.

My current particular wish for validation: Let me execute one non-static method at the end of the constructor. As my object is immutable, I don't care about setters and withers will call the constructor anyway. Don't call it validator and don't bother playing with manual constructors (as this is a lot of work for lombok and trivial for me).

--

You received this message because you are subscribed to the Google Groups "Project Lombok" group.

To unsubscribe from this group and stop receiving emails from it, send an email to project-lombo...@googlegroups.com.

[Marco Servetto]

To me, both solutions
1-the static method with injected parameters executed early, with a
'validation' feel
2-the instance method executed after the generated constructors, and
only those, with a 'hook only' feel
Would work perfectly fine and cover most of the boilerplate for most
scenarios where validation is actual boilerplate.
If the validation is 'triky' then it is one case when it is not boilerplate :)
However, in cases of deeply immutable classes or mutable classes with
only deeply immutable fields it is reasonably boilerplaty.

> To view this discussion on the web visit https://groups.google.com/d/msgid/project-lombok/CAGsWfGiHupKf9U8PksY8Lr1YCVr159G-%2BpvdLNLDyr5AP-%3DPkw%40mail.gmail.com.

[Martin Grajcar]

I'd much prefer a method be annotated with a no-args '@Postprocess' or '@Validator', with lombok figuring out what to do based on the arguments and staticness. I guess '@Validator private static validateLegalDrinkingAge(LocalDate birthDate, Locale country) {}' where crucially this method is inside a class def that has field 'LocalDate birthDate' and field 'Locale country' could work; lombok would know to invoke that inside all generated constructors (and possibly append it to all explicit ones), passing birthDate and country in, and lombok can even inject these in the setters, BEFORE the fieldupdate goes through (the setBirthDate method would then invoke vLDA with the param as first parameter and the existing value for the country field as second.

That's a wonderful idea. Syntactically nice, not prone to typos and not prone to forgetting to list a field needed for the validation.

Your belief that validation is hard especially for mutables is proven again because you really don't want to validate until after you've updated BOTH fields, if there's a validator that crosschecks both of them like this.

The proper way is to ditch mutability, call toBuilder() and validate in the constructor. However, it's not always possible. You may need in-place changes. Copying may be too slow or garbage creation may be unacceptable. Or repeating the validation for fields not involved in the change may take too long.

We can advise not to use validators like this for mutables, as long as lombok generates code that folks expect, even if that code is less useful than you might initially think, I'm okay with it.

What about

@ValidatedSetter

public setBirthDateAndCountry(LocalDate birthDate, Locale country) {

}

The method body must be empty, lombok inserts calls to all relevant validators (i.e., those working with either argument) and then sets all listed fields. Just an idea.

This would allow the user to set multiple fields at once without an intervening validation.

> Throw in a second annotation which updates the so annotated method to include ALLLLL the fields, in order, as parameters, and we might have something.
Or, much simpler,
if the @Validator annotation is on top of a no args method, then all
the fields are introduced as parameters.
Since a static @Validator 'needs' some parameters to make sense

Why ALLL? What about finding all simple names equal to a field name and adding just them? This would look much better in the outline....

 

[Marco Servetto]

> Why ALLL? What about finding all simple names equal to a field name and adding just them? This would look much better in the outline....

All is a good default. Is simple to understand and general. If you
want less then all, just declare them yourself.

[Reinier Zwitserloot]

On Monday, July 8, 2019 at 2:48:26 AM UTC+2, Maaartin G wrote:

Why ALLL? What about finding all simple names equal to a field name and adding just them? This would look much better in the outline....

because if we didn't inject all, your IDE's auto-complete feature* is not going to list any of them, as you're in a static context. You'd have to type it out by hand without guidance, and then as you finish typing the parameter poofs into existence.

*) This is gonna be tricky for the intellij plugin, I bet :(