Merged in SHIBUI-782 (pull request #173)

SHIBUI-782: fix filters delete bug

Approved-by: Shibui Jenkins <shibui.jenkins@gmail.com>

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<MetadataFilter> 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<MetadataFilter> filters, final String filterResourceId) {
         MetadataFilter persistedFilter = filters
                 .filter(f -> f.getResourceId().equals(filterResourceId))
-                .collect(Collectors.toList()).get(0);
+                .collect(toList()).get(0);
 
         return persistedFilter;
     }

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<String> createRequestHttpEntityFor(Closure jsonBodySupplier) {
         new HttpEntity<String>(jsonBodySupplier(), ['Content-Type': 'application/json'] as HttpHeaders)
     }

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()