Skip to content

Commit 2a6c2d3

Browse files
mukhiddin-yusufmukhiddin-yusuf
authored
fix(authority-source-files): Fix validation of authority source file prefix for PATCH request (#255)
* fix(authority-source-files): fix validation of authority source file prefix - do not allow to provide more than one preifx in request body for PATCH request - add validation for the prefix same as for POST request Closes: MODELINKS-208
1 parent 7cddc91 commit 2a6c2d3

File tree

10 files changed

+62
-39
lines changed

10 files changed

+62
-39
lines changed

‎NEWS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
* Fix empty links list propagation ([MODELINKS-166](https://issues.folio.org/browse/MODELINKS-166))
3333
* Fix base url of authority file after linking ([MODELINKS-192](https://folio-org.atlassian.net/browse/MODELINKS-192))
3434
* Fix authority source file sequence deletion ([MODELINKS-211](https://issues.folio.org/browse/MODELINKS-211))
35+
* Fix authority source file prefix validation for PATCH request ([MODELINKS-208](https://folio-org.atlassian.net/browse/MODELINKS-208))
3536

3637
### Tech Dept
3738
* Create custom Mockito verifies for Hibernate entities ([MODELINKS-209](https://issues.folio.org/browse/MODELINKS-209))

‎src/main/java/org/folio/entlinks/controller/converter/AuthoritySourceFileMapper.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -115,12 +115,12 @@ default Set<AuthoritySourceFileCode> toEntityCodes(List<String> codes) {
115115

116116
default Set<AuthoritySourceFileCode> toEntityCodes(AuthoritySourceFilePatchDto authoritySourceFileDto,
117117
AuthoritySourceFile authoritySourceFile) {
118-
var dtoCodes = authoritySourceFileDto.getCodes();
119-
if (dtoCodes == null) {
118+
var dtoCode = authoritySourceFileDto.getCode();
119+
if (dtoCode == null) {
120120
return authoritySourceFile.getAuthoritySourceFileCodes();
121121
}
122122

123-
return toEntityCodes(dtoCodes);
123+
return toEntityCodes(List.of(dtoCode));
124124
}
125125

126126
default AuthoritySourceFileCode toEntityCode(String code) {

‎src/main/java/org/folio/entlinks/controller/delegate/AuthoritySourceFileServiceDelegate.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -130,9 +130,8 @@ private void validatePatchRequest(AuthoritySourceFilePatchDto patchDto, Authorit
130130
errorParameters.add(new Parameter("name")
131131
.value(String.join(",", patchDto.getName())));
132132
}
133-
if (patchDto.getCodes() != null) {
134-
errorParameters.add(new Parameter("codes")
135-
.value(String.join(",", patchDto.getCodes())));
133+
if (patchDto.getCode() != null) {
134+
errorParameters.add(new Parameter("code").value(patchDto.getCode()));
136135
}
137136
if (patchDto.getHridManagement() != null && patchDto.getHridManagement().getStartNumber() != null) {
138137
errorParameters.add(new Parameter("hridManagement.startNumber")

‎src/main/java/org/folio/entlinks/service/authority/AuthoritySourceFileService.java

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import org.apache.commons.lang3.StringUtils;
1313
import org.folio.entlinks.controller.converter.AuthoritySourceFileMapper;
1414
import org.folio.entlinks.domain.entity.AuthoritySourceFile;
15+
import org.folio.entlinks.domain.entity.AuthoritySourceFileCode;
1516
import org.folio.entlinks.domain.repository.AuthorityRepository;
1617
import org.folio.entlinks.domain.repository.AuthoritySourceFileRepository;
1718
import org.folio.entlinks.exception.AuthoritySourceFileHridException;
@@ -121,10 +122,7 @@ public AuthoritySourceFile create(AuthoritySourceFile entity) {
121122
public AuthoritySourceFile update(UUID id, AuthoritySourceFile modified) {
122123
log.debug("update:: Attempting to update AuthoritySourceFile [id: {}]", id);
123124

124-
if (!Objects.equals(id, modified.getId())) {
125-
throw new RequestBodyValidationException("Request should have id = " + id,
126-
List.of(new Parameter("id").value(String.valueOf(modified.getId()))));
127-
}
125+
validateOnUpdate(id, modified);
128126

129127
var existingEntity = repository.findById(id).orElseThrow(() -> new AuthoritySourceFileNotFoundException(id));
130128
if (modified.getVersion() < existingEntity.getVersion()) {
@@ -187,12 +185,23 @@ public boolean authoritiesExistForSourceFile(UUID sourceFileId) {
187185
}
188186

189187
private void validateOnCreate(AuthoritySourceFile entity) {
190-
for (var sourceFileCode : entity.getAuthoritySourceFileCodes()) {
191-
var code = sourceFileCode.getCode();
192-
if (StringUtils.isBlank(code) || !StringUtils.isAlpha(code)) {
193-
throw new RequestBodyValidationException("Authority Source File prefix should be non-empty sequence of letters",
194-
List.of(new Parameter("code").value(code)));
195-
}
188+
entity.getAuthoritySourceFileCodes().forEach(this::validateSourceFileCode);
189+
}
190+
191+
private void validateOnUpdate(UUID id, AuthoritySourceFile entity) {
192+
if (!Objects.equals(id, entity.getId())) {
193+
throw new RequestBodyValidationException("Request should have id = " + id,
194+
List.of(new Parameter("id").value(String.valueOf(entity.getId()))));
195+
}
196+
197+
entity.getAuthoritySourceFileCodes().forEach(this::validateSourceFileCode);
198+
}
199+
200+
private void validateSourceFileCode(AuthoritySourceFileCode sourceFileCode) {
201+
var code = sourceFileCode.getCode();
202+
if (StringUtils.isBlank(code) || !StringUtils.isAlpha(code)) {
203+
throw new RequestBodyValidationException("Authority Source File prefix should be non-empty sequence of letters",
204+
List.of(new Parameter("code").value(code)));
196205
}
197206
}
198207

‎src/main/resources/swagger.api/schemas/authority-source-file/authoritySourceFilePatchDto.yaml

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,11 @@ properties:
44
name:
55
type: string
66
description: Authority source file name
7-
codes:
8-
type: array
9-
description: List of identifying prefix
10-
items:
11-
type: string
12-
description: identifying prefix, i.e. 'n', 'D', 'fst'
7+
code:
8+
type: string
9+
minLength: 1
10+
maxLength: 25
11+
description: identifying prefix, i.e. 'n', 'D', 'fst'
1312
type:
1413
type: string
1514
description: Type of authority records stored in source file

‎src/test/java/org/folio/entlinks/controller/AuthoritySourceFilesControllerIT.java

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -316,32 +316,27 @@ void updateAuthoritySourceFilePartially_positive_entityGetUpdated() throws Excep
316316
.type("type1")
317317
.baseUrl("https://url/")
318318
.selectable(false)
319-
// remove code and insert code2 and code3
320-
.codes(List.of("code2", "code3"))
319+
.code("replacedCode")
321320
.hridManagement(new AuthoritySourceFilePatchDtoHridManagement().startNumber(hridStartNumber));
322321

323322
var created = doPostAndReturn(authoritySourceFilesEndpoint(), createDto, AuthoritySourceFileDto.class);
324323

325324
doPatch(authoritySourceFilesEndpoint(created.getId()), partiallyModified)
326325
.andExpect(status().isNoContent());
327326

328-
var content = doGet(authoritySourceFilesEndpoint(created.getId()))
327+
doGet(authoritySourceFilesEndpoint(created.getId()))
329328
.andExpect(jsonPath("source", is(SourceEnum.LOCAL.getValue())))
330329
.andExpect(jsonPath("name", is(partiallyModified.getName())))
331330
.andExpect(jsonPath("type", is(partiallyModified.getType())))
332331
.andExpect(jsonPath("baseUrl", is(partiallyModified.getBaseUrl())))
333332
.andExpect(jsonPath("selectable", is(partiallyModified.getSelectable())))
334-
.andExpect(jsonPath("codes", hasSize(2)))
333+
.andExpect(jsonPath("codes", is(List.of("replacedCode"))))
335334
.andExpect(jsonPath("_version", is(1)))
336335
.andExpect(jsonPath("hridManagement.startNumber", is(hridStartNumber)))
337336
.andExpect(jsonPath("metadata.createdDate", notNullValue()))
338337
.andExpect(jsonPath("metadata.updatedDate", notNullValue()))
339338
.andExpect(jsonPath("metadata.updatedByUserId", is(USER_ID)))
340-
.andExpect(jsonPath("metadata.createdByUserId", is(USER_ID)))
341-
.andReturn().getResponse().getContentAsString();
342-
var resultDto = objectMapper.readValue(content, AuthoritySourceFileDto.class);
343-
344-
assertThat(new HashSet<>(resultDto.getCodes()), equalTo(new HashSet<>(partiallyModified.getCodes())));
339+
.andExpect(jsonPath("metadata.createdByUserId", is(USER_ID)));
345340
}
346341

347342
@Test

‎src/test/java/org/folio/entlinks/controller/converter/AuthoritySourceFileMapperTest.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ void testPartialUpdate() {
8080
var patchDto = new AuthoritySourceFilePatchDto();
8181
patchDto.setName(UPDATED_NAME);
8282
patchDto.setType(UPDATED_TYPE);
83-
patchDto.setCodes(List.of(UPDATED_CODE));
83+
patchDto.setCode(UPDATED_CODE);
8484
patchDto.setBaseUrl(UPDATED_BASE_URL);
8585
patchDto.selectable(true);
8686
patchDto.hridManagement(new AuthoritySourceFilePatchDtoHridManagement().startNumber(5));
@@ -90,8 +90,7 @@ void testPartialUpdate() {
9090
assertThat(updatedFile).isNotNull();
9191
assertThat(updatedFile.getName()).isEqualTo(patchDto.getName());
9292
assertThat(updatedFile.getType()).isEqualTo(patchDto.getType());
93-
assertThat(updatedFile.getAuthoritySourceFileCodes().stream().map(AuthoritySourceFileCode::getCode).toList())
94-
.isEqualTo(patchDto.getCodes());
93+
assertThat(updatedFile.getAuthoritySourceFileCodes().iterator().next().getCode()).isEqualTo(patchDto.getCode());
9594
assertThat(updatedFile.getFullBaseUrl()).isEqualTo(patchDto.getBaseUrl());
9695
assertThat(updatedFile.isSelectable()).isEqualTo(patchDto.getSelectable());
9796
assertThat(updatedFile.getHridStartNumber()).isEqualTo(patchDto.getHridManagement().getStartNumber());

‎src/test/java/org/folio/entlinks/controller/delegate/AuthoritySourceFileServiceDelegateTest.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ void shouldNotPatchAuthoritySourceFile_whenSourceFolioOrAuthoritiesReferenced(Au
194194
.selectable(true)
195195
.version(1)
196196
.baseUrl("baseUrl")
197-
.codes(List.of("a", "b"))
197+
.code("a")
198198
.hridManagement(new AuthoritySourceFilePatchDtoHridManagement().startNumber(1));
199199
var expected = new RequestBodyValidationException(
200200
"Unable to patch. Authority source file source is FOLIO or it has authority references", errors);
@@ -348,9 +348,9 @@ void shouldNotDeleteConsortiumShadowCopy() {
348348
static Stream<Arguments> patchValidationFailureData() {
349349
return Stream.of(
350350
Arguments.of(AuthoritySourceFileSource.FOLIO, false, List.of(new Parameter("name").value("name"),
351-
new Parameter("codes").value("a,b"),
351+
new Parameter("code").value("a"),
352352
new Parameter("hridManagement.startNumber").value("1"))),
353-
Arguments.of(AuthoritySourceFileSource.LOCAL, true, List.of(new Parameter("codes").value("a,b"),
353+
Arguments.of(AuthoritySourceFileSource.LOCAL, true, List.of(new Parameter("code").value("a"),
354354
new Parameter("hridManagement.startNumber").value("1"))));
355355
}
356356

‎src/test/java/org/folio/entlinks/service/authority/AuthoritySourceFileServiceTest.java

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ void shouldCreateAuthoritySourceFile(String source) {
171171
}
172172

173173
@ParameterizedTest
174-
@ValueSource(strings = {"", "123", "#", "abc123", "abc def"})
174+
@ValueSource(strings = {"", "123", "#", "abc123", "abc def", "abc,def"})
175175
void shouldNotBePossibleToCreateAuthoritySourceFileWithInvalidCode(String code) {
176176
var sourceFileCode = new AuthoritySourceFileCode();
177177
sourceFileCode.setCode(code);
@@ -261,6 +261,27 @@ void shouldThrowExceptionEntityIdDiffersFromProvidedId() {
261261
verifyNoInteractions(repository);
262262
}
263263

264+
@ParameterizedTest
265+
@ValueSource(strings = {"", "123", "#", "abc123", "abc def", "abc,def"})
266+
void shouldThrowExceptionWhenProvidedInvalidSourceFileCode(String code) {
267+
var entity = new AuthoritySourceFile();
268+
UUID id = UUID.randomUUID();
269+
entity.setId(id);
270+
var sourceFileCode = new AuthoritySourceFileCode();
271+
sourceFileCode.setCode(code);
272+
entity.addCode(sourceFileCode);
273+
274+
var thrown = assertThrows(RequestBodyValidationException.class, () -> service.update(id, entity));
275+
276+
assertThat(thrown.getInvalidParameters()).hasSize(1);
277+
assertThat(thrown.getInvalidParameters().get(0).getKey()).isEqualTo("code");
278+
assertThat(thrown.getInvalidParameters().get(0).getValue())
279+
.isEqualTo(sourceFileCode.getCode());
280+
assertThat(thrown.getMessage())
281+
.isEqualTo("Authority Source File prefix should be non-empty sequence of letters");
282+
verifyNoInteractions(repository);
283+
}
284+
264285
@Test
265286
void shouldDeleteAuthoritySourceFile() {
266287
var id = UUID.randomUUID();

‎src/test/java/org/folio/support/TestDataUtils.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -306,7 +306,7 @@ public class AuthorityTestData {
306306
UUID.fromString("453e9a34-31a3-4f82-b3f5-1057f20b050e"),
307307
UUID.fromString("08c9fd60-d038-46bb-be83-45f93a8e53b7")};
308308
private static final Integer[] SOURCE_FILE_CODE_IDS = new Integer[] {1, 2, 3};
309-
private static final String[] SOURCE_FILE_CODES = new String[] {"code1", "code2", "code3"};
309+
private static final String[] SOURCE_FILE_CODES = new String[] {"codeOne", "codeTwo", "codeThree"};
310310
private static final String[] SOURCE_FILE_NAMES = new String[] {"name1", "name2", "name3"};
311311
private static final AuthoritySourceFileSource[] SOURCE_FILE_SOURCES = new AuthoritySourceFileSource[] {
312312
AuthoritySourceFileSource.LOCAL,

0 commit comments

Comments
 (0)