Skip to content

Commit

Permalink
fix: correct bean handling of authenticated user
Browse files Browse the repository at this point in the history
  • Loading branch information
mebo4b committed Oct 28, 2020
1 parent b5101f7 commit 03b45ea
Show file tree
Hide file tree
Showing 7 changed files with 94 additions and 75 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@
import static de.caritas.cob.userservice.liveservice.generated.web.model.EventType.DIRECTMESSAGE;
import static org.apache.commons.lang3.StringUtils.isNotBlank;

import de.caritas.cob.userservice.api.helper.AuthenticatedUser;
import de.caritas.cob.userservice.api.service.LogService;
import de.caritas.cob.userservice.liveservice.generated.web.LiveControllerApi;
import java.util.List;
import java.util.stream.Collectors;
import lombok.NonNull;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
Expand All @@ -20,6 +22,7 @@ public class LiveEventNotificationService {

private final @NonNull LiveControllerApi liveControllerApi;
private final @NonNull UserIdsProviderFactory userIdsProviderFactory;
private final @NonNull AuthenticatedUser authenticatedUser;

/**
* Collects all relevant user or consultant ids of chats and sessions and sends a new
Expand All @@ -30,7 +33,9 @@ public class LiveEventNotificationService {
public void sendLiveDirectMessageEventToUsers(String rcGroupId) {
if (isNotBlank(rcGroupId)) {
List<String> userIds = this.userIdsProviderFactory.byRocketChatGroup(rcGroupId)
.collectUserIds(rcGroupId);
.collectUserIds(rcGroupId).stream()
.filter(this::notInitiatingUser)
.collect(Collectors.toList());
try {
this.liveControllerApi.sendLiveEvent(userIds, DIRECTMESSAGE.toString());
} catch (RestClientException e) {
Expand All @@ -41,4 +46,8 @@ public void sendLiveDirectMessageEventToUsers(String rcGroupId) {
}
}

private boolean notInitiatingUser(String userId) {
return !userId.equals(this.authenticatedUser.getUserId());
}

}
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
package de.caritas.cob.userservice.api.service.liveevents;

import static java.util.Collections.emptyList;
import static java.util.Objects.requireNonNull;

import de.caritas.cob.userservice.api.exception.rocketchat.RocketChatGetGroupMembersException;
import de.caritas.cob.userservice.api.helper.AuthenticatedUser;
import de.caritas.cob.userservice.api.model.rocketchat.group.GroupMemberDTO;
import de.caritas.cob.userservice.api.repository.consultant.Consultant;
import de.caritas.cob.userservice.api.repository.user.User;
Expand All @@ -15,28 +13,20 @@
import java.util.List;
import java.util.Objects;
import java.util.stream.Collectors;
import org.springframework.beans.factory.annotation.Autowired;
import lombok.NonNull;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Component;

/**
* Provider to observe all assigned chat user ids instead of initiator.
*/
@Component
public class RelevantUserAccountIdsByChatProvider extends UserIdsProvider {
@RequiredArgsConstructor
public class RelevantUserAccountIdsByChatProvider implements UserIdsProvider {

private final RocketChatService rocketChatService;
private final UserService userService;
private final ConsultantService consultantService;

@Autowired
public RelevantUserAccountIdsByChatProvider(AuthenticatedUser authenticatedUser,
RocketChatService rocketChatService, UserService userService,
ConsultantService consultantService) {
super(authenticatedUser);
this.rocketChatService = requireNonNull(rocketChatService);
this.userService = requireNonNull(userService);
this.consultantService = requireNonNull(consultantService);
}
private final @NonNull RocketChatService rocketChatService;
private final @NonNull UserService userService;
private final @NonNull ConsultantService consultantService;

/**
* Collects all relevant user ids of a chat.
Expand All @@ -45,7 +35,7 @@ public RelevantUserAccountIdsByChatProvider(AuthenticatedUser authenticatedUser,
* @return a {@link List} containing all user ids to be notified
*/
@Override
List<String> collectUserIds(String rcGroupId) {
public List<String> collectUserIds(String rcGroupId) {
try {
return extractDependentUserIds(rcGroupId);
} catch (RocketChatGetGroupMembersException e) {
Expand All @@ -61,7 +51,6 @@ private List<String> extractDependentUserIds(String rcGroupId)
.map(GroupMemberDTO::get_id)
.map(this::toUserAccountId)
.filter(Objects::nonNull)
.filter(this::notInitiatingUser)
.collect(Collectors.toList());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,31 +2,24 @@

import static java.util.Collections.emptyList;
import static java.util.Objects.isNull;
import static java.util.Objects.requireNonNull;

import de.caritas.cob.userservice.api.helper.AuthenticatedUser;
import de.caritas.cob.userservice.api.repository.session.Session;
import de.caritas.cob.userservice.api.repository.session.SessionRepository;
import java.util.List;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.springframework.beans.factory.annotation.Autowired;
import lombok.NonNull;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Component;

/**
* Provider to observe assigned session user ids instead of initiator.
*/
@Component
public class RelevantUserAccountIdsBySessionProvider extends UserIdsProvider {
@RequiredArgsConstructor
public class RelevantUserAccountIdsBySessionProvider implements UserIdsProvider {

private final SessionRepository sessionRepository;

@Autowired
public RelevantUserAccountIdsBySessionProvider(AuthenticatedUser authenticatedUser,
SessionRepository sessionRepository) {
super(authenticatedUser);
this.sessionRepository = requireNonNull(sessionRepository);
}
private final @NonNull SessionRepository sessionRepository;

/**
* Collects the relevant user id of a session, if consultant wrote, id of user will be returned
Expand All @@ -36,7 +29,7 @@ public RelevantUserAccountIdsBySessionProvider(AuthenticatedUser authenticatedUs
* @return a {@link List} containing the user id to be notified
*/
@Override
List<String> collectUserIds(String rcGroupId) {
public List<String> collectUserIds(String rcGroupId) {
Session session = this.sessionRepository.findByGroupId(rcGroupId)
.orElse(this.sessionRepository.findByFeedbackGroupId(rcGroupId)
.orElse(null));
Expand All @@ -49,7 +42,6 @@ private List<String> extractDependentUserIds(Session session) {
return emptyList();
}
return Stream.of(session.getUser().getUserId(), session.getConsultant().getId())
.filter(this::notInitiatingUser)
.collect(Collectors.toList());
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,19 +1,10 @@
package de.caritas.cob.userservice.api.service.liveevents;

import de.caritas.cob.userservice.api.helper.AuthenticatedUser;
import java.util.List;
import lombok.NonNull;
import lombok.RequiredArgsConstructor;

@RequiredArgsConstructor
abstract class UserIdsProvider {
interface UserIdsProvider {

final @NonNull AuthenticatedUser authenticatedUser;
List<String> collectUserIds(String rcGroupId);

abstract List<String> collectUserIds(String rcGroupId);

boolean notInitiatingUser(String userId) {
return !userId.equals(this.authenticatedUser.getUserId());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

import static de.caritas.cob.userservice.liveservice.generated.web.model.EventType.DIRECTMESSAGE;
import static java.util.Arrays.asList;
import static java.util.Collections.emptyList;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
Expand All @@ -13,8 +14,10 @@
import static org.mockito.Mockito.when;
import static org.powermock.reflect.Whitebox.setInternalState;

import de.caritas.cob.userservice.api.helper.AuthenticatedUser;
import de.caritas.cob.userservice.api.service.LogService;
import de.caritas.cob.userservice.liveservice.generated.web.LiveControllerApi;
import java.util.List;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand All @@ -39,6 +42,12 @@ public class LiveEventNotificationServiceTest {
@Mock
private RelevantUserAccountIdsBySessionProvider bySessionProvider;

@Mock
private RelevantUserAccountIdsByChatProvider byChatProvider;

@Mock
private AuthenticatedUser authenticatedUser;

@Mock
private Logger logger;

Expand Down Expand Up @@ -86,4 +95,41 @@ public void sendLiveDirectMessageEventToUsers_Should_logError_When_apiCallFailes
verify(logger, times(1)).error(anyString(), anyString(), anyString());
}

@Test
public void sendLiveDirectMessageEventToUsers_Should_sendEventToAllUsersInsteadOfInitiatingUser() {
List<String> userIds = asList("id1", "id2", "id3", "id4");
when(this.byChatProvider.collectUserIds(any())).thenReturn(userIds);
when(this.userIdsProviderFactory.byRocketChatGroup(any())).thenReturn(this.byChatProvider);
when(this.authenticatedUser.getUserId()).thenReturn("id2");

this.liveEventNotificationService.sendLiveDirectMessageEventToUsers("group id");

List<String> expectedIds = asList("id1", "id3", "id4");
verify(this.liveControllerApi, times(1)).sendLiveEvent(eq(expectedIds),
eq(DIRECTMESSAGE.toString()));
}

@Test
public void sendLiveDirectMessageEventToUsers_Should_sendEmptyList_When_noIdsAreProvided() {
when(this.userIdsProviderFactory.byRocketChatGroup(any())).thenReturn(this.byChatProvider);

this.liveEventNotificationService.sendLiveDirectMessageEventToUsers("group id");

verify(this.liveControllerApi, times(1)).sendLiveEvent(eq(emptyList()),
eq(DIRECTMESSAGE.toString()));
}

@Test
public void sendLiveDirectMessageEventToUsers_Should_sendEventToAllUsers_When_initiatingUserIsAnother() {
List<String> userIds = asList("id1", "id2", "id3", "id4");
when(this.byChatProvider.collectUserIds(any())).thenReturn(userIds);
when(this.userIdsProviderFactory.byRocketChatGroup(any())).thenReturn(this.byChatProvider);
when(this.authenticatedUser.getUserId()).thenReturn("another");

this.liveEventNotificationService.sendLiveDirectMessageEventToUsers("group id");

verify(this.liveControllerApi, times(1)).sendLiveEvent(eq(userIds),
eq(DIRECTMESSAGE.toString()));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,6 @@ public class RelevantUserAccountIdsByChatProviderTest {
@InjectMocks
private RelevantUserAccountIdsByChatProvider byChatProvider;

@Mock
private AuthenticatedUser authenticatedUser;

@Mock
private RocketChatService rocketChatService;

Expand All @@ -54,7 +51,7 @@ public void collectUserIds_Should_returnEmptyList_When_rocketChatServiceThrowsEx
}

@Test
public void collectUserIds_Should_returnAllMergedDependingIdsInsteadOfAuthenticatedUser_When_rcGroupHasMembers()
public void collectUserIds_Should_returnAllMergedDependingIds_When_rcGroupHasMembers()
throws RocketChatGetGroupMembersException {
List<GroupMemberDTO> groupMembers = asList(
memberDTOWithRcId("rc1"), memberDTOWithRcId("rc2"), memberDTOWithRcId("rc3"));
Expand All @@ -65,13 +62,13 @@ public void collectUserIds_Should_returnAllMergedDependingIdsInsteadOfAuthentica
.thenReturn(Optional.of(userWithId("user1")));
when(this.userService.findUserByRcUserId(eq("rc3")))
.thenReturn(Optional.of(userWithId("user2")));
when(this.authenticatedUser.getUserId()).thenReturn("user2");

List<String> collectedUserIds = this.byChatProvider.collectUserIds("groupId");

assertThat(collectedUserIds, hasSize(2));
assertThat(collectedUserIds, hasSize(3));
assertThat(collectedUserIds.get(0), is("consultant1"));
assertThat(collectedUserIds.get(1), is("user1"));
assertThat(collectedUserIds.get(2), is("user2"));
}

private GroupMemberDTO memberDTOWithRcId(String rcId) {
Expand All @@ -91,7 +88,7 @@ private User userWithId(String userId) {
}

@Test
public void collectUserIds_Should_returnAllMergedDependingIdsInsteadOfAuthenticatedAndNotAvailableUser_When_rcGroupHasMembers()
public void collectUserIds_Should_returnAllMergedDependingIdsInsteadOfNotAvailableUser_When_rcGroupHasMembers()
throws RocketChatGetGroupMembersException {
List<GroupMemberDTO> groupMembers = asList(
memberDTOWithRcId("rc1"), memberDTOWithRcId("rc2"), memberDTOWithRcId("rc3"));
Expand All @@ -100,12 +97,12 @@ public void collectUserIds_Should_returnAllMergedDependingIdsInsteadOfAuthentica
.thenReturn(Optional.of(consultantWithId("consultant1")));
when(this.userService.findUserByRcUserId(eq("rc3")))
.thenReturn(Optional.of(userWithId("user2")));
when(this.authenticatedUser.getUserId()).thenReturn("user2");

List<String> collectedUserIds = this.byChatProvider.collectUserIds("groupId");

assertThat(collectedUserIds, hasSize(1));
assertThat(collectedUserIds, hasSize(2));
assertThat(collectedUserIds.get(0), is("consultant1"));
assertThat(collectedUserIds.get(1), is("user2"));
}

}
Loading

0 comments on commit 03b45ea

Please sign in to comment.