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 2ec5f01ab..7e20287be 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 @@ -27,10 +27,12 @@ import org.springframework.web.servlet.support.ServletUriComponentsBuilder; import java.net.URI; +import java.util.ArrayList; +import java.util.List; import java.util.function.Supplier; -import java.util.stream.Collectors; import java.util.stream.Stream; +import static java.util.stream.Collectors.toList; import static org.springframework.http.HttpStatus.NOT_FOUND; @RestController @@ -67,7 +69,7 @@ public ResponseEntity getAll(@PathVariable String metadataResolverId) { @Transactional(readOnly = true) public ResponseEntity getOne(@PathVariable String metadataResolverId, @PathVariable String resourceId) { MetadataResolver resolver = findResolverOrThrowHttp404(metadataResolverId); - return ResponseEntity.ok(findFilterOrThrowHttp404(resolver, resourceId)); + return ResponseEntity.ok(findFilterOrThrowHttp404(resourceId)); } @PostMapping("/Filters") @@ -130,12 +132,20 @@ public ResponseEntity delete(@PathVariable String metadataResolverId, @PathVariable String resourceId) { MetadataResolver resolver = findResolverOrThrowHttp404(metadataResolverId); + MetadataFilter filterToDelete = findFilterOrThrowHttp404(resourceId); + //TODO: consider implementing delete of filter directly from RDBMS via FilterRepository - boolean removed = resolver.getMetadataFilters().removeIf(f -> f.getResourceId().equals(resourceId)); + //This is currently the only way to correctly delete and manage resolver-filter relationship + //Until we implement a bi-directional relationship between them which turns out to be a much larger + //change that we need to make in the entire code base + List updatedFilters = new ArrayList<>(resolver.getMetadataFilters()); + boolean removed = updatedFilters.removeIf(f -> f.getResourceId().equals(resourceId)); if(!removed) { throw HTTP_404_CLIENT_ERROR_EXCEPTION.get(); } + resolver.setMetadataFilters(updatedFilters); repository.save(resolver); + filterRepository.delete(filterToDelete); //TODO: do we need to reload filters here?!? //metadataResolverService.reloadFilters(persistedMr.getName()); @@ -151,17 +161,18 @@ private MetadataResolver findResolverOrThrowHttp404(String resolverResourceId) { return resolver; } - private MetadataFilter findFilterOrThrowHttp404(MetadataResolver resolver, String filterResourceId) { - return resolver.getMetadataFilters().stream() - .filter(f -> f.getResourceId().equals(filterResourceId)) - .findFirst() - .orElseThrow(HTTP_404_CLIENT_ERROR_EXCEPTION); + private MetadataFilter findFilterOrThrowHttp404(String filterResourceId) { + MetadataFilter filter = filterRepository.findByResourceId(filterResourceId); + if(filter == null) { + throw HTTP_404_CLIENT_ERROR_EXCEPTION.get(); + } + return filter; } private MetadataFilter newlyPersistedFilter(Stream filters, final String filterResourceId) { MetadataFilter persistedFilter = filters .filter(f -> f.getResourceId().equals(filterResourceId)) - .collect(Collectors.toList()).get(0); + .collect(toList()).get(0); return persistedFilter; } diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersControllerIntegrationTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersControllerIntegrationTests.groovy index e65a7e963..06f9d0ca0 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersControllerIntegrationTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersControllerIntegrationTests.groovy @@ -126,6 +126,54 @@ class MetadataFiltersControllerIntegrationTests extends Specification { GETResultAfterDelete.statusCode.value() == 404 } + def "DELETE Filter with resolver having more than TWO filters attached"() { + given: 'MetadataResolver with 3 attached filters is available in data store' + def resolver = generator.buildRandomMetadataResolverOfType('FileBacked') + resolver.metadataFilters << generator.entityAttributesFilter() + resolver.metadataFilters << generator.entityAttributesFilter() + resolver.metadataFilters << generator.entityAttributesFilter() + resolver.metadataFilters << generator.entityAttributesFilter() + resolver.metadataFilters << generator.entityAttributesFilter() + resolver.metadataFilters << generator.entityAttributesFilter() + resolver.metadataFilters << generator.entityAttributesFilter() + def filter_THREE_ResourceId = resolver.metadataFilters[2].resourceId + def filter_SIX_ResourceId = resolver.metadataFilters[5].resourceId + def resolverResourceId = resolver.resourceId + metadataResolverRepository.save(resolver) + + when: 'GET resolver to count the original number of filters' + def originalResolverResult = this.restTemplate.getForEntity("$BASE_URI/$resolverResourceId", Map) + + then: + originalResolverResult.body.metadataFilters.size == 7 + + when: 'DELETE call is made for one of the filters and then GET call is made for the just deleted filter' + restTemplate.delete("$BASE_URI/$resolverResourceId/Filters/$filter_SIX_ResourceId") + def GETResultAfterDelete = this.restTemplate.getForEntity("$BASE_URI/$resolverResourceId/Filters/$filter_SIX_ResourceId", String) + + then: 'The deleted resource is gone' + GETResultAfterDelete.statusCodeValue == 404 + + and: 'GET resolver to count modified number of filters' + def resolverResult_2 = this.restTemplate.getForEntity("$BASE_URI/$resolverResourceId", Map) + + then: + resolverResult_2.body.metadataFilters.size == 6 + + and: 'DELETE call is made for one of the filters and then GET call is made for the just deleted filter' + restTemplate.delete("$BASE_URI/$resolverResourceId/Filters/$filter_THREE_ResourceId") + def GETResultAfterDelete_2 = this.restTemplate.getForEntity("$BASE_URI/$resolverResourceId/Filters/$filter_THREE_ResourceId", String) + + then: 'The deleted resource is gone' + GETResultAfterDelete_2.statusCodeValue == 404 + + and: 'GET resolver to count modified number of filters' + def resolverResult_3 = this.restTemplate.getForEntity("$BASE_URI/$resolverResourceId", Map) + + then: + resolverResult_3.body.metadataFilters.size == 5 + } + private HttpEntity createRequestHttpEntityFor(Closure jsonBodySupplier) { new HttpEntity(jsonBodySupplier(), ['Content-Type': 'application/json'] as HttpHeaders) } diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersControllerTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersControllerTests.groovy index 40c6cefef..bf622dfb5 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersControllerTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersControllerTests.groovy @@ -112,6 +112,7 @@ class MetadataFiltersControllerTests extends Specification { def expectedFilter = testObjectGenerator.entityAttributesFilter() metadataResolver.metadataFilters = [expectedFilter] 1 * metadataResolverRepository.findByResourceId(_) >> metadataResolver + 1 * metadataFilterRepository.findByResourceId(_) >> expectedFilter def expectedResourceId = expectedFilter.resourceId def expectedHttpResponseStatus = status().isOk()