[SHIBUI-1001]

Modified resolver initialization code to move shared code into a method. PUT and POST now both behave the same way with the same error message when there is a file not found exception.
TIER · Dec 3, 2018 · a350d33 · a350d33
1 parent 46526ab
commit a350d33
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 14 deletions.
diff --git a/...n/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java b/...n/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java
@@ -119,11 +119,9 @@ public ResponseEntity<?> create(@RequestBody MetadataResolver newResolver) throw
         MetadataResolver persistedResolver = resolverRepository.save(newResolver);
         positionOrderContainerService.appendPositionOrderForNew(persistedResolver);
 
-        //TODO: currently, the update call might explode, but the save works.. in which case, the UI never gets
-        // n valid response. This operation is not atomic. Should we return an error here?
-        if (persistedResolver.getDoInitialization()) {
-            org.opensaml.saml.metadata.resolver.MetadataResolver openSamlRepresentation = metadataResolverConverterService.convertToOpenSamlRepresentation(persistedResolver);
-            OpenSamlChainingMetadataResolverUtil.updateChainingMetadataResolver((OpenSamlChainingMetadataResolver) chainingMetadataResolver, openSamlRepresentation);
+        ResponseEntity initializationError = doResolverInitialization(persistedResolver);
+        if (initializationError != null) {
+            return initializationError;
         }
 
         return ResponseEntity.created(getResourceUriFor(persistedResolver)).body(persistedResolver);
@@ -151,15 +149,9 @@ public ResponseEntity<?> update(@PathVariable String resourceId, @RequestBody Me
 
         MetadataResolver persistedResolver = resolverRepository.save(updatedResolver);
 
-        if (persistedResolver.getDoInitialization()) {
-            org.opensaml.saml.metadata.resolver.MetadataResolver openSamlRepresentation = null;
-            try {
-                openSamlRepresentation = metadataResolverConverterService.convertToOpenSamlRepresentation(persistedResolver);
-            } catch (FileNotFoundException e) {
-                return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR)
-                        .body(new ErrorResponse(HttpStatus.INTERNAL_SERVER_ERROR.toString(), "label.file-doesnt-exist"));
-            }
-            OpenSamlChainingMetadataResolverUtil.updateChainingMetadataResolver((OpenSamlChainingMetadataResolver) chainingMetadataResolver, openSamlRepresentation);
+        ResponseEntity initializationError = doResolverInitialization(persistedResolver);
+        if (initializationError != null) {
+            return initializationError;
         }
 
         return ResponseEntity.ok(persistedResolver);
@@ -181,4 +173,18 @@ private static URI getResourceUriFor(MetadataResolver resolver) {
                 .build()
                 .toUri();
     }
+
+    private ResponseEntity<?> doResolverInitialization(MetadataResolver persistedResolver) throws ComponentInitializationException, ResolverException, IOException {
+        if (persistedResolver.getDoInitialization()) {
+            org.opensaml.saml.metadata.resolver.MetadataResolver openSamlRepresentation = null;
+            try {
+                openSamlRepresentation = metadataResolverConverterService.convertToOpenSamlRepresentation(persistedResolver);
+            } catch (FileNotFoundException e) {
+                return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR)
+                        .body(new ErrorResponse(HttpStatus.INTERNAL_SERVER_ERROR.toString(), "message.file-doesnt-exist"));
+            }
+            OpenSamlChainingMetadataResolverUtil.updateChainingMetadataResolver((OpenSamlChainingMetadataResolver) chainingMetadataResolver, openSamlRepresentation);
+        }
+        return null;
+    }
 }
diff --git a/backend/src/main/resources/i18n/messages.properties b/backend/src/main/resources/i18n/messages.properties
@@ -397,6 +397,7 @@ message.wizard-status=Step { index } of { length }
 message.entity-id-min-unique=You must add at least one entity id target and they must each be unique.
 message.required-for-scripts=Required for Scripts
 message.required-for-regex=Required for Regex
+message.file-doesnt-exist=The requested file to be processed does not exist on the server.
 
 tooltip.entity-id=Entity ID
 tooltip.service-provider-name=Service Provider Name (Dashboard Display Only)

diff --git a/backend/src/main/resources/i18n/messages_en.properties b/backend/src/main/resources/i18n/messages_en.properties
@@ -403,6 +403,7 @@ message.wizard-status=Step { index } of { length }
 message.entity-id-min-unique=You must add at least one entity id target and they must each be unique.
 message.required-for-scripts=Required for Scripts
 message.required-for-regex=Required for Regex
+message.file-doesnt-exist=The requested file to be processed does not exist on the server.
 
 tooltip.entity-id=Entity ID
 tooltip.service-provider-name=Service Provider Name (Dashboard Display Only)