fix: Unable to save outbound sms during async operation [DHIS2-18586]

dhis-2/dhis-api/src/main/java/org/hisp/dhis/message/MessageSender.java

@@ -35,7 +35,6 @@
 import org.hisp.dhis.outboundmessage.OutboundMessageResponse;
 import org.hisp.dhis.outboundmessage.OutboundMessageResponseSummary;
 import org.hisp.dhis.user.User;
-import org.springframework.util.concurrent.ListenableFuture;
 
 /**
  * @author Lars Helge Overland
@@ -68,10 +67,6 @@ Future<OutboundMessageResponse> sendMessageAsync(
    */
   OutboundMessageResponseSummary sendMessageBatch(OutboundMessageBatch batch);
 
-  /** sends message batch asynchronously */
-  ListenableFuture<OutboundMessageResponseSummary> sendMessageBatchAsync(
-      OutboundMessageBatch batch);
-
   /** To check if given service is configured and ready to use. */
   boolean isConfigured();
 

dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/message/EmailMessageSender.java

@@ -35,6 +35,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.Future;
 import java.util.stream.Collectors;
 import lombok.RequiredArgsConstructor;
@@ -61,9 +62,7 @@
 import org.hisp.dhis.user.UserSettingsService;
 import org.hisp.dhis.util.ObjectUtils;
 import org.springframework.scheduling.annotation.Async;
-import org.springframework.scheduling.annotation.AsyncResult;
 import org.springframework.stereotype.Service;
-import org.springframework.util.concurrent.ListenableFuture;
 
 /**
  * @author Lars Helge Overland
@@ -184,7 +183,7 @@ public OutboundMessageResponse sendMessage(
   public Future<OutboundMessageResponse> sendMessageAsync(
       String subject, String text, String footer, User sender, Set<User> users, boolean forceSend) {
     OutboundMessageResponse response = sendMessage(subject, text, footer, sender, users, forceSend);
-    return new AsyncResult<>(response);
+    return CompletableFuture.completedFuture(response);
   }
 
   @Override
@@ -269,13 +268,6 @@ public OutboundMessageResponseSummary sendMessageBatch(OutboundMessageBatch batc
     return generateSummary(statuses);
   }
 
-  @Override
-  public ListenableFuture<OutboundMessageResponseSummary> sendMessageBatchAsync(
-      OutboundMessageBatch batch) {
-    OutboundMessageResponseSummary summary = sendMessageBatch(batch);
-    return new AsyncResult<>(summary);
-  }
-
   @Override
   public boolean isConfigured() {
     return getEmailConfiguration().isOk();

dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/sms/config/SmsMessageSender.java

@@ -36,6 +36,7 @@
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
+import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.Future;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
@@ -55,12 +56,11 @@
 import org.hisp.dhis.sms.outbound.OutboundSmsService;
 import org.hisp.dhis.sms.outbound.OutboundSmsStatus;
 import org.hisp.dhis.system.util.SmsUtils;
+import org.hisp.dhis.user.AuthenticationService;
 import org.hisp.dhis.user.User;
 import org.hisp.dhis.user.UserSettingsService;
 import org.springframework.scheduling.annotation.Async;
-import org.springframework.scheduling.annotation.AsyncResult;
 import org.springframework.stereotype.Service;
-import org.springframework.util.concurrent.ListenableFuture;
 
 /**
  * @author Nguyen Kim Lai
@@ -87,6 +87,7 @@ public class SmsMessageSender implements MessageSender {
   private final UserSettingsService userSettingsService;
   private final OutboundSmsService outboundSmsService;
   private final SystemSettingsProvider settingsProvider;
+  private final AuthenticationService authenticationService;
 
   @Override
   public OutboundMessageResponse sendMessage(
@@ -122,7 +123,7 @@ public OutboundMessageResponse sendMessage(
   public Future<OutboundMessageResponse> sendMessageAsync(
       String subject, String text, String footer, User sender, Set<User> users, boolean forceSend) {
     OutboundMessageResponse response = sendMessage(subject, text, footer, sender, users, forceSend);
-    return new AsyncResult<>(response);
+    return CompletableFuture.completedFuture(response);
   }
 
   @Override
@@ -186,13 +187,6 @@ public OutboundMessageResponseSummary sendMessageBatch(OutboundMessageBatch batc
         NO_CONFIG, DeliveryChannel.SMS, OutboundMessageBatchStatus.ABORTED, batch.size());
   }
 
-  @Override
-  public ListenableFuture<OutboundMessageResponseSummary> sendMessageBatchAsync(
-      OutboundMessageBatch batch) {
-    OutboundMessageResponseSummary summary = sendMessageBatch(batch);
-    return new AsyncResult<>(summary);
-  }
-
   @Override
   public boolean isConfigured() {
     return gatewayAdminService.hasGateways();
@@ -292,7 +286,12 @@ private void handleResponse(OutboundMessageResponse status, OutboundSms sms) {
       sms.setStatus(OutboundSmsStatus.FAILED);
     }
 
-    outboundSmsService.save(sms);
+    try {
+      authenticationService.obtainSystemAuthentication();
+      outboundSmsService.save(sms);
+    } finally {
+      authenticationService.clearAuthentication();
+    }
     status.setDescription(gatewayResponse.getResponseMessage());
     status.setResponseObject(gatewayResponse);
   }

dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/sms/SmsMessageSenderTest.java

@@ -69,6 +69,7 @@
 import org.hisp.dhis.sms.config.SmsMessageSender;
 import org.hisp.dhis.sms.outbound.GatewayResponse;
 import org.hisp.dhis.sms.outbound.OutboundSmsService;
+import org.hisp.dhis.user.AuthenticationService;
 import org.hisp.dhis.user.User;
 import org.hisp.dhis.user.UserSettingsService;
 import org.junit.jupiter.api.BeforeEach;
@@ -104,6 +105,8 @@ class SmsMessageSenderTest {
 
   @Mock private SystemSettings settings;
 
+  @Mock private AuthenticationService authenticationService;
+
   private SmsGatewayConfig smsGatewayConfig;
 
   private OutboundMessageResponse okStatus;
@@ -148,7 +151,8 @@ public void initTest() {
             smsGateways,
             userSettingsService,
             outboundSmsService,
-            settingsProvider);
+            settingsProvider,
+            authenticationService);
   }
 
   private void mockGateway() {

dhis-2/dhis-support/dhis-support-test/src/main/java/org/hisp/dhis/test/message/DefaultFakeMessageSender.java

@@ -48,9 +48,7 @@
 import org.hisp.dhis.outboundmessage.OutboundMessageResponseSummary;
 import org.hisp.dhis.user.User;
 import org.springframework.context.annotation.Profile;
-import org.springframework.scheduling.annotation.AsyncResult;
 import org.springframework.stereotype.Service;
-import org.springframework.util.concurrent.ListenableFuture;
 
 /**
  * A {@link MessageSender} used in test setup that pretends to send messages and that gives access
@@ -137,12 +135,6 @@ public OutboundMessageResponseSummary sendMessageBatch(OutboundMessageBatch batc
     return summary;
   }
 
-  @Override
-  public ListenableFuture<OutboundMessageResponseSummary> sendMessageBatchAsync(
-      OutboundMessageBatch batch) {
-    return new AsyncResult<>(sendMessageBatch(batch));
-  }
-
   @Override
   public boolean isConfigured() {
     return true;

dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/message/MessageServiceTest.java

@@ -68,6 +68,7 @@ class MessageServiceTest extends PostgresIntegrationTestBase {
   void setUp() {
     sender = makeUser("S");
     userA = makeUser("A");
+    userA.setPhoneNumber("40342434");
     userB = makeUser("B");
     userService.addUser(sender);
     userService.addUser(userA);

dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/message/SmsMessageSenderTest.java

@@ -0,0 +1,114 @@
+/*
+ * Copyright (c) 2004-2024, University of Oslo
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ * Redistributions of source code must retain the above copyright notice, this
+ * list of conditions and the following disclaimer.
+ *
+ * Redistributions in binary form must reproduce the above copyright notice,
+ * this list of conditions and the following disclaimer in the documentation
+ * and/or other materials provided with the distribution.
+ * Neither the name of the HISP project nor the names of its contributors may
+ * be used to endorse or promote products derived from this software without
+ * specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR
+ * ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
+ * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package org.hisp.dhis.message;
+
+import static org.junit.jupiter.api.Assertions.assertAll;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+
+import java.util.List;
+import java.util.Set;
+import java.util.concurrent.TimeUnit;
+import org.awaitility.Awaitility;
+import org.hisp.dhis.feedback.BadRequestException;
+import org.hisp.dhis.feedback.ConflictException;
+import org.hisp.dhis.feedback.ForbiddenException;
+import org.hisp.dhis.sms.config.BulkSmsGatewayConfig;
+import org.hisp.dhis.sms.config.GatewayAdministrationService;
+import org.hisp.dhis.sms.config.SmsGatewayConfig;
+import org.hisp.dhis.sms.outbound.OutboundSms;
+import org.hisp.dhis.sms.outbound.OutboundSmsService;
+import org.hisp.dhis.test.integration.PostgresIntegrationTestBase;
+import org.hisp.dhis.user.User;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInstance;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.beans.factory.annotation.Qualifier;
+
+/**
+ * @author Zubair Asghar
+ */
+@TestInstance(TestInstance.Lifecycle.PER_CLASS)
+class SmsMessageSenderTest extends PostgresIntegrationTestBase {
+
+  @Autowired
+  @Qualifier("smsMessageSender")
+  private MessageSender smsMessageSender;
+
+  @Autowired private OutboundSmsService outboundSmsService;
+
+  @Autowired private GatewayAdministrationService gatewayAdministrationService;
+
+  private User sender;
+  private User userA;
+  private User userB;
+  private Set<User> users;
+
+  @BeforeAll
+  void setUp() throws ForbiddenException, ConflictException, BadRequestException {
+    sender = makeUser("S");
+    userA = makeUser("A");
+    userA.setPhoneNumber("40342434");
+    userB = makeUser("B");
+
+    userService.addUser(sender);
+    userService.addUser(userA);
+    userService.addUser(userB);
+
+    users = Set.of(userA, userB);
+
+    SmsGatewayConfig smsGatewayConfig = new BulkSmsGatewayConfig();
+    smsGatewayConfig.setDefault(true);
+    smsGatewayConfig.setUsername("user_uio");
+    smsGatewayConfig.setPassword("RefLt4N5<1");
+
+    gatewayAdministrationService.addGateway(smsGatewayConfig);
+  }
+
+  @Test
+  void shouldSendMessageAsyncAndSaveOutboundSms() {
+    smsMessageSender.sendMessageAsync("subject", "text_message", "footer", sender, users, true);
+
+    // Wait until the outbound SMS is persisted
+    Awaitility.await()
+        .atMost(10, TimeUnit.SECONDS)
+        .until(() -> !outboundSmsService.getAll().isEmpty());
+
+    List<OutboundSms> outboundSms = outboundSmsService.getAll();
+
+    assertAll(
+        "Verify outbound SMS creation",
+        () -> assertFalse(outboundSms.isEmpty(), "Outbound SMS list should not be empty"),
+        () ->
+            assertEquals(
+                "text_message",
+                outboundSms.get(0).getMessage(),
+                "Message text should match the sent message"));
+  }
+}