Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: make sure background threads check for not logged in current user #16402

Merged
merged 4 commits into from
Feb 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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));
}
}
Loading