SHIBUI-2270

Code clean up from Dima's code review comments
TIER · Sep 8, 2022 · b41c645 · b41c645
1 parent b6bd57c
commit b41c645
Show file tree

Hide file tree

Showing 40 changed files with 162 additions and 176 deletions.
diff --git a/.../tier/shibboleth/admin/ui/service/envers/EnversEntityDescriptorVersionServiceTests.groovy b/.../tier/shibboleth/admin/ui/service/envers/EnversEntityDescriptorVersionServiceTests.groovy
@@ -6,7 +6,7 @@ import edu.internet2.tier.shibboleth.admin.ui.configuration.Internationalization
 import edu.internet2.tier.shibboleth.admin.ui.configuration.SearchConfiguration
 import edu.internet2.tier.shibboleth.admin.ui.configuration.TestConfiguration
 import edu.internet2.tier.shibboleth.admin.ui.domain.EntityDescriptor
-import edu.internet2.tier.shibboleth.admin.ui.exception.EntityNotFoundException
+import edu.internet2.tier.shibboleth.admin.ui.exception.PersistentEntityNotFound
 import edu.internet2.tier.shibboleth.admin.ui.repository.EntityDescriptorRepository
 import edu.internet2.tier.shibboleth.admin.ui.repository.envers.EnversTestsSupport
 import edu.internet2.tier.shibboleth.admin.ui.service.EntityDescriptorService
@@ -121,7 +121,7 @@ class EnversEntityDescriptorVersionServiceTests extends Specification {
             def edRepresentation = entityDescriptorVersionService.findSpecificVersionOfEntityDescriptor(ed.resourceId, '1000')
             false
         }
-        catch (EntityNotFoundException expected) {
+        catch (PersistentEntityNotFound expected) {
             true
         }
     }

diff --git a/...oovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAMetadataResolverServiceImpl.groovy b/...oovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAMetadataResolverServiceImpl.groovy
@@ -26,7 +26,7 @@ import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.ResourceBackedMet
 import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.TemplateScheme
 import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.opensaml.OpenSamlChainingMetadataResolver
 import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.opensaml.Refilterable
-import edu.internet2.tier.shibboleth.admin.ui.exception.EntityNotFoundException
+import edu.internet2.tier.shibboleth.admin.ui.exception.PersistentEntityNotFound
 import edu.internet2.tier.shibboleth.admin.ui.exception.ForbiddenException
 import edu.internet2.tier.shibboleth.admin.ui.exception.InitializationException
 import edu.internet2.tier.shibboleth.admin.ui.opensaml.OpenSamlObjects
@@ -498,10 +498,10 @@ class JPAMetadataResolverServiceImpl implements MetadataResolverService {
         }
     }
 
-    public edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.MetadataResolver findByResourceId(String resourceId) throws EntityNotFoundException {
+    public edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.MetadataResolver findByResourceId(String resourceId) throws PersistentEntityNotFound {
         edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.MetadataResolver result = metadataResolverRepository.findByResourceId(resourceId)
         if (result == null ) {
-            throw new EntityNotFoundException("No Provider with resourceId[" + resourceId + "] was found")
+            throw new PersistentEntityNotFound("No Provider with resourceId[" + resourceId + "] was found")
         }
         return result
     }

diff --git a/...net2/tier/shibboleth/admin/ui/configuration/auto/MigrationTasksContextLoadedListener.java b/...net2/tier/shibboleth/admin/ui/configuration/auto/MigrationTasksContextLoadedListener.java
@@ -1,7 +1,5 @@
 package edu.internet2.tier.shibboleth.admin.ui.configuration.auto;
 
-import edu.internet2.tier.shibboleth.admin.ui.exception.EntityNotFoundException;
-import edu.internet2.tier.shibboleth.admin.ui.security.exception.InvalidGroupRegexException;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.ApplicationListener;
 import org.springframework.context.event.ContextRefreshedEvent;
@@ -16,8 +14,6 @@
 import edu.internet2.tier.shibboleth.admin.ui.security.service.IGroupService;
 import edu.internet2.tier.shibboleth.admin.ui.security.service.UserService;
 
-import java.util.List;
-
 /**
  * After the context loads, do any needed migration tasks
  */

diff --git a/...d/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/ActivateController.java b/...d/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/ActivateController.java
@@ -5,20 +5,18 @@
 import io.swagger.v3.oas.annotations.tags.Tag;
 import io.swagger.v3.oas.annotations.tags.Tags;
 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.PatchMapping;
 import org.springframework.web.bind.annotation.PathVariable;
-import org.springframework.web.bind.annotation.PutMapping;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RestController;
 
 import edu.internet2.tier.shibboleth.admin.ui.domain.exceptions.MetadataFileNotFoundException;
 import edu.internet2.tier.shibboleth.admin.ui.domain.filters.MetadataFilter;
 import edu.internet2.tier.shibboleth.admin.ui.domain.frontend.EntityDescriptorRepresentation;
 import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.MetadataResolver;
-import edu.internet2.tier.shibboleth.admin.ui.exception.EntityNotFoundException;
+import edu.internet2.tier.shibboleth.admin.ui.exception.PersistentEntityNotFound;
 import edu.internet2.tier.shibboleth.admin.ui.exception.ForbiddenException;
 import edu.internet2.tier.shibboleth.admin.ui.exception.InitializationException;
 import edu.internet2.tier.shibboleth.admin.ui.service.EntityDescriptorService;
@@ -41,23 +39,26 @@ public class ActivateController {
 
     @PatchMapping(path = "/entityDescriptor/{resourceId}/{mode}")
     @Transactional
-    public ResponseEntity<?> enableEntityDescriptor(@PathVariable String resourceId, @PathVariable String mode) throws EntityNotFoundException, ForbiddenException {
+    public ResponseEntity<?> enableEntityDescriptor(@PathVariable String resourceId, @PathVariable String mode) throws
+                    PersistentEntityNotFound, ForbiddenException {
         boolean status = "enable".equalsIgnoreCase(mode);
         EntityDescriptorRepresentation edr = entityDescriptorService.updateEntityDescriptorEnabledStatus(resourceId, status);
         return ResponseEntity.ok(edr);
     }
 
     @PatchMapping(path = "/MetadataResolvers/{metadataResolverId}/Filter/{resourceId}/{mode}")
     @Transactional
-    public ResponseEntity<?> enableFilter(@PathVariable String metadataResolverId, @PathVariable String resourceId, @PathVariable String mode) throws EntityNotFoundException, ForbiddenException, ScriptException {
+    public ResponseEntity<?> enableFilter(@PathVariable String metadataResolverId, @PathVariable String resourceId, @PathVariable String mode) throws
+                    PersistentEntityNotFound, ForbiddenException, ScriptException {
         boolean status = "enable".equalsIgnoreCase(mode);
         MetadataFilter persistedFilter = filterService.updateFilterEnabledStatus(metadataResolverId, resourceId, status);
         return ResponseEntity.ok(persistedFilter);
     }    
 
     @PatchMapping("/MetadataResolvers/{resourceId}/{mode}") 
     @Transactional
-    public ResponseEntity<?> enableProvider(@PathVariable String resourceId, @PathVariable String mode) throws EntityNotFoundException, ForbiddenException, MetadataFileNotFoundException, InitializationException {
+    public ResponseEntity<?> enableProvider(@PathVariable String resourceId, @PathVariable String mode) throws
+                    PersistentEntityNotFound, ForbiddenException, MetadataFileNotFoundException, InitializationException {
         boolean status = "enable".equalsIgnoreCase(mode);
         MetadataResolver existingResolver = metadataResolverService.findByResourceId(resourceId);
         existingResolver.setEnabled(status);

diff --git a/...main/java/edu/internet2/tier/shibboleth/admin/ui/controller/ActivateExceptionHandler.java b/...main/java/edu/internet2/tier/shibboleth/admin/ui/controller/ActivateExceptionHandler.java
@@ -12,15 +12,15 @@
 import org.springframework.web.servlet.mvc.method.annotation.ResponseEntityExceptionHandler;
 
 import edu.internet2.tier.shibboleth.admin.ui.domain.exceptions.MetadataFileNotFoundException;
-import edu.internet2.tier.shibboleth.admin.ui.exception.EntityNotFoundException;
+import edu.internet2.tier.shibboleth.admin.ui.exception.PersistentEntityNotFound;
 import edu.internet2.tier.shibboleth.admin.ui.exception.ForbiddenException;
 import edu.internet2.tier.shibboleth.admin.ui.exception.InitializationException;
 
 @ControllerAdvice(assignableTypes = {ActivateController.class})
 public class ActivateExceptionHandler extends ResponseEntityExceptionHandler {
 
-    @ExceptionHandler({ EntityNotFoundException.class })
-    public ResponseEntity<?> handleEntityNotFoundException(EntityNotFoundException e, WebRequest request) {
+    @ExceptionHandler({ PersistentEntityNotFound.class })
+    public ResponseEntity<?> handleEntityNotFoundException(PersistentEntityNotFound e, WebRequest request) {
         return ResponseEntity.status(HttpStatus.NOT_FOUND).body(new ErrorResponse(HttpStatus.NOT_FOUND, e.getMessage()));
     }
 
@@ -45,4 +45,4 @@ public ResponseEntity<?> handleScriptException(ScriptException e, WebRequest req
     }
 
 
-}
+}
diff --git a/...ain/java/edu/internet2/tier/shibboleth/admin/ui/controller/AttributeBundleController.java b/...ain/java/edu/internet2/tier/shibboleth/admin/ui/controller/AttributeBundleController.java
@@ -1,11 +1,8 @@
 package edu.internet2.tier.shibboleth.admin.ui.controller;
 
 import edu.internet2.tier.shibboleth.admin.ui.domain.AttributeBundle;
-import edu.internet2.tier.shibboleth.admin.ui.exception.EntityNotFoundException;
+import edu.internet2.tier.shibboleth.admin.ui.exception.PersistentEntityNotFound;
 import edu.internet2.tier.shibboleth.admin.ui.exception.ObjectIdExistsException;
-import edu.internet2.tier.shibboleth.admin.ui.security.exception.GroupDeleteException;
-import edu.internet2.tier.shibboleth.admin.ui.security.exception.GroupExistsConflictException;
-import edu.internet2.tier.shibboleth.admin.ui.security.model.Group;
 import edu.internet2.tier.shibboleth.admin.ui.service.AttributeBundleService;
 import io.swagger.v3.oas.annotations.tags.Tag;
 import io.swagger.v3.oas.annotations.tags.Tags;
@@ -42,7 +39,7 @@ public ResponseEntity<?> create(@RequestBody AttributeBundle bundle) throws Obje
     @Secured("ROLE_ADMIN")
     @DeleteMapping("/{resourceId}")
     @Transactional
-    public ResponseEntity<?> delete(@PathVariable String resourceId) throws EntityNotFoundException {
+    public ResponseEntity<?> delete(@PathVariable String resourceId) throws PersistentEntityNotFound {
         attributeBundleService.deleteDefinition(resourceId);
         return ResponseEntity.noContent().build();
     }
@@ -55,14 +52,14 @@ public ResponseEntity<?> getAll() {
 
     @GetMapping("/{resourceId}")
     @Transactional(readOnly = true)
-    public ResponseEntity<?> getOne(@PathVariable String resourceId) throws EntityNotFoundException {
+    public ResponseEntity<?> getOne(@PathVariable String resourceId) throws PersistentEntityNotFound {
         return ResponseEntity.ok(attributeBundleService.findByResourceId(resourceId));
     }
 
     @Secured("ROLE_ADMIN")
     @PutMapping
     @Transactional
-    public ResponseEntity<?> update(@RequestBody AttributeBundle bundle) throws EntityNotFoundException {
+    public ResponseEntity<?> update(@RequestBody AttributeBundle bundle) throws PersistentEntityNotFound {
         AttributeBundle result = attributeBundleService.updateBundle(bundle);
         return ResponseEntity.ok(result);
     }

diff --git a/...va/edu/internet2/tier/shibboleth/admin/ui/controller/AttributeBundleExceptionHandler.java b/...va/edu/internet2/tier/shibboleth/admin/ui/controller/AttributeBundleExceptionHandler.java
@@ -1,6 +1,6 @@
 package edu.internet2.tier.shibboleth.admin.ui.controller;
 
-import edu.internet2.tier.shibboleth.admin.ui.exception.EntityNotFoundException;
+import edu.internet2.tier.shibboleth.admin.ui.exception.PersistentEntityNotFound;
 import edu.internet2.tier.shibboleth.admin.ui.exception.ObjectIdExistsException;
 import org.springframework.http.HttpHeaders;
 import org.springframework.http.HttpStatus;
@@ -12,8 +12,8 @@
 
 @ControllerAdvice(assignableTypes = {AttributeBundleController.class})
 public class AttributeBundleExceptionHandler extends ResponseEntityExceptionHandler {
-    @ExceptionHandler({ EntityNotFoundException.class })
-    public ResponseEntity<?> handleEntityNotFoundException(EntityNotFoundException e, WebRequest request) {
+    @ExceptionHandler({ PersistentEntityNotFound.class })
+    public ResponseEntity<?> handleEntityNotFoundException(PersistentEntityNotFound e, WebRequest request) {
         return ResponseEntity.status(HttpStatus.NOT_FOUND).body(new ErrorResponse(HttpStatus.NOT_FOUND, e.getMessage()));
     }
 

diff --git a/...in/java/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorController.java b/...in/java/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorController.java
@@ -2,14 +2,13 @@
 
 import edu.internet2.tier.shibboleth.admin.ui.domain.EntityDescriptor;
 import edu.internet2.tier.shibboleth.admin.ui.domain.frontend.EntityDescriptorRepresentation;
-import edu.internet2.tier.shibboleth.admin.ui.exception.EntityNotFoundException;
+import edu.internet2.tier.shibboleth.admin.ui.exception.PersistentEntityNotFound;
 import edu.internet2.tier.shibboleth.admin.ui.exception.ForbiddenException;
 import edu.internet2.tier.shibboleth.admin.ui.exception.InvalidPatternMatchException;
 import edu.internet2.tier.shibboleth.admin.ui.exception.ObjectIdExistsException;
 import edu.internet2.tier.shibboleth.admin.ui.opensaml.OpenSamlObjects;
 import edu.internet2.tier.shibboleth.admin.ui.service.EntityDescriptorService;
 import edu.internet2.tier.shibboleth.admin.ui.service.EntityDescriptorVersionService;
-import io.swagger.v3.oas.annotations.Operation;
 import io.swagger.v3.oas.annotations.tags.Tag;
 import io.swagger.v3.oas.annotations.tags.Tags;
 import lombok.extern.slf4j.Slf4j;
@@ -34,7 +33,6 @@
 import javax.annotation.PostConstruct;
 import java.net.URI;
 import java.util.ConcurrentModificationException;
-import java.util.Optional;
 
 @RestController
 @RequestMapping("/api")
@@ -77,7 +75,7 @@ public ResponseEntity<?> create(@RequestBody EntityDescriptorRepresentation edRe
     @Secured("ROLE_ADMIN")
     @DeleteMapping(value = "/EntityDescriptor/{resourceId}")
     @Transactional
-    public ResponseEntity<?> deleteOne(@PathVariable String resourceId) throws ForbiddenException, EntityNotFoundException {
+    public ResponseEntity<?> deleteOne(@PathVariable String resourceId) throws ForbiddenException, PersistentEntityNotFound {
         entityDescriptorService.delete(resourceId);
         return ResponseEntity.noContent().build();
     }
@@ -90,7 +88,7 @@ public ResponseEntity<?> getAll() throws ForbiddenException {
 
     @GetMapping("/EntityDescriptor/{resourceId}/Versions")
     @Transactional
-    public ResponseEntity<?> getAllVersions(@PathVariable String resourceId) throws EntityNotFoundException, ForbiddenException {
+    public ResponseEntity<?> getAllVersions(@PathVariable String resourceId) throws PersistentEntityNotFound, ForbiddenException {
         // this "get by resource id" verifies that both the ED exists and the user has proper access, so needs to remain
         EntityDescriptor ed = entityDescriptorService.getEntityDescriptorByResourceId(resourceId);
         return ResponseEntity.ok(versionService.findVersionsForEntityDescriptor(ed.getResourceId()));
@@ -105,21 +103,22 @@ public Iterable<EntityDescriptorRepresentation> getDisabledAndNotOwnedByAdmin()
 
     @GetMapping("/EntityDescriptor/{resourceId}")
     @Transactional
-    public ResponseEntity<?> getOne(@PathVariable String resourceId) throws EntityNotFoundException, ForbiddenException {
+    public ResponseEntity<?> getOne(@PathVariable String resourceId) throws PersistentEntityNotFound, ForbiddenException {
         return ResponseEntity.ok(entityDescriptorService
                         .createRepresentationFromDescriptor(entityDescriptorService.getEntityDescriptorByResourceId(resourceId)));
     }
 
     @GetMapping(value = "/EntityDescriptor/{resourceId}", produces = "application/xml")
     @Transactional
-    public ResponseEntity<?> getOneXml(@PathVariable String resourceId) throws MarshallingException, EntityNotFoundException, ForbiddenException {
+    public ResponseEntity<?> getOneXml(@PathVariable String resourceId) throws MarshallingException, PersistentEntityNotFound, ForbiddenException {
         EntityDescriptor ed = entityDescriptorService.getEntityDescriptorByResourceId(resourceId);
         final String xml = this.openSamlObjects.marshalToXmlString(ed);
         return ResponseEntity.ok(xml);
     }
 
     @GetMapping("/EntityDescriptor/{resourceId}/Versions/{versionId}")
-    public ResponseEntity<?> getSpecificVersion(@PathVariable String resourceId, @PathVariable String versionId) throws EntityNotFoundException, ForbiddenException {
+    public ResponseEntity<?> getSpecificVersion(@PathVariable String resourceId, @PathVariable String versionId) throws
+                    PersistentEntityNotFound, ForbiddenException {
         // this "get by resource id" verifies that both the ED exists and the user has proper access, so needs to remain
         EntityDescriptor ed = entityDescriptorService.getEntityDescriptorByResourceId(resourceId);
         EntityDescriptorRepresentation result = versionService.findSpecificVersionOfEntityDescriptor(ed.getResourceId(), versionId);
@@ -146,7 +145,7 @@ public void initRestTemplate() {
     @PutMapping("/EntityDescriptor/{resourceId}")
     @Transactional
     public ResponseEntity<?> update(@RequestBody EntityDescriptorRepresentation edRepresentation, @PathVariable String resourceId)
-                    throws ForbiddenException, ConcurrentModificationException, EntityNotFoundException,
+                    throws ForbiddenException, ConcurrentModificationException, PersistentEntityNotFound,
                     InvalidPatternMatchException {
         edRepresentation.setId(resourceId); // This should be the same already, but just to be safe...
         EntityDescriptorRepresentation result = entityDescriptorService.update(edRepresentation);

diff --git a/...rnet2/tier/shibboleth/admin/ui/controller/EntityDescriptorControllerExceptionHandler.java b/...rnet2/tier/shibboleth/admin/ui/controller/EntityDescriptorControllerExceptionHandler.java
@@ -1,6 +1,6 @@
 package edu.internet2.tier.shibboleth.admin.ui.controller;
 
-import edu.internet2.tier.shibboleth.admin.ui.exception.EntityNotFoundException;
+import edu.internet2.tier.shibboleth.admin.ui.exception.PersistentEntityNotFound;
 import edu.internet2.tier.shibboleth.admin.ui.exception.ForbiddenException;
 import edu.internet2.tier.shibboleth.admin.ui.exception.InvalidPatternMatchException;
 import edu.internet2.tier.shibboleth.admin.ui.exception.ObjectIdExistsException;
@@ -22,8 +22,8 @@ public ResponseEntity<?> handleConcurrentModificationException(ConcurrentModific
         return ResponseEntity.status(HttpStatus.CONFLICT).body(new ErrorResponse(HttpStatus.CONFLICT, e.getMessage()));
     }
 
-    @ExceptionHandler({ EntityNotFoundException.class })
-    public ResponseEntity<?> handleEntityNotFoundException(EntityNotFoundException e, WebRequest request) {
+    @ExceptionHandler({ PersistentEntityNotFound.class })
+    public ResponseEntity<?> handleEntityNotFoundException(PersistentEntityNotFound e, WebRequest request) {
         return ResponseEntity.status(HttpStatus.NOT_FOUND).body(new ErrorResponse(HttpStatus.NOT_FOUND, e.getMessage()));
     }