From 5c8e75b571a5d1e5d337213a94b64833e72c9c9c Mon Sep 17 00:00:00 2001
From: Robert Goldmann <deadlocker@gmx.de>
Date: Mon, 18 Apr 2022 10:26:49 +0200
Subject: [PATCH] #663 - show error messages on error

---
 .../migration/MigrationController.java        | 14 +++-
 .../migration/MigrationService.java           | 25 ++----
 .../budgetmaster/migration/MigrationTask.java | 57 ++++++++++++--
 .../resources/languages/base_de.properties    |  1 +
 .../resources/languages/base_en.properties    |  2 +-
 .../src/main/resources/static/js/migration.js | 26 +------
 .../resources/templates/migration/status.ftl  | 25 +-----
 .../templates/migration/statusFragment.ftl    | 76 +++++++++++++++++--
 8 files changed, 146 insertions(+), 80 deletions(-)

diff --git a/BudgetMasterServer/src/main/java/de/deadlocker8/budgetmaster/migration/MigrationController.java b/BudgetMasterServer/src/main/java/de/deadlocker8/budgetmaster/migration/MigrationController.java
index 83f3f92b5..17cc1df1c 100644
--- a/BudgetMasterServer/src/main/java/de/deadlocker8/budgetmaster/migration/MigrationController.java
+++ b/BudgetMasterServer/src/main/java/de/deadlocker8/budgetmaster/migration/MigrationController.java
@@ -105,8 +105,18 @@ public class MigrationController extends BaseController
 	@GetMapping("/getStatus")
 	public String getMigrationStatus(Model model)
 	{
-		model.addAttribute(ModelAttributes.STATUS, migrationService.getMigrationStatus());
-		model.addAttribute(ModelAttributes.SUMMARY, migrationService.getSummary());
+		final MigrationStatus migrationStatus = migrationService.getMigrationStatus();
+		model.addAttribute(ModelAttributes.STATUS, migrationStatus);
+
+		if(migrationStatus == MigrationStatus.SUCCESS)
+		{
+			model.addAttribute(ModelAttributes.SUMMARY, migrationService.getSummary());
+		}
+		if(migrationStatus == MigrationStatus.ERROR)
+		{
+			model.addAttribute(ModelAttributes.SUMMARY, migrationService.getCollectedStdout());
+		}
+
 		return ReturnValues.STATUS_FRAGMENT;
 	}
 }
\ No newline at end of file
diff --git a/BudgetMasterServer/src/main/java/de/deadlocker8/budgetmaster/migration/MigrationService.java b/BudgetMasterServer/src/main/java/de/deadlocker8/budgetmaster/migration/MigrationService.java
index c2be6deb1..2f29e4ec9 100644
--- a/BudgetMasterServer/src/main/java/de/deadlocker8/budgetmaster/migration/MigrationService.java
+++ b/BudgetMasterServer/src/main/java/de/deadlocker8/budgetmaster/migration/MigrationService.java
@@ -14,18 +14,14 @@ import org.springframework.stereotype.Service;
 
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.util.ArrayList;
 import java.util.Date;
 import java.util.List;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
 
 @Service
 public class MigrationService
 {
 	public static final String PREVIOUS_DATABASE_FILE_NAME = "budgetmaster.mv.db";
 	public static final String PREVIOUS_DATABASE_FILE_NAME_WITHOUT_EXTENSION = "budgetmaster";
-	private static final Pattern PATTERN_SUMMARY = Pattern.compile(".*(\\[COMPLETED\\] Migrate .*)");
 
 	private final SettingsService settingsService;
 	private final Path applicationSupportFolder;
@@ -126,22 +122,11 @@ public class MigrationService
 
 	public List<String> getSummary()
 	{
-		final List<String> collectedStdout = migrationTask.getCollectedStdout();
-		if(collectedStdout == null)
-		{
-			return new ArrayList<>();
-		}
-
-		final List<String> summary = new ArrayList<>();
-		for(String line : collectedStdout)
-		{
-			final Matcher matcher = PATTERN_SUMMARY.matcher(line);
-			if(matcher.find())
-			{
-				summary.add(matcher.group(1));
-			}
-		}
+		return migrationTask.getSummary();
+	}
 
-		return summary;
+	public List<String> getCollectedStdout()
+	{
+		return migrationTask.getCollectedStdout();
 	}
 }
diff --git a/BudgetMasterServer/src/main/java/de/deadlocker8/budgetmaster/migration/MigrationTask.java b/BudgetMasterServer/src/main/java/de/deadlocker8/budgetmaster/migration/MigrationTask.java
index 8f6ca06e8..7e2930372 100644
--- a/BudgetMasterServer/src/main/java/de/deadlocker8/budgetmaster/migration/MigrationTask.java
+++ b/BudgetMasterServer/src/main/java/de/deadlocker8/budgetmaster/migration/MigrationTask.java
@@ -10,18 +10,27 @@ import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.StandardCopyOption;
 import java.text.MessageFormat;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 public class MigrationTask implements Runnable
 {
 	private static final Logger LOGGER = LoggerFactory.getLogger(MigrationTask.class);
 	private static final String BUDGET_MASTER_MIGRATOR_JAR = "BudgetMasterDatabaseMigrator.jar";
+	private static final Pattern PATTERN_SUMMARY = Pattern.compile(".*(\\[COMPLETED\\] Migrate .*)");
+
+	private static final int NUMBER_OF_MIGRATION_STEPS = 25;
 
 	private final Path applicationSupportFolder;
 
 	private MigrationArguments migrationArguments;
 	private MigrationStatus migrationStatus;
 	private List<String> collectedStdout;
+	private List<String> summary;
 
 	public MigrationTask(Path applicationSupportFolder)
 	{
@@ -33,6 +42,7 @@ public class MigrationTask implements Runnable
 	{
 		this.migrationArguments = migrationArguments;
 		this.collectedStdout = new ArrayList<>();
+		this.summary = new ArrayList<>();
 	}
 
 	@Override
@@ -40,12 +50,14 @@ public class MigrationTask implements Runnable
 	{
 		if(migrationArguments == null)
 		{
-			throw new RuntimeException("No migration arguments set!");
+			throw new IllegalArgumentException("No migration arguments set!");
 		}
 
 		if(migrationStatus != MigrationStatus.NOT_RUNNING)
 		{
-			throw new RuntimeException("Migration already performed!");
+			this.migrationStatus = MigrationStatus.NOT_RUNNING;
+			this.collectedStdout = new ArrayList<>();
+			this.summary = new ArrayList<>();
 		}
 
 		LOGGER.debug("Start migration...");
@@ -63,7 +75,15 @@ public class MigrationTask implements Runnable
 				Files.deleteIfExists(migratorPath);
 			}
 
-			setMigrationStatus(MigrationStatus.SUCCESS);
+			summary = collectSummary();
+			if(summary.size() == NUMBER_OF_MIGRATION_STEPS)
+			{
+				setMigrationStatus(MigrationStatus.SUCCESS);
+			}
+			else
+			{
+				setMigrationStatus(MigrationStatus.ERROR);
+			}
 		}
 		catch(MigrationException | IOException e)
 		{
@@ -137,7 +157,6 @@ public class MigrationTask implements Runnable
 		return commandResultOptional.get().replace("\\", "/");
 	}
 
-
 	public MigrationStatus getMigrationStatus()
 	{
 		return migrationStatus;
@@ -152,4 +171,32 @@ public class MigrationTask implements Runnable
 	{
 		return collectedStdout;
 	}
+
+	public List<String> getSummary()
+	{
+		return summary;
+	}
+
+	private List<String> collectSummary()
+	{
+		if(collectedStdout == null)
+		{
+			return new ArrayList<>();
+		}
+
+		LOGGER.debug("Collecting summary...");
+
+		final List<String> matchingLines = new ArrayList<>();
+		for(String line : collectedStdout)
+		{
+			final Matcher matcher = PATTERN_SUMMARY.matcher(line);
+			if(matcher.find())
+			{
+				matchingLines.add(matcher.group(1));
+			}
+		}
+
+		LOGGER.debug("Summary collected");
+		return matchingLines;
+	}
 }
diff --git a/BudgetMasterServer/src/main/resources/languages/base_de.properties b/BudgetMasterServer/src/main/resources/languages/base_de.properties
index 0fddc3c89..7f572b75a 100644
--- a/BudgetMasterServer/src/main/resources/languages/base_de.properties
+++ b/BudgetMasterServer/src/main/resources/languages/base_de.properties
@@ -653,3 +653,4 @@ migration.status.not.running=Migration wurde nicht gestartet.
 migration.status.running=Migration läuft...
 migration.status.success=Migration fertiggestellt!
 migration.status.error=Migration fehlgeschlagen!
+migration.status.summary=Details
diff --git a/BudgetMasterServer/src/main/resources/languages/base_en.properties b/BudgetMasterServer/src/main/resources/languages/base_en.properties
index b672a0143..4b9f52138 100644
--- a/BudgetMasterServer/src/main/resources/languages/base_en.properties
+++ b/BudgetMasterServer/src/main/resources/languages/base_en.properties
@@ -652,4 +652,4 @@ migration.status.not.running=Migration is not started.
 migration.status.running=Migration is running...
 migration.status.success=Migration finished!
 migration.status.error=Migration failed!
-
+migration.status.summary=Details
diff --git a/BudgetMasterServer/src/main/resources/static/js/migration.js b/BudgetMasterServer/src/main/resources/static/js/migration.js
index dc8c9c876..93ddf3025 100644
--- a/BudgetMasterServer/src/main/resources/static/js/migration.js
+++ b/BudgetMasterServer/src/main/resources/static/js/migration.js
@@ -5,30 +5,11 @@ $(document).ready(function()
 
 function getMigrationStatus()
 {
-    if(typeof migrationStatus === 'undefined')
+    if(typeof migrationStatus !== 'undefined')
     {
-        document.getElementById('progress-spinner').style.display = 'none';
-    }
-    else
-    {
-        switch(migrationStatus)
+        if(migrationStatus === 'SUCCESS' || migrationStatus === 'ERROR')
         {
-            case 'NOT_RUNNING':
-                document.getElementById('button-migration-home').style.display = 'none';
-                document.getElementById('progress-spinner').style.display = 'none';
-                break;
-            case 'SUCCESS':
-                document.getElementById('button-migration-home').style.display = '';
-                document.getElementById('progress-spinner').style.display = 'none';
-                break;
-            case 'RUNNING':
-                document.getElementById('button-migration-home').style.display = 'none';
-                document.getElementById('progress-spinner').style.display = '';
-                break;
-            default:
-                document.getElementById('button-migration-home').style.display = 'none';
-                document.getElementById('progress-spinner').style.display = 'none';
-                return;
+            return;
         }
     }
 
@@ -39,6 +20,7 @@ function getMigrationStatus()
         success: function(data)
         {
             $('#migration-status').html(data);
+            $('.collapsible').collapsible();
         },
         complete: function()
         {
diff --git a/BudgetMasterServer/src/main/resources/templates/migration/status.ftl b/BudgetMasterServer/src/main/resources/templates/migration/status.ftl
index f45365e36..20dbff686 100644
--- a/BudgetMasterServer/src/main/resources/templates/migration/status.ftl
+++ b/BudgetMasterServer/src/main/resources/templates/migration/status.ftl
@@ -4,6 +4,7 @@
         <@header.globals/>
         <@header.header "BudgetMaster - ${locale.getString('title.migration')}"/>
         <#import "/spring.ftl" as s>
+        <@header.style "collapsible"/>
     </head>
     <@header.body>
         <#import "../helpers/navbar.ftl" as navbar>
@@ -19,31 +20,7 @@
                 </div>
 
                 <div class="container center-align">
-                    <div class="row">
-                        <div class="col s12 m12 l8 offset-l2">
-                            <div class="preloader-wrapper small active" id="progress-spinner">
-                                <div class="spinner-layer spinner-blue-only">
-                                    <div class="circle-clipper left">
-                                        <div class="circle"></div>
-                                    </div>
-                                    <div class="gap-patch">
-                                        <div class="circle"></div>
-                                    </div>
-                                    <div class="circle-clipper right">
-                                        <div class="circle"></div>
-                                    </div>
-                                </div>
-                            </div>
-                        </div>
-                    </div>
-
                     <div id="migration-status" data-url="<@s.url '/migration/getStatus'/>"></div>
-
-                    <div class="row" id="button-migration-home">
-                        <div class="col s12 m12 l8 offset-l2">
-                            <@header.buttonLink url='/' icon='home' localizationKey='menu.home'/>
-                        </div>
-                    </div>
                 </div>
             </div>
         </main>
diff --git a/BudgetMasterServer/src/main/resources/templates/migration/statusFragment.ftl b/BudgetMasterServer/src/main/resources/templates/migration/statusFragment.ftl
index 830cbed60..0893d2e0f 100644
--- a/BudgetMasterServer/src/main/resources/templates/migration/statusFragment.ftl
+++ b/BudgetMasterServer/src/main/resources/templates/migration/statusFragment.ftl
@@ -8,13 +8,77 @@
     </div>
 </div>
 
-<div class="row left-align">
-    <div class="col s12 m12 l8 offset-l2">
-        <#list summary as summaryLine>
-            ${summaryLine}<br>
-        </#list>
+<#if status.name() == "RUNNING">
+    <div class="row">
+        <div class="col s12 m12 l8 offset-l2">
+            <div class="preloader-wrapper small active" id="progress-spinner">
+                <div class="spinner-layer spinner-blue-only">
+                    <div class="circle-clipper left">
+                        <div class="circle"></div>
+                    </div>
+                    <div class="gap-patch">
+                        <div class="circle"></div>
+                    </div>
+                    <div class="circle-clipper right">
+                        <div class="circle"></div>
+                    </div>
+                </div>
+            </div>
+        </div>
     </div>
-</div>
+</#if>
+
+<#if status.name() == "SUCCESS">
+    <div class="row">
+        <div class="col s12">
+            <ul class="collapsible">
+                <li>
+                    <div class="collapsible-header bold">
+                        <i class="fas fa-info"></i>
+                        ${locale.getString("migration.status.summary")}
+                    </div>
+                    <div class="collapsible-body">
+                        <table class="bordered">
+                            <#list summary as summaryLine>
+                                <tr>
+                                    <td>${summaryLine}</td>
+                                </tr>
+                            </#list>
+                        </table>
+                    </div>
+                </li>
+            </ul>
+        </div>
+    </div>
+
+    <div class="row" id="button-migration-home">
+        <div class="col s12 m12 l8 offset-l2">
+            <@header.buttonLink url='/' icon='home' localizationKey='menu.home'/>
+        </div>
+    </div>
+</#if>
+
+<#if status.name() == "ERROR">
+    <div class="row">
+        <div class="col s12">
+            <ul class="collapsible">
+                <li>
+                    <div class="collapsible-header bold">
+                        <i class="${status.getIcon()} ${status.getTextColor()}"></i>
+                        ${locale.getString("migration.status.summary")}
+                    </div>
+                    <div class="collapsible-body">
+                        <p class="left-align">
+                            <#list summary as summaryLine>
+                                ${summaryLine}<br>
+                            </#list>
+                        </p>
+                    </div>
+                </li>
+            </ul>
+        </div>
+    </div>
+</#if>
 
 <script>
     migrationStatus = "${status.name()}";
-- 
GitLab