From f6f34f69a8e9048f39f80346a26a60e37eef1337 Mon Sep 17 00:00:00 2001 From: Dmitriy Kopylenko Date: Wed, 8 Aug 2018 15:12:34 -0400 Subject: [PATCH 1/3] SHIBUI-734: DELETE filters --- .../controller/MetadataFiltersController.java | 87 +++++++++++++------ 1 file changed, 62 insertions(+), 25 deletions(-) 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 7d873afa6..89a33ea98 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 @@ -14,6 +14,8 @@ import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.transaction.annotation.Transactional; +import org.springframework.web.bind.annotation.DeleteMapping; +import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.PostMapping; @@ -21,12 +23,15 @@ 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.client.HttpClientErrorException; import org.springframework.web.servlet.support.ServletUriComponentsBuilder; import java.net.URI; import java.util.List; +import java.util.function.Supplier; import java.util.stream.Collectors; -import java.util.stream.Stream; + +import static org.springframework.http.HttpStatus.NOT_FOUND; @RestController @RequestMapping("/api/MetadataResolvers/{metadataResolverId}") @@ -40,33 +45,36 @@ public class MetadataFiltersController { @Autowired private MetadataResolverService metadataResolverService; + private static final Supplier HTTP_404_CLIENT_ERROR_EXCEPTION = () -> new HttpClientErrorException(NOT_FOUND); + + @ExceptionHandler + public ResponseEntity notFoundHandler(HttpClientErrorException ex) { + if(ex.getStatusCode() == NOT_FOUND) { + return ResponseEntity.notFound().build(); + } + throw ex; + } + @GetMapping("/Filters") @Transactional(readOnly = true) public ResponseEntity getAll(@PathVariable String metadataResolverId) { - MetadataResolver resolver = repository.findByResourceId(metadataResolverId); - if(resolver == null) { - return ResponseEntity.notFound().build(); - } + MetadataResolver resolver = findResolverOrThrowHttp404(metadataResolverId); + resolver.convertFiltersIntoTransientRepresentationIfNecessary(); return ResponseEntity.ok(resolver.getMetadataFilters()); } @GetMapping("/Filters/{resourceId}") + @Transactional(readOnly = true) public ResponseEntity getOne(@PathVariable String metadataResolverId, @PathVariable String resourceId) { - MetadataResolver resolver = repository.findByResourceId(metadataResolverId); - if(resolver == null) { - return ResponseEntity.notFound().build(); - } - return ResponseEntity.ok(resolver.getMetadataFilters().stream() - .filter(f -> f.getResourceId().equals(resourceId)) - .collect(Collectors.toList()).get(0)); + MetadataResolver resolver = findResolverOrThrowHttp404(metadataResolverId); + resolver.convertFiltersIntoTransientRepresentationIfNecessary(); + return ResponseEntity.ok(findFilterOrThrowHttp404(resolver, resourceId)); } @PostMapping("/Filters") + @Transactional public ResponseEntity create(@PathVariable String metadataResolverId, @RequestBody MetadataFilter createdFilter) { - MetadataResolver metadataResolver = repository.findByResourceId(metadataResolverId); - if(metadataResolver == null) { - return ResponseEntity.notFound().build(); - } + MetadataResolver metadataResolver = findResolverOrThrowHttp404(metadataResolverId); metadataResolver.getMetadataFilters().add(createdFilter); //convert before saving into database @@ -79,7 +87,7 @@ public ResponseEntity create(@PathVariable String metadataResolverId, @Reques metadataResolverService.reloadFilters(persistedMr.getName()); MetadataFilter persistedFilter = - convertIntoTransientRepresentationIfNecessary(persistedMr.getMetadataFilters().stream(), createdFilter.getResourceId()); + convertIntoTransientRepresentationIfNecessary(persistedMr, createdFilter.getResourceId()); return ResponseEntity .created(getResourceUriFor(persistedMr, createdFilter.getResourceId())) @@ -88,14 +96,12 @@ public ResponseEntity create(@PathVariable String metadataResolverId, @Reques } @PutMapping("/Filters/{resourceId}") + @Transactional public ResponseEntity update(@PathVariable String metadataResolverId, @PathVariable String resourceId, @RequestBody MetadataFilter updatedFilter) { - MetadataResolver metadataResolver = repository.findByResourceId(metadataResolverId); - if(metadataResolver == null) { - return ResponseEntity.notFound().build(); - } + MetadataResolver metadataResolver = findResolverOrThrowHttp404(metadataResolverId); if (!resourceId.equals(updatedFilter.getResourceId())) { return new ResponseEntity(HttpStatus.CONFLICT); @@ -132,18 +138,49 @@ public ResponseEntity update(@PathVariable String metadataResolverId, metadataResolverService.reloadFilters(persistedMr.getName()); MetadataFilter persistedFilter = - convertIntoTransientRepresentationIfNecessary(persistedMr.getMetadataFilters().stream(), updatedFilter.getResourceId()); + convertIntoTransientRepresentationIfNecessary(persistedMr, updatedFilter.getResourceId()); persistedFilter.setVersion(persistedFilter.hashCode()); return ResponseEntity.ok().body(persistedFilter); } - private MetadataFilter convertIntoTransientRepresentationIfNecessary(Stream filters, final String filterResourceId) { - MetadataFilter persistedFilter = filters + @DeleteMapping("/Filters/{resourceId}") + @Transactional + public ResponseEntity delete(@PathVariable String metadataResolverId, + @PathVariable String resourceId) { + + MetadataResolver resolver = findResolverOrThrowHttp404(metadataResolverId); + boolean removed = resolver.getMetadataFilters().removeIf(f -> f.getResourceId().equals(resourceId)); + if(!removed) { + throw HTTP_404_CLIENT_ERROR_EXCEPTION.get(); + } + repository.save(resolver); + MetadataResolver persistedMr = repository.findByResourceId(metadataResolverId); + + //TODO: do we need to reload filters here?!? + //metadataResolverService.reloadFilters(persistedMr.getName()); + + return ResponseEntity.noContent().build(); + } + + private MetadataResolver findResolverOrThrowHttp404(String resolverResourceId) { + MetadataResolver resolver = repository.findByResourceId(resolverResourceId); + if(resolver == null) { + throw HTTP_404_CLIENT_ERROR_EXCEPTION.get(); + } + return resolver; + } + + private MetadataFilter findFilterOrThrowHttp404(MetadataResolver resolver, String filterResourceId) { + return resolver.getMetadataFilters().stream() .filter(f -> f.getResourceId().equals(filterResourceId)) - .collect(Collectors.toList()).get(0); + .findFirst() + .orElseThrow(HTTP_404_CLIENT_ERROR_EXCEPTION); + } + private MetadataFilter convertIntoTransientRepresentationIfNecessary(MetadataResolver resolver, final String filterResourceId) { + MetadataFilter persistedFilter = findFilterOrThrowHttp404(resolver, filterResourceId); //convert before saving into database if(persistedFilter instanceof EntityAttributesFilter) { EntityAttributesFilter.class.cast(persistedFilter).intoTransientRepresentation(); From 447751f425c4a81b24ee69c2307df46c7b72a098 Mon Sep 17 00:00:00 2001 From: Dmitriy Kopylenko Date: Wed, 15 Aug 2018 16:00:50 -0400 Subject: [PATCH 2/3] Add test for delete filter --- .../controller/MetadataFiltersController.java | 42 ++++++++++--------- ...taFiltersControllerIntegrationTests.groovy | 23 ++++++++++ 2 files changed, 45 insertions(+), 20 deletions(-) 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 64e7a0fe6..605f948a9 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 @@ -15,6 +15,7 @@ import org.springframework.http.ResponseEntity; import org.springframework.transaction.annotation.Transactional; import org.springframework.web.bind.annotation.DeleteMapping; +import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.PostMapping; @@ -47,33 +48,31 @@ public class MetadataFiltersController { private static final Supplier HTTP_404_CLIENT_ERROR_EXCEPTION = () -> new HttpClientErrorException(NOT_FOUND); + @ExceptionHandler + public ResponseEntity notFoundHandler(HttpClientErrorException ex) { + if(ex.getStatusCode() == NOT_FOUND) { + return ResponseEntity.notFound().build(); + } + throw ex; + } + @GetMapping("/Filters") @Transactional(readOnly = true) public ResponseEntity getAll(@PathVariable String metadataResolverId) { - MetadataResolver resolver = repository.findByResourceId(metadataResolverId); - if(resolver == null) { - return ResponseEntity.notFound().build(); - } + MetadataResolver resolver = findResolverOrThrowHttp404(metadataResolverId); return ResponseEntity.ok(resolver.getMetadataFilters()); } @GetMapping("/Filters/{resourceId}") + @Transactional(readOnly = true) public ResponseEntity getOne(@PathVariable String metadataResolverId, @PathVariable String resourceId) { - MetadataResolver resolver = repository.findByResourceId(metadataResolverId); - if(resolver == null) { - return ResponseEntity.notFound().build(); - } - return ResponseEntity.ok(resolver.getMetadataFilters().stream() - .filter(f -> f.getResourceId().equals(resourceId)) - .collect(Collectors.toList()).get(0)); + MetadataResolver resolver = findResolverOrThrowHttp404(metadataResolverId); + return ResponseEntity.ok(findFilterOrThrowHttp404(resolver, resourceId)); } @PostMapping("/Filters") public ResponseEntity create(@PathVariable String metadataResolverId, @RequestBody MetadataFilter createdFilter) { - MetadataResolver metadataResolver = repository.findByResourceId(metadataResolverId); - if(metadataResolver == null) { - return ResponseEntity.notFound().build(); - } + MetadataResolver metadataResolver = findResolverOrThrowHttp404(metadataResolverId); metadataResolver.getMetadataFilters().add(createdFilter); MetadataResolver persistedMr = repository.save(metadataResolver); @@ -85,7 +84,6 @@ public ResponseEntity create(@PathVariable String metadataResolverId, @Reques return ResponseEntity .created(getResourceUriFor(persistedMr, createdFilter.getResourceId())) .body(persistedFilter); - } @PutMapping("/Filters/{resourceId}") @@ -97,10 +95,7 @@ public ResponseEntity update(@PathVariable String metadataResolverId, return ResponseEntity.notFound().build(); } - MetadataResolver metadataResolver = repository.findByResourceId(metadataResolverId); - if(metadataResolver == null) { - return ResponseEntity.notFound().build(); - } + MetadataResolver metadataResolver = findResolverOrThrowHttp404(metadataResolverId); // check to make sure that the relationship exists if (!metadataResolver.getMetadataFilters().contains(filterTobeUpdated)) { @@ -155,6 +150,13 @@ 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 newlyPersistedFilter(Stream filters, final String filterResourceId) { MetadataFilter persistedFilter = filters .filter(f -> f.getResourceId().equals(filterResourceId)) 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 557fdf56e..e65a7e963 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 @@ -103,6 +103,29 @@ class MetadataFiltersControllerIntegrationTests extends Specification { updatedResultFromPUT.statusCode.value() == 200 } + def "DELETE Filter"() { + given: 'MetadataResolver with attached filter is available in data store' + def resolver = generator.buildRandomMetadataResolverOfType('FileBacked') + resolver.metadataFilters << generator.entityAttributesFilter() + def filterResourceId = resolver.metadataFilters[0].resourceId + def resolverResourceId = resolver.resourceId + metadataResolverRepository.save(resolver) + + + when: 'GET request is made with resource Id matching the existing filter' + def result = this.restTemplate.getForEntity("$BASE_URI/$resolverResourceId/Filters/$filterResourceId", String) + + then: + result.statusCode.value() == 200 + + and: 'DELETE call is made and then GET call is made for the just deleted resource' + restTemplate.delete("$BASE_URI/$resolverResourceId/Filters/$filterResourceId") + def GETResultAfterDelete = this.restTemplate.getForEntity("$BASE_URI/$resolverResourceId/Filters/$filterResourceId", String) + + then: 'The deleted resource is gone' + GETResultAfterDelete.statusCode.value() == 404 + } + private HttpEntity createRequestHttpEntityFor(Closure jsonBodySupplier) { new HttpEntity(jsonBodySupplier(), ['Content-Type': 'application/json'] as HttpHeaders) } From 8c4945d377b47d2d620bef81a397b71fd9d71fe9 Mon Sep 17 00:00:00 2001 From: Dmitriy Kopylenko Date: Thu, 16 Aug 2018 15:19:38 -0400 Subject: [PATCH 3/3] Add TODO note about manipulating filters directly via FilterRepository --- .../admin/ui/controller/MetadataFiltersController.java | 1 + 1 file changed, 1 insertion(+) 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 605f948a9..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 @@ -130,6 +130,7 @@ public ResponseEntity delete(@PathVariable String metadataResolverId, @PathVariable String resourceId) { MetadataResolver resolver = findResolverOrThrowHttp404(metadataResolverId); + //TODO: consider implementing delete of filter directly from RDBMS via FilterRepository boolean removed = resolver.getMetadataFilters().removeIf(f -> f.getResourceId().equals(resourceId)); if(!removed) { throw HTTP_404_CLIENT_ERROR_EXCEPTION.get();