From 4f8da1200fa479792d1bc146a03555370a05bb55 Mon Sep 17 00:00:00 2001
From: =?utf8?q?Tim=20D=C3=BCsterhus?= <duesterhus@woltlab.com>
Date: Tue, 1 Sep 2020 17:20:18 +0200
Subject: [PATCH] Support running the style update script multiple times

---
 com.woltlab.wcf/package.xml                   |  5 ++---
 .../acp/update_com.woltlab.wcf_5.3_style.php  | 21 ++++++++++++++++---
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/com.woltlab.wcf/package.xml b/com.woltlab.wcf/package.xml
index f3ea210b97..df5dbb9856 100644
--- a/com.woltlab.wcf/package.xml
+++ b/com.woltlab.wcf/package.xml
@@ -72,9 +72,6 @@
 		<instruction type="userMenu" />
 		<instruction type="page" />
 		
-		<!-- TODO: Verify that this step is idempotent! -->
-		<instruction type="script" run="standalone">acp/update_com.woltlab.wcf_5.3_style.php</instruction>
-		
 		<instruction type="script" run="standalone">acp/update_com.woltlab.wcf_5.3_emailConfirmation.php</instruction>
 		
 		<!-- The internal data structure of sitemaps have been modified, this script must be run before
@@ -93,6 +90,8 @@
 		
 		<instruction type="script" run="standalone">acp/update_com.woltlab.wcf_5.3_invalidateMailForm.php</instruction>
 		
+		<instruction type="script" run="standalone">acp/update_com.woltlab.wcf_5.3_style.php</instruction>
+		
 		<!-- This SQL step purges the legacy package servers, this must come last to make sure that an early
 		     abort of the core upgrade does not brick the installation. -->
 		<instruction type="sql">update_5.3.sql</instruction>
diff --git a/wcfsetup/install/files/acp/update_com.woltlab.wcf_5.3_style.php b/wcfsetup/install/files/acp/update_com.woltlab.wcf_5.3_style.php
index b5669d8509..58f01481c1 100644
--- a/wcfsetup/install/files/acp/update_com.woltlab.wcf_5.3_style.php
+++ b/wcfsetup/install/files/acp/update_com.woltlab.wcf_5.3_style.php
@@ -19,11 +19,26 @@ foreach ($styleList as $style) {
 	$variables = $style->getVariables();
 	$styleEditor = new StyleEditor($style);
 	
-	
 	// 1) Move existing asset path folder out of the way.
 	// It's unlikely that one exists, but having an existing folder will create a small mess.
+	$assetBackupPath = FileUtil::removeTrailingSlash($style->getAssetPath()) . '.old53/';
 	if (file_exists($style->getAssetPath())) {
-		rename($style->getAssetPath(), FileUtil::removeTrailingSlash($style->getAssetPath()) . '.old53/');
+		if (file_exists($assetBackupPath)) {
+			// If the assetBackupPath exists then we have one of the following situations.
+			// 
+			// 1) The source installation is configured pretty strangely.
+			//    This situation can be ignored due to its unlikelyness.
+			// 2) The style was successfully migrated, but the update failed at a later point.
+			//    There is no harm in skipping this style.
+			// 3) The style was unsuccessfully migrated and failed in steps 3 to 7.
+			//    To reach this situation this script has to fail twice (except in the unlikely situation that the assetPath existed previously).
+			//    It's unlikely that it succeeds this time. To skipping this style is a sane option.
+			//    Also the administrator is able to manually fix up the style later, because nothing is deleted.
+			//
+			// Concluding: This is a sufficiently reliable test to check whether this script was executed.
+			continue;
+		}
+		rename($style->getAssetPath(), $assetBackupPath);
 	}
 	
 	// 2) Create asset folder.
@@ -34,7 +49,7 @@ foreach ($styleList as $style) {
 	$srcPath = FileUtil::addTrailingSlash(WCF_DIR.$style->imagePath);
 	if ($srcPath !== WCF_DIR && $srcPath !== WCF_DIR.'images/') {
 		if ($srcPath == $style->getAssetPath()) {
-			$srcPath = FileUtil::removeTrailingSlash($style->getAssetPath()) . '.old53/';
+			$srcPath = $assetBackupPath;
 		}
 		
 		$iterator = new \RecursiveIteratorIterator(
-- 
2.20.1