Merge branch 'master' into NOJIRA-lucene-search-fix

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

@@ -33,11 +33,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
@@ -77,7 +78,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")
@@ -169,12 +170,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());
@@ -190,17 +199,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

@@ -119,6 +119,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()

ui/src/app/metadata/domain/action/entity.action.ts

@@ -6,7 +6,10 @@ export const PREVIEW_ENTITY = '[Domain] Preview Entity';
 export class PreviewEntity implements Action {
     readonly type = PREVIEW_ENTITY;
 
-    constructor(public payload: MetadataEntity) { }
+    constructor(public payload: {
+        id: string,
+        entity: MetadataEntity
+    }) { }
 }
 
 export type Actions =

ui/src/app/metadata/domain/effect/entity.effect.spec.ts

@@ -48,7 +48,7 @@ describe('Entity Effects', () => {
         it('should open a modal window for a filter', fakeAsync(() => {
             spyOn(modal, 'open').and.returnValue({componentInstance: <any>{}});
             spyOn(idService, 'preview').and.returnValue(of('<foo></foo>'));
-            effects.openModal(new EntityAttributesFilterEntity());
+            effects.openModal({ id: 'foo', entity: new EntityAttributesFilterEntity()});
             expect(idService.preview).toHaveBeenCalled();
             tick(10);
             expect(modal.open).toHaveBeenCalled();
@@ -57,7 +57,7 @@ describe('Entity Effects', () => {
         it('should open a modal window for a provider', fakeAsync(() => {
             spyOn(modal, 'open').and.returnValue({ componentInstance: <any>{} });
             spyOn(providerService, 'preview').and.returnValue(of('<foo></foo>'));
-            effects.openModal(new FileBackedHttpMetadataResolver());
+            effects.openModal({id: 'foo', entity: new FileBackedHttpMetadataResolver()});
             expect(providerService.preview).toHaveBeenCalled();
             tick(10);
             expect(modal.open).toHaveBeenCalled();

ui/src/app/metadata/domain/effect/entity.effect.ts

@@ -20,7 +20,7 @@ export class EntityEffects {
     previewEntityXml$ = this.actions$.pipe(
         ofType<entityActions.PreviewEntity>(entityActions.PREVIEW_ENTITY),
         map(action => action.payload),
-        tap(entity => this.openModal(entity))
+        tap(prev => this.openModal(prev))
     );
 
     constructor(
@@ -30,9 +30,10 @@ export class EntityEffects {
         private entityService: EntityIdService
     ) { }
 
-    openModal(entity: MetadataEntity): void {
-        let request: Observable<string> = entity.kind === MetadataTypes.FILTER ?
-            this.entityService.preview(entity.getId()) : this.providerService.preview(entity.getId());
+    openModal(prev: { id: string, entity: MetadataEntity }): void {
+        let { id, entity } = prev,
+            request: Observable<string> = entity.kind === MetadataTypes.FILTER ?
+            this.entityService.preview(id) : this.providerService.preview(id);
         request.subscribe(xml => {
             let modal = this.modalService.open(PreviewDialogComponent, {
                 size: 'lg',

ui/src/app/metadata/filter/container/edit-filter.component.html

@@ -29,6 +29,7 @@
                 [schema]="schema$ | async"
                 [model]="model$ | async"
                 [validators]="definition.getValidators()"
+                [actions]="actions"
                 (onChange)="valueChangeSubject.next($event)"
                 (onErrorChange)="statusChangeSubject.next($event)"></sf-form>
         </div>

ui/src/app/metadata/filter/container/edit-filter.component.ts

@@ -1,11 +1,11 @@
 import { Component } from '@angular/core';
 import { Store } from '@ngrx/store';
-import { Subject, Observable, of } from 'rxjs';
+import { Subject, Observable } from 'rxjs';
 
 import * as fromFilter from '../reducer';
-import { MetadataFilterTypes, EntityAttributesFilter } from '../model';
+import { MetadataFilterTypes } from '../model';
 import { FormDefinition } from '../../../wizard/model';
-import { MetadataFilter, MetadataEntity } from '../../domain/model';
+import { MetadataFilter } from '../../domain/model';
 import { SchemaService } from '../../../schema-form/service/schema.service';
 import { UpdateFilterRequest } from '../action/collection.action';
 import { CancelCreateFilter, UpdateFilterChanges } from '../action/filter.action';
@@ -32,6 +32,8 @@ export class EditFilterComponent {
     filter: MetadataFilter;
     isValid: boolean;
 
+    actions: any;
+
     constructor(
         private store: Store<fromFilter.State>,
         private schemaService: SchemaService
@@ -50,6 +52,12 @@ export class EditFilterComponent {
         this.store
             .select(fromFilter.getFilter)
             .subscribe(filter => this.filter = filter);
+
+        this.actions = {
+            preview: (property: any, parameters: any) => {
+                this.preview(parameters.id);
+            }
+        };
     }
 
     save(): void {
@@ -60,8 +68,11 @@ export class EditFilterComponent {
         this.store.dispatch(new CancelCreateFilter());
     }
 
-    preview(entity: MetadataFilter): void {
-        this.store.dispatch(new PreviewEntity(new EntityAttributesFilterEntity(entity)));
+    preview(id: string): void {
+        this.store.dispatch(new PreviewEntity({
+            id,
+            entity: new EntityAttributesFilterEntity(this.filter)
+        }));
     }
 }
 

ui/src/app/metadata/manager/container/dashboard-resolvers-list.component.ts

@@ -79,7 +79,7 @@ export class DashboardResolversListComponent implements OnInit {
     }
 
     openPreviewDialog(entity: MetadataEntity): void {
-        this.store.dispatch(new PreviewEntity(entity));
+        this.store.dispatch(new PreviewEntity({ id: entity.getId(), entity }));
     }
 
     deleteResolver(entity: MetadataResolver): void {

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

@@ -6,6 +6,7 @@ export enum ProviderCollectionActionTypes {
     UPDATE_PROVIDER_REQUEST = '[Metadata Provider] Update Request',
     UPDATE_PROVIDER_SUCCESS = '[Metadata Provider] Update Success',
     UPDATE_PROVIDER_FAIL = '[Metadata Provider] Update Fail',
+    UPDATE_PROVIDER_CONFLICT = '[Metadata Provider] Update Conflict',
 
     LOAD_PROVIDER_REQUEST = '[Metadata Provider Collection] Provider Load REQUEST',
     LOAD_PROVIDER_SUCCESS = '[Metadata Provider Collection] Provider Load SUCCESS',
@@ -91,6 +92,12 @@ export class UpdateProviderFail implements Action {
     constructor(public payload: MetadataProvider) { }
 }
 
+export class UpdateProviderConflict implements Action {
+    readonly type = ProviderCollectionActionTypes.UPDATE_PROVIDER_FAIL;
+
+    constructor(public payload: MetadataProvider) { }
+}
+
 export class AddProviderRequest implements Action {
     readonly type = ProviderCollectionActionTypes.ADD_PROVIDER_REQUEST;
 
@@ -195,6 +202,7 @@ export type ProviderCollectionActionsUnion =
     | UpdateProviderRequest
     | UpdateProviderSuccess
     | UpdateProviderFail
+    | UpdateProviderConflict
     | SetOrderProviderRequest
     | SetOrderProviderSuccess
     | SetOrderProviderFail

ui/src/app/metadata/provider/component/provider-wizard-summary.component.html

@@ -1,12 +1,12 @@
 <div class="row">
     <div class="col-xl-6 col-xs-12" *ngFor="let sections of columns">
-        <section class="px-3" *ngFor="let section of sections; let i = index;">
+        <section class="px-3" *ngFor="let section of sections; let i = index;" role="list">
             <button (click)="gotoPage(section.id)" class="tag tag-success tag-sm my-4 text-left" [attr.aria-label]="section.label" role="button">
                 <span class="index">{{ section.index }}</span>
                 <ng-container>{{ section.label }}</ng-container>
             </button>
             <ng-container *ngFor="let prop of section.properties">
-                <summary-property [property]="prop"></summary-property>
+                <summary-property [property]="prop" role="listitem"></summary-property>
             </ng-container>
         </section>
     </div>

ui/src/app/metadata/provider/component/summary-property.component.html

@@ -1,4 +1,4 @@
-<div role="listitem" class="mb-3" *ngIf="property">
+<div class="mb-3" *ngIf="property">
     <ng-container [ngSwitch]="property.type">
         <ng-container *ngSwitchDefault>
             <ng-container *ngIf="property.name">

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

@@ -6,6 +6,7 @@ import { NgbDropdownModule } from '@ng-bootstrap/ng-bootstrap';
 import { ProviderSelectComponent } from './provider-select.component';
 import * as fromRoot from '../reducer';
 import * as fromWizard from '../../../wizard/reducer';
+import { MetadataProvider } from '../../domain/model';
 
 @Component({
     template: `
@@ -53,4 +54,15 @@ describe('Provider Select Component', () => {
     it('should instantiate the component', async(() => {
         expect(app).toBeTruthy();
     }));
+
+    describe('setDefinition method', () => {
+        it('should not dispatch an action if no provider is defined', () => {
+            app.setDefinition(null);
+            expect(store.dispatch).not.toHaveBeenCalled();
+        });
+        it('should dispatch an action if a provider is defined', () => {
+            app.setDefinition({} as MetadataProvider);
+            expect(store.dispatch).toHaveBeenCalled();
+        });
+    });
 });

ui/src/app/metadata/provider/container/provider-select.component.ts

@@ -33,14 +33,15 @@ export class ProviderSelectComponent implements OnDestroy {
 
         this.provider$ = this.store.select(fromProviders.getSelectedProvider).pipe(skipWhile(p => !p));
 
-        this.provider$
-            .subscribe(provider => {
-                if (provider) {
-                    this.store.dispatch(new SetDefinition({
-                        ...MetadataProviderEditorTypes.find(def => def.type === provider['@type'])
-                    }));
-                }
-            });
+        this.provider$.subscribe(provider => this.setDefinition(provider));
+    }
+
+    setDefinition(provider: MetadataProvider): void {
+        if (provider) {
+            this.store.dispatch(new SetDefinition({
+                ...MetadataProviderEditorTypes.find(def => def.type === provider['@type'])
+            }));
+        }
     }
 
     ngOnDestroy() {

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

@@ -19,6 +19,7 @@ import {
     UpdateProviderRequest,
     UpdateProviderSuccess,
     UpdateProviderFail,
+    UpdateProviderConflict,
     GetOrderProviderRequest,
     GetOrderProviderSuccess,
     GetOrderProviderFail,
@@ -44,7 +45,7 @@ export class CollectionEffects {
 
     @Effect()
     openContention$ = this.actions$.pipe(
-        ofType<UpdateProviderFail>(ProviderCollectionActionTypes.UPDATE_PROVIDER_FAIL),
+        ofType<UpdateProviderConflict>(ProviderCollectionActionTypes.UPDATE_PROVIDER_CONFLICT),
         map(action => action.payload),
         withLatestFrom(this.store.select(fromProvider.getSelectedProvider)),
         switchMap(([changes, current]) =>
@@ -118,7 +119,7 @@ export class CollectionEffects {
                 .update(provider)
                 .pipe(
                     map(p => new UpdateProviderSuccess({id: p.id, changes: p})),
-                    catchError((e) => of(new UpdateProviderFail(provider)))
+                    catchError((e) => e.status === 409 ? of(new UpdateProviderConflict(provider)) : of(new UpdateProviderFail(provider)))
                 )
         )
     );

ui/src/app/schema-form/registry.ts

@@ -16,6 +16,7 @@ import { CustomArrayComponent } from './widget/array/array.component';
 import { CustomIntegerComponent } from './widget/number/number.component';
 import { FilterTargetComponent } from './widget/filter-target/filter-target.component';
 import { ChecklistComponent } from './widget/check/checklist.component';
+import { IconButtonComponent } from './widget/button/icon-button.component';
 
 
 export class CustomWidgetRegistry extends WidgetRegistry {
@@ -55,6 +56,8 @@ export class CustomWidgetRegistry extends WidgetRegistry {
 
         this.register('filter-target', FilterTargetComponent);
 
+        this.register('icon-button', IconButtonComponent);
+
         /* NGX-Form */
         this.register('range', RangeWidget);