Prefer Flux#take(long, boolean) over Flux#take(long) to limit upstream generation
#314
Conversation
|
Suggested commit message: |
|
|
||
| @AfterTemplate | ||
| Flux<T> after(Flux<T> flux, long n) { | ||
| return flux.take(n, /* limitRequest= */ true); |
There was a problem hiding this comment.
Gotta love the self-check profile, didn't expect this to show up in the after-template.
Fun fact; if we really want to add this comment in the actual output of applying a Refaster rule, we can use Refaster#emitComment or Refaster#emitCommentBefore.
There was a problem hiding this comment.
Yep! We don't have BooleanParameter enabled internally, so I think it's fine as-is 👍
| @@ -144,6 +144,18 @@ Mono<S> after(Mono<T> mono, S object) { | |||
| } | |||
| } | |||
|
|
|||
| static final class FluxTakeGenerationLimit<T> { | |||
There was a problem hiding this comment.
| static final class FluxTakeGenerationLimit<T> { | |
| static final class FluxTake<T> { |
There was a problem hiding this comment.
We use content of the after-template to determine how we should name the Refaster rule. We can simply write FluxTake here as we see in the after-template.
(We are working on a naming algorithm that will be enforced.)
| @@ -144,6 +144,18 @@ Mono<S> after(Mono<T> mono, S object) { | |||
| } | |||
| } | |||
|
|
|||
| static final class FluxTakeGenerationLimit<T> { | |||
There was a problem hiding this comment.
Let's add a Javadoc here as well :).
|
I'm thinking about the suggested commit message. I think it'd be better to add that information to the Javadoc as this information is good to have in the project. Otherwise it would get "lost" in the git history. So I'd go for moving it to there and simplify the suggested commit message. WDYT @eric-picnic ? |
I agree. I have moved it from the suggested commit message to the Javadoc of |
In Reactor versions prior to 3.5.0, `Flux#take(n)` makes an unbounded request upstream, and is equivalent to `Flux#take(n, false)`. In 3.5.0, the behavior of `Flux#take(n)` will change to that of `Flux#take(n, true)`. The intent with this Refaster rule is to get the new behavior before upgrading to Reactor 3.5.0.
There was a problem hiding this comment.
Nice! Sweet to see this suggestion I had made quite some time ago make it to EPS! 🚀
Sweet work @eric-picnic 💪 Congrats on the first contribution here.
error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ReactorRules.java
Show resolved
Hide resolved
Stephan202
force-pushed
the
eric-picnic/limit-flux-take
branch
from
3eacc3b to
46ab26b
Compare
|
|
||
| @AfterTemplate | ||
| Flux<T> after(Flux<T> flux, long n) { | ||
| return flux.take(n, /* limitRequest= */ true); |
There was a problem hiding this comment.
Yep! We don't have BooleanParameter enabled internally, so I think it's fine as-is 👍
| /** | ||
| * Prefer {@link Flux#take(long, boolean)} over {@link Flux#take(long)}. | ||
| * | ||
| * <p>In Reactor versions prior to 3.5.0, `Flux#take(long)` makes an unbounded request upstream, |
There was a problem hiding this comment.
Since it's Javadoc we generally use {@code instead of backticks.
| * <p>The intent with this Refaster rule is to get the new behavior before upgrading to Reactor | ||
| * 3.5.0. |
There was a problem hiding this comment.
Once on 3.6.0 we'll want to simplify the code in the opposite direction 😄. I'll add a comment.
| * <p>The intent with this Refaster rule is to get the new behavior before upgrading to Reactor | ||
| * 3.5.0. | ||
| */ | ||
| static final class FluxTake<T> { |
There was a problem hiding this comment.
This is one of the few (perhaps the first?) Refaster rules that is intentionally not behavior preserving. A good opportunity to use the @Description and @Severity annotations introduced in #255. 😄
error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ReactorRules.java
Show resolved
Hide resolved
rickie
changed the title
Flux#take(n, true) over Flux#take(n) to limit generationFlux#take(long, boolean) over Flux#take(long) to limit upstream generation
In Reactor versions prior to 3.5.0,
Flux#take(n)makes an unbounded request upstream, and is equivalent toFlux#take(n, false). In 3.5.0, the behavior ofFlux#take(n)will change to that ofFlux#take(n, true).The intent with this Refaster rule is to get the new behavior before upgrading to Reactor 3.5.0.