From c6723e483b7d8166f19349273a218945503c0057 Mon Sep 17 00:00:00 2001 From: Dmitriy Kopylenko Date: Wed, 13 Jun 2018 12:58:34 -0400 Subject: [PATCH] SHIBUI-522: filter controller refactoring WIP --- .../admin/ui/ShibbolethUiApplication.java | 20 ++++++++++ .../controller/MetadataFiltersController.java | 39 ++++++++++++------- .../filters/EntityAttributesFilter.java | 13 ++++++- .../MetadataResolverRepository.java | 3 ++ .../MetadataFiltersControllerTests.groovy | 15 +++---- .../admin/ui/util/TestObjectGenerator.groovy | 2 +- 6 files changed, 67 insertions(+), 25 deletions(-) diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/ShibbolethUiApplication.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/ShibbolethUiApplication.java index bffe130ad..1fc74cf9d 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/ShibbolethUiApplication.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/ShibbolethUiApplication.java @@ -1,12 +1,18 @@ package edu.internet2.tier.shibboleth.admin.ui; +import edu.internet2.tier.shibboleth.admin.ui.repository.MetadataResolverRepository; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.SpringApplication; import org.springframework.boot.autoconfigure.SpringBootApplication; import org.springframework.boot.autoconfigure.domain.EntityScan; import org.springframework.boot.builder.SpringApplicationBuilder; +import org.springframework.boot.context.event.ApplicationStartedEvent; import org.springframework.boot.web.servlet.support.SpringBootServletInitializer; +import org.springframework.context.annotation.Profile; +import org.springframework.context.event.EventListener; import org.springframework.data.jpa.repository.config.EnableJpaAuditing; import org.springframework.scheduling.annotation.EnableScheduling; +import org.springframework.stereotype.Component; @SpringBootApplication @EntityScan(basePackages = "edu.internet2.tier.shibboleth.admin.ui.domain") @@ -23,4 +29,18 @@ public static void main(String... args) { SpringApplication.run(ShibbolethUiApplication.class, args); } + @Component + @Profile("dev") + public static class MetadataResolversResourceIdEmitter { + + @Autowired + MetadataResolverRepository metadataResolverRepository; + + @EventListener + void showMetadataResolversResourceIds(ApplicationStartedEvent e) { + metadataResolverRepository.findAll() + .forEach(it -> System.out.println(String.format("MetadataResolver [%s: %s]", it.getName(), it.getResourceId()))); + } + } + } 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 7e1684fab..d11598a8d 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 @@ -37,32 +37,36 @@ public class MetadataFiltersController { @Autowired private MetadataResolverRepository repository; - @Autowired - private FilterService filterService; - @Autowired private MetadataResolverService metadataResolverService; @GetMapping("/Filters") @Transactional(readOnly = true) - public Iterable getAll(@PathVariable String metadataResolverId) { - // TODO: implement lookup based on metadataResolverId once we have more than one - return repository.findAll().iterator().next().getMetadataFilters(); + public ResponseEntity getAll(@PathVariable String metadataResolverId) { + MetadataResolver resolver = repository.findByResourceId(metadataResolverId); + if(resolver == null) { + return ResponseEntity.notFound().build(); + } + return ResponseEntity.ok(resolver.getMetadataFilters()); } @GetMapping("/Filters/{resourceId}") public ResponseEntity getOne(@PathVariable String metadataResolverId, @PathVariable String resourceId) { - // TODO: implement lookup based on metadataResolverId once we have more than one - // TODO: should we check that we found exactly one filter (as in the update method below)? If not, error? - return ResponseEntity.ok(repository.findAll().iterator().next().getMetadataFilters().stream() + 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)); } @PostMapping("/Filters") public ResponseEntity create(@PathVariable String metadataResolverId, @RequestBody MetadataFilter createdFilter) { - //TODO: replace with get by metadataResolverId once we have more than one - MetadataResolver metadataResolver = repository.findAll().iterator().next(); + MetadataResolver metadataResolver = repository.findByResourceId(metadataResolverId); + if(metadataResolver == null) { + return ResponseEntity.notFound().build(); + } metadataResolver.getMetadataFilters().add(createdFilter); //convert before saving into database @@ -88,8 +92,14 @@ public ResponseEntity update(@PathVariable String metadataResolverId, @PathVariable String resourceId, @RequestBody MetadataFilter updatedFilter) { - //TODO: replace with get by metadataResolverId once we have more than one - MetadataResolver metadataResolver = repository.findAll().iterator().next(); + MetadataResolver metadataResolver = repository.findByResourceId(metadataResolverId); + if(metadataResolver == null) { + return ResponseEntity.notFound().build(); + } + + if (!resourceId.equals(updatedFilter.getResourceId())) { + return new ResponseEntity(HttpStatus.CONFLICT); + } List filters = metadataResolver.getMetadataFilters().stream() @@ -121,9 +131,12 @@ public ResponseEntity update(@PathVariable String metadataResolverId, metadataResolverService.reloadFilters(persistedMr.getName()); + int version = updatedFilter.getVersion(); MetadataFilter persistedFilter = convertIntoTransientRepresentationIfNecessary(persistedMr.getMetadataFilters().stream(), updatedFilter.getResourceId()); + int persitedVersion = persistedFilter.getVersion(); + return ResponseEntity.ok().body(persistedFilter); } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/EntityAttributesFilter.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/EntityAttributesFilter.java index 4d2e6ab84..53cd07956 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/EntityAttributesFilter.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/EntityAttributesFilter.java @@ -18,6 +18,8 @@ import javax.persistence.PreUpdate; import javax.persistence.Transient; import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; import java.util.List; import static edu.internet2.tier.shibboleth.admin.util.ModelRepresentationConversions.getAttributeListFromAttributeReleaseList; @@ -60,11 +62,18 @@ public void intoTransientRepresentation() { @PrePersist @PreUpdate public void fromTransientRepresentation() { - this.attributes.clear(); List attributeList = new ArrayList<>(); attributeList.addAll(getAttributeListFromAttributeReleaseList(this.attributeRelease)); attributeList.addAll(getAttributeListFromRelyingPartyOverridesRepresentation(this.relyingPartyOverrides)); - this.attributes.addAll((List)(List)attributeList); + if(!attributeList.isEmpty()) { + //attributeList.sort(Comparator.comparing(org.opensaml.saml.saml2.core.Attribute::getName)); + this.attributes = (List) (List) attributeList; + } + + /*if(!attributeList.isEmpty()) { + this.attributes.clear(); + this.attributes.addAll((List)(List)attributeList); + }*/ } } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/repository/MetadataResolverRepository.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/repository/MetadataResolverRepository.java index fc415aef1..07835ea9a 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/repository/MetadataResolverRepository.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/repository/MetadataResolverRepository.java @@ -7,5 +7,8 @@ * Repository to manage {@link MetadataResolver} instances. */ public interface MetadataResolverRepository extends CrudRepository { + MetadataResolver findByName(String name); + + MetadataResolver findByResourceId(String resourceId); } 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 3c0a0ab8d..8981c146b 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 @@ -66,7 +66,6 @@ class MetadataFiltersControllerTests extends Specification { controller = new MetadataFiltersController ( repository: metadataResolverRepository, - filterService: filterService, metadataResolverService: new MetadataResolverService() { @Override void reloadFilters(String metadataResolverName) { @@ -88,8 +87,7 @@ class MetadataFiltersControllerTests extends Specification { def metadataResolver = new MetadataResolver() List expectedContent = testObjectGenerator.buildAllTypesOfFilterList() metadataResolver.setMetadataFilters(expectedContent) - List metadataResolverList = [metadataResolver] - 1 * metadataResolverRepository.findAll() >> metadataResolverList + 1 * metadataResolverRepository.findByResourceId(_) >> metadataResolver def expectedHttpResponseStatus = status().isOk() def expectedResponseContentType = APPLICATION_JSON_UTF8 @@ -109,7 +107,7 @@ class MetadataFiltersControllerTests extends Specification { def metadataResolver = new MetadataResolver() def expectedFilter = testObjectGenerator.entityAttributesFilter() metadataResolver.metadataFilters = [expectedFilter] - 1 * metadataResolverRepository.findAll() >> [metadataResolver] + 1 * metadataResolverRepository.findByResourceId(_) >> metadataResolver def expectedResourceId = expectedFilter.resourceId def expectedHttpResponseStatus = status().isOk() @@ -127,8 +125,6 @@ class MetadataFiltersControllerTests extends Specification { def "FilterController.create creates the desired filter"() { given: - controller.filterService = mockFilterService // so we can control ids - def randomFilter = testObjectGenerator.entityAttributesFilter() def metadataResolver = new MetadataResolver() metadataResolver.setResourceId(randomGenerator.randomId()) @@ -138,7 +134,7 @@ class MetadataFiltersControllerTests extends Specification { metadataResolverWithFilter.metadataFilters = metadataResolver.metadataFilters.collect() metadataResolverWithFilter.getMetadataFilters().add(randomFilter) - 1 * metadataResolverRepository.findAll() >> [metadataResolver] + 1 * metadataResolverRepository.findByResourceId(_) >> metadataResolver 1 * metadataResolverRepository.save(_) >> metadataResolverWithFilter def expectedMetadataResolverUUID = metadataResolver.getResourceId() @@ -180,7 +176,7 @@ class MetadataFiltersControllerTests extends Specification { updatedMetadataResolver.setMetadataFilters(originalMetadataResolver.getMetadataFilters().collect()) updatedMetadataResolver.getMetadataFilters().add(updatedFilter) - 1 * metadataResolverRepository.findAll() >> [originalMetadataResolver] + 1 * metadataResolverRepository.findByResourceId(_) >> originalMetadataResolver 1 * metadataResolverRepository.save(_) >> updatedMetadataResolver def filterUUID = updatedFilter.getResourceId() @@ -193,6 +189,7 @@ class MetadataFiltersControllerTests extends Specification { then: def expectedJson = new JsonSlurper().parseText(postedJsonBody) + def hashcode = updatedFilter.hashCode() expectedJson << [version: updatedFilter.hashCode()] result.andExpect(status().isOk()) .andExpect(content().json(JsonOutput.toJson(expectedJson), true)) @@ -210,7 +207,7 @@ class MetadataFiltersControllerTests extends Specification { originalMetadataResolver.setMetadataFilters(testObjectGenerator.buildAllTypesOfFilterList()) originalMetadataResolver.getMetadataFilters().add(randomFilter) - 1 * metadataResolverRepository.findAll() >> [originalMetadataResolver] + 1 * metadataResolverRepository.findByResourceId(_) >> originalMetadataResolver def filterUUID = randomFilter.getResourceId() diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/util/TestObjectGenerator.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/util/TestObjectGenerator.groovy index 4027d8739..1b020c499 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/util/TestObjectGenerator.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/util/TestObjectGenerator.groovy @@ -92,7 +92,7 @@ class TestObjectGenerator { it.name = entityAttributesFilter.name it.resourceId = entityAttributesFilter.resourceId it.setEntityAttributesFilterTarget(entityAttributesFilter.entityAttributesFilterTarget) - it.setAttributes(new ArrayList(entityAttributesFilter.attributes)) + it.setAttributes(entityAttributesFilter.attributes) it.intoTransientRepresentation() it }