diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersController.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersController.java index 2b4cceb2c..2ec5f01ab 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersController.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersController.java @@ -48,7 +48,6 @@ public class MetadataFiltersController { private static final Supplier HTTP_404_CLIENT_ERROR_EXCEPTION = () -> new HttpClientErrorException(NOT_FOUND); - //TODO: refactor to use RestControllerSupport class @ExceptionHandler public ResponseEntity notFoundHandler(HttpClientErrorException ex) { if(ex.getStatusCode() == NOT_FOUND) { @@ -101,7 +100,6 @@ public ResponseEntity update(@PathVariable String metadataResolverId, // check to make sure that the relationship exists if (!metadataResolver.getMetadataFilters().contains(filterTobeUpdated)) { // TODO: find a better response - // TODO: refactor to use RestControllerSupport class return new ResponseEntity<>(HttpStatus.BAD_REQUEST); } @@ -145,7 +143,6 @@ public ResponseEntity delete(@PathVariable String metadataResolverId, return ResponseEntity.noContent().build(); } - //TODO: refactor to use RestControllerSupport class private MetadataResolver findResolverOrThrowHttp404(String resolverResourceId) { MetadataResolver resolver = repository.findByResourceId(resolverResourceId); if(resolver == null) { diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersPositionOrderController.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersPositionOrderController.java index bc3ccbffd..4c7d70ec0 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersPositionOrderController.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersPositionOrderController.java @@ -13,12 +13,10 @@ import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RestController; -import org.springframework.web.bind.annotation.RestControllerAdvice; import java.util.ArrayList; -import java.util.Arrays; +import java.util.Comparator; import java.util.List; -import java.util.Optional; import java.util.stream.Collectors; import static java.util.stream.Collectors.toList; @@ -43,23 +41,33 @@ public ResponseEntity updateFiltersPositionOrder(@PathVariable String metadat MetadataResolver resolver = restControllersSupport.findResolverOrThrowHttp404(metadataResolverId); List currentFilters = resolver.getMetadataFilters(); - List reOrderedFilters = new ArrayList<>(); - - filtersResourceIds.forEach(it -> - currentFilters.stream() - .filter(f -> f.getResourceId().equals(it)) - .findFirst() - .ifPresent(reOrderedFilters::add) - ); - - if(currentFilters.size() == reOrderedFilters.size()) { - resolver.setMetadataFilters(reOrderedFilters); - metadataResolverRepository.save(resolver); - return ResponseEntity.noContent().build(); + + //Check for bad data upfront. We could avoid this check and take wrong size and/or filter ids and blindly pass to sort below. + //In that case, the sort operation will silently NOT do anything and leave original filters order, + //but we will not be able to indicate to calling clients HTTP 400 in that case. + if ((filtersResourceIds.size() != currentFilters.size()) || + (!currentFilters.stream() + .map(MetadataFilter::getResourceId) + .collect(toList()) + .containsAll(filtersResourceIds))) { + + return ResponseEntity + .badRequest() + .body("Number of filters to reorder or filters resource ids do not match current filters"); } - return ResponseEntity - .badRequest() - .body("Number of filters to reorder or filters resource ids do not match current filters"); + + //This is needed in order to set reference to persistent filters collection to be able to merge the persistent collection + //Otherwise if we manipulate the original collection directly and try to save, we'll get RDBMS constraint violation exception + List reOrderedFilters = new ArrayList<>(currentFilters); + + //Main re-ordering operation + reOrderedFilters.sort(Comparator.comparingInt(f -> filtersResourceIds.indexOf(f.getResourceId()))); + + //re-set the reference and save to DB + resolver.setMetadataFilters(reOrderedFilters); + metadataResolverRepository.save(resolver); + + return ResponseEntity.noContent().build(); } @GetMapping diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersPositionOrderControllerIntegrationTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersPositionOrderControllerIntegrationTests.groovy index c1718a0fd..4d163a660 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersPositionOrderControllerIntegrationTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersPositionOrderControllerIntegrationTests.groovy @@ -114,7 +114,7 @@ class MetadataFiltersPositionOrderControllerIntegrationTests extends Specificati resolverResult.body.metadataFilters.collect {it.resourceId} == originalFiltersPosition and: 'POST is made to re-order filters position with invalid resource ids' - def reorderPOSTResult_2 = reorderFilters(resolver.resourceId, ['invalid', 'resource ids']) + def reorderPOSTResult_2 = reorderFilters(resolver.resourceId, [resolver.secondFilterResourceId, 'invalid-id']) then: 'Request completed successfully with 400' reorderPOSTResult_2.statusCodeValue == 400