Randgalt / record-builder

Record builder generator for Java records
Apache License 2.0
758 stars 55 forks source link

Validations for the withers #47

Closed danp11 closed 3 years ago

danp11 commented 3 years ago

Hi Jordan

I'm playing around with new validation opportunities that you just have implemented. Very nice :-)

Howevere, It seems to be a possible to sneak thru the validation when using the withers methods.

For example when letting RequiredRecord implement RequiredRecordBuilder.With and RequiredRecord2 implement RequiredRecord2Builder.With

I expect these two unit test to pass:

    void testNotNullsWithNewProperty() {
        var valid = RequiredRecordBuilder.builder().hey("hey").i(1).build();
        Assertions.assertThrows(NullPointerException.class, () -> valid.withHey(null));
    }

    @Test
    void testValidationWithNewProperty() {
        var valid = RequiredRecord2Builder.builder().hey("hey").i(1).build();
        Assertions.assertThrows(ValidationException.class, () -> valid.withHey(null));
    }

For the @RecordBuilder.Options(interpretNotNulls = true) I guess it just as easy to add Objects.reuireNonNull in appropriate with-methods

but how to handle @RecordBuilder.Options(useValidationApi = true) ?

We dont want to perform a full validation of the entiere object via the builders static "new" method but instead only validate the actual property.

Please let me know your thoughts around this and I'll be happy to start with an initial PR.

Regards Dan

Randgalt commented 3 years ago

Yes, I agree. For validation, you can validate individual fields (referenced as "properties" in the spec) via: https://stackoverflow.com/a/21961677/2048051. A PR would be appreciated.

danp11 commented 3 years ago

Hi again,

I will start with the PR in a week or so, even if it is a small one. I'm working fulltime and now when it is European championship in soccer each night it is hard to find sparetime for hobby-coding :-)

I do believe that you have built something that will be very useful when apps updates to later Java version in the future so I'm very intrested in the progress, eventhough Im a bit slow at the moment :-)

Randgalt commented 3 years ago

I'm watching the Euros too. I can do the PR if you don't have time. Let me know.

danp11 commented 3 years ago

Ah nice :-) I thought you had the south-american championship now also? Tonight the Swedes are playing :-) against Ukraine.

I looked into to make code changes for the 'interpretNotNulls = true' and the Withers by adding the addNullCheckCodeBlock() from the add1WithMethod() but I couldn't get it to work properly. (Didn't try more than 45 minutes though :-)

If it is a 15 minutes fix for you, then please implement it and I can make a review.

Go Sweden tonight! :-) Which is your favourite team?

/Dan

Randgalt commented 3 years ago

I was living in Panamá but I live in Portugal now. But, they're out so I'll support England (I'm an EPL fan: ManU).

danp11 commented 3 years ago

Ok, your github profile says Panama that's why I asked about the S-A championship. Gamestart for England in 2 hours your time then:-) Later