Merge branch 'master' into SHIBUI-660

backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersController.java

@@ -14,19 +14,25 @@
 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;
 import org.springframework.web.bind.annotation.PutMapping;
 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.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}")
 public class MetadataFiltersController {
@@ -40,33 +46,33 @@ public class MetadataFiltersController {
     @Autowired
     private FilterRepository filterRepository;
 
+    private static final Supplier<HttpClientErrorException> 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);
 
@@ -78,7 +84,6 @@ public ResponseEntity<?> create(@PathVariable String metadataResolverId, @Reques
         return ResponseEntity
                 .created(getResourceUriFor(persistedMr, createdFilter.getResourceId()))
                 .body(persistedFilter);
-
     }
 
     @PutMapping("/Filters/{resourceId}")
@@ -90,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)) {
@@ -122,6 +124,40 @@ public ResponseEntity<?> update(@PathVariable String metadataResolverId,
         return ResponseEntity.ok().body(persistedFilter);
     }
 
+    @DeleteMapping("/Filters/{resourceId}")
+    @Transactional
+    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();
+        }
+        repository.save(resolver);
+
+        //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))
+                .findFirst()
+                .orElseThrow(HTTP_404_CLIENT_ERROR_EXCEPTION);
+    }
+
     private MetadataFilter newlyPersistedFilter(Stream<MetadataFilter> filters, final String filterResourceId) {
         MetadataFilter persistedFilter = filters
                 .filter(f -> f.getResourceId().equals(filterResourceId))
@@ -180,4 +216,4 @@ private static URI getResourceUriFor(MetadataResolver mr, String filterResourceI
                 .build()
                 .toUri();
     }
-}
+}

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

ui/src/app/metadata/domain/service/filter.service.ts

@@ -18,7 +18,6 @@ export class MetadataFilterService {
     }
 
     find(providerId: string, filterId: string): Observable<MetadataFilter> {
-        // console.log(id);
         return this.http.get<MetadataFilter>(`${this.base}${this.endpoint}/${providerId}/Filters/${ filterId }`);
     }
 
@@ -27,7 +26,10 @@ export class MetadataFilterService {
     }
 
     save(providerId: string, filter: MetadataFilter): Observable<MetadataFilter> {
-        console.log(providerId, filter);
         return this.http.post<MetadataFilter>(`${this.base}${this.endpoint}/${providerId}/Filters`, filter);
     }
+
+    remove(providerId: string, filterId: string): Observable<string> {
+        return this.http.delete<string>(`${this.base}${this.endpoint}/${providerId}/Filters/${ filterId }`);
+    }
 }

ui/src/app/metadata/filter/action/collection.action.ts

@@ -99,19 +99,19 @@ export class AddFilterFail implements Action {
 export class RemoveFilterRequest implements Action {
     readonly type = FilterCollectionActionTypes.REMOVE_FILTER_REQUEST;
 
-    constructor(public payload: MetadataFilter) { }
+    constructor(public payload: string) { }
 }
 
 export class RemoveFilterSuccess implements Action {
     readonly type = FilterCollectionActionTypes.REMOVE_FILTER_SUCCESS;
 
-    constructor(public payload: MetadataFilter) { }
+    constructor(public payload: string) { }
 }
 
 export class RemoveFilterFail implements Action {
     readonly type = FilterCollectionActionTypes.REMOVE_FILTER_FAIL;
 
-    constructor(public payload: MetadataFilter) { }
+    constructor(public error: Error) { }
 }
 
 export type FilterCollectionActionsUnion =

ui/src/app/metadata/filter/effect/collection.effect.ts

@@ -14,6 +14,7 @@ import { MetadataFilter } from '../../domain/model';
 import { removeNulls } from '../../../shared/util';
 import { EntityAttributesFilterEntity } from '../../domain/entity/filter/entity-attributes-filter';
 import { MetadataFilterService } from '../../domain/service/filter.service';
+import { SelectProviderRequest } from '../../provider/action/collection.action';
 
 /* istanbul ignore next */
 @Injectable()
@@ -96,6 +97,14 @@ export class FilterCollectionEffects {
                 );
         })
     );
+    @Effect()
+    updateFilterSuccessReloadProvider$ = this.actions$.pipe(
+        ofType<actions.UpdateFilterSuccess>(FilterCollectionActionTypes.UPDATE_FILTER_SUCCESS),
+        map(action => action.payload),
+        withLatestFrom(this.store.select(fromProvider.getSelectedProviderId).pipe(skipWhile(id => !id))),
+        map(([filter, providerId]) => new SelectProviderRequest(providerId))
+    );
+
     @Effect({ dispatch: false })
     updateFilterSuccessRedirect$ = this.actions$.pipe(
         ofType<actions.UpdateFilterSuccess>(FilterCollectionActionTypes.UPDATE_FILTER_SUCCESS),
@@ -104,6 +113,27 @@ export class FilterCollectionEffects {
         tap(([filter, provider]) => this.router.navigate(['/', 'metadata', 'provider', provider, 'filters']))
     );
 
+    @Effect()
+    removeFilterRequest$ = this.actions$.pipe(
+        ofType<actions.RemoveFilterRequest>(FilterCollectionActionTypes.REMOVE_FILTER_REQUEST),
+        map(action => action.payload),
+        withLatestFrom(this.store.select(fromProvider.getSelectedProviderId).pipe(skipWhile(id => !id))),
+        switchMap(([filterId, providerId]) =>
+            this.filterService.remove(providerId, filterId).pipe(
+                map(removed => new actions.RemoveFilterSuccess(removed)),
+                catchError(err => of(new actions.RemoveFilterFail(err)))
+            )
+        )
+    );
+
+    @Effect()
+    removeFilterSuccess$ = this.actions$.pipe(
+        ofType<actions.RemoveFilterSuccess>(FilterCollectionActionTypes.REMOVE_FILTER_SUCCESS),
+        map(action => action.payload),
+        withLatestFrom(this.store.select(fromProvider.getSelectedProviderId).pipe(skipWhile(id => !id))),
+        map(([filter, providerId]) => new actions.LoadFilterRequest(providerId))
+    );
+
     constructor(
         private actions$: Actions,
         private router: Router,

ui/src/app/metadata/filter/reducer/collection.reducer.spec.ts

@@ -10,7 +10,8 @@ import {
     UpdateFilterRequest,
     AddFilterSuccess,
     AddFilterFail,
-    UpdateFilterFail
+    UpdateFilterFail,
+    RemoveFilterFail
 } from '../action/collection.action';
 import { EntityAttributesFilterEntity } from '../../domain/entity/filter/entity-attributes-filter';
 
@@ -93,6 +94,13 @@ describe('Filter Reducer', () => {
         });
     });
 
+    describe(`${FilterCollectionActionTypes.REMOVE_FILTER_FAIL}`, () => {
+        it('should set saving to false', () => {
+            const action = new RemoveFilterFail(new Error('foo'));
+            expect(reducer(snapshot, action).saving).toBe(false);
+        });
+    });
+
     describe(`${FilterCollectionActionTypes.UPDATE_FILTER_SUCCESS}`, () => {
         it('should update the filter in the collection', () => {
             spyOn(fromFilter.adapter, 'updateOne').and.callThrough();

ui/src/app/metadata/filter/reducer/collection.reducer.ts

@@ -58,13 +58,28 @@ export function reducer(state = initialState, action: FilterCollectionActionsUni
 
         case FilterCollectionActionTypes.ADD_FILTER_SUCCESS:
         case FilterCollectionActionTypes.ADD_FILTER_FAIL:
+        case FilterCollectionActionTypes.REMOVE_FILTER_FAIL:
         case FilterCollectionActionTypes.UPDATE_FILTER_FAIL: {
             return {
                 ...state,
                 saving: false
             };
         }
 
+        case FilterCollectionActionTypes.REMOVE_FILTER_SUCCESS: {
+            return adapter.removeOne(action.payload, {
+                ...state,
+                saving: false
+            });
+        }
+
+        case FilterCollectionActionTypes.REMOVE_FILTER_REQUEST: {
+            return {
+                ...state,
+                saving: true
+            };
+        }
+
         case FilterCollectionActionTypes.SELECT_FILTER_REQUEST: {
             return {
                 ...state,

ui/src/app/metadata/filter/reducer/index.ts

@@ -52,7 +52,7 @@ export const getAllFilters = createSelector(getCollectionState, fromCollection.s
 export const getCollectionSaving = createSelector(getCollectionState, fromCollection.getIsSaving);
 
 export const notAddtlFilters = ['RequiredValidUntil', 'SignatureValidation', 'EntityRoleWhiteList'];
-export const filterTypeFn = filters => filters.filter(f => notAddtlFilters.indexOf(f['@type']) === -1);
+export const filterTypeFn = filters => [...filters.filter(f => notAddtlFilters.indexOf(f['@type']) === -1)];
 
 export const getAdditionalFilters = createSelector(getAllFilters, filterTypeFn);
 

ui/src/app/metadata/provider/container/provider-filter-list.component.html

@@ -40,7 +40,7 @@
                             <th>Filter Type</th>
                             <th>Enabled?</th>
                             <th>Edit</th>
-                            <!--<th>Delete</th>-->
+                            <th>Delete</th>
                         </tr>
                     </thead>
                     <tbody>
@@ -79,14 +79,12 @@
                                     <span class="sr-only">Edit</span>
                                 </a>
                             </td>
-                            <!--
                             <td class="td-sm">
-                                <button class="btn btn-link">
+                                <button class="btn btn-link" (click)="remove(filter.resourceId)">
                                     <i class="fa fa-trash fa-lg sr-hidden text-danger"></i>
                                     <span class="sr-only">Delete</span>
                                 </button>
                             </td>
-                            -->
                         </tr>
                     </tbody>
                 </table>

ui/src/app/metadata/provider/container/provider-filter-list.component.spec.ts

@@ -59,4 +59,11 @@ describe('Provider Filter List Component', () => {
     it('should instantiate the component', async(() => {
         expect(app).toBeTruthy();
     }));
+
+    describe('remove method', () => {
+        it('should dispatch an action to the store', () => {
+            app.remove('foo');
+            expect(store.dispatch).toHaveBeenCalled();
+        });
+    });
 });

ui/src/app/metadata/provider/container/provider-filter-list.component.ts

@@ -7,7 +7,7 @@ import * as fromFilter from '../../filter/reducer';
 import { MetadataFilter, MetadataProvider } from '../../domain/model';
 import { NAV_FORMATS } from '../component/provider-editor-nav.component';
 import { SetIndex } from '../../../wizard/action/wizard.action';
-import { UpdateFilterRequest, LoadFilterRequest } from '../../filter/action/collection.action';
+import { UpdateFilterRequest, LoadFilterRequest, RemoveFilterRequest } from '../../filter/action/collection.action';
 
 @Component({
     selector: 'provider-filter-list',
@@ -30,7 +30,9 @@ export class ProviderFilterListComponent implements OnDestroy {
         this.filters$ = this.store.select(fromFilter.getAdditionalFilters);
         this.provider$ = this.store.select(fromProvider.getSelectedProvider).pipe(skipWhile(p => !p));
         this.provider$
-            .pipe(takeUntil(this.ngUnsubscribe))
+            .pipe(
+                takeUntil(this.ngUnsubscribe)
+            )
             .subscribe(p => {
                 this.store.dispatch(new LoadFilterRequest(p.resourceId));
             });
@@ -44,6 +46,10 @@ export class ProviderFilterListComponent implements OnDestroy {
         this.store.dispatch(new UpdateFilterRequest({ ...filter, filterEnabled: !filter.filterEnabled }));
     }
 
+    remove(id: string): void {
+        this.store.dispatch(new RemoveFilterRequest(id));
+    }
+
     ngOnDestroy(): void {
         this.ngUnsubscribe.next();
         this.ngUnsubscribe.complete();