Skip to content

Commit

Permalink
fix: make sure background threads check for not logged in current user (
Browse files Browse the repository at this point in the history
#16402)

* fix: make sure background threads check for null/not logged in current user
  • Loading branch information
netroms authored Feb 6, 2024
1 parent 7c5bce7 commit a9f65d5
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ public class DefaultMessageService implements MessageService {
@Override
@Transactional
public long sendTicketMessage(String subject, String text, String metaData) {
User currentUser = userService.getUserByUsername(CurrentUserUtil.getCurrentUsername());
User currentUser = getCurrentUserOrNull();

MessageConversationParams params =
new MessageConversationParams.Builder()
Expand All @@ -133,7 +133,7 @@ public long sendPrivateMessage(
String text,
String metaData,
Set<FileResource> attachments) {
User currentUser = userService.getUserByUsername(CurrentUserUtil.getCurrentUsername());
User currentUser = getCurrentUserOrNull();

MessageConversationParams params =
new MessageConversationParams.Builder()
Expand Down Expand Up @@ -215,7 +215,13 @@ public long sendMessage(MessageConversationParams params, UserDetails actingUser
@Override
@Transactional
public long sendMessage(MessageConversationParams params) {
return sendMessage(params, CurrentUserUtil.getCurrentUserDetails());
UserDetails currentUserDetails;
if (CurrentUserUtil.hasCurrentUser()) {
currentUserDetails = CurrentUserUtil.getCurrentUserDetails();
} else {
currentUserDetails = new SystemUser();
}
return sendMessage(params, currentUserDetails);
}

@Override
Expand Down Expand Up @@ -254,7 +260,7 @@ public void sendReply(
String metaData,
boolean internal,
Set<FileResource> attachments) {
User sender = userService.getUserByUsername(CurrentUserUtil.getCurrentUsername());
User sender = getCurrentUserOrNull();

Message message = new Message(text, metaData, sender, internal);

Expand Down Expand Up @@ -288,7 +294,7 @@ public long sendCompletenessMessage(CompleteDataSetRegistration registration) {

UserGroup userGroup = dataSet.getNotificationRecipients();

User sender = userService.getUserByUsername(CurrentUserUtil.getCurrentUsername());
User sender = getCurrentUserOrNull();

// data set completed through sms
if (sender == null) {
Expand Down Expand Up @@ -367,7 +373,7 @@ public MessageConversation getMessageConversation(String uid) {
return null;
}

User currentUser = userService.getUserByUsername(CurrentUserUtil.getCurrentUsername());
User currentUser = getCurrentUserOrNull();

mc.setFollowUp(mc.isFollowUp(currentUser));
mc.setRead(mc.isRead(currentUser));
Expand All @@ -378,7 +384,7 @@ public MessageConversation getMessageConversation(String uid) {
@Override
@Transactional(readOnly = true)
public long getUnreadMessageConversationCount() {
User currentUser = userService.getUserByUsername(CurrentUserUtil.getCurrentUsername());
User currentUser = getCurrentUserOrNull();
return messageConversationStore.getUnreadUserMessageConversationCount(currentUser);
}

Expand All @@ -391,15 +397,15 @@ public long getUnreadMessageConversationCount(User user) {
@Override
@Transactional(readOnly = true)
public List<MessageConversation> getMessageConversations() {
User currentUser = userService.getUserByUsername(CurrentUserUtil.getCurrentUsername());
User currentUser = getCurrentUserOrNull();
return messageConversationStore.getMessageConversations(
currentUser, null, false, false, null, null);
}

@Override
@Transactional(readOnly = true)
public List<MessageConversation> getMessageConversations(int first, int max) {
User currentUser = userService.getUserByUsername(CurrentUserUtil.getCurrentUsername());
User currentUser = getCurrentUserOrNull();
return messageConversationStore.getMessageConversations(
currentUser, null, false, false, first, max);
}
Expand Down Expand Up @@ -437,7 +443,7 @@ public void deleteMessages(User user) {
@Override
@Transactional(readOnly = true)
public List<UserMessage> getLastRecipients(int first, int max) {
User currentUser = userService.getUserByUsername(CurrentUserUtil.getCurrentUsername());
User currentUser = getCurrentUserOrNull();
return messageConversationStore.getLastRecipients(currentUser, first, max);
}

Expand Down Expand Up @@ -519,4 +525,12 @@ private Object constructUrl(
String expectedBaseUrlFormat = removeAnyTrailingSlash(baseUrl);
return expectedBaseUrlFormat + MESSAGE_PATH + messageType + "/" + uid;
}

private User getCurrentUserOrNull() {
if (CurrentUserUtil.hasCurrentUser()) {
String currentUsername = CurrentUserUtil.getCurrentUsername();
return userService.getUserByUsername(currentUsername);
}
return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
*/
package org.hisp.dhis.message;

import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
Expand Down Expand Up @@ -192,4 +193,18 @@ void testGetMessageConversations() {
assertTrue(conversations.contains(conversationB));
assertFalse(conversations.contains(conversationC));
}

@Test
void testSendMessageWithNoCurrentUser() {
clearSecurityContext();
MessageConversationParams params =
new MessageConversationParams.Builder()
.withRecipients(Set.of(userA))
.withSubject("subject")
.withText("text")
.withMessageType(MessageType.SYSTEM)
.build();

assertDoesNotThrow(() -> messageService.sendMessage(params));
}
}

0 comments on commit a9f65d5

Please sign in to comment.