Skip to content

Commit

Permalink
MODCONSKC-60. Fix issue with adding consortia-configuration in member…
Browse files Browse the repository at this point in the history
… tenants
  • Loading branch information
SerhiiNosko committed Jan 21, 2025
1 parent 30da556 commit 3d26205
Show file tree
Hide file tree
Showing 8 changed files with 60 additions and 72 deletions.
1 change: 1 addition & 0 deletions descriptors/ModuleDescriptor-template.json
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@
"consortia.tenants.item.post"
],
"modulePermissions": [
"consortia.consortia-configuration.item.post",
"perms.users.item.put",
"perms.users.item.post",
"perms.users.assign.immutable",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package org.folio.consortia.client;

import org.folio.consortia.domain.dto.ConsortiaConfiguration;
import org.folio.spring.config.FeignClientConfiguration;
import org.springframework.cloud.openfeign.FeignClient;
import org.springframework.http.MediaType;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestBody;

@FeignClient(name = "consortia-configuration" , configuration = FeignClientConfiguration.class)
public interface ConsortiaConfigurationClient {

@PostMapping(consumes = MediaType.APPLICATION_JSON_VALUE)
void saveConfiguration(@RequestBody ConsortiaConfiguration configuration);
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,10 @@ public interface ConsortiaConfigurationService {

/**
* Save new configuration with central tenant id as value.
* This configuration will be stored in requested tenant schema
* This configuration will be stored in requested tenant schema.
* Will override configuration if already exists.
*
* @param centralTenantId id of central tenant for requested tenant
* @throws ResourceAlreadyExistException if configuration already exists
*/
ConsortiaConfiguration createConfiguration(String centralTenantId) throws ResourceAlreadyExistException;

/**
* Check if there exists a central tenant configuration;
* if not then save new configuration with central tenant id as value.
* This configuration will be stored in requested tenant schema
*
* @param centralTenantId id of central tenant for requested tenant
*/
void createConfigurationIfNeeded(String centralTenantId);
ConsortiaConfiguration createConfiguration(String centralTenantId);
}
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
package org.folio.consortia.service.impl;

import java.util.List;
import java.util.function.Supplier;

import lombok.RequiredArgsConstructor;
import lombok.extern.log4j.Log4j2;
import org.folio.consortia.domain.dto.ConsortiaConfiguration;
import org.folio.consortia.domain.entity.ConsortiaConfigurationEntity;
import org.folio.consortia.exception.ResourceAlreadyExistException;
import org.folio.consortia.exception.ResourceNotFoundException;
import org.folio.consortia.repository.ConsortiaConfigurationRepository;
import org.folio.consortia.service.ConsortiaConfigurationService;
Expand All @@ -20,8 +18,6 @@
@Service
@RequiredArgsConstructor
public class ConsortiaConfigurationServiceImpl implements ConsortiaConfigurationService {
private static final String CONSORTIA_CONFIGURATION_EXIST_MSG_TEMPLATE =
"System can not have more than one configuration record";
private final ConsortiaConfigurationRepository configurationRepository;
private final ConversionService converter;
private final FolioExecutionContext folioExecutionContext;
Expand All @@ -39,21 +35,9 @@ public String getCentralTenantId(String requestTenantId) {
}

@Override
public ConsortiaConfiguration createConfiguration(String centralTenantId) throws ResourceAlreadyExistException {
return createConfiguration(centralTenantId, () -> {
throw new ResourceAlreadyExistException(CONSORTIA_CONFIGURATION_EXIST_MSG_TEMPLATE);
});
}

@Override
public void createConfigurationIfNeeded(String centralTenantId) {
createConfiguration(centralTenantId, this::getConsortiaConfiguration);
}

private ConsortiaConfiguration createConfiguration(String centralTenantId, Supplier<ConsortiaConfiguration> supplierIfConfigExists) {
public ConsortiaConfiguration createConfiguration(String centralTenantId) {
if (configurationRepository.count() > 0) {
log.info("createConfiguration:: Configuration already exists for central tenant: '{}'", centralTenantId);
return supplierIfConfigExists.get();
log.info("createConfiguration:: Override existing consortia configuration with centralTenantId: '{}'", centralTenantId);
}
ConsortiaConfigurationEntity configuration = new ConsortiaConfigurationEntity();
configuration.setCentralTenantId(centralTenantId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@
import static org.folio.consortia.service.impl.CustomFieldServiceImpl.ORIGINAL_TENANT_ID_NAME;
import static org.folio.consortia.utils.HelperUtils.checkIdenticalOrThrow;

import java.util.List;
import java.util.Objects;
import java.util.UUID;

import org.apache.commons.collections4.map.CaseInsensitiveMap;
import org.apache.commons.lang3.ObjectUtils;
import org.folio.consortia.client.ConsortiaConfigurationClient;
import org.folio.consortia.client.UserTenantsClient;
import org.folio.consortia.domain.dto.ConsortiaConfiguration;
import org.folio.consortia.domain.dto.Tenant;
import org.folio.consortia.domain.dto.TenantCollection;
import org.folio.consortia.domain.dto.TenantDetails;
Expand All @@ -30,6 +30,7 @@
import org.folio.consortia.service.TenantManager;
import org.folio.consortia.service.TenantService;
import org.folio.consortia.service.UserService;
import org.folio.consortia.utils.TenantContextUtils;
import org.folio.spring.FolioExecutionContext;
import org.folio.spring.context.ExecutionContextBuilder;
import org.folio.spring.scope.FolioExecutionContextSetter;
Expand All @@ -52,7 +53,7 @@ public class TenantManagerImpl implements TenantManager {

private final TenantService tenantService;
private final ConsortiumService consortiumService;
private final ConsortiaConfigurationService consortiaConfigurationService;
private final ConsortiaConfigurationClient configurationClient;
private final SyncPrimaryAffiliationService syncPrimaryAffiliationService;
private final UserService userService;
private final CapabilitiesUserService capabilitiesUserService;
Expand Down Expand Up @@ -171,11 +172,12 @@ private Tenant addNewTenant(UUID consortiumId, Tenant tenantDto, UUID adminUserI

var finalShadowAdminUser = shadowAdminUser;
// switch to context of the desired tenant and apply all necessary setup

var allHeaders = new CaseInsensitiveMap<>(folioExecutionContext.getOkapiHeaders());
allHeaders.put("x-okapi-tenant", List.of(tenantDto.getId()));
try (var ignored = new FolioExecutionContextSetter(folioExecutionContext.getFolioModuleMetadata(), allHeaders)) {
consortiaConfigurationService.createConfigurationIfNeeded(centralTenantId);
try (var ignored = new FolioExecutionContextSetter(TenantContextUtils.prepareContextForTenant(tenantDto.getId(),
folioExecutionContext.getFolioModuleMetadata(), folioExecutionContext))) {
// Use self-invocation to avoid making calls to other DB schemas and prevent issues
// with SQL connections tied to the original tenant gathered from connection pool, which would prevent tenant switching
configurationClient.saveConfiguration(createConsortiaConfigurationBody(centralTenantId));
log.info("save:: consortia configuration was created in tenant '{}'", tenantDto.getId());
if (!tenantDto.getIsCentral() && isUserTenantsEmpty()) {
createUserTenantWithDummyUser(tenantDto.getId(), centralTenantId, consortiumId);
createShadowAdminWithPermissions(finalShadowAdminUser);
Expand Down Expand Up @@ -267,4 +269,10 @@ private void createShadowAdminWithPermissions(User user) {
capabilitiesUserService.createWithPermissionSetsFromFile(userOptional.getId(), SHADOW_ADMIN_PERMISSION_SETS_FILE_PATH);
}

private ConsortiaConfiguration createConsortiaConfigurationBody(String tenantId) {
ConsortiaConfiguration configuration = new ConsortiaConfiguration();
configuration.setCentralTenantId(tenantId);
return configuration;
}

}
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package org.folio.consortia.controller;

import static org.folio.consortia.support.EntityUtils.createConsortiaConfiguration;
import static org.folio.consortia.support.EntityUtils.createTenant;
import static org.folio.consortia.support.EntityUtils.createTenantDetailsEntity;
import static org.folio.consortia.support.EntityUtils.createTenantEntity;
Expand Down Expand Up @@ -34,6 +33,7 @@
import java.util.UUID;
import org.folio.consortia.base.BaseIT;
import org.folio.consortia.client.CapabilitySetsClient;
import org.folio.consortia.client.ConsortiaConfigurationClient;
import org.folio.consortia.client.UserCapabilitySetsClient;
import org.folio.consortia.client.UserTenantsClient;
import org.folio.consortia.client.UsersClient;
Expand All @@ -54,7 +54,6 @@
import org.folio.consortia.repository.TenantDetailsRepository;
import org.folio.consortia.repository.TenantRepository;
import org.folio.consortia.repository.UserTenantRepository;
import org.folio.consortia.service.ConsortiaConfigurationService;
import org.folio.consortia.service.SyncPrimaryAffiliationService;
import org.folio.consortia.service.TenantService;
import org.folio.consortia.service.UserService;
Expand Down Expand Up @@ -96,7 +95,7 @@ class TenantControllerTest extends BaseIT {
@MockBean
UserTenantRepository userTenantRepository;
@MockBean
ConsortiaConfigurationService configurationService;
ConsortiaConfigurationClient consortiaConfigurationClient;
@MockBean
KafkaService kafkaService;
@MockBean
Expand Down Expand Up @@ -169,7 +168,7 @@ void shouldSaveTenant(String contentString) throws Exception {
when(tenantDetailsRepository.save(any(TenantDetailsEntity.class))).thenReturn(tenantDetailsEntity);
when(tenantRepository.findCentralTenant()).thenReturn(Optional.of(centralTenant));
doNothing().when(syncPrimaryAffiliationService).syncPrimaryAffiliations(any(UUID.class), anyString(), anyString());
when(configurationService.createConfiguration(CENTRAL_TENANT_ID)).thenReturn(createConsortiaConfiguration(CENTRAL_TENANT_ID));
doNothing().when(consortiaConfigurationClient).saveConfiguration(any());

this.mockMvc.perform(
post("/consortia/7698e46-c3e3-11ed-afa1-0242ac120002/tenants?adminUserId=" + adminUser.getId())
Expand Down Expand Up @@ -272,7 +271,7 @@ void shouldGet4xxErrorWhileSaving(String contentString) throws Exception {

doReturn(new User()).when(usersKeycloakClient).getUsersByUserId(any());
when(tenantRepository.findCentralTenant()).thenReturn(Optional.of(centralTenant));
when(configurationService.createConfiguration(CENTRAL_TENANT_ID)).thenReturn(createConsortiaConfiguration(CENTRAL_TENANT_ID));
doNothing().when(consortiaConfigurationClient).saveConfiguration(any());

this.mockMvc.perform(post("/consortia/7698e46-c3e3-11ed-afa1-0242ac120002/tenants?adminUserId=111841e3-e6fb-4191-9fd8-5674a5107c34")
.headers(headers).content(contentString))
Expand Down Expand Up @@ -311,7 +310,7 @@ void shouldThrownMethodArgumentNotValidException(String contentString) throws Ex
when(consortiumRepository.existsById(consortiumId)).thenReturn(true);
when(tenantRepository.existsById(any(String.class))).thenReturn(false);
when(tenantRepository.findCentralTenant()).thenReturn(Optional.of(centralTenant));
when(configurationService.createConfiguration(CENTRAL_TENANT_ID)).thenReturn(createConsortiaConfiguration(CENTRAL_TENANT_ID));
doNothing().when(consortiaConfigurationClient).saveConfiguration(any());

Set<ConstraintViolation<?>> constraintViolations = new HashSet<>();
constraintViolations.add(mock(ConstraintViolation.class));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package org.folio.consortia.service;

import org.folio.consortia.domain.entity.ConsortiaConfigurationEntity;
import org.folio.consortia.exception.ResourceAlreadyExistException;
import org.folio.consortia.exception.ResourceNotFoundException;
import org.folio.consortia.repository.ConsortiaConfigurationRepository;
import org.folio.consortia.service.impl.ConsortiaConfigurationServiceImpl;
Expand All @@ -27,7 +26,6 @@
import static org.folio.consortia.support.EntityUtils.createConsortiaConfigurationEntity;
import static org.folio.consortia.support.EntityUtils.createOkapiHeaders;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
Expand Down Expand Up @@ -82,37 +80,27 @@ void shouldSaveConfigValue() {

when(configurationRepository.save(any())).thenReturn(configuration);
when(configurationRepository.count()).thenReturn(0L);
when(conversionService.convert(configuration, ConsortiaConfiguration.class))
.thenReturn(createConsortiaConfiguration(CENTRAL_TENANT_ID));

configurationService.createConfiguration(CENTRAL_TENANT_ID);

verify(configurationRepository, times(1)).save(any());
}

@Test
void shouldReturnConfigValueWhenSavingIfExists() {
void shouldReSaveConfigValueWhenItExistsAlready() {
ConsortiaConfigurationEntity configuration = createConsortiaConfigurationEntity(CENTRAL_TENANT_ID);

when(folioExecutionContext.getOkapiHeaders()).thenReturn(createOkapiHeaders());
when(configurationRepository.findAll()).thenReturn(List.of(configuration));
when(configurationRepository.count()).thenReturn(1L);
when(conversionService.convert(configuration, ConsortiaConfiguration.class))
.thenReturn(createConsortiaConfiguration(CENTRAL_TENANT_ID));

configurationService.createConfigurationIfNeeded(CENTRAL_TENANT_ID);

verify(configurationRepository, never()).save(configuration);
}

@Test
void shouldThrowResourceAlreadyExistExceptionErrorWhileSavingConfigValue() {
ConsortiaConfigurationEntity configuration = createConsortiaConfigurationEntity(CENTRAL_TENANT_ID);

when(configurationRepository.save(any())).thenReturn(configuration);
when(configurationRepository.count()).thenReturn(1L);

Assertions.assertThrows(ResourceAlreadyExistException.class,
() -> configurationService.createConfiguration(CENTRAL_TENANT_ID));

verify(configurationRepository, times(0)).save(any());
configurationService.createConfiguration(CENTRAL_TENANT_ID);

verify(configurationRepository, times(1)).save(any());
}

@Test
Expand Down
20 changes: 11 additions & 9 deletions src/test/java/org/folio/consortia/service/TenantManagerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@
import java.util.Optional;
import java.util.UUID;
import java.util.concurrent.Callable;
import org.folio.consortia.client.ConsortiaConfigurationClient;
import org.folio.consortia.client.UserTenantsClient;
import org.folio.consortia.domain.dto.ConsortiaConfiguration;
import org.folio.consortia.domain.dto.Tenant;
import org.folio.consortia.domain.dto.TenantDetails;
import org.folio.consortia.domain.dto.User;
Expand Down Expand Up @@ -85,7 +87,7 @@ class TenantManagerTest {
@Spy
private FolioExecutionContext folioExecutionContext = getFolioExecutionContext();
@Mock
private ConsortiaConfigurationService consortiaConfigurationService;
private ConsortiaConfigurationClient consortiaConfigurationClient;
@Mock
private CapabilitiesUserService capabilitiesUserService;
@Mock
Expand All @@ -111,7 +113,7 @@ class TenantManagerTest {
@BeforeEach
void setUp() {
tenantService = new TenantServiceImpl(tenantRepository, userTenantRepository, tenantDetailsRepository, conversionService, consortiumService, folioExecutionContext);
tenantManager = new TenantManagerImpl(tenantService, consortiumService, consortiaConfigurationService, syncPrimaryAffiliationService, userService, capabilitiesUserService,
tenantManager = new TenantManagerImpl(tenantService, consortiumService, consortiaConfigurationClient, syncPrimaryAffiliationService, userService, capabilitiesUserService,
customFieldService, cleanupService, lockService, userTenantsClient, systemUserScopedExecutionService, executionContextBuilder, folioExecutionContext);
}

Expand Down Expand Up @@ -163,7 +165,7 @@ void shouldSaveNotCentralTenantWithNewUserAndPermissions() {
when(tenantRepository.existsById(any())).thenReturn(false);
when(tenantRepository.findCentralTenant()).thenReturn(Optional.of(centralTenant));
when(tenantDetailsRepository.save(any(TenantDetailsEntity.class))).thenReturn(localTenantDetailsEntity);
doNothing().when(consortiaConfigurationService).createConfigurationIfNeeded(TENANT_ID);
doNothing().when(consortiaConfigurationClient).saveConfiguration(any(ConsortiaConfiguration.class));
doNothing().when(userTenantsClient).postUserTenant(any());
when(userTenantsClient.getUserTenants()).thenReturn(new UserTenantCollection(List.of(), 0));
when(conversionService.convert(localTenantDetailsEntity, Tenant.class)).thenReturn(tenant);
Expand All @@ -179,7 +181,7 @@ void shouldSaveNotCentralTenantWithNewUserAndPermissions() {

verify(userService, times(1)).prepareShadowUser(UUID.fromString(adminUser.getId()), "diku");
verify(userTenantRepository, times(1)).save(any());
verify(consortiaConfigurationService).createConfigurationIfNeeded(any());
verify(consortiaConfigurationClient).saveConfiguration(any());
verify(userTenantsClient).postUserTenant(any());
verify(userService, times(1)).createUser(any());
verify(lockService).lockTenantSetupWithinTransaction();
Expand All @@ -201,7 +203,7 @@ void shouldSaveCentralTenantWithExistingAndPermissions() {
when(tenantRepository.existsById(any())).thenReturn(false);
when(tenantRepository.findCentralTenant()).thenReturn(Optional.of(centralTenant));
when(tenantDetailsRepository.save(any(TenantDetailsEntity.class))).thenReturn(tenantDetailsEntity);
doNothing().when(consortiaConfigurationService).createConfigurationIfNeeded(TENANT_ID);
doNothing().when(consortiaConfigurationClient).saveConfiguration(any());
doNothing().when(userTenantsClient).postUserTenant(any());
when(conversionService.convert(tenantDetailsEntity, Tenant.class)).thenReturn(tenant);
when(folioExecutionContext.getTenantId()).thenReturn("diku");
Expand All @@ -219,7 +221,7 @@ void shouldSaveCentralTenantWithExistingAndPermissions() {

var tenant1 = tenantManager.save(CONSORTIUM_ID, UUID.randomUUID(), tenant);

verify(consortiaConfigurationService).createConfigurationIfNeeded(any());
verify(consortiaConfigurationClient).saveConfiguration(any());
verify(lockService).lockTenantSetupWithinTransaction();

verify(userService, never()).prepareShadowUser(any(), any());
Expand Down Expand Up @@ -253,7 +255,7 @@ void shouldReAddSoftDeletedTenant() {

var actualTenant = tenantManager.save(CONSORTIUM_ID, UUID.randomUUID(), newTenant);

verifyNoInteractions(consortiaConfigurationService);
verifyNoInteractions(consortiaConfigurationClient);
verifyNoInteractions(lockService);

verifyNoInteractions(userService);
Expand Down Expand Up @@ -483,14 +485,14 @@ void testAddNewTenantWithExistentUserTenant() {
when(userService.prepareShadowUser(any(UUID.class), anyString())).thenReturn(adminUser);
when(userTenantRepository.save(any(UserTenantEntity.class))).thenReturn(new UserTenantEntity());

doNothing().when(consortiaConfigurationService).createConfigurationIfNeeded(TENANT_ID);
doNothing().when(consortiaConfigurationClient).saveConfiguration(any());
when(userTenantsClient.getUserTenants()).thenReturn(new UserTenantCollection(List.of(), 1));

var tenant1 = tenantManager.save(CONSORTIUM_ID, UUID.fromString(adminUser.getId()), tenant);

verify(userService, times(1)).prepareShadowUser(UUID.fromString(adminUser.getId()), "diku");
verify(userTenantRepository, times(1)).save(any());
verify(consortiaConfigurationService).createConfigurationIfNeeded(any());
verify(consortiaConfigurationClient).saveConfiguration(any());
verify(lockService).lockTenantSetupWithinTransaction();
verify(userTenantsClient, never()).postUserTenant(any());
verify(userService, never()).getById(any());
Expand Down

0 comments on commit 3d26205

Please sign in to comment.