Skip to content

Commit 70c0149

Browse files
BKadirkhodjaevBKadirkhodjaev
committed
[EUREKA-561-5]. Simplify location validation, update tests
1 parent 641a1e6 commit 70c0149

File tree

5 files changed

+106
-66
lines changed

5 files changed

+106
-66
lines changed

‎src/main/java/org/folio/helper/PurchaseOrderHelper.java‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -235,12 +235,12 @@ public Future<Void> updateOrder(CompositePurchaseOrder compPO, boolean deleteHol
235235
boolean isTransitionToOpen = isTransitionToOpen(poFromStorage, compPO);
236236
return orderValidationService.validateOrderForUpdate(compPO, poFromStorage, deleteHoldings, requestContext)
237237
.compose(v -> {
238-
var poLineFutures = new ArrayList<Future<Void>>();
239238
if (isTransitionToOpen) {
240239
if (CollectionUtils.isEmpty(compPO.getCompositePoLines())) {
241240
compPO.setCompositePoLines(clonedPoFromStorage.getCompositePoLines());
242241
}
243242
}
243+
var poLineFutures = new ArrayList<Future<Void>>();
244244
logger.info("updateOrder:: POL size: {}", compPO.getCompositePoLines());
245245
if (!compPO.getCompositePoLines().isEmpty()) {
246246
compPO.getCompositePoLines().forEach(poLine -> {
@@ -249,12 +249,10 @@ public Future<Void> updateOrder(CompositePurchaseOrder compPO, boolean deleteHol
249249
.findFirst().orElse(null);
250250
logger.info("updateOrder:: Stored POL found: {}", Objects.nonNull(compPoLineFromStorage));
251251
if (Objects.nonNull(compPoLineFromStorage)) {
252-
var updatedLocations = poLine.getLocations();
253-
var storedLocations = compPoLineFromStorage.getLocations();
252+
var updatedLocations = compPoLineFromStorage.getLocations();
254253
logger.info("updateOrder:: Stored POL poLineId: {}", poLine.getId());
255-
logger.info("updateOrder:: Stored POL storedLocations: {}", JsonArray.of(storedLocations).encodePrettily());
256254
logger.info("updateOrder:: Stored POL updatedLocations: {}", JsonArray.of(updatedLocations).encodePrettily());
257-
poLineFutures.add(compositePoLineValidationService.validateUserUnaffiliatedLocationUpdates(poLine.getId(), updatedLocations, storedLocations, requestContext));
255+
poLineFutures.add(compositePoLineValidationService.validateUserUnaffiliatedLocationUpdates(poLine.getId(), updatedLocations, requestContext));
258256
}
259257
});
260258
}

‎src/main/java/org/folio/helper/PurchaseOrderLineHelper.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,7 @@ public Future<Void> updateOrderLine(CompositePoLine compOrderLine, RequestContex
278278
.compose(compOrder -> protectionService.isOperationRestricted(compOrder.getAcqUnitIds(), UPDATE, requestContext)
279279
.compose(v -> purchaseOrderLineService.validateAndNormalizeISBNAndProductType(Collections.singletonList(compOrderLine), requestContext))
280280
.compose(v -> validateAccessProviders(compOrderLine, requestContext))
281-
.compose(v -> compositePoLineValidationService.validateUserUnaffiliatedLocationUpdates(compOrderLine.getId(), compOrderLine.getLocations(), poLineFromStorage.getLocations(), requestContext))
281+
.compose(v -> compositePoLineValidationService.validateUserUnaffiliatedLocationUpdates(compOrderLine.getId(), compOrderLine.getLocations(), requestContext))
282282
.compose(v -> expenseClassValidationService.validateExpenseClassesForOpenedOrder(compOrder, Collections.singletonList(compOrderLine), requestContext))
283283
.compose(v -> processPoLineEncumbrances(compOrder, compOrderLine, poLineFromStorage, requestContext)))
284284
.map(v -> compOrderLine.withPoLineNumber(poLineFromStorage.getPoLineNumber())) // PoLine number must not be modified during PoLine update, set original value

‎src/main/java/org/folio/service/orders/CompositePoLineValidationService.java‎

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@
2424

2525
import io.vertx.core.Future;
2626
import org.apache.commons.collections4.CollectionUtils;
27-
import org.apache.commons.collections4.SetUtils;
2827
import org.apache.commons.lang3.StringUtils;
28+
import org.apache.http.HttpStatus;
2929
import org.apache.logging.log4j.LogManager;
3030
import org.apache.logging.log4j.Logger;
3131
import org.folio.orders.utils.HelperUtils;
@@ -345,27 +345,20 @@ private void validateReceivingWorkflowForBindary(CompositePoLine poLine, List<Er
345345
}
346346
}
347347

348-
public Future<Void> validateUserUnaffiliatedLocationUpdates(String poLineId, List<Location> updatedPoLineLocations,
349-
List<Location> storedPoLineLocations, RequestContext requestContext) {
348+
public Future<Void> validateUserUnaffiliatedLocationUpdates(String poLineId, List<Location> locations, RequestContext requestContext) {
350349
return getUserTenantsIfNeeded(requestContext)
351350
.compose(userTenants -> {
352351
if (CollectionUtils.isEmpty(userTenants)) {
353352
logger.info("validateUserUnaffiliatedLocationUpdates:: User tenants is empty");
354353
return Future.succeededFuture();
355354
}
356-
var storageUnaffiliatedLocations = extractUnaffiliatedLocations(storedPoLineLocations, userTenants);
357-
var updatedUnaffiliatedLocations = extractUnaffiliatedLocations(updatedPoLineLocations, userTenants);
358-
logger.info("validateUserUnaffiliatedLocationUpdates:: Found unaffiliated POL location tenant ids, poLineId: '{}', stored: '{}', updated: '{}'",
359-
poLineId,
360-
storageUnaffiliatedLocations.stream().map(Location::getTenantId).distinct().toList(),
361-
updatedUnaffiliatedLocations.stream().map(Location::getTenantId).distinct().toList());
362-
if (!SetUtils.isEqualSet(storageUnaffiliatedLocations, updatedUnaffiliatedLocations)) {
363-
logger.info("validateUserUnaffiliatedLocationUpdates:: User is not affiliated with all locations on the POL, poLineId: '{}'",
364-
poLineId);
365-
return Future.failedFuture(new HttpException(422, ErrorCodes.LOCATION_UPDATE_WITHOUT_AFFILIATION));
355+
var uniqueUnAffiliatedLocations = extractUnaffiliatedLocations(locations, userTenants).stream()
356+
.map(Location::getTenantId).distinct().toList();
357+
if (CollectionUtils.isNotEmpty(uniqueUnAffiliatedLocations)) {
358+
logger.info("validateUserUnaffiliatedLocationUpdates:: User has unaffiliated locations on the POL, poLineId: {}, unique locations: {}", poLineId, uniqueUnAffiliatedLocations);
359+
return Future.failedFuture(new HttpException(HttpStatus.SC_UNPROCESSABLE_ENTITY, ErrorCodes.LOCATION_UPDATE_WITHOUT_AFFILIATION));
366360
}
367-
logger.info("validateUserUnaffiliatedLocationUpdates:: User is affiliated with all locations on the POL, poLineId: '{}'",
368-
poLineId);
361+
logger.info("validateUserUnaffiliatedLocationUpdates:: User is affiliated with all {} locations on the POL, poLineId: {}", locations.size(), poLineId);
369362
return Future.succeededFuture();
370363
});
371364
}

‎src/test/java/org/folio/helper/PurchaseOrderHelperTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ void testPutPendingCompositeOrder() throws IOException {
201201
doReturn(succeededFuture(null))
202202
.when(encumbranceService).updateEncumbrancesOrderStatusAndReleaseIfClosed(any(CompositePurchaseOrder.class), eq(requestContext));
203203
doReturn(succeededFuture(null))
204-
.when(compositePoLineValidationService).validateUserUnaffiliatedLocationUpdates(anyString(), any(), any(), eq(requestContext));
204+
.when(compositePoLineValidationService).validateUserUnaffiliatedLocationUpdates(anyString(), any(), eq(requestContext));
205205

206206
// When
207207
Future<Void> future = purchaseOrderHelper.putCompositeOrderById(compPO.getId(), deleteHoldings, compPO, requestContext);

‎src/test/java/org/folio/service/orders/CompositePoLineValidationServiceTest.java‎

Lines changed: 93 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package org.folio.service.orders;
22

33
import static io.vertx.core.Future.succeededFuture;
4-
import static org.folio.TestUtils.getLocationsForTenants;
54
import static org.folio.rest.core.exceptions.ErrorCodes.*;
65
import static org.folio.rest.jaxrs.model.CompositePoLine.OrderFormat.OTHER;
76
import static org.folio.rest.jaxrs.model.CompositePoLine.OrderFormat.P_E_MIX;
@@ -12,19 +11,20 @@
1211
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
1312
import static org.junit.jupiter.api.Assertions.assertTrue;
1413
import static org.junit.jupiter.api.Assertions.fail;
15-
import static org.mockito.ArgumentMatchers.any;
1614
import static org.mockito.ArgumentMatchers.anyString;
1715
import static org.mockito.ArgumentMatchers.eq;
1816
import static org.mockito.Mockito.doReturn;
17+
import static org.mockito.Mockito.when;
1918

20-
import java.lang.reflect.InvocationTargetException;
2119
import java.util.List;
2220
import java.util.Optional;
2321
import java.util.Set;
2422
import java.util.UUID;
2523
import java.util.stream.Collectors;
2624

2725
import io.vertx.core.Future;
26+
import io.vertx.junit5.VertxExtension;
27+
import io.vertx.junit5.VertxTestContext;
2828
import org.folio.models.consortium.ConsortiumConfiguration;
2929
import org.folio.rest.core.exceptions.ErrorCodes;
3030
import org.folio.rest.core.exceptions.HttpException;
@@ -35,33 +35,31 @@
3535
import org.folio.rest.jaxrs.model.Error;
3636
import org.folio.rest.jaxrs.model.Location;
3737
import org.folio.rest.jaxrs.model.Physical;
38-
import org.folio.rest.jaxrs.model.PoLine;
3938
import org.folio.service.consortium.ConsortiumConfigurationService;
4039
import org.folio.service.consortium.ConsortiumUserTenantsRetriever;
4140
import org.folio.service.finance.expenceclass.ExpenseClassValidationService;
4241
import org.junit.jupiter.api.AfterEach;
4342
import org.junit.jupiter.api.BeforeEach;
4443
import org.junit.jupiter.api.DisplayName;
4544
import org.junit.jupiter.api.Test;
45+
import org.junit.jupiter.api.extension.ExtendWith;
4646
import org.mockito.InjectMocks;
4747
import org.mockito.Mock;
4848
import org.mockito.MockitoAnnotations;
4949

50+
@ExtendWith(VertxExtension.class)
5051
public class CompositePoLineValidationServiceTest {
52+
53+
@Mock private ExpenseClassValidationService expenseClassValidationService;
54+
@Mock private ConsortiumConfigurationService consortiumConfigurationService;
55+
@Mock private ConsortiumUserTenantsRetriever consortiumUserTenantsRetriever;
56+
@Mock private RequestContext requestContext;
57+
@InjectMocks private CompositePoLineValidationService compositePoLineValidationService;
58+
5159
private AutoCloseable mockitoMocks;
52-
@Mock
53-
private ExpenseClassValidationService expenseClassValidationService;
54-
@Mock
55-
private ConsortiumConfigurationService consortiumConfigurationService;
56-
@Mock
57-
private ConsortiumUserTenantsRetriever consortiumUserTenantsRetriever;
58-
@InjectMocks
59-
private CompositePoLineValidationService compositePoLineValidationService;
60-
@Mock
61-
private RequestContext requestContext;
6260

6361
@BeforeEach
64-
public void initMocks(){
62+
public void initMocks() {
6563
mockitoMocks = MockitoAnnotations.openMocks(this);
6664
}
6765

@@ -169,7 +167,6 @@ void shouldReturnErrorIfIncorrectOrderFormatWhenBindaryActive() {
169167
assertEquals(ORDER_FORMAT_INCORRECT_FOR_BINDARY_ACTIVE.getCode(), errors.get(0).getCode());
170168
}
171169

172-
173170
@Test
174171
void shouldReturnErrorIfIncorrectCreateInventoryWhenBindaryActive() {
175172
var compositePoLine = new CompositePoLine()
@@ -382,7 +379,6 @@ void testValidatePoLineWithZeroQuantitiesWithoutLocations() {
382379
assertThat(errors, hasSize(0));
383380
}
384381

385-
386382
private Set<String> errorsToCodes(List<Error> errors) {
387383
return errors
388384
.stream()
@@ -391,38 +387,91 @@ private Set<String> errorsToCodes(List<Error> errors) {
391387
}
392388

393389
@Test
394-
void testValidateUserUnaffiliatedLocationUpdates() throws NoSuchMethodException, InvocationTargetException, IllegalAccessException {
395-
var locationsStored = getLocationsForTenants(List.of("tenant1", "tenant2", "tenant3"));
396-
var locationsUpdated = getLocationsForTenants(List.of("tenant1", "tenant3"));
397-
var storagePoLine = new PoLine().withLocations(locationsStored);
398-
var updatedPoLine = new CompositePoLine().withLocations(locationsUpdated);
390+
void testValidateUserUnaffiliatedLocationUpdates(VertxTestContext testContext) {
391+
var locationsUpdated = List.of(
392+
new Location().withLocationId(UUID.randomUUID().toString()).withTenantId("tenant1"),
393+
new Location().withLocationId(UUID.randomUUID().toString()).withTenantId("tenant2"));
394+
var updatedPoLineId = UUID.randomUUID().toString();
395+
396+
when(consortiumConfigurationService.getConsortiumConfiguration(eq(requestContext)))
397+
.thenReturn(Future.succeededFuture(Optional.empty()));
398+
when(consortiumUserTenantsRetriever.getUserTenants(eq("consortiumId"), anyString(), eq(requestContext)))
399+
.thenReturn(Future.succeededFuture(List.of("tenant1")));
400+
401+
compositePoLineValidationService.validateUserUnaffiliatedLocationUpdates(updatedPoLineId, locationsUpdated, requestContext)
402+
.onComplete(testContext.succeeding(result -> testContext.verify(testContext::completeNow)));
403+
}
399404

400-
doReturn(succeededFuture(Optional.of(new ConsortiumConfiguration("tenant1", "consortiumId"))))
401-
.when(consortiumConfigurationService).getConsortiumConfiguration(any(RequestContext.class));
402-
doReturn(succeededFuture(List.of("tenant1", "tenant2")))
403-
.when(consortiumUserTenantsRetriever).getUserTenants(eq("consortiumId"), eq("centralTenantId"), any(RequestContext.class));
405+
@Test
406+
void testValidateUserUnaffiliatedLocationUpdatesAllValidLocations(VertxTestContext testContext) {
407+
var locationsUpdated = List.of(
408+
new Location().withLocationId(UUID.randomUUID().toString()).withTenantId("tenant1"),
409+
new Location().withLocationId(UUID.randomUUID().toString()).withTenantId("tenant2"));
410+
var updatedPoLineId = UUID.randomUUID().toString();
411+
412+
when(consortiumConfigurationService.getConsortiumConfiguration(eq(requestContext)))
413+
.thenReturn(Future.succeededFuture(Optional.of(new ConsortiumConfiguration("tenant1", "consortiumId"))));
414+
when(consortiumUserTenantsRetriever.getUserTenants(eq("consortiumId"), anyString(), eq(requestContext)))
415+
.thenReturn(Future.succeededFuture(List.of("tenant1", "tenant2")));
416+
417+
compositePoLineValidationService.validateUserUnaffiliatedLocationUpdates(updatedPoLineId, locationsUpdated, requestContext)
418+
.onComplete(testContext.succeeding(result -> testContext.verify(testContext::completeNow)));
419+
}
404420

405-
var future = compositePoLineValidationService.validateUserUnaffiliatedLocationUpdates(updatedPoLine.getId(), updatedPoLine.getLocations(), storagePoLine.getLocations(), requestContext);
421+
@Test
422+
void testValidateUserUnaffiliatedLocationUpdatesAllValidDuplicateTenantLocations(VertxTestContext testContext) {
423+
var locationsUpdated = List.of(
424+
new Location().withLocationId(UUID.randomUUID().toString()).withTenantId("tenant1"),
425+
new Location().withLocationId(UUID.randomUUID().toString()).withTenantId("tenant2"),
426+
new Location().withLocationId(UUID.randomUUID().toString()).withTenantId("tenant2"));
427+
var updatedPoLineId = UUID.randomUUID().toString();
428+
429+
when(consortiumConfigurationService.getConsortiumConfiguration(eq(requestContext)))
430+
.thenReturn(Future.succeededFuture(Optional.of(new ConsortiumConfiguration("tenant1", "consortiumId"))));
431+
when(consortiumUserTenantsRetriever.getUserTenants(eq("consortiumId"), anyString(), eq(requestContext)))
432+
.thenReturn(Future.succeededFuture(List.of("tenant1", "tenant2")));
433+
434+
compositePoLineValidationService.validateUserUnaffiliatedLocationUpdates(updatedPoLineId, locationsUpdated, requestContext)
435+
.onComplete(testContext.succeeding(result -> testContext.verify(testContext::completeNow)));
436+
}
406437

407-
assertTrue(future.succeeded());
438+
@Test
439+
void testValidateUserUnaffiliatedLocationUpdatesOneValidAndOneInvalidLocations(VertxTestContext testContext) {
440+
var locationsUpdated = List.of(
441+
new Location().withLocationId(UUID.randomUUID().toString()).withTenantId("tenant1"),
442+
new Location().withLocationId(UUID.randomUUID().toString()).withTenantId("tenant2"));
443+
var updatedPoLineId = UUID.randomUUID().toString();
444+
445+
when(consortiumConfigurationService.getConsortiumConfiguration(eq(requestContext)))
446+
.thenReturn(Future.succeededFuture(Optional.of(new ConsortiumConfiguration("tenant1", "consortiumId"))));
447+
when(consortiumUserTenantsRetriever.getUserTenants(eq("consortiumId"), anyString(), eq(requestContext)))
448+
.thenReturn(Future.succeededFuture(List.of("tenant1")));
449+
450+
compositePoLineValidationService.validateUserUnaffiliatedLocationUpdates(updatedPoLineId, locationsUpdated, requestContext)
451+
.onComplete(testContext.failing(cause -> testContext.verify(() -> {
452+
assertInstanceOf(HttpException.class, cause);
453+
assertTrue(cause.getMessage().contains(ErrorCodes.LOCATION_UPDATE_WITHOUT_AFFILIATION.getDescription()));
454+
testContext.completeNow();
455+
})));
408456
}
409457

410458
@Test
411-
void testValidateUserUnaffiliatedLocationUpdatesInvalid() {
412-
var locationsStored = getLocationsForTenants(List.of("tenant1", "tenant2", "tenant3"));
413-
var locationsUpdated = getLocationsForTenants(List.of("tenant1", "tenant3"));
414-
locationsUpdated.get(1).withQuantityPhysical(10);
415-
var storagePoLine = new PoLine().withLocations(locationsStored);
416-
var updatedPoLine = new CompositePoLine().withLocations(locationsUpdated);
417-
418-
doReturn(succeededFuture(Optional.of(new ConsortiumConfiguration("tenant1", "consortiumId"))))
419-
.when(consortiumConfigurationService).getConsortiumConfiguration(any(RequestContext.class));
420-
doReturn(succeededFuture(List.of("tenant1")))
421-
.when(consortiumUserTenantsRetriever).getUserTenants(eq("consortiumId"), anyString(), any(RequestContext.class));
422-
423-
var future = compositePoLineValidationService.validateUserUnaffiliatedLocationUpdates(updatedPoLine.getId(), updatedPoLine.getLocations(), storagePoLine.getLocations(), requestContext);
424-
425-
assertTrue(future.failed());
426-
assertInstanceOf(HttpException.class, future.cause());
459+
void testValidateUserUnaffiliatedLocationUpdatesTwoInvalidLocations(VertxTestContext testContext) {
460+
var locationsUpdated = List.of(
461+
new Location().withLocationId(UUID.randomUUID().toString()).withTenantId("tenant1"),
462+
new Location().withLocationId(UUID.randomUUID().toString()).withTenantId("tenant2"));
463+
var updatedPoLineId = UUID.randomUUID().toString();
464+
465+
when(consortiumConfigurationService.getConsortiumConfiguration(eq(requestContext)))
466+
.thenReturn(Future.succeededFuture(Optional.of(new ConsortiumConfiguration("tenant1", "consortiumId"))));
467+
when(consortiumUserTenantsRetriever.getUserTenants(eq("consortiumId"), anyString(), eq(requestContext)))
468+
.thenReturn(Future.succeededFuture(List.of("tenant3")));
469+
470+
compositePoLineValidationService.validateUserUnaffiliatedLocationUpdates(updatedPoLineId, locationsUpdated, requestContext)
471+
.onComplete(testContext.failing(cause -> testContext.verify(() -> {
472+
assertInstanceOf(HttpException.class, cause);
473+
assertTrue(cause.getMessage().contains(ErrorCodes.LOCATION_UPDATE_WITHOUT_AFFILIATION.getDescription()));
474+
testContext.completeNow();
475+
})));
427476
}
428477
}

0 commit comments

Comments
 (0)