chibenwa commented on code in PR #3198:
URL: https://github.com/apache/james-project/pull/3198#discussion_r4153973738
##########
protocols/imap/src/main/java/org/apache/james/imap/processor/IdleProcessor.java:
##########
@@ -77,87 +79,156 @@ public void configure(ImapConfiguration imapConfiguration)
{
super.configure(imapConfiguration);
this.heartbeatInterval =
imapConfiguration.idleTimeIntervalAsDuration();
- this.enableIdle = imapConfiguration.isEnableIdle();
+ this.enableIdle = imapConfiguration.isEnableIdle() &&
!heartbeatInterval.isZero() && !heartbeatInterval.isNegative();
}
@Override
protected Mono<Void> processRequestReactive(IdleRequest request,
ImapSession session, Responder responder) {
- CountDownLatch countDownLatch = new CountDownLatch(1);
- return Mono.fromRunnable(() -> idle(request, session, responder,
countDownLatch))
- .then(unsolicitedResponses(session, responder, false))
+ Responder safeResponder = session.threadSafe(responder);
+ SelectedMailbox selectedMailbox = session.getSelected();
+ Sinks.One<Void> idleReadySink = Sinks.one();
+ AtomicBoolean idleActive = new AtomicBoolean(true);
+ AtomicBoolean lineHandlerAdded = new AtomicBoolean(false);
+
+ IdleMailboxListener idleListener = selectedMailbox != null
+ ? new IdleMailboxListener(session, safeResponder, idleReadySink,
idleActive)
+ : null;
Review Comment:
```suggestion
Optional<IdleMailboxListener> idleListener =
Optionnal.ofNullable(selectedMailbox)
.map(mailbox -> new IdleMailboxListener(session, safeResponder,
idleReadySink, idleActive));
```
I would rather handle the fact that this can be missing from the type system
and not rely on null in the code
----------------
Even better: can we run IDLE if no mailbox is selected ? Maybe not...
I propose
```suggestion
if (selectedMailbox != null) {
// RESPOND BAD
// INFO log tried to IDLE while no mailbox is selected (with MDC
context...)
// return MONO
}
IdleMailboxListener idleListener = new IdleMailboxListener(session,
safeResponder, idleReadySink, idleActive);
// as today
```
And remove downstream null guards.
WDYT ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
