From 4f1d0adc4d4c0141388d765eafcf30f994326417 Mon Sep 17 00:00:00 2001 From: Gethin James Date: Tue, 14 Feb 2012 10:50:04 +0000 Subject: [PATCH] FIXED : ALF-12777: MMT should not install AMPs which override pre-existing files in the war file, unless -force is provided The MMT is moving toward more of a validation phase (checks things, calculate changes) then an execution phase (makes the changes). git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@33880 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- .../module/tool/ModuleManagementTool.java | 244 ++++++++++-------- .../module/tool/ModuleManagementToolTest.java | 40 +-- source/test-resources/module/test.war | Bin 9022 -> 8917 bytes source/test-resources/module/test_v4.amp | Bin 0 -> 4978 bytes 4 files changed, 166 insertions(+), 118 deletions(-) create mode 100644 source/test-resources/module/test_v4.amp diff --git a/source/java/org/alfresco/repo/module/tool/ModuleManagementTool.java b/source/java/org/alfresco/repo/module/tool/ModuleManagementTool.java index 0526f88466..21580fb38f 100644 --- a/source/java/org/alfresco/repo/module/tool/ModuleManagementTool.java +++ b/source/java/org/alfresco/repo/module/tool/ModuleManagementTool.java @@ -26,6 +26,7 @@ import java.io.InputStream; import java.io.OutputStream; import java.util.Date; import java.util.Map; +import java.util.Map.Entry; import java.util.Properties; import org.alfresco.error.AlfrescoRuntimeException; @@ -196,7 +197,7 @@ public class ModuleManagementTool implements LogOutput * @param warFileLocation the location of the WAR file into which the AMP file is to be installed. * @param preview indicates whether this should be a preview install. This means that the process of * installation will be followed and reported, but the WAR file will not be modified. - * @param forceInstall indicates whether the installed files will be replaces reguarless of the currently installed + * @param forceInstall indicates whether the installed files will be replaces regardless of the currently installed * version of the AMP. Generally used during development of the AMP. * @param backupWAR indicates whether we should backup the war we are modifying or not */ @@ -240,105 +241,29 @@ public class ModuleManagementTool implements LogOutput // Try to find an installed module by the ID ModuleDetails installedModuleDetails = warHelper.getModuleDetailsOrAlias(theWar, installingModuleDetails); - // Now clean up the old instance - if (installedModuleDetails != null) - { - String installedId = installedModuleDetails.getId(); - VersionNumber installedVersion = installedModuleDetails.getVersion(); - - int compareValue = installedVersion.compareTo(installingVersion); - if (compareValue > 0) - { - // Trying to install an earlier version of the extension - outputMessage("WARNING: A later version of this module is already installed in the WAR. Installation skipped. "+ - "You could force the installation by passing the -force option.",false); - return; - } - - if (forceInstall == true) - { - // Warn of forced install - outputMessage("WARNING: The installation of this module is being forced. All files will be removed and replaced regardless of exiting versions present.",false); - } - - if (compareValue == 0) - { - // Trying to install the same extension version again - outputMessage("WARNING: This version of this module is already installed in the WAR..upgrading.",false); - } - - if (forceInstall == true || compareValue <= 0) - { - - // Trying to update the extension, old files need to cleaned before we proceed - outputMessage("Clearing out files relating to version '" + installedVersion + "' of module '" + installedId + "'",false); - uninstallModule(installedId, warFileLocation, preview, true); - } - } + uninstallIfNecessary(warFileLocation, installedModuleDetails, preview, forceInstall, installingVersion); - // Check if a custom mapping file has been defined - Properties fileMappingProperties = null; - Properties customFileMappingProperties = getCustomFileMappings(ampFileLocation); - if (customFileMappingProperties == null) - { - fileMappingProperties = defaultFileMappingProperties; - } - else - { - fileMappingProperties = new Properties(); - // A custom mapping file was present. Check if it must inherit the default mappings. - String inheritDefaultStr = customFileMappingProperties.getProperty(PROP_INHERIT_DEFAULT, "true"); - if (inheritDefaultStr.equalsIgnoreCase("true")) - { - fileMappingProperties.putAll(defaultFileMappingProperties); - } - fileMappingProperties.putAll(customFileMappingProperties); - fileMappingProperties.remove(PROP_INHERIT_DEFAULT); - } - - // Copy the files from the AMP file into the WAR file outputMessage("Adding files relating to version '" + installingVersion + "' of module '" + installingId + "'"); InstalledFiles installedFiles = new InstalledFiles(warFileLocation, installingId); - for (Map.Entry entry : fileMappingProperties.entrySet()) - { - // The file mappings are expected to start with "/" - String mappingSource = (String) entry.getKey(); - if (mappingSource.length() == 0 || !mappingSource.startsWith("/")) - { - throw new AlfrescoRuntimeException("File mapping sources must start with '/', but was: " + mappingSource); - } - String mappingTarget = (String) entry.getValue(); - if (mappingTarget.length() == 0 || !mappingTarget.startsWith("/")) - { - throw new AlfrescoRuntimeException("File mapping targets must start with '/' but was '" + mappingTarget + "'"); - } - - mappingSource = mappingSource.trim(); //trim whitespace - mappingTarget = mappingTarget.trim(); //trim whitespace - - // Run throught the files one by one figuring out what we are going to do during the copy - copyToWar(ampFileLocation, warFileLocation, mappingSource, mappingTarget, installedFiles, preview); - - if (preview == false) - { - // Get a reference to the source folder (if it isn't present don't do anything) - File source = new File(ampFileLocation + "/" + mappingSource, DETECTOR_AMP_AND_WAR); - if (source != null && source.list() != null) - { - // Get a reference to the destination folder - File destination = new File(warFileLocation + "/" + mappingTarget, DETECTOR_AMP_AND_WAR); - if (destination == null) - { - throw new ModuleManagementToolException("The destination folder '" + mappingTarget + "' as specified in mapping properties does not exist in the war"); - } - // Do the bulk copy since this is quicker than copying files one by one - destination.copyAllFrom(source); - } - } - } + + Properties directoryChanges = calculateChanges(ampFileLocation, warFileLocation, preview, forceInstall, installedFiles); if (preview == false) { + //Now actually do the changes + if (directoryChanges != null && directoryChanges.size() > 0) + { + for (Entry entry : directoryChanges.entrySet()) + { + + File destination = new File((String) entry.getValue(), DETECTOR_AMP_AND_WAR); + File source = new File((String) entry.getKey(), DETECTOR_AMP_AND_WAR); + //Do the bulk copy since this is quicker than copying files one by one + //The changes aren't actuall "committed" until the File.update() is called (below) + destination.copyAllFrom(source); + } + } + // Save the installed file list installedFiles.save(); @@ -375,6 +300,109 @@ public class ModuleManagementTool implements LogOutput } } + private void uninstallIfNecessary(String warFileLocation, ModuleDetails installedModuleDetails, boolean preview, + boolean forceInstall, VersionNumber installingVersion) + { + // Now clean up the old instance + if (installedModuleDetails != null) + { + String installedId = installedModuleDetails.getId(); + VersionNumber installedVersion = installedModuleDetails.getVersion(); + + int compareValue = installedVersion.compareTo(installingVersion); + if (compareValue > 0) + { + // Trying to install an earlier version of the extension + outputMessage("WARNING: A later version of this module is already installed in the WAR. Installation skipped. "+ + "You could force the installation by passing the -force option.",false); + return; + } + + if (forceInstall == true) + { + // Warn of forced install + outputMessage("WARNING: The installation of this module is being forced. All files will be removed and replaced regardless of exiting versions present.",false); + } + + if (compareValue == 0) + { + // Trying to install the same extension version again + outputMessage("WARNING: This version of this module is already installed in the WAR..upgrading.",false); + } + + if (forceInstall == true || compareValue <= 0) + { + + // Trying to update the extension, old files need to cleaned before we proceed + outputMessage("Clearing out files relating to version '" + installedVersion + "' of module '" + installedId + "'",false); + uninstallModule(installedId, warFileLocation, preview, true); + } + } + } + + /** + */ + private Properties calculateChanges(String ampFileLocation, String warFileLocation, boolean preview, + boolean forceInstall, InstalledFiles installedFiles) throws IOException + { + Properties dirChanges = new Properties(); + + // Check if a custom mapping file has been defined + Properties fileMappingProperties = null; + Properties customFileMappingProperties = getCustomFileMappings(ampFileLocation); + if (customFileMappingProperties == null) + { + fileMappingProperties = defaultFileMappingProperties; + } + else + { + fileMappingProperties = new Properties(); + // A custom mapping file was present. Check if it must inherit the default mappings. + String inheritDefaultStr = customFileMappingProperties.getProperty(PROP_INHERIT_DEFAULT, "true"); + if (inheritDefaultStr.equalsIgnoreCase("true")) + { + fileMappingProperties.putAll(defaultFileMappingProperties); + } + fileMappingProperties.putAll(customFileMappingProperties); + fileMappingProperties.remove(PROP_INHERIT_DEFAULT); + } + + // Copy the files from the AMP file into the WAR file + for (Map.Entry entry : fileMappingProperties.entrySet()) + { + // The file mappings are expected to start with "/" + String mappingSource = (String) entry.getKey(); + if (mappingSource.length() == 0 || !mappingSource.startsWith("/")) + { + throw new AlfrescoRuntimeException("File mapping sources must start with '/', but was: " + mappingSource); + } + String mappingTarget = (String) entry.getValue(); + if (mappingTarget.length() == 0 || !mappingTarget.startsWith("/")) + { + throw new AlfrescoRuntimeException("File mapping targets must start with '/' but was '" + mappingTarget + "'"); + } + + mappingSource = mappingSource.trim(); //trim whitespace + mappingTarget = mappingTarget.trim(); //trim whitespace + + // Run throught the files one by one figuring out what we are going to do during the copy + calculateCopyToWar(ampFileLocation, warFileLocation, mappingSource, mappingTarget, installedFiles, preview, forceInstall); + + // Get a reference to the source folder (if it isn't present don't do anything) + File source = new File(ampFileLocation + "/" + mappingSource, DETECTOR_AMP_AND_WAR); + if (source != null && source.list() != null) + { + // Add to the list of directory changes so we can implement the changes later. + String sourceDir = ampFileLocation + mappingSource; + String destinationDir = warFileLocation + mappingTarget; + dirChanges.put(sourceDir, destinationDir); + } + + } + + return dirChanges; + } + private void backupWar(String warFileLocation, boolean backupWAR) { @@ -511,9 +539,11 @@ public class ModuleManagementTool implements LogOutput * @param destinationDir the directory in the WAR to copy to. It must start with "/". * @param installedFiles a list of the currently installed files * @param preview indicates whether this is a preview install or not + * @param forceInstall indicates whether the installed files will be replaces regardless of the currently installed + * version of the AMP. * @throws IOException throws any IOExpceptions thar are raised */ - private void copyToWar(String ampFileLocation, String warFileLocation, String sourceDir, String destinationDir, InstalledFiles installedFiles, boolean preview) + private void calculateCopyToWar(String ampFileLocation, String warFileLocation, String sourceDir, String destinationDir, InstalledFiles installedFiles, boolean preview, boolean forceInstall) throws IOException { if (sourceDir.length() == 0 || !sourceDir.startsWith("/")) @@ -555,12 +585,22 @@ public class ModuleManagementTool implements LogOutput } else { - // Backup file about to be updated - backupLocation = BACKUP_DIR + "/" + generateGuid() + ".bin"; - if (preview == false) + if (forceInstall) { - File backupFile = new File(warFileLocation + backupLocation, DETECTOR_AMP_AND_WAR); - backupFile.copyFrom(destinationChild); + // Backup file about to be updated + backupLocation = BACKUP_DIR + "/" + generateGuid() + ".bin"; + if (preview == false) + { + File backupFile = new File(warFileLocation + backupLocation, DETECTOR_AMP_AND_WAR); + backupFile.copyFrom(destinationChild); + } + } else { + //Not a forced install, there is an existing file in the war, lets rollback the transaction, + //throw an error and explain the problem. +// File. +// ZipController zipController = ZipController.getInstance(warFile); +// zipController.reset(); + throw new ModuleManagementToolException("ERROR: The amp will overwrite an existing file in the war '" + destinationDir + "/" + sourceChild.getName() + "'. Execution halted. By specifying -force , you can force installation of AMP regardless of the current war state."); } } @@ -583,8 +623,8 @@ public class ModuleManagementTool implements LogOutput mkdir = true; } - copyToWar(ampFileLocation, warFileLocation, sourceDir + "/" + sourceChild.getName(), - destinationDir + "/" + sourceChild.getName(), installedFiles, preview); + calculateCopyToWar(ampFileLocation, warFileLocation, sourceDir + "/" + sourceChild.getName(), + destinationDir + "/" + sourceChild.getName(), installedFiles, preview, forceInstall); if (mkdir == true) { installedFiles.addMkdir(destinationDir + "/" + sourceChild.getName()); diff --git a/source/java/org/alfresco/repo/module/tool/ModuleManagementToolTest.java b/source/java/org/alfresco/repo/module/tool/ModuleManagementToolTest.java index 70751c2ca4..52957f09b3 100644 --- a/source/java/org/alfresco/repo/module/tool/ModuleManagementToolTest.java +++ b/source/java/org/alfresco/repo/module/tool/ModuleManagementToolTest.java @@ -81,19 +81,9 @@ public class ModuleManagementToolTest extends TestCase InstalledFiles installed0 = new InstalledFiles(warLocation, "test"); installed0.load(); assertNotNull(installed0); - assertEquals(8, installed0.getAdds().size()); + assertEquals(9, installed0.getAdds().size()); assertEquals(1, installed0.getMkdirs().size()); - assertEquals(1, installed0.getUpdates().size()); - String backup = null; - String orig = null; - for (Map.Entry update : installed0.getUpdates().entrySet()) - { - checkContentsOfFile(warLocation + update.getKey(), "VERSIONONE"); - checkContentsOfFile(warLocation + update.getValue(), "ORIGIONAL"); - backup = update.getValue(); - orig = update.getKey(); - } - + // Try and install same version try { @@ -129,7 +119,6 @@ public class ModuleManagementToolTest extends TestCase files3.add("/scripts/test.js"); files3.add("/jsp/test.jsp"); files3.add("/extra.txt"); - files3.add(backup); checkForFileNonExistance(warLocation, files3); // Check the intstalled files @@ -139,9 +128,6 @@ public class ModuleManagementToolTest extends TestCase assertEquals(8, installed1.getAdds().size()); assertEquals(1, installed1.getMkdirs().size()); assertEquals(0, installed1.getUpdates().size()); - - // Ensure the file has been reverted as it isnt updated in the v2.0 - checkContentsOfFile(warLocation + orig, "ORIGIONAL"); /** * Try and install an earlier version over a later version @@ -269,6 +255,28 @@ public class ModuleManagementToolTest extends TestCase } } + public void testExistingFilesInWar() throws Exception + { + manager.setVerbose(true); + + String warLocation = getFileLocation(".war", "module/test.war"); + String ampLocation = getFileLocation(".amp", "module/test_v4.amp"); + + try + { + this.manager.installModule(ampLocation, warLocation, false, false, true); + } + catch(ModuleManagementToolException exception) + { + assertTrue(exception.getMessage().contains("will overwrite an existing file in the war")); + } + + this.manager.installModule(ampLocation, warLocation, false, true, true); //Now force it + checkContentsOfFile(warLocation + "/jsp/relogin.jsp", "VERSIONONE"); + checkContentsOfFile(warLocation + "/css/main.css", "p{margin-bottom:1em;}"); + + } + public void testWhiteSpaceInCustomMapping() throws Exception { diff --git a/source/test-resources/module/test.war b/source/test-resources/module/test.war index 803d4244813af421005b50611d70960e6257091b..1c723ba0539b84edc8194f9dc2178dee068169e5 100644 GIT binary patch delta 80 zcmdnzcGY#m6XDIGA}zd=FDj}`-mAzl`LCiWa{uo8exCb&?)&#R*>Up- za>Ia8bi6@86qvw{%faIdZ=B?(!patp;ftvo=@Z^%J61W{uI0>S;@9 zyQ*ZZ=UK5-G_&PCYw4YrDw##`+*FHC5l?(jGqQ&1Bhx1plpPu7QLaMBk<|5 zqmz9J=eNfQogsQEa1;0$9dohNX~tW&--J3V)IqO}i>Z3Rb(BR;GDxM}ns8 zw7L>;aKW+9rQJNIpSDsiS1oz9Yk2m(fQ_Mijo9UVuUsvIRtK8)-a9(^v$bf=9-d5# z!v+~0<6W*-_uowp=1(k^H;E~ym-BQGq$Iqrcxww>_ST#{PqTMc6%@5%M4g*Y2%A?z zpw)uzSt}28T;98zzelENOr^RG?_{Iph95q0yQ%lkinxKSdV$)w!-%9FmQuX%)8r$@ zNQi}!$0C!+C9BHHp(try5f)RhQ@B|f$F;(3?ItFMkMJ$c|R0Sawiim z1lab$iE6rU;v!GR#u>*|qnbTK-~X0sTWLMs_V~8&x$nm0aPy+ij+_3e`z_ZS?o9 z8CMx&dNoZR2L%pB-;hnKuJN#JT8SAYPdoZoTYgpe4I2?F7TqOZSSono&|FvQ+5T#`qm+Z}NRbmRf9oZVy<>kH$u1xpZpR``#8ZFm33x zEM_R4jdNg@Ak&-QJ$FZzgzLP3rNN`KjT3%$M&3y!)xIp3pPi@XMwZ42r^oLTmh*Jn zc`PfUcJzmooKwFoCJ!8*e6ekestAkH;B~=;u7^H-Btf=5>Dihx-jyOGIN5I2RHk)A zM%%MhnxuA@#BUO&n4R-}&`La5l?Tst6YmtVA6FEIbUIL^diHgPd69>NqutZVT}`XO z2Qzh6d4|iKzBVZ-HGa-4IOe(1V3lxNv~pQFniL+8Rdi4`qRv02FDpSctOfBp;SJd* zM_&Q?c-!TR1!a7~U(U;LD|ndPSY6Se5*ssQleRtEzSzLq8c1HGa|M1?g1XrF;zhGh zPerd_`F+))mI8j0ii-UPg&B#J4aTKKO@rBa(lDgDutjfUzEA@Fp%|Q|T8WT($DAag z4X^cjPduzG^q3E7D<0js5{ulKqoR9uhKrqo8+)3-hXnJ-63AFGN6TRVc^D<4kqCA( za7uEDh2lQQYf+*o)|eAm1nXN^P*{*%HnRP6^hwgfOm_fz3Yg-+gxU*7!7xo>3Q)Dh zc-Bvw?phZ0GP9k{6lpol*KxM$`RYFBxxW5vnyOjqx4Z7~s9zR(B4aG#`>Wx}Q^~oh zD5e1pZ}0mbiiB3<%3BPd`}Bbb`AQ|Pgoaj*`?hhj;C+k4LicU+7SGR;@Fzp30hPey zBMxRr^Q?4B;J+t;37!b32b98Na0D%2fqz;mNJWUj+R?9Fw>!wy)!eFW9yaN?Cb^D& zVl$h%N3wckygDW+yPeu_=NrXlyId^k#TUJ8mqId5+M3Q~xh(Wqeki(-Uo`k7{T1v$ zggRgeoWd-NKvyPVz=Jm@AU>=SU@1mWhzL9}0*i>iVGtOEe>j;!#3NYsdF$8^d4q5bAW;r7+;@w(^&lzTsbNJ#uy5GqDtLH*9AjzGEZM@1jU&ed68(X+=%NV?DP zmB)DK-Snf-PWXvW9p7&?J87e8!C#6{;w3Pi?|-xwMXWiJ^z55d-ujF+p2>7PTKJ}J zo%*|}bgzuO64=38HoB3&ILH-Tx-3hPv}2n~lvbN){v?}}J+ov~&ykvhU4QSCsw(|Nvy{n89xY}|jTadSJipp~a6$*SXVCUP&muic0AP^Pmeq`sq~F zSbqhEltQy8Q|Fs_CGFMj2G>@@UCTaP52T*i>|ECnsz=O*l&)V(J;NM0 z9zQG5_C>eG*0K7Sm?7jaj%&I|($!y!7C9ye1PGiT0|iQvu%W~tAUA=vhn0(~jiZC3 zgBAOJZjABAoCcv#fXQM12KfIMV6fAJHHGX?!i7;dtIx(reGKwPNmjJ}4c7lYSX|pA zxi@bV;9C$HV$^kWaP{eBxHSm8W6%IWyMZMO#PL$x8VwR}Xn<%}V7U?f>E*b!67cN^ z4NyrEu-rlkm{mZf0X1#?mV`7RsD%l2f1~NIFH26{o4qmEx_N;Xw8e`9QUZl&Gj)>p&`5Y*Yeh!)4oU5^G4Cr@1hXPig2>YDVU>AZ0=rkJm h{BuwGr{{Hk1CnLi$qo+WI1DBQ{JFpZ&LRSS{TEzD!NC9k literal 0 HcmV?d00001