Validate before storing, and make clear that method has side effects (storing)

2 files changed, 6 insertions, 10 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
index b70eb8d6fa7..b3158102182 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
@@ -331,13 +331,9 @@ public class ApplicationController {
                 validateRun(application.get(), zone, platformVersion, applicationVersion);
             }
 
-            validate(applicationPackage.deploymentSpec());
-
             // Update application with information from application package
-            if ( ! preferOldestVersion && ! application.get().deploymentJobs().builtInternally()) {
-                application = withUpdatedConfig(application, applicationPackage);
-                store(application); // store missing information even if we fail deployment below
-            }
+            if ( ! preferOldestVersion && ! application.get().deploymentJobs().builtInternally())
+                application = storeWithUpdatedConfig(application, applicationPackage);
 
             // Assign global rotation
             application = withRotation(application, zone);
@@ -360,8 +356,9 @@ public class ApplicationController {
     }
 
     /** Stores the deployment spec and validation overrides from the application package, and runs cleanup. */
-    public LockedApplication withUpdatedConfig(LockedApplication application, ApplicationPackage applicationPackage) {
-        // Store information about application package
+    public LockedApplication storeWithUpdatedConfig(LockedApplication application, ApplicationPackage applicationPackage) {
+        validate(applicationPackage.deploymentSpec());
+
         application = application.with(applicationPackage.deploymentSpec());
         application = application.with(applicationPackage.validationOverrides());
 
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java
index e9b0267a443..994f47ab004 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java
@@ -227,8 +227,7 @@ public class JobController {
                         });
             }
 
-            controller.applications().store(controller.applications().withUpdatedConfig(application.withBuiltInternally(true),
-                    new ApplicationPackage(applicationPackage)));
+            controller.applications().storeWithUpdatedConfig(application.withBuiltInternally(true), new ApplicationPackage(applicationPackage));
 
             notifyOfNewSubmission(id, revision, run);
         });