Skip to content

Commit

Permalink
fix: Unable to save outbound sms during async operation [DHIS2-18586] (
Browse files Browse the repository at this point in the history
  • Loading branch information
zubaira authored Dec 13, 2024
1 parent 85f55a7 commit ad491fb
Show file tree
Hide file tree
Showing 7 changed files with 132 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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(
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -104,6 +105,8 @@ class SmsMessageSenderTest {

@Mock private SystemSettings settings;

@Mock private AuthenticationService authenticationService;

private SmsGatewayConfig smsGatewayConfig;

private OutboundMessageResponse okStatus;
Expand Down Expand Up @@ -148,7 +151,8 @@ public void initTest() {
smsGateways,
userSettingsService,
outboundSmsService,
settingsProvider);
settingsProvider,
authenticationService);
}

private void mockGateway() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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"));
}
}

0 comments on commit ad491fb

Please sign in to comment.