Skip to content

Commit

Permalink
MODCON-73. Removed unnecessary folioExecution.getInstance() and setti…
Browse files Browse the repository at this point in the history
…ng context again in scope of method. (#67)

* [MODCON - 73] - Removed unnecessary folioExecution.getInstance() and setting context again in scope of method.

* [MODCON - 73] - Removed unnecessary folioExecution.getInstance() and setting context again in scope of method.

* [MODCON - 73] - Removed unnecessary when(folioExecution.getInstance()).... from Tests

* [MODCON - 73] - Removed unnecessary when(folioExecution.getInstance()).... from Tests

* [MODCON - 73] - Minor improvements

* [MODCON - 73] - Minor improvements
  • Loading branch information
azizbekxm authored Jul 13, 2023
1 parent 4a202d5 commit c3c925d
Show file tree
Hide file tree
Showing 10 changed files with 12 additions and 60 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,7 @@ public class ConsortiaConfigurationServiceImpl implements ConsortiaConfiguration

@Override
public ConsortiaConfiguration getConsortiaConfiguration() {
FolioExecutionContext currentContext = (FolioExecutionContext) folioExecutionContext.getInstance();
String requestedTenantId = getTenantIdFromHeader(currentContext);
String requestedTenantId = getTenantIdFromHeader(folioExecutionContext);
ConsortiaConfigurationEntity configuration = getConfiguration(requestedTenantId);
return converter.convert(configuration, ConsortiaConfiguration.class);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ public TenantEntity getByTenantId(String tenantId) {
public Tenant save(UUID consortiumId, UUID adminUserId, Tenant tenantDto) {
log.info("save:: Trying to save a tenant by consortiumId '{}', tenant object with id '{}' and isCentral={}", consortiumId,
tenantDto.getId(), tenantDto.getIsCentral());
FolioExecutionContext currentTenantContext = (FolioExecutionContext) folioExecutionContext.getInstance();

// validation part
checkTenantNotExistsAndConsortiumExistsOrThrow(consortiumId, tenantDto.getId());
Expand All @@ -109,7 +108,7 @@ public Tenant save(UUID consortiumId, UUID adminUserId, Tenant tenantDto) {
} else {
checkAdminUserIdPresentOrThrow(adminUserId);
centralTenantId = getCentralTenantId();
shadowAdminUser = userService.prepareShadowUser(adminUserId, currentTenantContext.getTenantId());
shadowAdminUser = userService.prepareShadowUser(adminUserId, folioExecutionContext.getTenantId());
userTenantRepository.save(createUserTenantEntity(consortiumId, shadowAdminUser, tenantDto));
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package org.folio.consortia.service.impl;

import static org.folio.consortia.utils.TenantContextUtils.prepareContextForTenant;

import java.util.UUID;

import org.apache.commons.lang3.ObjectUtils;
Expand All @@ -16,15 +14,12 @@
import org.folio.consortia.service.UserAffiliationService;
import org.folio.consortia.service.UserTenantService;
import org.folio.spring.FolioExecutionContext;
import org.folio.spring.FolioModuleMetadata;
import org.folio.spring.scope.FolioExecutionContextSetter;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

import com.fasterxml.jackson.databind.ObjectMapper;

import lombok.AllArgsConstructor;
import lombok.SneakyThrows;
import lombok.extern.log4j.Log4j2;

@Service
Expand All @@ -37,14 +32,11 @@ public class UserAffiliationServiceImpl implements UserAffiliationService {
private final TenantService tenantService;
private final KafkaService kafkaService;
private final FolioExecutionContext folioExecutionContext;
private final FolioModuleMetadata folioModuleMetadata;
private final ObjectMapper objectMapper = new ObjectMapper();

@Override
@SneakyThrows
@Transactional
public void createPrimaryUserAffiliation(String eventPayload) {
FolioExecutionContext currentContext = (FolioExecutionContext) folioExecutionContext.getInstance();
String centralTenantId = folioExecutionContext.getTenantId();
try {
var userEvent = objectMapper.readValue(eventPayload, UserEvent.class);
Expand Down Expand Up @@ -72,18 +64,14 @@ public void createPrimaryUserAffiliation(String eventPayload) {
PrimaryAffiliationEvent affiliationEvent = createPrimaryAffiliationEvent(userEvent, centralTenantId);
String data = objectMapper.writeValueAsString(affiliationEvent);

// context is changed in save() method and context is empty after save() method, so we need to set context again.
try (var context = new FolioExecutionContextSetter(prepareContextForTenant(centralTenantId, folioModuleMetadata, currentContext))) {
kafkaService.send(KafkaService.Topic.CONSORTIUM_PRIMARY_AFFILIATION_CREATED, consortiaTenant.getConsortiumId().toString(), data);
}
kafkaService.send(KafkaService.Topic.CONSORTIUM_PRIMARY_AFFILIATION_CREATED, consortiaTenant.getConsortiumId().toString(), data);
log.info("Primary affiliation has been set for the user: {}", userEvent.getUserDto().getId());
} catch (Exception e) {
log.error("Exception occurred while creating primary affiliation", e);
}
}

@Override
@SneakyThrows
@Transactional
public void updatePrimaryUserAffiliation(String eventPayload) {
String centralTenantId = folioExecutionContext.getTenantId();
Expand Down Expand Up @@ -117,10 +105,8 @@ public void updatePrimaryUserAffiliation(String eventPayload) {
}

@Override
@SneakyThrows
@Transactional
public void deletePrimaryUserAffiliation(String eventPayload) {
FolioExecutionContext currentContext = (FolioExecutionContext) folioExecutionContext.getInstance();
String centralTenantId = folioExecutionContext.getTenantId();
try {
var userEvent = objectMapper.readValue(eventPayload, UserEvent.class);
Expand All @@ -138,10 +124,7 @@ public void deletePrimaryUserAffiliation(String eventPayload) {
PrimaryAffiliationEvent affiliationEvent = createPrimaryAffiliationEvent(userEvent, centralTenantId);
String data = objectMapper.writeValueAsString(affiliationEvent);

// context is changed in deleteShadowUsers() method and context is empty after deleteShadowUsers() method, so we need to set context again.
try (var context = new FolioExecutionContextSetter(prepareContextForTenant(centralTenantId, folioModuleMetadata, currentContext))) {
kafkaService.send(KafkaService.Topic.CONSORTIUM_PRIMARY_AFFILIATION_DELETED, consortiaTenant.getConsortiumId().toString(), data);
}
kafkaService.send(KafkaService.Topic.CONSORTIUM_PRIMARY_AFFILIATION_DELETED, consortiaTenant.getConsortiumId().toString(), data);
log.info("Primary affiliation has been deleted for the user: {}", userEvent.getUserDto().getId());
} catch (Exception e) {
log.error("Exception occurred while deleting primary affiliation", e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public void deleteById(String userId) {
}

public User prepareShadowUser(UUID userId, String tenantId) {
try (var context = new FolioExecutionContextSetter(prepareContextForTenant(tenantId, folioModuleMetadata, (FolioExecutionContext) folioExecutionContext.getInstance()))) {
try (var context = new FolioExecutionContextSetter(prepareContextForTenant(tenantId, folioModuleMetadata, folioExecutionContext))) {
log.info("prepareShadowUser:: Try to get user of tenant={} ", folioExecutionContext.getTenantId());
User user = new User();
User userOptional = usersClient.getUsersByUserId(userId.toString());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,6 @@ public UserTenantCollection getByUserId(UUID consortiumId, UUID userId, Integer
@Transactional
public UserTenant save(UUID consortiumId, UserTenant userTenantDto, boolean isSystemUserContextRequired) {
log.debug("Going to save user with id: {} into tenant: {}", userTenantDto.getUserId(), userTenantDto.getTenantId());
FolioExecutionContext currentTenantContext = (FolioExecutionContext) folioExecutionContext.getInstance();
String currentTenantId = folioExecutionContext.getTenantId();
consortiumService.checkConsortiumExistsOrThrow(consortiumId);

Optional<UserTenantEntity> userTenant = userTenantRepository.findByUserIdAndIsPrimary(userTenantDto.getUserId(), IS_PRIMARY_TRUE);
Expand All @@ -123,18 +121,15 @@ public UserTenant save(UUID consortiumId, UserTenant userTenantDto, boolean isSy
if (isSystemUserContextRequired) {
createOrUpdateShadowUserWithSystemUserContext(userTenantDto.getUserId(), shadowUser, userTenantDto);
} else {
createOrUpdateShadowUserWithRequestedContext(userTenantDto.getUserId(), shadowUser, userTenantDto, currentTenantContext);
createOrUpdateShadowUserWithRequestedContext(userTenantDto.getUserId(), shadowUser, userTenantDto, folioExecutionContext);
}

try (var context = new FolioExecutionContextSetter(prepareContextForTenant(currentTenantId, folioModuleMetadata, currentTenantContext))) {
UserTenantEntity userTenantEntity = toEntity(userTenantDto, consortiumId, shadowUser);
userTenantRepository.save(userTenantEntity);
log.info("User affiliation added and user created or activated for user id: {} in the tenant: {}",
userTenantDto.getUserId(), userTenantDto.getTenantId());
UserTenantEntity userTenantEntity = toEntity(userTenantDto, consortiumId, shadowUser);
userTenantRepository.save(userTenantEntity);
log.info("User affiliation added and user created or activated for user id: {} in the tenant: {}", userTenantDto.getUserId(), userTenantDto.getTenantId());

return converter.convert(userTenantEntity, UserTenant.class);
}
}
return converter.convert(userTenantEntity, UserTenant.class);
}

@Override
@Transactional
Expand Down Expand Up @@ -168,7 +163,6 @@ public UserTenantEntity getByUserIdAndTenantId(UUID userId, String tenantId) {
@Transactional
public void deleteByUserIdAndTenantId(UUID consortiumId, String tenantId, UUID userId) {
log.debug("Going to delete user affiliation for user id: {} in the tenant: {}", userId.toString(), tenantId);
FolioExecutionContext currentTenantContext = (FolioExecutionContext) folioExecutionContext.getInstance();

consortiumService.checkConsortiumExistsOrThrow(consortiumId);
UserTenantEntity userTenantEntity = userTenantRepository.findByUserIdAndTenantId(userId, tenantId)
Expand All @@ -182,7 +176,7 @@ public void deleteByUserIdAndTenantId(UUID consortiumId, String tenantId, UUID u

userTenantRepository.deleteByUserIdAndTenantId(userId, tenantId);

try (var context = new FolioExecutionContextSetter(prepareContextForTenant(tenantId, folioModuleMetadata, currentTenantContext))) {
try (var context = new FolioExecutionContextSetter(prepareContextForTenant(tenantId, folioModuleMetadata, folioExecutionContext))) {
User user = userService.getById(userId);
deactivateUser(user);
log.info("User affiliation deleted and user deactivated for user id: {} in the tenant: {}", userId.toString(), tenantId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ void shouldGetConfigValueByGetConsortiaConfiguration() {
when(conversionService.convert(configuration, ConsortiaConfiguration.class))
.thenReturn(createConsortiaConfiguration(CENTRAL_TENANT_ID));
when(folioExecutionContext.getTenantId()).thenReturn("testtenant1");
when(folioExecutionContext.getInstance()).thenReturn(folioExecutionContext);
Map<String, Collection<String>> okapiHeaders = new HashMap<>();
okapiHeaders.put(XOkapiHeaders.TENANT, List.of("testtenant1"));
when(folioExecutionContext.getOkapiHeaders()).thenReturn(okapiHeaders);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,6 @@ void shouldSaveNotCentralTenantWithNewUserAndPermissions() {
when(conversionService.convert(tenantEntity1, Tenant.class)).thenReturn(tenant);
doReturn(folioExecutionContext).when(contextHelper).getSystemUserFolioExecutionContext(anyString());
when(folioExecutionContext.getTenantId()).thenReturn("diku");
when(folioExecutionContext.getInstance()).thenReturn(folioExecutionContext);

var tenant1 = tenantService.save(consortiumId, UUID.randomUUID(), tenant);

Expand Down Expand Up @@ -195,7 +194,6 @@ void shouldSaveCentralTenantWithExistingAndPermissions() throws JsonProcessingEx
when(conversionService.convert(tenantEntity1, Tenant.class)).thenReturn(tenant);
doReturn(folioExecutionContext).when(contextHelper).getSystemUserFolioExecutionContext(anyString());
when(folioExecutionContext.getTenantId()).thenReturn("diku");
when(folioExecutionContext.getInstance()).thenReturn(folioExecutionContext);
Map<String, Collection<String>> okapiHeaders = new HashMap<>();
okapiHeaders.put(XOkapiHeaders.TENANT, List.of("diku"));
when(folioExecutionContext.getOkapiHeaders()).thenReturn(okapiHeaders);
Expand Down Expand Up @@ -223,7 +221,6 @@ void shouldUpdateTenant() {

Map<String, Collection<String>> okapiHeaders = new HashMap<>();
okapiHeaders.put(XOkapiHeaders.TENANT, List.of("diku"));
when(folioExecutionContext.getInstance()).thenReturn(folioExecutionContext);
when(folioExecutionContext.getOkapiHeaders()).thenReturn(okapiHeaders);

when(consortiumRepository.existsById(consortiumId)).thenReturn(true);
Expand Down Expand Up @@ -359,7 +356,6 @@ void shouldNotSaveTenantForDuplicateId() {
when(conversionService.convert(tenantEntity1, Tenant.class)).thenReturn(tenant);
when(tenantRepository.findCentralTenant()).thenReturn(Optional.of(centralTenant));
when(folioExecutionContext.getTenantId()).thenReturn("diku");
when(folioExecutionContext.getInstance()).thenReturn(folioExecutionContext);
Map<String, Collection<String>> okapiHeaders = new HashMap<>();
okapiHeaders.put(XOkapiHeaders.TENANT, List.of("diku"));
when(folioExecutionContext.getOkapiHeaders()).thenReturn(okapiHeaders);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ void primaryAffiliationAddedSuccessfullyTest() {

when(tenantService.getByTenantId(anyString())).thenReturn(te);
doNothing().when(consortiumService).checkConsortiumExistsOrThrow(any());
when(folioExecutionContext.getInstance()).thenReturn(folioExecutionContext);
when(folioExecutionContext.getTenantId()).thenReturn("diku");
Map<String, Collection<String>> map = createOkapiHeaders();
when(folioExecutionContext.getOkapiHeaders()).thenReturn(map);
Expand All @@ -97,7 +96,6 @@ void primaryAffiliationAddedSuccessfullyTestToCentralTenant() {

when(tenantService.getByTenantId(anyString())).thenReturn(te);
doNothing().when(consortiumService).checkConsortiumExistsOrThrow(any());
when(folioExecutionContext.getInstance()).thenReturn(folioExecutionContext);
when(folioExecutionContext.getTenantId()).thenReturn("diku");
Map<String, Collection<String>> map = createOkapiHeaders();
when(folioExecutionContext.getOkapiHeaders()).thenReturn(map);
Expand Down Expand Up @@ -168,7 +166,6 @@ void primaryAffiliationSuccessfullyDeletedTest() {

when(tenantService.getByTenantId(anyString())).thenReturn(te);
doNothing().when(consortiumService).checkConsortiumExistsOrThrow(any());
when(folioExecutionContext.getInstance()).thenReturn(folioExecutionContext);
when(folioExecutionContext.getTenantId()).thenReturn("diku");
Map<String, Collection<String>> map = createOkapiHeaders();
when(folioExecutionContext.getOkapiHeaders()).thenReturn(map);
Expand All @@ -188,7 +185,6 @@ void kafkaMessageFailedWhenDeletingTest() {
when(tenantService.getByTenantId(anyString())).thenReturn(te);
doNothing().when(consortiumService).checkConsortiumExistsOrThrow(any());
doThrow(new RuntimeException("Unable to send message to Kafka")).when(kafkaService).send(any(), anyString(), any());
when(folioExecutionContext.getInstance()).thenReturn(folioExecutionContext);
when(folioExecutionContext.getTenantId()).thenReturn("diku");
Map<String, Collection<String>> map = createOkapiHeaders();
when(folioExecutionContext.getOkapiHeaders()).thenReturn(map);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ void shouldDeleteUser() {
@Test
void shouldThrowNotFoundWhilePrepareShadowUser() {
when(folioExecutionContext.getTenantId()).thenReturn("diku");
when(folioExecutionContext.getInstance()).thenReturn(folioExecutionContext);
Map<String, Collection<String>> okapiHeaders = new HashMap<>();
okapiHeaders.put(XOkapiHeaders.TENANT, List.of("diku"));
when(folioExecutionContext.getOkapiHeaders()).thenReturn(okapiHeaders);
Expand All @@ -71,7 +70,6 @@ void shouldThrowNotFoundWhilePrepareShadowUser() {
@Test
void shouldPrepareShadowUser() {
when(folioExecutionContext.getTenantId()).thenReturn("diku");
when(folioExecutionContext.getInstance()).thenReturn(folioExecutionContext);
Map<String, Collection<String>> okapiHeaders = new HashMap<>();
okapiHeaders.put(XOkapiHeaders.TENANT, List.of("diku"));
when(folioExecutionContext.getOkapiHeaders()).thenReturn(okapiHeaders);
Expand Down
Loading

0 comments on commit c3c925d

Please sign in to comment.