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

[MODCON-130] - Adjust process to add ECS tenant with soft deleted functionality #140

Merged
merged 22 commits into from
Dec 27, 2023
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
cb6ab68
[MODCON-127] - Implement ECS tenant soft deletion functionality
azizbekxm Dec 18, 2023
b109ee4
[MODCON-127] - Implement ECS tenant soft deletion functionality
azizbekxm Dec 18, 2023
eb949c1
[MODCON-127] - Implement ECS tenant soft deletion functionality
azizbekxm Dec 19, 2023
56e0403
[MODCON-127] - Covered with Unit Tests
azizbekxm Dec 19, 2023
2a7b946
[MODCON-127] - Fixed unit tests
azizbekxm Dec 19, 2023
71dc94d
[MODCON-127] - Covered with Unit Tests
azizbekxm Dec 19, 2023
472ee38
[MODCON-127] - Added filter to get user methods
azizbekxm Dec 20, 2023
7e9ff61
[MODCON-130] - Adjust process to add ECS tenant with soft deleted fun…
azizbekxm Dec 20, 2023
d85d1a6
[MODCON-130] - Added tests
azizbekxm Dec 21, 2023
edad534
[MODCON-130] - Minor improvements
azizbekxm Dec 22, 2023
6524bab
[MODCON-130] - Minor improvements
azizbekxm Dec 22, 2023
1db47a1
[MODCON-130] - Fixed code smell
azizbekxm Dec 21, 2023
20ee3d6
Merge branch 'master' into MODCON-130-1
azizbekxm Dec 22, 2023
b6504ea
[MODCON-130] - Minor improvements
azizbekxm Dec 22, 2023
8850bd6
[MODCON-130] - Fixed issue relate to PUT request and other improvements
azizbekxm Dec 23, 2023
52c5d03
[MODCON - 130] - Adjust process to add ECS tenant with soft deleted …
azizbekxm Dec 25, 2023
26ea836
[MODCON - 130] - Adjust process to add ECS tenant with soft deleted …
azizbekxm Dec 25, 2023
7b10f54
[MODCON - 130] - Fixed Tests
azizbekxm Dec 25, 2023
4c4035c
[MODCON - 130] - Fixed Tests
azizbekxm Dec 25, 2023
b058c6a
[MODCON - 130] - Minor improvement
azizbekxm Dec 25, 2023
b47723c
[MODCON - 130] - Fixed Tests
azizbekxm Dec 25, 2023
e9cefb1
[MODCON - 130] - Reverted new validation for code and name uniqueness
azizbekxm Dec 26, 2023
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 @@ -14,6 +14,7 @@ public Tenant convert(TenantDetailsEntity source) {
tenant.setCode(source.getCode());
tenant.setName(source.getName());
tenant.setIsCentral(source.getIsCentral());
tenant.setIsDeleted(source.getIsDeleted());
return tenant;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ public TenantDetails convert(TenantDetailsEntity source) {
.code(source.getCode())
.name(source.getName())
.isCentral(source.getIsCentral())
.isDeleted(source.getIsDeleted())
.setupStatus(source.getSetupStatus());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,16 @@
@Repository
public interface UserTenantRepository extends JpaRepository<UserTenantEntity, UUID> {

@Query("SELECT ut FROM UserTenantEntity ut WHERE ut.tenant.isDeleted IS NULL OR ut.tenant.isDeleted= FALSE")
@Query("SELECT ut FROM UserTenantEntity ut WHERE ut.tenant.isDeleted= FALSE")
Page<UserTenantEntity> getAll(Pageable pageable);

@Query("SELECT ut FROM UserTenantEntity ut WHERE ut.userId= ?1 AND (ut.tenant.isDeleted IS NULL OR ut.tenant.isDeleted= FALSE)")
@Query("SELECT ut FROM UserTenantEntity ut WHERE ut.userId= ?1 AND ut.tenant.isDeleted= FALSE")
Page<UserTenantEntity> findByUserId(UUID userId, Pageable pageable);

@Query("SELECT ut FROM UserTenantEntity ut WHERE ut.userId= ?1")
Page<UserTenantEntity> findAnyByUserId(UUID userId, Pageable pageable);

@Query("SELECT ut FROM UserTenantEntity ut WHERE ut.username= ?1 AND ut.tenant.id= ?2 AND (ut.tenant.isDeleted IS NULL OR ut.tenant.isDeleted= FALSE)")
@Query("SELECT ut FROM UserTenantEntity ut WHERE ut.username= ?1 AND ut.tenant.id= ?2 AND ut.tenant.isDeleted= FALSE")
SerhiiNosko marked this conversation as resolved.
Show resolved Hide resolved
Optional<UserTenantEntity> findByUsernameAndTenantId(String username, String tenantId);

@Query("SELECT ut FROM UserTenantEntity ut WHERE ut.userId= ?1 AND ut.tenant.id= ?2")
Expand Down
4 changes: 4 additions & 0 deletions src/main/java/org/folio/consortia/service/TenantService.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ public interface TenantService {

/**
* Inserts single tenant based on consortiumId.
* Method checks whether requesting tenant is soft deleted or new tenant.
* For re-adding soft deleted tenant,
* tenant is_deleted flag will be changed to false and dummy user will be created in mod_users.user-tenants table
* For new tenant, all necessary actions will be done.
*
* @param consortiumId the consortiumId
* @param tenantDto the tenantDto
Expand Down
135 changes: 91 additions & 44 deletions src/main/java/org/folio/consortia/service/impl/TenantServiceImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -120,18 +120,47 @@ public TenantEntity getByTenantId(String tenantId) {
@Override
@Transactional
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());
log.info("save:: Trying to save a tenant with id={}, consortiumId={} and isCentral={}", tenantDto.getId(),
consortiumId, tenantDto.getIsCentral());
validateConsortiumAndTenantForSaveOperation(consortiumId, tenantDto);

// validation part
checkTenantNotExistsAndConsortiumExistsOrThrow(consortiumId, tenantDto.getId());
checkCodeAndNameUniqueness(tenantDto);
if (tenantDto.getIsCentral()) {
checkCentralTenantExistsOrThrow();
var requestingTenant = tenantRepository.findById(tenantDto.getId());

// checked whether tenant exists or not.
return requestingTenant.isPresent() ? reAddSoftDeletedTenant(consortiumId, requestingTenant.get())
: addNewTenant(consortiumId, tenantDto, adminUserId);
}

private Tenant reAddSoftDeletedTenant(UUID consortiumId, TenantEntity existedTenant) {
log.info("reAddSoftDeletedTenant:: Re-adding soft deleted tenant with id={}", existedTenant.getId());
validateExistedTenant(existedTenant);

existedTenant.setIsDeleted(false);
var savedTenant = saveTenant(consortiumId, converter.convert(existedTenant, Tenant.class), SetupStatusEnum.IN_PROGRESS);

String centralTenantId = getCentralTenantId();
var hasFailedAffiliations = false;
try (var ignored = new FolioExecutionContextSetter(contextHelper.getSystemUserFolioExecutionContext(existedTenant.getId()))) {
createUserTenantWithDummyUser(existedTenant.getId(), centralTenantId, consortiumId);
log.info("reAddSoftDeletedTenant:: Dummy user re-created in user-tenants table");
} catch (Exception e) {
hasFailedAffiliations = true;
log.error("Failed to create dummy user with centralTenantId: {}, tenant: {}" +
" and error message: {}", centralTenantId, existedTenant.getId(), e.getMessage(), e);
}
updateTenantSetupStatus(existedTenant.getId(), centralTenantId, hasFailedAffiliations ?
SetupStatusEnum.COMPLETED_WITH_ERRORS : SetupStatusEnum.COMPLETED);

return savedTenant;
}

private Tenant addNewTenant(UUID consortiumId, Tenant tenantDto, UUID adminUserId) {
log.info("addNewTenant:: Creating new tenant with id={}, consortiumId={}, and adminUserId={}",
tenantDto.getId(), consortiumId, adminUserId);
validateCodeAndNameUniqueness(tenantDto);

// save tenant to db
lockService.lockTenantSetupWithinTransaction();
tenantDto.setIsDeleted(false);
Tenant savedTenant = saveTenant(consortiumId, tenantDto, SetupStatusEnum.IN_PROGRESS);

// save admin user tenant association for non-central tenant
Expand All @@ -147,7 +176,7 @@ public Tenant save(UUID consortiumId, UUID adminUserId, Tenant tenantDto) {
userTenantRepository.save(createUserTenantEntity(consortiumId, shadowAdminUser, tenantDto));
// creating shadow user of consortia system user of central tenant with same permissions.
var centralSystemUser = userService.getByUsername(systemUserUsername)
.orElseThrow(() -> new ResourceNotFoundException("systemUserUsername", systemUserUsername));
.orElseThrow(() -> new ResourceNotFoundException("systemUserUsername", systemUserUsername));
shadowSystemUser = userService.prepareShadowUser(UUID.fromString(centralSystemUser.getId()), folioExecutionContext.getTenantId());
userTenantRepository.save(createUserTenantEntity(consortiumId, shadowSystemUser, tenantDto));
}
Expand All @@ -171,16 +200,20 @@ public Tenant save(UUID consortiumId, UUID adminUserId, Tenant tenantDto) {
@Override
@Transactional
public Tenant update(UUID consortiumId, String tenantId, Tenant tenantDto) {
consortiumService.checkConsortiumExistsOrThrow(consortiumId);
checkTenantExistsOrThrow(tenantId);
checkIdenticalOrThrow(tenantId, tenantDto.getId(), TENANTS_IDS_NOT_MATCHED_ERROR_MSG);
log.debug("update:: Trying to update tenant '{}' in consortium '{}'", tenantId, consortiumId);
var existedTenant = tenantRepository.findById(tenantId)
.orElseThrow(() -> new ResourceNotFoundException("id", tenantId));

validateTenantForUpdateOperation(consortiumId, tenantId, tenantDto, existedTenant);
// isDeleted flag cannot be changed by put request
tenantDto.setIsDeleted(existedTenant.getIsDeleted());
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After found issue relate to ablitiy of updating isCentral and isDeleted field, introduced improvement in order to prevent future unwanted changes

return updateTenant(consortiumId, tenantDto);
}

@Override
@Transactional(propagation = Propagation.REQUIRES_NEW)
public void updateTenantSetupStatus(String tenantId, String centralTenantId, SetupStatusEnum setupStatus) {
try (var ctx = new FolioExecutionContextSetter(prepareContextForTenant(centralTenantId,
try (var ignored = new FolioExecutionContextSetter(prepareContextForTenant(centralTenantId,
folioExecutionContext.getFolioModuleMetadata(), folioExecutionContext))) {
tenantDetailsRepository.setSetupStatusByTenantId(setupStatus, tenantId);
log.info("updateTenantSetupStatus:: tenant id={} status updated to {}", tenantId, setupStatus);
Expand All @@ -196,9 +229,8 @@ public void delete(UUID consortiumId, String tenantId) {
if (tenant.isEmpty()) {
throw new ResourceNotFoundException("id", tenantId);
}
if (Boolean.TRUE.equals(tenant.get().getIsCentral())) {
throw new IllegalArgumentException(String.format("central tenant '%s' cannot be deleted", tenantId));
}

validateTenantForDeleteOperation(tenant.get());

var softDeletedTenant = tenant.get();
softDeletedTenant.setIsDeleted(true);
Expand All @@ -221,22 +253,21 @@ private Tenant saveTenant(UUID consortiumId, Tenant tenantDto, SetupStatusEnum s
}

private Tenant updateTenant(UUID consortiumId, Tenant tenantDto) {
log.debug("updateTenant:: Trying to update tenant with consoritumId={} and tenant with id={}", consortiumId, tenantDto);
TenantEntity entity = toTenantEntity(consortiumId, tenantDto);
TenantEntity updatedTenant = tenantRepository.save(entity);
log.info("updateTenant:: Tenant '{}' successfully updated", updatedTenant.getId());
return converter.convert(updatedTenant, Tenant.class);
}

/*
Dummy user will be used to support cross-tenant requests checking in mod-authtoken,
if user-tenant table contains some record in institutional tenant - it means mod-consortia enabled for
this tenant and will allow cross-tenant request.

@param tenantId tenant id
@param centralTenantId central tenant id
@param consortiumId consortium id
*/
/**
* Dummy user will be used to support cross-tenant requests checking in mod-authtoken,
* if user-tenant table contains some record in institutional tenant - it means mod-consortia enabled for
* this tenant and will allow cross-tenant request.
*
* @param tenantId tenant id
* @param centralTenantId central tenant id
* @param consortiumId consortium id
*/
private void createUserTenantWithDummyUser(String tenantId, String centralTenantId, UUID consortiumId) {
UserTenant userTenant = new UserTenant();
userTenant.setId(UUID.randomUUID());
Expand All @@ -250,22 +281,6 @@ private void createUserTenantWithDummyUser(String tenantId, String centralTenant
userTenantsClient.postUserTenant(userTenant);
}

private void checkTenantNotExistsAndConsortiumExistsOrThrow(UUID consortiumId, String tenantId) {
consortiumService.checkConsortiumExistsOrThrow(consortiumId);
if (tenantRepository.existsById(tenantId)) {
throw new ResourceAlreadyExistException("id", tenantId);
}
}

private void checkCodeAndNameUniqueness(Tenant tenant) {
if (tenantRepository.existsByName(tenant.getName())) {
throw new ResourceAlreadyExistException("name", tenant.getName());
}
if (tenantRepository.existsByCode(tenant.getCode())) {
throw new ResourceAlreadyExistException("code", tenant.getCode());
}
}

@Override
public void checkTenantExistsOrThrow(String tenantId) {
if (!tenantRepository.existsById(tenantId)) {
Expand All @@ -289,12 +304,44 @@ public void checkTenantsAndConsortiumExistsOrThrow(UUID consortiumId, List<Strin
}
}

private void checkCentralTenantExistsOrThrow() {
if (tenantRepository.existsByIsCentralTrue()) {
private void validateTenantForUpdateOperation(UUID consortiumId, String tenantId, Tenant tenantDto, TenantEntity existedTenant) {
consortiumService.checkConsortiumExistsOrThrow(consortiumId);
checkIdenticalOrThrow(tenantId, tenantDto.getId(), TENANTS_IDS_NOT_MATCHED_ERROR_MSG);
if (tenantDto.getIsCentral() != existedTenant.getIsCentral())
throw new IllegalArgumentException(String.format("'isCentral' field cannot be changed. It should be '%s'", existedTenant.getIsCentral()));
}

private void validateConsortiumAndTenantForSaveOperation(UUID consortiumId, Tenant tenantDto) {
consortiumService.checkConsortiumExistsOrThrow(consortiumId);
if (tenantDto.getIsCentral() && tenantRepository.existsByIsCentralTrue()) {
throw new ResourceAlreadyExistException("isCentral", "true");
}
}

private void validateCodeAndNameUniqueness(Tenant tenant) {
if (tenantRepository.existsByName(tenant.getName())) {
throw new ResourceAlreadyExistException("name", tenant.getName());
}
if (tenantRepository.existsByCode(tenant.getCode())) {
throw new ResourceAlreadyExistException("code", tenant.getCode());
}
}

private void validateExistedTenant(TenantEntity tenant) {
if (Boolean.FALSE.equals(tenant.getIsDeleted())) {
throw new ResourceAlreadyExistException("id", tenant.getId());
}
}

private void validateTenantForDeleteOperation(TenantEntity tenant) {
if (Boolean.TRUE.equals(tenant.getIsDeleted())) {
throw new IllegalArgumentException(String.format("Tenant [%s] has already been soft deleted.", tenant.getId()));
}
if (Boolean.TRUE.equals(tenant.getIsCentral())) {
throw new IllegalArgumentException(String.format("Central tenant [%s] cannot be deleted.", tenant.getId()));
}
}

private void checkAdminUserIdPresentOrThrow(UUID adminUserId) {
if (Objects.isNull(adminUserId)) {
log.warn("checkAdminUserIdPresentOrThrow:: adminUserId is not present");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
<changeSet id="MOCON-127@add-is-deleted-flag" author="azizbekxm">
<addColumn tableName="tenant">
<column name="is_deleted" type="boolean" defaultValueBoolean="false">
<constraints unique="false" />
<constraints unique="false" nullable="false"/>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this added because there shouldn't be null

</column>
</addColumn>
</changeSet>
Expand Down
1 change: 1 addition & 0 deletions src/main/resources/permissions/system-user-permissions.csv
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ perms.users.assign.immutable
perms.users.assign.mutable
perms.users.get
user-tenants.item.post
user-tenants.item.delete
consortia.sync-primary-affiliations.item.post
consortia.create-primary-affiliations.item.post
departments.item.post
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@
@EntityScan(basePackageClasses = TenantEntity.class)
class TenantControllerTest extends BaseIT {
private static final String TENANT_REQUEST_BODY = "{\"id\":\"diku1234\",\"code\":\"TST\",\"name\":\"diku_tenant_name1234\", \"isCentral\":false}";
private static final String CENTRAL_TENANT_REQUEST_BODY = "{\"id\":\"diku1234\",\"code\":\"TST\",\"name\":\"diku_tenant_name1234\", \"isCentral\":true}";
private static final String CONSORTIUM_ID = "7698e46-c3e3-11ed-afa1-0242ac120002";
private static final String CENTRAL_TENANT_ID = "diku";
public static final String SYNC_PRIMARY_AFFILIATIONS_URL = "/consortia/%s/tenants/%s/sync-primary-affiliations?centralTenantId=%s";
Expand Down Expand Up @@ -181,6 +182,32 @@ void shouldSaveTenant(String contentString) throws Exception {
.andExpect(status().isCreated());
}

@ParameterizedTest
@ValueSource(strings = {TENANT_REQUEST_BODY})
void shouldReAddSoftDeletedTenant(String contentString) throws Exception {
var headers = defaultHeaders();
String adminUser = UUID.randomUUID().toString();
TenantEntity centralTenant = createTenantEntity(CENTRAL_TENANT_ID, CENTRAL_TENANT_ID, "AAA", true);
var existedTenant = createTenantEntity("diku1234", "diku_tenant_name1234");
existedTenant.setIsDeleted(true);

var tenantDetailsEntity = new TenantDetailsEntity();
tenantDetailsEntity.setConsortiumId(centralTenant.getConsortiumId());
tenantDetailsEntity.setId("diku1234");

doNothing().when(userTenantsClient).postUserTenant(any());
when(consortiumRepository.existsById(any())).thenReturn(true);
when(tenantRepository.findById("diku1234")).thenReturn(Optional.of(existedTenant));
when(tenantDetailsRepository.save(any(TenantDetailsEntity.class))).thenReturn(tenantDetailsEntity);
when(tenantRepository.findCentralTenant()).thenReturn(Optional.of(centralTenant));
doReturn(folioExecutionContext).when(contextHelper).getSystemUserFolioExecutionContext(anyString());

this.mockMvc.perform(
post("/consortia/7698e46-c3e3-11ed-afa1-0242ac120002/tenants?adminUserId=" + adminUser)
.headers(headers).content(contentString))
.andExpect(status().isCreated());
}

@ParameterizedTest
@ValueSource(strings = {TENANT_REQUEST_BODY})
void shouldUpdateTenant(String contentString) throws Exception {
Expand Down Expand Up @@ -318,21 +345,37 @@ void shouldThrownMethodArgumentNotValidException(String contentString) throws Ex


@ParameterizedTest
@ValueSource(strings = {TENANT_REQUEST_BODY})
void shouldGet4xxErrorWhileSavingDuplicateName(String contentString) throws Exception {
@ValueSource(strings = {CENTRAL_TENANT_REQUEST_BODY})
void shouldGet4xxErrorWhileSavingDuplicateCentralTenant(String contentString) throws Exception {
var headers = defaultHeaders();
UUID consortiumId = UUID.fromString(CONSORTIUM_ID);
TenantEntity centralTenant = createTenantEntity(CENTRAL_TENANT_ID, CENTRAL_TENANT_ID, "TTA", true);
PermissionUser permissionUser = new PermissionUser();
PermissionUserCollection permissionUserCollection = new PermissionUserCollection();
permissionUserCollection.setPermissionUsers(List.of(permissionUser));

doReturn(new User()).when(usersClient).getUserById(any());
doReturn(permissionUserCollection).when(permissionsClient).get(any());
when(consortiumRepository.existsById(consortiumId)).thenReturn(true);
when(tenantRepository.existsById(any(String.class))).thenReturn(true);
when(tenantRepository.existsByIsCentralTrue()).thenReturn(true);
when(tenantRepository.findCentralTenant()).thenReturn(Optional.of(centralTenant));
doNothing().when(configurationClient).saveConfiguration(createConsortiaConfiguration(CENTRAL_TENANT_ID));

this.mockMvc.perform(
post("/consortia/7698e46-c3e3-11ed-afa1-0242ac120002/tenants?adminUserId=111841e3-e6fb-4191-9fd8-5674a5107c34")
.headers(headers).content(contentString))
.andExpectAll(
status().is4xxClientError(),
jsonPath("$.errors[0].message", is("Object with isCentral [true] is already presented in the system")),
jsonPath("$.errors[0].code", is("DUPLICATE_ERROR")));
}

@ParameterizedTest
@ValueSource(strings = {TENANT_REQUEST_BODY})
void shouldGet4xxErrorWhileSavingExistingTenant(String contentString) throws Exception {
var headers = defaultHeaders();
UUID consortiumId = UUID.fromString(CONSORTIUM_ID);
var existedTenant = createTenantEntity("diku1234", "diku_tenant_name1234");
existedTenant.setIsDeleted(false);

when(consortiumRepository.existsById(consortiumId)).thenReturn(true);
when(tenantRepository.existsById(any(String.class))).thenReturn(true);
when(tenantRepository.findById(anyString())).thenReturn(Optional.of(existedTenant));

this.mockMvc.perform(
post("/consortia/7698e46-c3e3-11ed-afa1-0242ac120002/tenants?adminUserId=111841e3-e6fb-4191-9fd8-5674a5107c34")
Expand Down
Loading