Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[mail] Ensure SMTP thing status reflects connection status #17817

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

[mail] Ensure SMTP thing status reflects connection status #17817

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
675c5fd
Split session initialization from mail sending
clinique Nov 28, 2024
d124f52
Small corrections
clinique Nov 28, 2024
4e808cd
Close the session transport
clinique Nov 28, 2024
8ab739a
Apply spotless
clinique Nov 28, 2024
a8a0604
Better identification of the connexion issue. Sending an email tries …
clinique Nov 29, 2024
536fc67
Code cleansing
clinique Nov 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions ...ab.binding.mail/src/main/java/org/openhab/binding/mail/internal/MailBindingConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
*/
package org.openhab.binding.mail.internal;

import java.util.Arrays;
import java.util.HashSet;
import java.util.Set;

import org.eclipse.jdt.annotation.NonNullByDefault;
Expand All @@ -34,8 +32,8 @@ public class MailBindingConstants {
public static final ThingTypeUID THING_TYPE_IMAPSERVER = new ThingTypeUID(BINDING_ID, "imap");
public static final ThingTypeUID THING_TYPE_POP3SERVER = new ThingTypeUID(BINDING_ID, "pop3");

public static final Set<ThingTypeUID> SUPPORTED_THING_TYPES = new HashSet<>(
Arrays.asList(THING_TYPE_SMTPSERVER, THING_TYPE_IMAPSERVER, THING_TYPE_POP3SERVER));
public static final Set<ThingTypeUID> SUPPORTED_THING_TYPES = Set.of(THING_TYPE_SMTPSERVER, THING_TYPE_IMAPSERVER,
THING_TYPE_POP3SERVER);

public static final ChannelTypeUID CHANNEL_TYPE_UID_FOLDER_MAILCOUNT = new ChannelTypeUID(BINDING_ID, "mailcount");
public static final ChannelTypeUID CHANNEL_TYPE_UID_MAIL_CONTENT = new ChannelTypeUID(BINDING_ID, "content");
Expand Down
103 changes: 69 additions & 34 deletions ...org.openhab.binding.mail/src/main/java/org/openhab/binding/mail/internal/SMTPHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,20 @@
import javax.activation.PatchedMailcapCommandMap;
import javax.mail.MessagingException;
import javax.mail.Part;
import javax.mail.Session;
import javax.mail.internet.MimeMultipart;

import org.apache.commons.mail.DefaultAuthenticator;
import org.apache.commons.mail.Email;
import org.apache.commons.mail.EmailException;
import org.apache.commons.mail.SimpleEmail;
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.binding.mail.internal.action.SendMailActions;
import org.openhab.binding.mail.internal.config.SMTPConfig;
import org.openhab.core.thing.ChannelUID;
import org.openhab.core.thing.Thing;
import org.openhab.core.thing.ThingStatus;
import org.openhab.core.thing.ThingStatusDetail;
import org.openhab.core.thing.binding.BaseThingHandler;
import org.openhab.core.thing.binding.ThingHandlerService;
import org.openhab.core.types.Command;
Expand All @@ -45,13 +48,15 @@
* @author Jan N. Klug - Initial contribution
* @author Hans-Jörg Merk - Fixed UnsupportedDataTypeException - Originally by Jan N. Klug
* - Fix sending HTML/Multipart mail - Originally by Jan N. Klug
* @author Gaël L'hopital - Added session initialization for thing status check
*/
@NonNullByDefault
public class SMTPHandler extends BaseThingHandler {
private final Logger logger = LoggerFactory.getLogger(SMTPHandler.class);
private final PatchedMailcapCommandMap commandMap = new PatchedMailcapCommandMap();

private @NonNullByDefault({}) SMTPConfig config;
private String sender = "";
private @Nullable Session session;

public SMTPHandler(Thing thing) {
super(thing);
Expand All @@ -63,48 +68,82 @@ public void handleCommand(ChannelUID channelUID, Command command) {

@Override
public void initialize() {
config = getConfigAs(SMTPConfig.class);
updateStatus(ThingStatus.UNKNOWN);

if (config.port == 0) {
if (config.security == ServerSecurity.SSL) {
config.port = 465;
} else {
config.port = 25;
}
SMTPConfig config = getConfigAs(SMTPConfig.class);
if (config.sender instanceof String confSender) {
sender = confSender;
}

Email mail = new SimpleEmail();
mail.setHostName(config.hostname);

int port = config.port != 0 ? config.port : config.security == ServerSecurity.SSL ? 465 : 25;
switch (config.security) {
case SSL:
mail.setSSLOnConnect(true);
mail.setSslSmtpPort(Integer.toString(port));
break;
case STARTTLS:
mail.setStartTLSEnabled(true);
mail.setStartTLSRequired(true);
mail.setSmtpPort(port);
break;
case PLAIN:
mail.setSmtpPort(port);
}

if (!(config.username.isEmpty() || config.password.isEmpty())) {
mail.setAuthentication(config.username, config.password);
}

Session localSession;
try {
localSession = mail.getMailSession();
localSession.getTransport().connect();
} catch (EmailException | MessagingException e) {
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR, e.getMessage());
return;
}

this.session = localSession;
updateStatus(ThingStatus.ONLINE);
}

@Override
public void dispose() {
Session localSession = session;
if (localSession != null) {
try {
localSession.getTransport().close();
} catch (MessagingException ignore) {
}
session = null;
}
}

/**
* use this server to send a mail
*
* @param mail the Email that needs to be sent
* @return true if successful, false if failed
*/
public boolean sendMail(Email mail) {
Session localSession = session;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you do it like this a single failed login (or connection problem) during initialization leads to an "forever unable to send mail" state. I don't think that's really a good idea.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@J-N-K : you're correct. Changed.

if (localSession == null) {
logger.warn("Thing {} is not ONLINE, can't send mail", thing.getUID());
return false;
}

mail.setMailSession(localSession);

try {
if (mail.getFromAddress() == null) {
mail.setFrom(config.sender);
}
mail.setHostName(config.hostname);
switch (config.security) {
case SSL:
mail.setSSLOnConnect(true);
mail.setSslSmtpPort(config.port.toString());
break;
case STARTTLS:
mail.setStartTLSEnabled(true);
mail.setStartTLSRequired(true);
mail.setSmtpPort(config.port);
break;
case PLAIN:
mail.setSmtpPort(config.port);
}
if (!config.username.isEmpty() && !config.password.isEmpty()) {
mail.setAuthenticator(new DefaultAuthenticator(config.username, config.password));
if (mail.getFromAddress() == null && !sender.isEmpty()) {
mail.setFrom(sender);
} else {
logger.warn("No sender defined, can't send mail");
return false;
}

mail.buildMimeMessage();

// fix command map not available
Expand All @@ -127,11 +166,7 @@ public boolean sendMail(Email mail) {
mail.sendMimeMessage();
} catch (MessagingException | EmailException e) {
Throwable cause = e.getCause();
if (cause != null) {
logger.warn("{}", cause.toString());
} else {
logger.warn("{}", e.getMessage());
}
logger.warn("{}", cause != null ? cause.toString() : e.getMessage());
return false;
}
return true;
Expand Down
2 changes: 1 addition & 1 deletion ...enhab.binding.mail/src/main/java/org/openhab/binding/mail/internal/config/BaseConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
@NonNullByDefault
public class BaseConfig {
public @Nullable String hostname;
public Integer port = 0;
public int port = 0;
public String username = "";
public String password = "";
public ServerSecurity security = ServerSecurity.PLAIN;
Expand Down