Merge pull request #6901 from eleftherias/gh-6885-http-basic-dsl

[eleftherias]

Issue: gh-5557
Fixes: gh-6885

[rwinch]

Is there a reason to use this vs using java.util.function.Consumer?

NOTE: Much of the java configuration was written when we supported JDK 1.5+. Now that we require at least JDK 8 I think Consumer makes sense. Perhaps you were following the patterns that existed in the older code is why you created this interface?

[eleftherias]

The java.util.function.Consumer does't allow exceptions to be thrown from the accept method.
In our case the realmName method on HttpBasicConfigurer throws an exception, so you would not be able to use that method in a lambda with the Java Consumer.
I have seen a custom consumer in Spring Boot and I assume it is for the same reason.
https://github.com/spring-projects/spring-boot/blob/36c1c051b8106371d37f8a747cea0e369a8e5c84/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/context/runner/ContextConsumer.java

[rwinch]

Thanks for the pointer. That makes sense. A few thoughts:

• I think we should change the signature to allow throwing Throwable rather than Exception
• I'm thinking we may want to make this more central (i.e. within core) since it is likely we will need to use this in other places.

[eleftherias]

@rwinch I moved the consumer to the core module and renamed it. I also made the Throwable a type argument so that it doesn't need to bubble up as a Throwable in the calling methods.

[rwinch]

I think I'd prefer to revert and potentially use a generic noop Consumer. The reason is that withDefaults is exposed for those not using lambda's and becomes confusing since the defaults are already applied by default. Another reason is that way we don't need a withDefaults for every configurer that we have.

I know we already talked about creating a method reference for users, but in hindsight I'd suggest we just require users to create an empty lambda. The JDK doesn't even provide one. We can always introduce a generic no op consumer later on.

[eleftherias]

I agree.
I believe users could still do the following to keep the defaults.

http
    .httpBasic(httpBasicConfigurer -> {});

[rwinch]

I'd fix this formatting to have newlines in it. I know there are other places we are missing newlines, but that occurred by mistake.

[rwinch]

I'd just omit this from the configuration as we now recommend exposing a UserDetailsService via a Bean. This also allows the user to focus on this specific configuration

[eleftherias]

I've updated the Javadoc with both suggestions.

[rwinch]

Sorry this went in a full circle. However, now that we have a custom interface again, perhaps we can reintroduce something like

<T> Customizer<T> withDefaults() {
    return t -> {};
}

[sbrannen]

That would need to be static -- right?

[eleftherias]

@rwinch I added withDefaults to the Customizer.
@sbrannen Yes, the method is static.

[rwinch]

Thanks @eleftherias! This is now merged into master