Skip to content

Commit 389aa7b

Browse files
committed
fix: address PR 311 review feedback
- MailService: switch to @PostConstruct to cache JavaMailSender once at startup; warning logged a single time without PII instead of per-call with recipient address (GDPR/noise concern raised by Copilot review). - MailServiceTest: call init() after construction to simulate @PostConstruct; no-sender tests use a dedicated service instance. - README: clarify spring.security.oauth2.enabled is a framework property (not a standard Spring Security key) and is opt-in only.
1 parent 4ff1741 commit 389aa7b

3 files changed

Lines changed: 35 additions & 36 deletions

File tree

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ Follow these steps to get up and running with the Spring User Framework in your
261261
```
262262

263263
**Notes:**
264-
- `spring-boot-starter-oauth2-client` is required even if you don't plan to enable social login. The framework's security chain wires OAuth2 user services at startup; the dependency must be on the classpath so the classes resolve. OAuth2 login itself remains disabled by default (`spring.security.oauth2.enabled=false`).
264+
- `spring-boot-starter-oauth2-client` is required even if you don't plan to use social login. The framework's security chain wires OAuth2 user services at startup; the dependency must be on the classpath so the classes resolve. The OAuth2 login flow itself is disabled by default and opt-in — set `spring.security.oauth2.enabled=true` in your application properties only when you configure OAuth2 provider credentials (this is a framework property, not a standard Spring Security key).
265265
- `spring-retry` needs an explicit version because Spring Boot's BOM may not manage this artifact. The version shown matches what the framework is built against.
266266

267267
### Step 2: Database Configuration

src/main/java/com/digitalsanctuary/spring/user/mail/MailService.java

Lines changed: 19 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.digitalsanctuary.spring.user.mail;
22

33
import java.util.Map;
4+
import jakarta.annotation.PostConstruct;
45
import org.springframework.beans.factory.ObjectProvider;
56
import org.springframework.beans.factory.annotation.Value;
67
import org.springframework.mail.MailException;
@@ -20,18 +21,20 @@
2021
* emails.
2122
*
2223
* <p>Email is treated as optional: if no {@link JavaMailSender} bean is available (typically because {@code spring.mail.host} is not configured),
23-
* send operations log a warning and return without throwing, so the application starts and runs normally with email-dependent features degraded.</p>
24+
* a single warning is logged at startup and all send operations silently no-op, so the application starts and runs normally with email-dependent
25+
* features degraded.</p>
2426
*/
2527
@Slf4j
2628
@Service
2729
public class MailService {
2830

29-
/** Provider for the mail sender — resolved lazily so the bean is optional. */
3031
private final ObjectProvider<JavaMailSender> mailSenderProvider;
3132

32-
/** The mail content builder. */
3333
private final MailContentBuilder mailContentBuilder;
3434

35+
/** Resolved once at startup; null when JavaMailSender is not configured. */
36+
private JavaMailSender resolvedSender;
37+
3538
/** The from address. */
3639
@Value("${user.mail.fromAddress}")
3740
private String fromAddress;
@@ -48,17 +51,15 @@ public MailService(ObjectProvider<JavaMailSender> mailSenderProvider, MailConten
4851
}
4952

5053
/**
51-
* Resolve the {@link JavaMailSender}, or log a warning and return null when none is available.
52-
*
53-
* @param to the recipient (for log context only)
54-
* @return the sender, or {@code null} if not configured
54+
* Resolves and caches the {@link JavaMailSender} once at startup. Logs a single warning when no sender is available so operators are informed
55+
* without flooding logs during normal operation.
5556
*/
56-
private JavaMailSender resolveMailSender(String to) {
57-
JavaMailSender sender = mailSenderProvider.getIfAvailable();
58-
if (sender == null) {
59-
log.warn("Email send to '{}' skipped: JavaMailSender is not configured. Set 'spring.mail.host' to enable email sending.", to);
57+
@PostConstruct
58+
void init() {
59+
resolvedSender = mailSenderProvider.getIfAvailable();
60+
if (resolvedSender == null) {
61+
log.warn("JavaMailSender is not configured — email sending is disabled. Set 'spring.mail.host' to enable.");
6062
}
61-
return sender;
6263
}
6364

6465
/**
@@ -72,12 +73,10 @@ private JavaMailSender resolveMailSender(String to) {
7273
@Retryable(retryFor = {MailException.class}, maxAttempts = 3,
7374
backoff = @Backoff(delay = 1000, multiplier = 2))
7475
public void sendSimpleMessage(String to, String subject, String text) {
75-
log.debug("Attempting to send simple email to: {}", to);
76-
77-
JavaMailSender sender = resolveMailSender(to);
78-
if (sender == null) {
76+
if (resolvedSender == null) {
7977
return;
8078
}
79+
log.debug("Attempting to send simple email to: {}", to);
8180

8281
MimeMessagePreparator messagePreparator = mimeMessage -> {
8382
MimeMessageHelper messageHelper = new MimeMessageHelper(mimeMessage);
@@ -87,7 +86,7 @@ public void sendSimpleMessage(String to, String subject, String text) {
8786
messageHelper.setText(text, true);
8887
};
8988

90-
sender.send(messagePreparator);
89+
resolvedSender.send(messagePreparator);
9190
log.debug("Successfully sent simple email to: {}", to);
9291
}
9392

@@ -103,12 +102,10 @@ public void sendSimpleMessage(String to, String subject, String text) {
103102
@Retryable(retryFor = {MailException.class}, maxAttempts = 3,
104103
backoff = @Backoff(delay = 1000, multiplier = 2))
105104
public void sendTemplateMessage(String to, String subject, Map<String, Object> variables, String templatePath) {
106-
log.debug("Attempting to send template email to: {}, template: {}", to, templatePath);
107-
108-
JavaMailSender sender = resolveMailSender(to);
109-
if (sender == null) {
105+
if (resolvedSender == null) {
110106
return;
111107
}
108+
log.debug("Attempting to send template email to: {}, template: {}", to, templatePath);
112109

113110
MimeMessagePreparator messagePreparator = mimeMessage -> {
114111
MimeMessageHelper messageHelper = new MimeMessageHelper(mimeMessage);
@@ -121,7 +118,7 @@ public void sendTemplateMessage(String to, String subject, Map<String, Object> v
121118
messageHelper.setText(content, true);
122119
};
123120

124-
sender.send(messagePreparator);
121+
resolvedSender.send(messagePreparator);
125122
log.debug("Successfully sent template email to: {}", to);
126123
}
127124

src/test/java/com/digitalsanctuary/spring/user/mail/MailServiceTest.java

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,10 @@ class MailServiceTest {
5656

5757
@BeforeEach
5858
void setUp() {
59-
// Provider returns the mock mailSender by default; individual tests can override to simulate missing config.
59+
// Provider returns the mock mailSender by default.
6060
lenient().when(mailSenderProvider.getIfAvailable()).thenReturn(mailSender);
6161
mailService = new MailService(mailSenderProvider, mailContentBuilder);
62+
mailService.init(); // simulate @PostConstruct — caches the resolved sender
6263
ReflectionTestUtils.setField(mailService, "fromAddress", FROM_ADDRESS);
6364

6465
// Setup default mock behavior
@@ -514,29 +515,30 @@ void shouldHandleTemplateBuilderException() throws Exception {
514515
@DisplayName("Missing JavaMailSender Tests")
515516
class MissingMailSenderTests {
516517

518+
private MailService unconfiguredMailService;
519+
520+
@BeforeEach
521+
void setUpUnconfigured() {
522+
// Separate service instance where the sender is absent from startup.
523+
when(mailSenderProvider.getIfAvailable()).thenReturn(null);
524+
unconfiguredMailService = new MailService(mailSenderProvider, mailContentBuilder);
525+
unconfiguredMailService.init();
526+
ReflectionTestUtils.setField(unconfiguredMailService, "fromAddress", FROM_ADDRESS);
527+
}
528+
517529
@Test
518530
@DisplayName("sendSimpleMessage should no-op when JavaMailSender is not configured")
519531
void sendSimpleMessageNoOpsWhenSenderMissing() {
520-
// Given
521-
when(mailSenderProvider.getIfAvailable()).thenReturn(null);
522-
523-
// When
524-
mailService.sendSimpleMessage(TO_ADDRESS, SUBJECT, "Body");
532+
unconfiguredMailService.sendSimpleMessage(TO_ADDRESS, SUBJECT, "Body");
525533

526-
// Then
527534
verify(mailSender, never()).send(any(MimeMessagePreparator.class));
528535
}
529536

530537
@Test
531538
@DisplayName("sendTemplateMessage should no-op when JavaMailSender is not configured")
532539
void sendTemplateMessageNoOpsWhenSenderMissing() {
533-
// Given
534-
when(mailSenderProvider.getIfAvailable()).thenReturn(null);
540+
unconfiguredMailService.sendTemplateMessage(TO_ADDRESS, SUBJECT, new HashMap<>(), "email/test");
535541

536-
// When
537-
mailService.sendTemplateMessage(TO_ADDRESS, SUBJECT, new HashMap<>(), "email/test");
538-
539-
// Then
540542
verify(mailSender, never()).send(any(MimeMessagePreparator.class));
541543
verify(mailContentBuilder, never()).build(anyString(), any(Context.class));
542544
}

0 commit comments

Comments
 (0)