From ea3b0e9c61d0304bb17817c6c9d169264d3224fa Mon Sep 17 00:00:00 2001 From: Bill Smith Date: Thu, 24 May 2018 12:45:29 -0700 Subject: [PATCH] [SHIBUI-517] Updates to controller and tests. More tests incoming. --- ...eBackedHttpMetadataProviderController.java | 39 +++++++++++-- .../admin/ui/domain/AbstractAuditable.java | 2 + .../HttpMetadataResolverAttributes.java | 2 + .../ui/domain/resolvers/MetadataResolver.java | 8 ++- .../ReloadableMetadataResolverAttributes.java | 2 + ...HttpMetadataResolverRepositoryTests.groovy | 57 ++++++++++++++++++- 6 files changed, 102 insertions(+), 8 deletions(-) diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/FileBackedHttpMetadataProviderController.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/FileBackedHttpMetadataProviderController.java index 56fa92729..572c728ae 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/FileBackedHttpMetadataProviderController.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/FileBackedHttpMetadataProviderController.java @@ -2,7 +2,10 @@ import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.FileBackedHttpMetadataResolver; import edu.internet2.tier.shibboleth.admin.ui.repository.FileBackedHttpMetadataResolverRepository; +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; import org.springframework.transaction.annotation.Transactional; import org.springframework.web.bind.annotation.DeleteMapping; @@ -23,6 +26,7 @@ @RestController @RequestMapping("/api/MetadataProvider/FileBackedHttp") public class FileBackedHttpMetadataProviderController { + private static final Logger logger = LoggerFactory.getLogger(FileBackedHttpMetadataProviderController.class); @Autowired FileBackedHttpMetadataResolverRepository repository; @@ -39,33 +43,56 @@ public ResponseEntity deleteByResourceId(@PathVariable String resourceId) { @GetMapping("/name/{metadataProviderName}") @Transactional(readOnly = true) public ResponseEntity getOneByName(@PathVariable String metadataProviderName) { - return ResponseEntity.ok(repository.findByName(metadataProviderName)); + FileBackedHttpMetadataResolver resolver = repository.findByResourceId(metadataProviderName); + if (resolver == null) { + return ResponseEntity.notFound().build(); + } else { + resolver.setVersion(resolver.hashCode()); + return ResponseEntity.ok(resolver); + } } @GetMapping("/{resourceId}") @Transactional(readOnly = true) public ResponseEntity getOneByResourceId(@PathVariable String resourceId) { - return ResponseEntity.ok(repository.findByResourceId(resourceId)); + FileBackedHttpMetadataResolver resolver = repository.findByResourceId(resourceId); + if (resolver == null) { + return ResponseEntity.notFound().build(); + } else { + resolver.setVersion(resolver.hashCode()); + return ResponseEntity.ok(resolver); + } } @PostMapping public ResponseEntity create(@RequestBody FileBackedHttpMetadataResolver resolver) { - //TODO: Check for duplicates based on name + if (repository.findByName(resolver.getName()) != null) { + return ResponseEntity.status(HttpStatus.CONFLICT).build(); + } + FileBackedHttpMetadataResolver persistedResolver = repository.save(resolver); + persistedResolver.setVersion(persistedResolver.hashCode()); return ResponseEntity .created(getResourceUriFor(persistedResolver)) .body(persistedResolver); - } @PutMapping public ResponseEntity update(@RequestBody FileBackedHttpMetadataResolver resolver) { FileBackedHttpMetadataResolver existingResolver = repository.findByResourceId(resolver.getResourceId()); - //TODO: Handle not found. - //TODO: Handle contention. + + if (existingResolver == null) { + return ResponseEntity.notFound().build(); + } + + if (existingResolver.hashCode() != resolver.getVersion()) { + logger.info("Comparing: " + existingResolver.hashCode() + " with " + resolver.getVersion()); + return ResponseEntity.status(HttpStatus.CONFLICT).build(); + } resolver.setAudId(existingResolver.getAudId()); + //TODO: Do we need to set anything else? dates? FileBackedHttpMetadataResolver updatedResolver = repository.save(resolver); diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/AbstractAuditable.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/AbstractAuditable.java index 8f530bdd6..1d23d5113 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/AbstractAuditable.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/AbstractAuditable.java @@ -1,5 +1,6 @@ package edu.internet2.tier.shibboleth.admin.ui.domain; +import lombok.EqualsAndHashCode; import org.hibernate.annotations.CreationTimestamp; import org.hibernate.annotations.UpdateTimestamp; import org.springframework.data.annotation.CreatedBy; @@ -20,6 +21,7 @@ @MappedSuperclass @EntityListeners(AuditingEntityListener.class) +@EqualsAndHashCode public abstract class AbstractAuditable implements Auditable { @Id diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/HttpMetadataResolverAttributes.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/HttpMetadataResolverAttributes.java index cc880cc32..437516ffd 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/HttpMetadataResolverAttributes.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/HttpMetadataResolverAttributes.java @@ -1,6 +1,7 @@ package edu.internet2.tier.shibboleth.admin.ui.domain.resolvers; import lombok.AllArgsConstructor; +import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.NoArgsConstructor; import lombok.Setter; @@ -16,6 +17,7 @@ @AllArgsConstructor @Getter @Setter +@EqualsAndHashCode public class HttpMetadataResolverAttributes { private String httpClientRef; 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 f8ad0c216..4deebcbcb 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 @@ -15,13 +15,15 @@ import javax.persistence.InheritanceType; import javax.persistence.OneToMany; +import javax.persistence.OrderColumn; +import javax.persistence.Transient; import java.util.ArrayList; import java.util.List; import java.util.UUID; @Entity @Inheritance(strategy = InheritanceType.TABLE_PER_CLASS) -@EqualsAndHashCode(callSuper = true) +@EqualsAndHashCode(callSuper = true, exclude={"version"}) @NoArgsConstructor @Getter @Setter @@ -47,5 +49,9 @@ public class MetadataResolver extends AbstractAuditable { private Boolean satisfyAnyPredicates; @OneToMany(cascade = CascadeType.ALL) + @OrderColumn private List metadataFilters = new ArrayList<>(); + + @Transient + private int version; } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/ReloadableMetadataResolverAttributes.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/ReloadableMetadataResolverAttributes.java index ad21f7a23..cddf5b935 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/ReloadableMetadataResolverAttributes.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/ReloadableMetadataResolverAttributes.java @@ -1,6 +1,7 @@ package edu.internet2.tier.shibboleth.admin.ui.domain.resolvers; import lombok.AllArgsConstructor; +import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.NoArgsConstructor; import lombok.Setter; @@ -12,6 +13,7 @@ @AllArgsConstructor @Getter @Setter +@EqualsAndHashCode public class ReloadableMetadataResolverAttributes { private String parserPoolRef; diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/repository/FileBackedHttpMetadataResolverRepositoryTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/repository/FileBackedHttpMetadataResolverRepositoryTests.groovy index 0cef56367..d4aea60ba 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/repository/FileBackedHttpMetadataResolverRepositoryTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/repository/FileBackedHttpMetadataResolverRepositoryTests.groovy @@ -1,5 +1,6 @@ package edu.internet2.tier.shibboleth.admin.ui.repository +import com.fasterxml.jackson.databind.ObjectMapper import edu.internet2.tier.shibboleth.admin.ui.configuration.CoreShibUiConfiguration import edu.internet2.tier.shibboleth.admin.ui.configuration.MetadataResolverConfiguration import edu.internet2.tier.shibboleth.admin.ui.configuration.SearchConfiguration @@ -12,21 +13,28 @@ import org.springframework.beans.factory.annotation.Autowired import org.springframework.boot.autoconfigure.domain.EntityScan import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest import org.springframework.data.jpa.repository.config.EnableJpaRepositories +import org.springframework.test.annotation.DirtiesContext import org.springframework.test.context.ContextConfiguration import spock.lang.Specification -import static edu.internet2.tier.shibboleth.admin.ui.domain.EntityAttributesFilterTarget.EntityAttributesFilterTargetType.* +import javax.persistence.EntityManager + +import static edu.internet2.tier.shibboleth.admin.ui.domain.EntityAttributesFilterTarget.EntityAttributesFilterTargetType.ENTITY import static edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.HttpMetadataResolverAttributes.HttpCachingType.memory @DataJpaTest @ContextConfiguration(classes=[CoreShibUiConfiguration, SearchConfiguration, MetadataResolverConfiguration]) @EnableJpaRepositories(basePackages = ["edu.internet2.tier.shibboleth.admin.ui"]) @EntityScan("edu.internet2.tier.shibboleth.admin.ui") +@DirtiesContext(methodMode = DirtiesContext.MethodMode.BEFORE_METHOD) class FileBackedHttpMetadataResolverRepositoryTests extends Specification { @Autowired FileBackedHttpMetadataResolverRepository repositoryUnderTest + @Autowired + EntityManager entityManager + def "file backed http metadata resolver instances persist OK"() { when: def mdr = new FileBackedHttpMetadataResolver().with { @@ -69,4 +77,51 @@ class FileBackedHttpMetadataResolverRepositoryTests extends Specification { item.httpMetadataResolverAttributes.httpCaching == memory item.reloadableMetadataResolverAttributes.indexesRef == "indexesSpringBeanId" } + + def "FileBackedHttpMetadataResolver hashcode works as desired"() { + given: + // TODO: Ask JJ why an empty reloadableMetadataResolverAttributes object results in a null object for item2 below + def resolverJson = '''{ + "name": "name", + "requireValidMetadata": true, + "failFastInitialization": true, + "sortKey": 7, + "criterionPredicateRegistryRef": "criterionPredicateRegistryRef", + "useDefaultPredicateRegistry": true, + "satisfyAnyPredicates": true, + "metadataFilters": [], + "reloadableMetadataResolverAttributes": { + }, + "httpMetadataResolverAttributes": { + "httpClientRef": "httpClientRef", + "connectionRequestTimeout": "connectionRequestTimeout", + "requestTimeout": "requestTimeout", + "socketTimeout": "socketTimeout", + "disregardTLSCertificate": true, + "tlsTrustEngineRef": "tlsTrustEngineRef", + "httpClientSecurityParametersRef": "httpClientSecurityParametersRef", + "proxyHost": "proxyHost", + "proxyPort": "proxyPort", + "proxyUser": "proxyUser", + "proxyPassword": "proxyPassword", + "httpCaching": "none", + "httpCacheDirectory": "httpCacheDirectory", + "httpMaxCacheEntries": 1, + "httpMaxCacheEntrySize": 2 + } +}''' + + when: + def resolver = new ObjectMapper().readValue(resolverJson.bytes, FileBackedHttpMetadataResolver) + def persistedResolver = repositoryUnderTest.save(resolver) + entityManager.flush() + + then: + entityManager.clear() + def item1 = repositoryUnderTest.findByResourceId(persistedResolver.resourceId) + entityManager.clear() + def item2 = repositoryUnderTest.findByResourceId(persistedResolver.resourceId) + + item1.hashCode() == item2.hashCode() + } }