Skip to content

Commit

Permalink
fix(authority-source-files): adjust logic for consortium tenants (#247)
Browse files Browse the repository at this point in the history
Closes: MODELINKS-201
  • Loading branch information
psmagin authored Feb 9, 2024
1 parent cb8b1c2 commit d29b796
Show file tree
Hide file tree
Showing 10 changed files with 97 additions and 64 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.folio.entlinks.service.consortium.UserTenantsService;
import org.folio.entlinks.service.consortium.propagation.ConsortiumPropagationService;
import org.folio.spring.FolioExecutionContext;
import org.folio.spring.service.SystemUserScopedExecutionService;
import org.folio.tenant.domain.dto.Parameter;
import org.springframework.stereotype.Service;

Expand All @@ -33,14 +34,13 @@
public class AuthoritySourceFileServiceDelegate {

private static final String URL_PROTOCOL_PATTERN = "^(https?://www\\.|https?://|www\\.)";
private static final String CREATE_ACTION = "create";
private static final String NEXT_HRID_ACTION = "next HRID";

private final AuthoritySourceFileService service;
private final AuthoritySourceFileMapper mapper;
private final UserTenantsService tenantsService;
private final ConsortiumPropagationService<AuthoritySourceFile> propagationService;
private final FolioExecutionContext context;
private final SystemUserScopedExecutionService executionService;

public AuthoritySourceFileDtoCollection getAuthoritySourceFiles(Integer offset, Integer limit, String cqlQuery) {
var entities = service.getAll(offset, limit, cqlQuery);
Expand All @@ -54,7 +54,7 @@ public AuthoritySourceFileDto getAuthoritySourceFileById(UUID id) {

public AuthoritySourceFileDto createAuthoritySourceFile(AuthoritySourceFilePostDto authoritySourceFile) {
log.debug("create:: Attempting to create AuthoritySourceFile [createDto: {}]", authoritySourceFile);
validateActionRightsForTenant(CREATE_ACTION);
validateActionRightsForTenant(DomainEventType.CREATE);
var entity = mapper.toEntity(authoritySourceFile);
normalizeBaseUrl(entity);
var created = service.create(entity);
Expand All @@ -68,7 +68,7 @@ public AuthoritySourceFileDto createAuthoritySourceFile(AuthoritySourceFilePostD
public void patchAuthoritySourceFile(UUID id, AuthoritySourceFilePatchDto partiallyModifiedDto) {
log.debug("patch:: Attempting to patch AuthoritySourceFile [id: {}, patchDto: {}]", id, partiallyModifiedDto);
var existingEntity = service.getById(id);
validateModifyPossibility(DomainEventType.UPDATE, existingEntity);
validateActionRightsForTenant(DomainEventType.UPDATE);
validatePatchRequest(partiallyModifiedDto, existingEntity);

var partialEntityUpdate = new AuthoritySourceFile(existingEntity);
Expand All @@ -81,17 +81,16 @@ public void patchAuthoritySourceFile(UUID id, AuthoritySourceFilePatchDto partia

public void deleteAuthoritySourceFileById(UUID id) {
var entity = service.getById(id);
validateModifyPossibility(DomainEventType.DELETE, entity);
validateActionRightsForTenant(DomainEventType.DELETE);

service.deleteById(id);
propagationService.propagate(entity, DELETE, context.getTenantId());
}

public AuthoritySourceFileHridDto getAuthoritySourceFileNextHrid(UUID id) {
log.debug("nextHrid:: Attempting to get next AuthoritySourceFile HRID [id: {}]", id);
validateActionRightsForTenant(NEXT_HRID_ACTION);

var hrid = service.nextHrid(id);
var tenantId = tenantsService.getCentralTenant(context.getTenantId()).orElse(context.getTenantId());
var hrid = executionService.executeSystemUserScoped(tenantId, () -> service.nextHrid(id));

return new AuthoritySourceFileHridDto().id(id).hrid(hrid);
}
Expand All @@ -107,7 +106,7 @@ private void normalizeBaseUrl(AuthoritySourceFile entity) {
}
}

private void validateActionRightsForTenant(String action) {
private void validateActionRightsForTenant(DomainEventType action) {
var tenantId = context.getTenantId();
var centralTenantId = tenantsService.getCentralTenant(tenantId);
if (centralTenantId.isPresent() && !tenantId.equals(centralTenantId.get())) {
Expand All @@ -116,13 +115,6 @@ private void validateActionRightsForTenant(String action) {
}
}

private void validateModifyPossibility(DomainEventType eventType, AuthoritySourceFile entity) {
if (entity.isConsortiumShadowCopy()) {
throw new RequestBodyValidationException(eventType.name() + " is not applicable to consortium shadow copy",
List.of(new Parameter("id").value(String.valueOf(entity.getId()))));
}
}

private void validatePatchRequest(AuthoritySourceFilePatchDto patchDto, AuthoritySourceFile existing) {
var errorParameters = new LinkedList<Parameter>();
var hasAuthorityReferences = service.authoritiesExistForSourceFile(existing.getId());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,14 +130,6 @@ void markNotNew() {
this.isNew = false;
}

public void markAsConsortiumShadowCopy() {
this.setSource(AuthoritySourceFileSource.CONSORTIUM);
}

public boolean isConsortiumShadowCopy() {
return AuthoritySourceFileSource.CONSORTIUM.equals(this.getSource());
}

public String getFullBaseUrl() {
if (baseUrl == null) {
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,5 @@
public enum AuthoritySourceFileSource {
FOLIO,

LOCAL,

CONSORTIUM;
LOCAL
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ public ConsortiumAuthoritySourceFilePropagationService(AuthoritySourceFileServic
}

protected void doPropagation(AuthoritySourceFile sourceFile, PropagationType propagationType) {
sourceFile.markAsConsortiumShadowCopy();
switch (propagationType) {
case CREATE -> sourceFileService.create(sourceFile);
case UPDATE -> sourceFileService.update(sourceFile.getId(), sourceFile);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,33 @@
ALTER COLUMN source TYPE AuthoritySourceType using UPPER(source)::AuthoritySourceType;</sql>
</changeSet>

<changeSet id="MODELINKS-170@@create-authority_source_file_source-type" author="Pavlo_Smahin">
<preConditions onFail="MARK_RAN">
<sqlCheck expectedResult="0">
SELECT COUNT(*)
FROM pg_type t
JOIN pg_catalog.pg_namespace n ON n.oid = t.typnamespace
WHERE t.typname = 'authority_source_file_source' AND n.nspname = '${database.defaultSchemaName}';
</sqlCheck>
</preConditions>

<comment>Create authority_source_file_source enum</comment>

<sql>CREATE TYPE authority_source_file_source AS ENUM ('FOLIO', 'LOCAL')</sql>
</changeSet>

<changeSet id="MODELINKS-184@@migrate-authority-source-file-source-type-to-authority_source_file_source" author="Viacheslav_Kolesnyk">
<preConditions onFail="MARK_RAN">
<tableExists tableName="authority_source_file"/>
</preConditions>

<comment>Change 'source' column type from AuthoritySourceType to authority_source_file_source for authority_source_file</comment>
<sql>UPDATE authority_source_file SET source = 'LOCAL' WHERE source = 'CONSORTIUM';</sql>
<sql>ALTER TABLE authority_source_file
ALTER COLUMN source TYPE authority_source_file_source
USING (source::text)::authority_source_file_source;</sql>
<sql>DROP TYPE IF EXISTS AuthoritySourceType</sql>
</changeSet>


</databaseChangeLog>
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ properties:
enum:
- folio
- local
- consortium
selectable:
type: boolean
hridManagement:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import java.util.List;
import java.util.Optional;
import java.util.UUID;
import java.util.concurrent.Callable;
import java.util.stream.Stream;
import org.folio.entlinks.controller.converter.AuthoritySourceFileMapper;
import org.folio.entlinks.domain.dto.AuthoritySourceFileDto;
Expand All @@ -35,6 +36,7 @@
import org.folio.entlinks.service.consortium.UserTenantsService;
import org.folio.entlinks.service.consortium.propagation.ConsortiumAuthoritySourceFilePropagationService;
import org.folio.spring.FolioExecutionContext;
import org.folio.spring.service.SystemUserScopedExecutionService;
import org.folio.spring.testing.type.UnitTest;
import org.folio.support.TestDataUtils;
import org.folio.tenant.domain.dto.Parameter;
Expand All @@ -48,14 +50,12 @@
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.springframework.data.domain.Page;
import org.springframework.data.domain.PageImpl;

@UnitTest
@ExtendWith(MockitoExtension.class)
class AuthoritySourceFileServiceDelegateTest {


private static final String SANITIZED_BASE_URL = "id.loc.gov/authorities/test-source/";

@Mock
Expand All @@ -68,6 +68,8 @@ class AuthoritySourceFileServiceDelegateTest {
private ConsortiumAuthoritySourceFilePropagationService propagationService;
@Mock
private FolioExecutionContext context;
@Mock
private SystemUserScopedExecutionService executionService;

@InjectMocks
private AuthoritySourceFileServiceDelegate delegate;
Expand All @@ -80,7 +82,7 @@ void shouldGetSourceFileCollectionByQuery() {
var expectedCollection = new AuthoritySourceFileDtoCollection();
when(service.getAll(any(Integer.class), any(Integer.class), any(String.class)))
.thenReturn(new PageImpl<>(List.of()));
when(mapper.toAuthoritySourceFileCollection(any(Page.class))).thenReturn(expectedCollection);
when(mapper.toAuthoritySourceFileCollection(any())).thenReturn(expectedCollection);

var sourceFiles = delegate.getAuthoritySourceFiles(0, 100, "cql.allRecords=1");

Expand Down Expand Up @@ -134,8 +136,7 @@ void shouldNormalizeBaseUrlForSourceFileCreateOnConsortiumCentralTenant() {
expected.setSequenceName("sequence_name");
expected.setHridStartNumber(dto.getHridManagement().getStartNumber());

when(context.getTenantId()).thenReturn(CENTRAL_TENANT_ID);
when(tenantsService.getCentralTenant(CENTRAL_TENANT_ID)).thenReturn(Optional.of(CENTRAL_TENANT_ID));
mockAsConsortiumCentralTenant();
when(mapper.toEntity(dto)).thenReturn(expected);
when(service.create(expected)).thenReturn(expected);
when(mapper.toDto(expected)).thenReturn(new AuthoritySourceFileDto());
Expand All @@ -159,12 +160,12 @@ void shouldNormalizeBaseUrlForSourceFilePartialUpdate() {
var expected = new AuthoritySourceFile(existing);
expected.setBaseUrl(SANITIZED_BASE_URL);

mockAsNonConsortiumTenant();
when(service.getById(existing.getId())).thenReturn(existing);
when(service.authoritiesExistForSourceFile(existing.getId())).thenReturn(true);
when(mapper.partialUpdate(any(AuthoritySourceFilePatchDto.class), any(AuthoritySourceFile.class)))
.thenAnswer(i -> i.getArguments()[1]);
when(service.update(any(UUID.class), any(AuthoritySourceFile.class))).thenAnswer(i -> i.getArguments()[1]);
when(context.getTenantId()).thenReturn(TENANT_ID);
var dto = new AuthoritySourceFilePatchDto().baseUrl(INPUT_BASE_URL);

delegate.patchAuthoritySourceFile(existing.getId(), dto);
Expand Down Expand Up @@ -253,12 +254,11 @@ void shouldDeleteAuthoritySourceFileById() {
void shouldNotCreateForConsortiumMemberTenant() {
var dto = new AuthoritySourceFilePostDto();

when(context.getTenantId()).thenReturn(TENANT_ID);
when(tenantsService.getCentralTenant(TENANT_ID)).thenReturn(Optional.of(CENTRAL_TENANT_ID));
mockAsConsortiumMemberTenant();

var exc = assertThrows(RequestBodyValidationException.class, () -> delegate.createAuthoritySourceFile(dto));

assertThat(exc.getMessage()).isEqualTo("Action 'create' is not supported for consortium member tenant");
assertThat(exc.getMessage()).isEqualTo("Action 'CREATE' is not supported for consortium member tenant");
assertThat(exc.getInvalidParameters()).hasSize(1);
assertThat(exc.getInvalidParameters().get(0))
.matches(param -> param.getKey().equals("tenantId") && param.getValue().equals(TENANT_ID));
Expand All @@ -270,9 +270,11 @@ void shouldNotCreateForConsortiumMemberTenant() {
void shouldGetNextHrid() {
var id = UUID.randomUUID();
var code = "CODE10";
when(context.getTenantId()).thenReturn(TENANT_ID);
when(tenantsService.getCentralTenant(TENANT_ID)).thenReturn(Optional.empty());

mockAsNonConsortiumTenant();
when(service.nextHrid(id)).thenReturn(code);
when(executionService.executeSystemUserScoped(eq(TENANT_ID), any()))
.thenAnswer(invocation -> ((Callable<?>) invocation.getArgument(1)).call());

var hridDto = delegate.getAuthoritySourceFileNextHrid(id);

Expand All @@ -283,36 +285,40 @@ void shouldGetNextHrid() {
}

@Test
void shouldNotNextHridForConsortiumMemberTenant() {
when(context.getTenantId()).thenReturn(TENANT_ID);
when(tenantsService.getCentralTenant(TENANT_ID)).thenReturn(Optional.of(CENTRAL_TENANT_ID));

void shouldGetNextHridForConsortiumMemberTenant_withSwitchToCentralTenant() {
var id = UUID.randomUUID();
var exc = assertThrows(RequestBodyValidationException.class, () -> delegate.getAuthoritySourceFileNextHrid(id));
var code = "CODE10";

assertThat(exc.getMessage()).isEqualTo("Action 'next HRID' is not supported for consortium member tenant");
assertThat(exc.getInvalidParameters()).hasSize(1);
assertThat(exc.getInvalidParameters().get(0))
.matches(param -> param.getKey().equals("tenantId") && param.getValue().equals(TENANT_ID));
verifyNoInteractions(service);
mockAsConsortiumMemberTenant();
when(service.nextHrid(id)).thenReturn(code);
when(executionService.executeSystemUserScoped(eq(CENTRAL_TENANT_ID), any()))
.thenAnswer(invocation -> ((Callable<?>) invocation.getArgument(1)).call());

var hridDto = delegate.getAuthoritySourceFileNextHrid(id);

assertThat(hridDto)
.extracting(AuthoritySourceFileHridDto::getId, AuthoritySourceFileHridDto::getHrid)
.containsExactly(id, code);
verify(service).nextHrid(id);
}

@Test
void shouldNotUpdateConsortiumShadowCopy() {
var existing = TestDataUtils.AuthorityTestData.authoritySourceFile(0);
existing.setSource(AuthoritySourceFileSource.CONSORTIUM);
existing.setSource(AuthoritySourceFileSource.FOLIO);
var id = existing.getId();
var dto = new AuthoritySourceFilePatchDto();

mockAsConsortiumMemberTenant();
when(service.getById(existing.getId())).thenReturn(existing);

var exc = assertThrows(RequestBodyValidationException.class,
() -> delegate.patchAuthoritySourceFile(id, dto));

assertThat(exc.getMessage()).isEqualTo("UPDATE is not applicable to consortium shadow copy");
assertThat(exc.getMessage()).isEqualTo("Action 'UPDATE' is not supported for consortium member tenant");
assertThat(exc.getInvalidParameters()).hasSize(1);
assertThat(exc.getInvalidParameters().get(0))
.matches(param -> param.getKey().equals("id") && param.getValue().equals(String.valueOf(id)));
.matches(param -> param.getKey().equals("tenantId") && param.getValue().equals(TENANT_ID));
verifyNoInteractions(mapper);
verifyNoMoreInteractions(service);
verifyNoInteractions(propagationService);
Expand All @@ -321,18 +327,19 @@ void shouldNotUpdateConsortiumShadowCopy() {
@Test
void shouldNotDeleteConsortiumShadowCopy() {
var existing = TestDataUtils.AuthorityTestData.authoritySourceFile(0);
existing.setSource(AuthoritySourceFileSource.CONSORTIUM);
existing.setSource(AuthoritySourceFileSource.FOLIO);
var id = existing.getId();

mockAsConsortiumMemberTenant();
when(service.getById(existing.getId())).thenReturn(existing);

var exc = assertThrows(RequestBodyValidationException.class,
() -> delegate.deleteAuthoritySourceFileById(id));

assertThat(exc.getMessage()).isEqualTo("DELETE is not applicable to consortium shadow copy");
assertThat(exc.getMessage()).isEqualTo("Action 'DELETE' is not supported for consortium member tenant");
assertThat(exc.getInvalidParameters()).hasSize(1);
assertThat(exc.getInvalidParameters().get(0))
.matches(param -> param.getKey().equals("id") && param.getValue().equals(String.valueOf(id)));
.matches(param -> param.getKey().equals("tenantId") && param.getValue().equals(TENANT_ID));
verifyNoMoreInteractions(service);
verifyNoInteractions(propagationService);
}
Expand All @@ -345,4 +352,19 @@ static Stream<Arguments> patchValidationFailureData() {
Arguments.of(AuthoritySourceFileSource.LOCAL, true, List.of(new Parameter("codes").value("a,b"),
new Parameter("hridManagement.startNumber").value("1"))));
}

private void mockAsNonConsortiumTenant() {
when(context.getTenantId()).thenReturn(TENANT_ID);
when(tenantsService.getCentralTenant(TENANT_ID)).thenReturn(Optional.empty());
}

private void mockAsConsortiumCentralTenant() {
when(context.getTenantId()).thenReturn(CENTRAL_TENANT_ID);
when(tenantsService.getCentralTenant(CENTRAL_TENANT_ID)).thenReturn(Optional.of(CENTRAL_TENANT_ID));
}

private void mockAsConsortiumMemberTenant() {
when(context.getTenantId()).thenReturn(TENANT_ID);
when(tenantsService.getCentralTenant(TENANT_ID)).thenReturn(Optional.of(CENTRAL_TENANT_ID));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import static java.util.Collections.emptyList;
import static org.assertj.core.api.Assertions.assertThat;
import static org.folio.entlinks.domain.entity.AuthoritySourceFileSource.CONSORTIUM;
import static org.folio.entlinks.domain.entity.AuthoritySourceFileSource.LOCAL;
import static org.folio.support.TestDataUtils.AuthorityTestData.authoritySourceFile;
import static org.junit.jupiter.api.Assertions.assertEquals;
Expand Down Expand Up @@ -155,7 +154,7 @@ void shouldNotFindAuthoritySourceFileForNullName() {
}

@ParameterizedTest
@ValueSource(strings = {"LOCAL", "CONSORTIUM"})
@ValueSource(strings = {"LOCAL", "FOLIO"})
void shouldCreateAuthoritySourceFile(String source) {
var code = new AuthoritySourceFileCode();
code.setCode("code");
Expand Down Expand Up @@ -197,7 +196,7 @@ void shouldUpdateAuthoritySourceFileModifiableFields(Integer existingHridStartNu
var id = existing.getId();
var modified = authoritySourceFile(1);
modified.setId(id);
modified.setSource(CONSORTIUM);
modified.setSource(LOCAL);

var expected = new AuthoritySourceFile(modified);
expected.setSource(existing.getSource());
Expand Down
Loading

0 comments on commit d29b796

Please sign in to comment.