Skip to content

Commit

Permalink
fix: process incoming sms via jobs
Browse files Browse the repository at this point in the history
* UserDetails refactoring broke SMS processing as SMS processing was
  done in another thread that does not have the currentUser set. The
  processing code uses the manager which requires the currentUser to be
  set.
* Process incoming SMS via jobs instead to fix the above. Every incoming
  sms will result in a job executed as the user sending the sms
* Override MessageSender for Email and SmsMessageSender with
  FakeMessageSender so we can assert on error sms responses.
* Test that we can delete a tracker event via an sms and that we
  respond with an sms if the event cannot be deleted.
  • Loading branch information
teleivo committed Sep 3, 2024
1 parent 8907027 commit 770aa51
Show file tree
Hide file tree
Showing 19 changed files with 521 additions and 543 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,16 @@
package org.hisp.dhis.outboundmessage;

import java.util.Set;
import lombok.EqualsAndHashCode;
import lombok.Getter;
import lombok.ToString;

/**
* @author Zubair <[email protected]>
*/
@Getter
@ToString
@EqualsAndHashCode
public class OutboundMessage {
private String subject;

Expand All @@ -45,26 +51,14 @@ public OutboundMessage(String subject, String text, Set<String> recipients) {
this.recipients = recipients;
}

public String getText() {
return text;
}

public void setText(String text) {
this.text = text;
}

public Set<String> getRecipients() {
return recipients;
}

public void setRecipients(Set<String> recipients) {
this.recipients = recipients;
}

public String getSubject() {
return subject;
}

public void setSubject(String subject) {
this.subject = subject;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
import org.hisp.dhis.scheduling.parameters.MonitoringJobParameters;
import org.hisp.dhis.scheduling.parameters.PredictorJobParameters;
import org.hisp.dhis.scheduling.parameters.PushAnalysisJobParameters;
import org.hisp.dhis.scheduling.parameters.SmsInboundProcessingJobParameters;
import org.hisp.dhis.scheduling.parameters.SmsJobParameters;
import org.hisp.dhis.scheduling.parameters.SqlViewUpdateParameters;
import org.hisp.dhis.scheduling.parameters.TestJobParameters;
Expand Down Expand Up @@ -95,6 +96,7 @@ public enum JobType {
*/
MOCK(MockJobParameters.class),
SMS_SEND(SmsJobParameters.class),
SMS_INBOUND_PROCESSING(SmsInboundProcessingJobParameters.class),
TRACKER_IMPORT_JOB(),
TRACKER_IMPORT_NOTIFICATION_JOB(),
TRACKER_IMPORT_RULE_ENGINE_JOB(),
Expand Down Expand Up @@ -238,7 +240,8 @@ public boolean isUsingContinuousExecution() {
|| this == ANALYTICS_TABLE
|| this == TRACKER_IMPORT_JOB
|| this == DATA_INTEGRITY
|| this == DATA_INTEGRITY_DETAILS;
|| this == DATA_INTEGRITY_DETAILS
|| this == SMS_INBOUND_PROCESSING;
}

public boolean hasJobParameters() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,22 @@
* (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.sms;
package org.hisp.dhis.scheduling.parameters;

import org.hisp.dhis.sms.incoming.IncomingSms;
import com.fasterxml.jackson.annotation.JsonProperty;
import lombok.Getter;
import lombok.NoArgsConstructor;
import lombok.Setter;
import org.hisp.dhis.scheduling.JobParameters;

public interface MessageQueue {
void put(IncomingSms message);
@Getter
@Setter
@NoArgsConstructor
public class SmsInboundProcessingJobParameters implements JobParameters {
@JsonProperty(required = true)
private long smsId;

IncomingSms get();

void remove(IncomingSms message);

void initialize();
public SmsInboundProcessingJobParameters(long smsId) {
this.smsId = smsId;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,7 @@
*/
package org.hisp.dhis.sms.incoming;

import java.util.Date;
import java.util.List;
import org.hisp.dhis.user.User;

/** Service providing support for retrieving incoming SMSes. */
public interface IncomingSmsService {
Expand All @@ -51,8 +49,6 @@ public interface IncomingSmsService {

long save(IncomingSms sms);

long save(String message, String originator, String gateway, Date receivedTime, User user);

List<IncomingSms> getSmsByStatus(SmsMessageStatus status, String originator);

List<IncomingSms> getSmsByStatus(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,13 @@
import org.hisp.dhis.i18n.I18nManager;
import org.hisp.dhis.i18n.ui.resourcebundle.DefaultResourceBundleManager;
import org.hisp.dhis.i18n.ui.resourcebundle.ResourceBundleManager;
import org.hisp.dhis.message.EmailMessageSender;
import org.hisp.dhis.message.MessageSender;
import org.hisp.dhis.outboundmessage.DefaultOutboundMessageBatchService;
import org.hisp.dhis.setting.DefaultStyleManager;
import org.hisp.dhis.setting.StyleManager;
import org.hisp.dhis.setting.SystemSettingManager;
import org.hisp.dhis.sms.config.SmsMessageSender;
import org.hisp.dhis.user.UserSettingService;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.scheduling.concurrent.ThreadPoolTaskScheduler;
Expand Down Expand Up @@ -77,7 +76,8 @@ public StyleManager styleManager(

@Bean("org.hisp.dhis.outboundmessage.OutboundMessageService")
public DefaultOutboundMessageBatchService defaultOutboundMessageBatchService(
SmsMessageSender smsMessageSender, EmailMessageSender emailMessageSender) {
@Qualifier("smsMessageSender") MessageSender smsMessageSender,
@Qualifier("emailMessageSender") MessageSender emailMessageSender) {
Map<DeliveryChannel, MessageSender> channels = new HashMap<>();
channels.put(DeliveryChannel.SMS, smsMessageSender);
channels.put(DeliveryChannel.EMAIL, emailMessageSender);
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -27,42 +27,20 @@
*/
package org.hisp.dhis.sms.incoming;

import static com.google.common.base.Preconditions.checkNotNull;

import java.util.Date;
import java.util.List;
import lombok.RequiredArgsConstructor;
import org.apache.commons.lang3.StringUtils;
import org.hisp.dhis.sms.MessageQueue;
import org.hisp.dhis.user.User;
import org.springframework.context.annotation.Lazy;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@RequiredArgsConstructor
@Service("org.hisp.dhis.sms.incoming.IncomingSmsService")
public class DefaultIncomingSmsService implements IncomingSmsService {
private static final String DEFAULT_GATEWAY = "default";

// -------------------------------------------------------------------------
// Dependencies
// -------------------------------------------------------------------------

private final IncomingSmsStore incomingSmsStore;

private final MessageQueue incomingSmsQueue;

public DefaultIncomingSmsService(
IncomingSmsStore incomingSmsStore, @Lazy MessageQueue incomingSmsQueue) {
checkNotNull(incomingSmsQueue);
checkNotNull(incomingSmsStore);

this.incomingSmsStore = incomingSmsStore;
this.incomingSmsQueue = incomingSmsQueue;
}

// -------------------------------------------------------------------------
// Implementation
// -------------------------------------------------------------------------

@Override
@Transactional(readOnly = true)
public List<IncomingSms> getAll() {
Expand All @@ -83,37 +61,14 @@ public long save(IncomingSms sms) {
} else {
sms.setSentDate(new Date());
}

sms.setReceivedDate(new Date());

sms.setGatewayId(StringUtils.defaultIfBlank(sms.getGatewayId(), DEFAULT_GATEWAY));

incomingSmsStore.save(sms);
incomingSmsQueue.put(sms);
return sms.getId();
}

@Override
@Transactional
public long save(
String message, String originator, String gateway, Date receivedTime, User user) {
IncomingSms sms = new IncomingSms();
sms.setText(message);
sms.setOriginator(originator);
sms.setGatewayId(gateway);
sms.setCreatedBy(user);

if (receivedTime != null) {
sms.setSentDate(receivedTime);
} else {
sms.setSentDate(new Date());
}

sms.setReceivedDate(new Date());

return save(sms);
}

@Override
@Transactional
public void delete(long id) {
IncomingSms incomingSms = incomingSmsStore.get(id);
Expand Down
Loading

0 comments on commit 770aa51

Please sign in to comment.