From 1a4064e9ed83f3eaa920c8da5534a7ed844c6c9e Mon Sep 17 00:00:00 2001 From: Dmitriy Kopylenko Date: Tue, 31 Jul 2018 16:46:51 -0400 Subject: [PATCH 1/7] SHIBUI-680 WIP --- .../ui/controller/MetadataResolversController.java | 13 +++++++------ .../ui/domain/filters/EntityAttributesFilter.java | 3 --- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java index 2b1ddc6b5..52f1a50ac 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java @@ -103,10 +103,11 @@ public ResponseEntity create(@RequestBody MetadataResolver newResolver) { } newResolver.convertFiltersFromTransientRepresentationIfNecessary(); - MetadataResolver persistedResolver = resolverRepository.save(newResolver); + resolverRepository.save(newResolver); + MetadataResolver persistedResolver = resolverRepository.findByResourceId(newResolver.getResourceId()); positionOrderContainerService.appendPositionOrderForNew(persistedResolver); - persistedResolver.updateVersion(); + persistedResolver.updateVersion(); persistedResolver.convertFiltersIntoTransientRepresentationIfNecessary(); return ResponseEntity.created(getResourceUriFor(persistedResolver)).body(persistedResolver); } @@ -131,12 +132,12 @@ public ResponseEntity update(@PathVariable String resourceId, @RequestBody Me updatedResolver.setAudId(existingResolver.getAudId()); - //If one needs to update filters, it should be dealt with via filters endpoints - updatedResolver.setMetadataFilters(existingResolver.getMetadataFilters()); + updatedResolver.convertFiltersFromTransientRepresentationIfNecessary(); + resolverRepository.save(updatedResolver); + MetadataResolver persistedResolver = resolverRepository.findByResourceId(updatedResolver.getResourceId()); - MetadataResolver persistedResolver = resolverRepository.save(updatedResolver); persistedResolver.updateVersion(); - + persistedResolver.convertFiltersFromTransientRepresentationIfNecessary(); return ResponseEntity.ok(persistedResolver); } 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 0208f92c4..a62ed9a24 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 @@ -51,15 +51,12 @@ public EntityAttributesFilter() { @Transient private RelyingPartyOverridesRepresentation relyingPartyOverrides; - @PostLoad public void intoTransientRepresentation() { this.attributeRelease = getAttributeReleaseListFromAttributeList(this.attributes); this.relyingPartyOverrides = getRelyingPartyOverridesRepresentationFromAttributeList(attributes); updateVersion(); } - @PrePersist - @PreUpdate public void fromTransientRepresentation() { List attributeList = new ArrayList<>(); attributeList.addAll(getAttributeListFromAttributeReleaseList(this.attributeRelease)); From 741c86258bd494e1c77bab148a9758b1bbbc42e5 Mon Sep 17 00:00:00 2001 From: Dmitriy Kopylenko Date: Mon, 13 Aug 2018 12:00:05 -0400 Subject: [PATCH 2/7] Unit test to demo the incorrect version when updating resolver --- ...ResolversControllerIntegrationTests.groovy | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversControllerIntegrationTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversControllerIntegrationTests.groovy index 25071cb98..3ceb55417 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversControllerIntegrationTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversControllerIntegrationTests.groovy @@ -255,6 +255,33 @@ class MetadataResolversControllerIntegrationTests extends Specification { createdResolver.metadataFilters[0] instanceof EntityAttributesFilter } + def "PUT MetadataResolver with one EntityAttributesFilters attached and check version -> /api/MetadataResolvers"() { + given: 'MetadataResolver with attached entity attributes is available in data store' + def resolver = generator.buildRandomMetadataResolverOfType('FileBacked') + resolver.metadataFilters << generator.entityAttributesFilter() + def resolverResourceId = resolver.resourceId + metadataResolverRepository.save(resolver) + + when: 'GET request is made with resource Id matching the existing resolver' + def result = this.restTemplate.getForEntity("$BASE_URI/$resolverResourceId", String) + def existingMetadataResolverMap = new JsonSlurper().parseText(result.body) + def existingMetadataVersion = existingMetadataResolverMap.version + + and: 'PUT call is made with' + existingMetadataResolverMap.name = 'Updated' + def updatedResultFromPUT = this.restTemplate.exchange( + "$BASE_URI/${existingMetadataResolverMap.resourceId}", + PUT, + createRequestHttpEntityFor { JsonOutput.toJson(existingMetadataResolverMap) }, + String) + def updatedResultFromGET = this.restTemplate.getForEntity("$BASE_URI/$resolverResourceId", String) + def updatedVersionReturnedFromPUT = new JsonSlurper().parseText(updatedResultFromPUT.body).version + def updatedVersionReturnedFromGET = new JsonSlurper().parseText(updatedResultFromGET.body).version + + then: + updatedVersionReturnedFromPUT == updatedVersionReturnedFromGET + } + private HttpEntity createRequestHttpEntityFor(Closure jsonBodySupplier) { new HttpEntity(jsonBodySupplier(), ['Content-Type': 'application/json'] as HttpHeaders) } From 221133de60a2acb84408e25eebea93f960be3bfa Mon Sep 17 00:00:00 2001 From: Dmitriy Kopylenko Date: Tue, 14 Aug 2018 09:28:56 -0400 Subject: [PATCH 3/7] Incremental changes related to hashcod/version refactoring based on code review --- .../ui/controller/MetadataResolversController.java | 14 ++++---------- .../ui/domain/resolvers/MetadataResolver.java | 13 +++++++++---- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java index 52f1a50ac..15ae5d236 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java @@ -61,7 +61,6 @@ public ResponseEntity unableToParseJson(Exception ex) { @Transactional(readOnly = true) public ResponseEntity getAll() { List resolvers = positionOrderContainerService.getAllMetadataResolversInDefinedOrderOrUnordered(); - resolvers.forEach(MetadataResolver::updateVersion); return ResponseEntity.ok(resolvers); } @@ -86,7 +85,6 @@ public ResponseEntity getOne(@PathVariable String resourceId) { if (resolver == null) { return ResponseEntity.notFound().build(); } - resolver.updateVersion(); return ResponseEntity.ok(resolver); } @@ -103,11 +101,9 @@ public ResponseEntity create(@RequestBody MetadataResolver newResolver) { } newResolver.convertFiltersFromTransientRepresentationIfNecessary(); - resolverRepository.save(newResolver); - MetadataResolver persistedResolver = resolverRepository.findByResourceId(newResolver.getResourceId()); + MetadataResolver persistedResolver = resolverRepository.save(newResolver); positionOrderContainerService.appendPositionOrderForNew(persistedResolver); - persistedResolver.updateVersion(); persistedResolver.convertFiltersIntoTransientRepresentationIfNecessary(); return ResponseEntity.created(getResourceUriFor(persistedResolver)).body(persistedResolver); } @@ -119,9 +115,9 @@ public ResponseEntity update(@PathVariable String resourceId, @RequestBody Me if (existingResolver == null) { return ResponseEntity.notFound().build(); } - if (existingResolver.hashCode() != updatedResolver.getVersion()) { + if (existingResolver.getVersion() != updatedResolver.getVersion()) { log.info("Metadata Resolver version conflict. Latest resolver in database version: {}. Resolver version sent from UI: {}", - existingResolver.hashCode(), updatedResolver.getVersion()); + existingResolver.getVersion(), updatedResolver.getVersion()); return ResponseEntity.status(HttpStatus.CONFLICT).build(); } @@ -133,10 +129,8 @@ public ResponseEntity update(@PathVariable String resourceId, @RequestBody Me updatedResolver.setAudId(existingResolver.getAudId()); updatedResolver.convertFiltersFromTransientRepresentationIfNecessary(); - resolverRepository.save(updatedResolver); - MetadataResolver persistedResolver = resolverRepository.findByResourceId(updatedResolver.getResourceId()); + MetadataResolver persistedResolver = resolverRepository.save(updatedResolver); - persistedResolver.updateVersion(); persistedResolver.convertFiltersFromTransientRepresentationIfNecessary(); return ResponseEntity.ok(persistedResolver); } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/MetadataResolver.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/MetadataResolver.java index 7d0fc03d1..cb3b7c9d9 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/MetadataResolver.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/MetadataResolver.java @@ -1,5 +1,6 @@ package edu.internet2.tier.shibboleth.admin.ui.domain.resolvers; +import com.fasterxml.jackson.annotation.JsonGetter; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonSubTypes; import com.fasterxml.jackson.annotation.JsonTypeInfo; @@ -73,10 +74,14 @@ public class MetadataResolver extends AbstractAuditable { private List metadataFilters = new ArrayList<>(); @Transient - private int version; - - public void updateVersion() { - this.version = hashCode(); + private Integer version; + + @JsonGetter("version") + public int getVersion() { + if (this.version != null && this.version != 0 ) { + return this.version; + } + return this.hashCode(); } public void convertFiltersIntoTransientRepresentationIfNecessary() { From 2b6988242775f3e288d8f71c7df131bd992e088f Mon Sep 17 00:00:00 2001 From: Dmitriy Kopylenko Date: Tue, 14 Aug 2018 10:55:17 -0400 Subject: [PATCH 4/7] Refactoring filters persistance --- .../controller/MetadataFiltersController.java | 58 ++++------- .../filters/EntityAttributesFilter.java | 9 +- .../ui/domain/filters/MetadataFilter.java | 15 ++- ...taFiltersControllerIntegrationTests.groovy | 97 +++++++++++++++++++ 4 files changed, 134 insertions(+), 45 deletions(-) create mode 100644 backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersControllerIntegrationTests.groovy 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..a6491fa86 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 @@ -6,10 +6,10 @@ import edu.internet2.tier.shibboleth.admin.ui.domain.filters.RequiredValidUntilFilter; import edu.internet2.tier.shibboleth.admin.ui.domain.filters.SignatureValidationFilter; import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.MetadataResolver; +import edu.internet2.tier.shibboleth.admin.ui.repository.FilterRepository; import edu.internet2.tier.shibboleth.admin.ui.repository.MetadataResolverRepository; import edu.internet2.tier.shibboleth.admin.ui.service.MetadataResolverService; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; + import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; @@ -24,7 +24,6 @@ import org.springframework.web.servlet.support.ServletUriComponentsBuilder; import java.net.URI; -import java.util.List; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -32,14 +31,15 @@ @RequestMapping("/api/MetadataResolvers/{metadataResolverId}") public class MetadataFiltersController { - private static Logger LOGGER = LoggerFactory.getLogger(MetadataFiltersController.class); - @Autowired private MetadataResolverRepository repository; @Autowired private MetadataResolverService metadataResolverService; + @Autowired + private FilterRepository filterRepository; + @GetMapping("/Filters") @Transactional(readOnly = true) public ResponseEntity getAll(@PathVariable String metadataResolverId) { @@ -78,8 +78,7 @@ public ResponseEntity create(@PathVariable String metadataResolverId, @Reques // we reload the filters here after save metadataResolverService.reloadFilters(persistedMr.getName()); - MetadataFilter persistedFilter = - convertIntoTransientRepresentationIfNecessary(persistedMr.getMetadataFilters().stream(), createdFilter.getResourceId()); + MetadataFilter persistedFilter = newlyPersistedFilter(persistedMr.getMetadataFilters().stream(), createdFilter.getResourceId()); return ResponseEntity .created(getResourceUriFor(persistedMr, createdFilter.getResourceId())) @@ -91,30 +90,28 @@ public ResponseEntity create(@PathVariable String metadataResolverId, @Reques public ResponseEntity update(@PathVariable String metadataResolverId, @PathVariable String resourceId, @RequestBody MetadataFilter updatedFilter) { + MetadataFilter filterTobeUpdated = filterRepository.findByResourceId(resourceId); + if (filterTobeUpdated == null) { + return ResponseEntity.notFound().build(); + } MetadataResolver metadataResolver = repository.findByResourceId(metadataResolverId); if(metadataResolver == null) { return ResponseEntity.notFound().build(); } - if (!resourceId.equals(updatedFilter.getResourceId())) { - return new ResponseEntity(HttpStatus.CONFLICT); + // check to make sure that the relationship exists + if (!metadataResolver.getMetadataFilters().contains(filterTobeUpdated)) { + // TODO: find a better response + return new ResponseEntity<>(HttpStatus.BAD_REQUEST); } - List filters = - metadataResolver.getMetadataFilters().stream() - .filter(f -> f.getResourceId().equals(updatedFilter.getResourceId())) - .collect(Collectors.toList()); - if (filters.size() > 1) { - // TODO: I don't think this should ever happen, but... if it does... - // do something? throw exception, return error? - LOGGER.warn("More than one filter was found for id {}! This is probably a bad thing.\n" + - "We're going to go ahead and use the first one, but .. look in to this!", updatedFilter.getResourceId()); + if (!resourceId.equals(updatedFilter.getResourceId())) { + return new ResponseEntity(HttpStatus.CONFLICT); } - MetadataFilter filterTobeUpdated = filters.get(0); // Verify we're the only one attempting to update the filter - if (updatedFilter.getVersion() != filterTobeUpdated.hashCode()) { + if (updatedFilter.getVersion() != filterTobeUpdated.getVersion()) { return new ResponseEntity(HttpStatus.CONFLICT); } @@ -122,32 +119,19 @@ public ResponseEntity update(@PathVariable String metadataResolverId, filterTobeUpdated.setFilterEnabled(updatedFilter.isFilterEnabled()); updateConcreteFilterTypeData(filterTobeUpdated, updatedFilter); - //convert before saving into database - if(filterTobeUpdated instanceof EntityAttributesFilter) { - EntityAttributesFilter.class.cast(filterTobeUpdated).fromTransientRepresentation(); - } + MetadataFilter persistedFilter = filterRepository.save(filterTobeUpdated); - MetadataResolver persistedMr = repository.save(metadataResolver); - - metadataResolverService.reloadFilters(persistedMr.getName()); - - MetadataFilter persistedFilter = - convertIntoTransientRepresentationIfNecessary(persistedMr.getMetadataFilters().stream(), updatedFilter.getResourceId()); - - persistedFilter.setVersion(persistedFilter.hashCode()); + // TODO: this is wrong + metadataResolverService.reloadFilters(metadataResolver.getName()); return ResponseEntity.ok().body(persistedFilter); } - private MetadataFilter convertIntoTransientRepresentationIfNecessary(Stream filters, final String filterResourceId) { + private MetadataFilter newlyPersistedFilter(Stream filters, final String filterResourceId) { MetadataFilter persistedFilter = filters .filter(f -> f.getResourceId().equals(filterResourceId)) .collect(Collectors.toList()).get(0); - //convert before saving into database - if(persistedFilter instanceof EntityAttributesFilter) { - EntityAttributesFilter.class.cast(persistedFilter).intoTransientRepresentation(); - } return 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 a62ed9a24..e5e923918 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 @@ -51,19 +51,22 @@ public EntityAttributesFilter() { @Transient private RelyingPartyOverridesRepresentation relyingPartyOverrides; + @PostLoad public void intoTransientRepresentation() { this.attributeRelease = getAttributeReleaseListFromAttributeList(this.attributes); this.relyingPartyOverrides = getRelyingPartyOverridesRepresentationFromAttributeList(attributes); - updateVersion(); } + @PrePersist + @PreUpdate public void fromTransientRepresentation() { List attributeList = new ArrayList<>(); attributeList.addAll(getAttributeListFromAttributeReleaseList(this.attributeRelease)); attributeList.addAll(getAttributeListFromRelyingPartyOverridesRepresentation(this.relyingPartyOverrides)); if(!attributeList.isEmpty()) { - this.attributes = (List) (List) attributeList; + this.attributes.clear(); + this.attributes.addAll((List) (List) attributeList); } } -} +} \ No newline at end of file diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/MetadataFilter.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/MetadataFilter.java index 0347de1f7..a80d538c9 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/MetadataFilter.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/MetadataFilter.java @@ -1,5 +1,6 @@ package edu.internet2.tier.shibboleth.admin.ui.domain.filters; +import com.fasterxml.jackson.annotation.JsonGetter; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonSubTypes; import com.fasterxml.jackson.annotation.JsonTypeInfo; @@ -46,9 +47,13 @@ public class MetadataFilter extends AbstractAuditable { private boolean filterEnabled; @Transient - private int version; - - public void updateVersion() { - this.version = hashCode(); + private transient Integer version; + + @JsonGetter("version") + public int getVersion() { + if (version != null && version != 0) { + return this.version; + } + return this.hashCode(); } -} +} \ No newline at end of file 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 new file mode 100644 index 000000000..1804ff6f1 --- /dev/null +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersControllerIntegrationTests.groovy @@ -0,0 +1,97 @@ +package edu.internet2.tier.shibboleth.admin.ui.controller + +import com.fasterxml.jackson.databind.ObjectMapper +import com.fasterxml.jackson.databind.SerializationFeature +import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule +import edu.internet2.tier.shibboleth.admin.ui.repository.MetadataResolverRepository +import edu.internet2.tier.shibboleth.admin.ui.util.TestObjectGenerator +import edu.internet2.tier.shibboleth.admin.util.AttributeUtility +import groovy.json.JsonOutput +import groovy.json.JsonSlurper +import org.opensaml.saml.metadata.resolver.ChainingMetadataResolver +import org.opensaml.saml.metadata.resolver.MetadataResolver +import org.springframework.beans.factory.annotation.Autowired +import org.springframework.boot.test.context.SpringBootTest +import org.springframework.boot.test.context.TestConfiguration +import org.springframework.boot.test.web.client.TestRestTemplate +import org.springframework.context.annotation.Bean +import org.springframework.http.HttpEntity +import org.springframework.http.HttpHeaders +import org.springframework.test.context.ActiveProfiles +import spock.lang.Specification + +import static org.springframework.http.HttpMethod.PUT + +/** + * @author Dmitriy Kopylenko + */ +@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) +@ActiveProfiles("no-auth") +class MetadataFiltersControllerIntegrationTests extends Specification { + + @Autowired + private TestRestTemplate restTemplate + + @Autowired + MetadataResolverRepository metadataResolverRepository + + @Autowired + AttributeUtility attributeUtility + + ObjectMapper mapper + TestObjectGenerator generator + + JsonSlurper jsonSlurper = new JsonSlurper() + + static BASE_URI = '/api/MetadataResolvers' + + def setup() { + generator = new TestObjectGenerator(attributeUtility) + mapper = new ObjectMapper() + mapper.enable(SerializationFeature.INDENT_OUTPUT) + mapper.registerModule(new JavaTimeModule()) + } + + def cleanup() { + metadataResolverRepository.deleteAll() + } + + def "PUT EntityAttributesFilter"() { + given: 'MetadataResolver with attached entity attributes 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) + def existingFilterMap = jsonSlurper.parseText(result.body) + + and: 'PUT call is made with unmodified filter state' + def updatedResultFromPUT = this.restTemplate.exchange( + "$BASE_URI/$resolverResourceId/Filters/$filterResourceId", + PUT, + createRequestHttpEntityFor { JsonOutput.toJson(existingFilterMap) }, String) + + then: + updatedResultFromPUT.statusCode.value() == 200 + } + + private HttpEntity createRequestHttpEntityFor(Closure jsonBodySupplier) { + new HttpEntity(jsonBodySupplier(), ['Content-Type': 'application/json'] as HttpHeaders) + } + + @TestConfiguration + static class Config { + @Bean + MetadataResolver metadataResolver() { + new ChainingMetadataResolver().with { + it.id = 'tester' + it.initialize() + return it + } + } + } +} \ No newline at end of file From e83f17fb5cd2ca4547584afb176b115597f110a6 Mon Sep 17 00:00:00 2001 From: Dmitriy Kopylenko Date: Tue, 14 Aug 2018 11:22:33 -0400 Subject: [PATCH 5/7] Add test for updated entity attributes filter --- ...taFiltersControllerIntegrationTests.groovy | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) 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 1804ff6f1..557fdf56e 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 @@ -79,6 +79,30 @@ class MetadataFiltersControllerIntegrationTests extends Specification { updatedResultFromPUT.statusCode.value() == 200 } + def "PUT EntityAttributesFilter and update it"() { + given: 'MetadataResolver with attached entity attributes 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) + def existingFilterMap = jsonSlurper.parseText(result.body) + + and: 'PUT call is made with modified filter state' + existingFilterMap.name = 'Entity Attributes Filter Updated' + def updatedResultFromPUT = this.restTemplate.exchange( + "$BASE_URI/$resolverResourceId/Filters/$filterResourceId", + PUT, + createRequestHttpEntityFor { JsonOutput.toJson(existingFilterMap) }, String) + + then: + updatedResultFromPUT.statusCode.value() == 200 + } + private HttpEntity createRequestHttpEntityFor(Closure jsonBodySupplier) { new HttpEntity(jsonBodySupplier(), ['Content-Type': 'application/json'] as HttpHeaders) } From 70c7b4651ec0750b8b5e67d4ab53c1f881e0188e Mon Sep 17 00:00:00 2001 From: Dmitriy Kopylenko Date: Tue, 14 Aug 2018 15:25:54 -0400 Subject: [PATCH 6/7] Fix test --- .../MetadataFiltersControllerTests.groovy | 38 +++++++++++-------- 1 file changed, 22 insertions(+), 16 deletions(-) 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 0443a7e54..e28a05cb3 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 @@ -9,6 +9,7 @@ import edu.internet2.tier.shibboleth.admin.ui.configuration.SearchConfiguration import edu.internet2.tier.shibboleth.admin.ui.domain.filters.EntityAttributesFilter import edu.internet2.tier.shibboleth.admin.ui.domain.filters.MetadataFilter import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.MetadataResolver +import edu.internet2.tier.shibboleth.admin.ui.repository.FilterRepository import edu.internet2.tier.shibboleth.admin.ui.repository.MetadataResolverRepository import edu.internet2.tier.shibboleth.admin.ui.service.FilterService import edu.internet2.tier.shibboleth.admin.ui.service.MetadataResolverService @@ -53,6 +54,8 @@ class MetadataFiltersControllerTests extends Specification { def metadataResolverRepository = Mock(MetadataResolverRepository) + def metadataFilterRepository = Mock(FilterRepository) + def controller def mockMvc @@ -67,6 +70,7 @@ class MetadataFiltersControllerTests extends Specification { controller = new MetadataFiltersController ( repository: metadataResolverRepository, + filterRepository: metadataFilterRepository, metadataResolverService: new MetadataResolverService() { @Override void reloadFilters(String metadataResolverName) { @@ -98,8 +102,8 @@ class MetadataFiltersControllerTests extends Specification { then: result.andExpect(expectedHttpResponseStatus) - .andExpect(content().contentType(expectedResponseContentType)) - .andExpect(content().json(mapper.writeValueAsString(expectedContent))) + .andExpect(content().contentType(expectedResponseContentType)) + .andExpect(content().json(mapper.writeValueAsString(expectedContent))) } def "FilterController.getOne gets the desired filter"() { @@ -156,11 +160,11 @@ class MetadataFiltersControllerTests extends Specification { .andExpect(header().string(expectedResponseHeader, containsString(expectedResponseHeaderValue))) where: - filterType | _ - 'entityAttributes' | _ - 'entityRoleWhiteList' | _ - 'signatureValidation' | _ - 'requiredValidUntil' | _ + filterType | _ + 'entityAttributes' | _ + 'entityRoleWhiteList' | _ + 'signatureValidation' | _ + 'requiredValidUntil' | _ } @Unroll @@ -173,7 +177,7 @@ class MetadataFiltersControllerTests extends Specification { def postedJsonBody = mapper.writeValueAsString(updatedFilter) def originalMetadataResolver = new MetadataResolver() - originalMetadataResolver.setResourceId(randomGenerator.randomId()) + originalMetadataResolver.setResourceId('foo') originalMetadataResolver.setMetadataFilters(testObjectGenerator.buildAllTypesOfFilterList()) originalMetadataResolver.metadataFilters.add(originalFilter) @@ -183,7 +187,8 @@ class MetadataFiltersControllerTests extends Specification { updatedMetadataResolver.getMetadataFilters().add(updatedFilter) 1 * metadataResolverRepository.findByResourceId(_) >> originalMetadataResolver - 1 * metadataResolverRepository.save(_) >> updatedMetadataResolver + 1 * metadataFilterRepository.findByResourceId(_) >> originalFilter + 1 * metadataFilterRepository.save(_) >> updatedFilter def filterUUID = updatedFilter.getResourceId() @@ -198,16 +203,16 @@ class MetadataFiltersControllerTests extends Specification { if (filterType == 'entityAttributes') { EntityAttributesFilter.cast(updatedFilter).fromTransientRepresentation() } - expectedJson << [version: updatedFilter.hashCode()] + expectedJson << [version: updatedFilter.getVersion()] result.andExpect(status().isOk()) .andExpect(content().json(JsonOutput.toJson(expectedJson), true)) where: - filterType | _ - 'entityAttributes' | _ - 'entityRoleWhiteList' | _ - 'signatureValidation' | _ - 'requiredValidUntil' | _ + filterType | _ + 'entityAttributes' | _ + 'entityRoleWhiteList' | _ + 'signatureValidation' | _ + 'requiredValidUntil' | _ } def "FilterController.update filter 409's if the version numbers don't match"() { @@ -223,6 +228,7 @@ class MetadataFiltersControllerTests extends Specification { originalMetadataResolver.getMetadataFilters().add(randomFilter) 1 * metadataResolverRepository.findByResourceId(_) >> originalMetadataResolver + 1 * metadataFilterRepository.findByResourceId(_) >> randomFilter def filterUUID = randomFilter.getResourceId() @@ -235,4 +241,4 @@ class MetadataFiltersControllerTests extends Specification { then: result.andExpect(status().is(409)) } -} +} \ No newline at end of file From b3f93bebfd3981f413d23cf6e6c2f6d2d549e9f6 Mon Sep 17 00:00:00 2001 From: Dmitriy Kopylenko Date: Wed, 15 Aug 2018 09:10:12 -0400 Subject: [PATCH 7/7] Refactor persistence implementation for attribute filter --- .../controller/MetadataFiltersController.java | 5 --- .../MetadataResolversController.java | 4 --- .../filters/EntityAttributesFilter.java | 32 ++++++++++--------- .../ui/domain/resolvers/MetadataResolver.java | 18 ----------- .../MetadataFiltersControllerTests.groovy | 3 -- ...ymorphicFiltersJacksonHandlingTests.groovy | 1 - .../MetadataResolverRepositoryTests.groovy | 5 --- 7 files changed, 17 insertions(+), 51 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 a6491fa86..52122b64f 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 @@ -68,11 +68,6 @@ public ResponseEntity create(@PathVariable String metadataResolverId, @Reques return ResponseEntity.notFound().build(); } metadataResolver.getMetadataFilters().add(createdFilter); - - //convert before saving into database - if(createdFilter instanceof EntityAttributesFilter) { - EntityAttributesFilter.class.cast(createdFilter).fromTransientRepresentation(); - } MetadataResolver persistedMr = repository.save(metadataResolver); // we reload the filters here after save diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java index 15ae5d236..be6ab51a4 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java @@ -100,11 +100,9 @@ public ResponseEntity create(@RequestBody MetadataResolver newResolver) { return validationErrorResponse; } - newResolver.convertFiltersFromTransientRepresentationIfNecessary(); MetadataResolver persistedResolver = resolverRepository.save(newResolver); positionOrderContainerService.appendPositionOrderForNew(persistedResolver); - persistedResolver.convertFiltersIntoTransientRepresentationIfNecessary(); return ResponseEntity.created(getResourceUriFor(persistedResolver)).body(persistedResolver); } @@ -128,10 +126,8 @@ public ResponseEntity update(@PathVariable String resourceId, @RequestBody Me updatedResolver.setAudId(existingResolver.getAudId()); - updatedResolver.convertFiltersFromTransientRepresentationIfNecessary(); MetadataResolver persistedResolver = resolverRepository.save(updatedResolver); - persistedResolver.convertFiltersFromTransientRepresentationIfNecessary(); return ResponseEntity.ok(persistedResolver); } 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 e5e923918..8492e745d 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 @@ -14,8 +14,6 @@ import javax.persistence.OneToOne; import javax.persistence.OrderColumn; import javax.persistence.PostLoad; -import javax.persistence.PrePersist; -import javax.persistence.PreUpdate; import javax.persistence.Transient; import java.util.ArrayList; @@ -48,25 +46,29 @@ public EntityAttributesFilter() { @Transient private List attributeRelease = new ArrayList<>(); + public void setAttributeRelease(List attributeRelease) { + this.attributeRelease = attributeRelease; + this.rebuildAttributes(); + } + @Transient private RelyingPartyOverridesRepresentation relyingPartyOverrides; + public void setRelyingPartyOverrides(RelyingPartyOverridesRepresentation relyingPartyOverridesRepresentation) { + this.relyingPartyOverrides = relyingPartyOverridesRepresentation; + this.rebuildAttributes(); + } + + //TODO: yeah, I'm not too happy, either + private void rebuildAttributes() { + this.attributes.clear(); + this.attributes.addAll((List) (List)getAttributeListFromAttributeReleaseList(this.attributeRelease)); + this.attributes.addAll((List) (List)getAttributeListFromRelyingPartyOverridesRepresentation(this.relyingPartyOverrides)); + } + @PostLoad public void intoTransientRepresentation() { this.attributeRelease = getAttributeReleaseListFromAttributeList(this.attributes); this.relyingPartyOverrides = getRelyingPartyOverridesRepresentationFromAttributeList(attributes); } - - @PrePersist - @PreUpdate - public void fromTransientRepresentation() { - List attributeList = new ArrayList<>(); - attributeList.addAll(getAttributeListFromAttributeReleaseList(this.attributeRelease)); - attributeList.addAll(getAttributeListFromRelyingPartyOverridesRepresentation(this.relyingPartyOverrides)); - - if(!attributeList.isEmpty()) { - this.attributes.clear(); - this.attributes.addAll((List) (List) attributeList); - } - } } \ No newline at end of file diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/MetadataResolver.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/MetadataResolver.java index cb3b7c9d9..599503f75 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/MetadataResolver.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/MetadataResolver.java @@ -5,7 +5,6 @@ import com.fasterxml.jackson.annotation.JsonSubTypes; import com.fasterxml.jackson.annotation.JsonTypeInfo; import edu.internet2.tier.shibboleth.admin.ui.domain.AbstractAuditable; -import edu.internet2.tier.shibboleth.admin.ui.domain.filters.EntityAttributesFilter; import edu.internet2.tier.shibboleth.admin.ui.domain.filters.MetadataFilter; import lombok.EqualsAndHashCode; import lombok.Getter; @@ -25,8 +24,6 @@ import java.util.List; import java.util.UUID; -import static java.util.stream.Collectors.toList; - @Entity @Inheritance(strategy = InheritanceType.TABLE_PER_CLASS) @EqualsAndHashCode(callSuper = true, exclude = {"version"}) @@ -83,19 +80,4 @@ public int getVersion() { } return this.hashCode(); } - - public void convertFiltersIntoTransientRepresentationIfNecessary() { - getAvailableEntityAttributesFilters().forEach(EntityAttributesFilter::intoTransientRepresentation); - } - - public void convertFiltersFromTransientRepresentationIfNecessary() { - getAvailableEntityAttributesFilters().forEach(EntityAttributesFilter::fromTransientRepresentation); - } - - private List getAvailableEntityAttributesFilters() { - return this.metadataFilters.stream() - .filter(EntityAttributesFilter.class::isInstance) - .map(EntityAttributesFilter.class::cast) - .collect(toList()); - } } 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 e28a05cb3..40c6cefef 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 @@ -200,9 +200,6 @@ class MetadataFiltersControllerTests extends Specification { then: def expectedJson = new JsonSlurper().parseText(postedJsonBody) - if (filterType == 'entityAttributes') { - EntityAttributesFilter.cast(updatedFilter).fromTransientRepresentation() - } expectedJson << [version: updatedFilter.getVersion()] result.andExpect(status().isOk()) .andExpect(content().json(JsonOutput.toJson(expectedJson), true)) diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/domain/filters/PolymorphicFiltersJacksonHandlingTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/domain/filters/PolymorphicFiltersJacksonHandlingTests.groovy index 56112a350..7841e045b 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/domain/filters/PolymorphicFiltersJacksonHandlingTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/domain/filters/PolymorphicFiltersJacksonHandlingTests.groovy @@ -73,7 +73,6 @@ class PolymorphicFiltersJacksonHandlingTests extends Specification { def simulatedPrePersistentFilter = new EntityAttributesFilter() simulatedPrePersistentFilter.attributeRelease = simulatedPersistentFilter.attributeRelease simulatedPrePersistentFilter.relyingPartyOverrides = simulatedPersistentFilter.relyingPartyOverrides - simulatedPrePersistentFilter.fromTransientRepresentation() expect: simulatedPersistentFilter.attributes.size() == simulatedPrePersistentFilter.attributes.size() diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/repository/MetadataResolverRepositoryTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/repository/MetadataResolverRepositoryTests.groovy index 646d3e4aa..872182ce8 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/repository/MetadataResolverRepositoryTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/repository/MetadataResolverRepositoryTests.groovy @@ -88,9 +88,6 @@ class MetadataResolverRepositoryTests extends Specification { } MetadataResolver metadataResolver = metadataResolverRepository.findAll().iterator().next() - //convert before saving into database - filter.fromTransientRepresentation() - metadataResolver.getMetadataFilters().add(filter) MetadataResolver persistedMr = metadataResolverRepository.save(metadataResolver) @@ -141,8 +138,6 @@ class MetadataResolverRepositoryTests extends Specification { filterToBeUpdated.relyingPartyOverrides = filter.relyingPartyOverrides filterToBeUpdated.attributeRelease = filter.attributeRelease - //convert before saving into database - filterToBeUpdated.fromTransientRepresentation() entityManager.clear() persistedMr = metadataResolverRepository.save(metadataResolver)