From db16db989336e58eaa76963e77ce324587f7f43c Mon Sep 17 00:00:00 2001 From: Lluis Sanchez Date: Fri, 16 Sep 2022 14:16:24 +0200 Subject: Fix add-in downgrade issue In some cases when add-ins are downgraded the add-in database may be left in an invalid status. This happened because the add-in scanner did not properly compare the versions of the old and the new add-in, so it did not uninstall the old add-in when versions were different. This resulted on the old and new descriptions of the add-in to be kept in the database, and it might generate add-in dependency errors. This also happened when upgrading, but since the engine always loads add-ins with the highest version it doesn't matter if there is one with lower version also registered. The solution is to cache information about the old add-in version, and when comparing ids, use that information. Added unit tests. --- .../Mono.Addins.Database/AddinRegistryUpdater.cs | 69 +++++++++++++++---- .../Mono.Addins.Database/AddinScanFolderInfo.cs | 17 ++++- .../Mono.Addins.Database/AddinScanResult.cs | 4 +- Mono.Addins/Mono.Addins.Database/AddinScanner.cs | 8 +-- .../Mono.Addins.Description/ModuleDescription.cs | 2 +- Test/UnitTests/TestScanDataFileGeneration.cs | 77 +++++++++++++++++++++- 6 files changed, 156 insertions(+), 21 deletions(-) diff --git a/Mono.Addins/Mono.Addins.Database/AddinRegistryUpdater.cs b/Mono.Addins/Mono.Addins.Database/AddinRegistryUpdater.cs index 517a348..760c4e8 100644 --- a/Mono.Addins/Mono.Addins.Database/AddinRegistryUpdater.cs +++ b/Mono.Addins/Mono.Addins.Database/AddinRegistryUpdater.cs @@ -25,6 +25,7 @@ // THE SOFTWARE. using System; using System.IO; +using System.Linq; namespace Mono.Addins.Database { @@ -34,6 +35,10 @@ namespace Mono.Addins.Database AddinScanFolderInfo currentFolderInfo; AddinScanResult scanResult; + // Folder info object that already existed. It is usually the same as currentFolderInfo, since + // folder info objects are reused, but not always. + AddinScanFolderInfo oldFolderInfo; + public AddinRegistryUpdater (AddinDatabase database, AddinScanResult scanResult): base (database) { this.database = database; @@ -71,6 +76,7 @@ namespace Mono.Addins.Database bool sharedFolder = domain == AddinDatabase.GlobalDomain; bool isNewFolder = folderInfo == null; bool folderHasIndex = dirScanDataIndex != null; + bool oldFolderInfoWasSet = false; if (isNewFolder) { // No folder info. It is the first time this folder is scanned. @@ -81,8 +87,14 @@ namespace Mono.Addins.Database } else if (folderInfo.FolderHasScanDataIndex != folderHasIndex) { // A scan data index appeared or disappeared. The information in folderInfo is not reliable. // Update the folder info and regenerate everything. + + // Keep a copy of the old folder info, to be used to retrieve the old status of the add-ins. + oldFolderInfo = folderInfo; + oldFolderInfoWasSet = true; + folderInfo = new AddinScanFolderInfo (oldFolderInfo); scanResult.RegenerateRelationData = true; folderInfo.Reset (); + scanResult.RegisterModifiedFolderInfo (folderInfo); folderInfo.FolderHasScanDataIndex = folderHasIndex; } @@ -119,14 +131,44 @@ namespace Mono.Addins.Database if (monitor.LogLevel > 1) monitor.Log ("Checking: " + path); + currentFolderInfo = folderInfo; + if (dirScanDataIndex != null) { // Instead of scanning the folder, just register the files in the index - foreach (var file in dirScanDataIndex.Files) - RegisterFileToScan (monitor, file.FileName, folderInfo, file); + if (oldFolderInfo != null && !oldFolderInfo.FolderHasScanDataIndex) + { + // There was no scan index in the previous scan but there is one in this scan. + // The old folder info doesn't contain the info for all files, just for the files in this folder. + // Since the new scan has an index, it can contain references to files not directly in this folder, + // so for those files we need to find their corresponding old folder info. + + // We group by folder so that we only need to query for folderInfo once per folder + foreach (var folder in dirScanDataIndex.Files.GroupBy(f => Path.GetDirectoryName(f.FileName))) + { + AddinScanFolderInfo oldFolderInfoForIncludedFolder; + if (folder.Key != path) + { + // The file does not belong to this folder, so we need to get the folderInfo from + // the right folder + database.GetFolderInfoForPath(monitor, folder.Key, out oldFolderInfoForIncludedFolder); + } + else + { + // The file belongs to the folder being visited, so oldFolderInfo is correct + oldFolderInfoForIncludedFolder = oldFolderInfo; + } + foreach(var file in folder) + RegisterFileToScan(monitor, file.FileName, file, oldFolderInfoForIncludedFolder); + } + } + else + { + foreach (var file in dirScanDataIndex.Files) + RegisterFileToScan(monitor, file.FileName, file, oldFolderInfo); + } foreach (var file in dirScanDataIndex.Assemblies) scanResult.AssemblyIndex.AddAssemblyLocation (file); } else { - currentFolderInfo = folderInfo; base.OnVisitFolder (monitor, path, domain, recursive); @@ -136,23 +178,26 @@ namespace Mono.Addins.Database scanResult.ChangesFound = true; if (scanResult.CheckOnly) return; - database.DeleteFolderInfo (monitor, folderInfo); + database.DeleteFolderInfo (monitor, oldFolderInfo ?? currentFolderInfo); } } // Look for deleted add-ins. - UpdateDeletedAddins (monitor, folderInfo); + UpdateDeletedAddins (monitor, oldFolderInfo ?? currentFolderInfo); + + if (oldFolderInfoWasSet) + oldFolderInfo = null; } protected override void OnVisitAddinManifestFile (IProgressStatus monitor, string file) { - RegisterFileToScan (monitor, file, currentFolderInfo, null); + RegisterFileToScan (monitor, file, null, oldFolderInfo); } protected override void OnVisitAssemblyFile (IProgressStatus monitor, string file) { - RegisterFileToScan (monitor, file, currentFolderInfo, null); + RegisterFileToScan (monitor, file, null, oldFolderInfo); scanResult.AssemblyIndex.AddAssemblyLocation (file); } @@ -172,17 +217,17 @@ namespace Mono.Addins.Database } } - void RegisterFileToScan (IProgressStatus monitor, string file, AddinScanFolderInfo folderInfo, AddinScanData scanData) + void RegisterFileToScan (IProgressStatus monitor, string file, AddinScanData scanData, AddinScanFolderInfo oldFolderInfo) { - AddinFileInfo finfo = folderInfo.GetAddinFileInfo (file); + AddinFileInfo finfo = (oldFolderInfo ?? currentFolderInfo).GetAddinFileInfo (file); bool added = false; - if (finfo != null && (!finfo.IsAddin || finfo.Domain == folderInfo.GetDomain (finfo.IsRoot)) && !finfo.HasChanged (FileSystem, scanData?.MD5) && !scanResult.RegenerateAllData) { + if (finfo != null && (!finfo.IsAddin || finfo.Domain == currentFolderInfo.GetDomain (finfo.IsRoot)) && !finfo.HasChanged (FileSystem, scanData?.MD5) && !scanResult.RegenerateAllData) { if (finfo.ScanError) { // Always schedule the file for scan if there was an error in a previous scan. // However, don't set ChangesFound=true, in this way if there isn't any other // change in the registry, the file won't be scanned again. - scanResult.AddFileToScan (file, folderInfo, scanData); + scanResult.AddFileToScan (file, currentFolderInfo, finfo, scanData); added = true; } @@ -201,7 +246,7 @@ namespace Mono.Addins.Database scanResult.ChangesFound = true; if (!scanResult.CheckOnly && !added) - scanResult.AddFileToScan (file, folderInfo, scanData); + scanResult.AddFileToScan (file, currentFolderInfo, finfo, scanData); } } } diff --git a/Mono.Addins/Mono.Addins.Database/AddinScanFolderInfo.cs b/Mono.Addins/Mono.Addins.Database/AddinScanFolderInfo.cs index 944922d..a3a8900 100644 --- a/Mono.Addins/Mono.Addins.Database/AddinScanFolderInfo.cs +++ b/Mono.Addins/Mono.Addins.Database/AddinScanFolderInfo.cs @@ -58,6 +58,16 @@ namespace Mono.Addins.Database this.folder = folder; } + public AddinScanFolderInfo (AddinScanFolderInfo other) + { + files = new Hashtable (other.files); + folder = other.folder; + fileName = other.fileName; + domain = other.domain; + sharedFolder = other.sharedFolder; + FolderHasScanDataIndex = other.FolderHasScanDataIndex; + } + public string FileName { get { return fileName; } } @@ -269,8 +279,13 @@ namespace Mono.Addins.Database public bool HasChanged (AddinFileSystemExtension fs, string md5) { - if (md5 != null && ScanDataMD5 != null) + // Special case: if an md5 is stored, this method can only return a valid result + // if compared with another md5. If no md5 is provided for comparison, then always consider + // the file to be changed. + + if (ScanDataMD5 != null) return md5 != ScanDataMD5; + return fs.GetLastWriteTime (File) != LastScan; } diff --git a/Mono.Addins/Mono.Addins.Database/AddinScanResult.cs b/Mono.Addins/Mono.Addins.Database/AddinScanResult.cs index 85b02e2..1917ce3 100644 --- a/Mono.Addins/Mono.Addins.Database/AddinScanResult.cs +++ b/Mono.Addins/Mono.Addins.Database/AddinScanResult.cs @@ -86,11 +86,12 @@ namespace Mono.Addins.Database RemovedAddins.Add (addinId); } - public void AddFileToScan (string file, AddinScanFolderInfo folderInfo, AddinScanData scanData) + public void AddFileToScan (string file, AddinScanFolderInfo folderInfo, AddinFileInfo oldFileInfo, AddinScanData scanData) { FileToScan di = new FileToScan (); di.File = file; di.AddinScanFolderInfo = folderInfo; + di.OldFileInfo = oldFileInfo; di.ScanDataMD5 = scanData?.MD5; FilesToScan.Add (di); RegisterModifiedFolderInfo (folderInfo); @@ -119,6 +120,7 @@ namespace Mono.Addins.Database { public string File; public AddinScanFolderInfo AddinScanFolderInfo; + public AddinFileInfo OldFileInfo; public string ScanDataMD5; } diff --git a/Mono.Addins/Mono.Addins.Database/AddinScanner.cs b/Mono.Addins/Mono.Addins.Database/AddinScanner.cs index bab121f..3c389c4 100644 --- a/Mono.Addins/Mono.Addins.Database/AddinScanner.cs +++ b/Mono.Addins/Mono.Addins.Database/AddinScanner.cs @@ -188,7 +188,7 @@ namespace Mono.Addins.Database if (config.LocalId.Length == 0) { // Generate an internal id for this add-in - config.LocalId = database.GetUniqueAddinId (file, (fi != null ? fi.AddinId : null), config.Namespace, config.Version); + config.LocalId = database.GetUniqueAddinId (file, fi?.AddinId, config.Namespace, config.Version); config.HasUserId = false; } @@ -232,11 +232,11 @@ namespace Mono.Addins.Database // If the scanned file results in an add-in version different from the one obtained from // previous scans, the old add-in needs to be uninstalled. - if (fi != null && fi.IsAddin && fi.AddinId != config.AddinId) { - database.UninstallAddin (monitor, folderInfo.Domain, fi.AddinId, fi.File, scanResult); + if (fileToScan.OldFileInfo != null && fileToScan.OldFileInfo.IsAddin && fileToScan.OldFileInfo.AddinId != config.AddinId) { + database.UninstallAddin (monitor, folderInfo.Domain, fileToScan.OldFileInfo.AddinId, fileToScan.OldFileInfo.File, scanResult); // If the add-in version has changed, regenerate everything again since old data can't be reused - if (Addin.GetIdName (fi.AddinId) == Addin.GetIdName (config.AddinId)) + if (Addin.GetIdName (fileToScan.OldFileInfo.AddinId) == Addin.GetIdName (config.AddinId)) scanResult.RegenerateRelationData = true; } diff --git a/Mono.Addins/Mono.Addins.Description/ModuleDescription.cs b/Mono.Addins/Mono.Addins.Description/ModuleDescription.cs index dfbd169..6184449 100644 --- a/Mono.Addins/Mono.Addins.Description/ModuleDescription.cs +++ b/Mono.Addins/Mono.Addins.Description/ModuleDescription.cs @@ -391,7 +391,7 @@ namespace Mono.Addins.Description writer.WriteValue ("DataFiles", NormalizePaths (DataFiles)); writer.WriteValue ("Dependencies", Dependencies); writer.WriteValue ("Extensions", Extensions); - writer.WriteValue ("IgnorePaths", NormalizePaths (ignorePaths)); + writer.WriteValue ("IgnorePaths", NormalizePaths (IgnorePaths)); } internal override void Read (BinaryXmlReader reader) diff --git a/Test/UnitTests/TestScanDataFileGeneration.cs b/Test/UnitTests/TestScanDataFileGeneration.cs index bb5c713..8fb64a7 100644 --- a/Test/UnitTests/TestScanDataFileGeneration.cs +++ b/Test/UnitTests/TestScanDataFileGeneration.cs @@ -103,8 +103,6 @@ namespace UnitTests Assert.Contains ("SimpleApp.Ext2,0.1.0", addins); Assert.Contains ("SimpleApp.Ext3,0.1.0", addins); Assert.Contains ("SimpleApp.Ext4,0.1.0", addins); - - AddinEngine engine = new AddinEngine (); } [Test] @@ -233,6 +231,81 @@ namespace UnitTests engine.Shutdown (); } + + [Test] + [TestCase (true, true, TestName = "DowngradeAddins - with scan data")] + [TestCase (true, false, TestName = "DowngradeAddins - from scan to no scan data")] + [TestCase (false, true, TestName = "DowngradeAddins - from no scan to scan data")] + [TestCase (false, false, TestName = "DowngradeAddins - with no scan data")] + public void DowngradeAddins (bool hasScaIndexBefore, bool hasScanIndexAfter) + { + // Tests that the database is properly updated when add-ins are downgraded. + + var dir = Util.GetSampleDirectory ("ScanDataFilesTest"); + + AddinRegistry registry; + + if (hasScaIndexBefore) { + // Generate the scan data files before initializing the engine + registry = GetRegistry (dir); + registry.GenerateAddinScanDataFiles (new ConsoleProgressStatus (true), recursive: true); + registry.Dispose (); + } + + AddinEngine engine = new AddinEngine (); + engine.Initialize (Path.Combine (dir, "Config"), Path.Combine (dir, "UserAddins"), null, Path.Combine (dir, "App")); + registry = engine.Registry; + + var addins = registry.GetAddins ().Select (a => a.Id).ToArray (); + Assert.AreEqual (4, addins.Length); + Assert.Contains ("SimpleApp.Ext1,0.1.0", addins); + Assert.Contains ("SimpleApp.Ext2,0.1.0", addins); + Assert.Contains ("SimpleApp.Ext3,0.1.0", addins); + Assert.Contains ("SimpleApp.Ext4,0.1.0", addins); + engine.Shutdown (); + + // Downgrade add-ins + + SetAddinVersions (dir, "0.1.0", "0.0.1"); + + if (hasScanIndexAfter) { + // Regenerate the data files + registry = GetRegistry (dir); + registry.GenerateAddinScanDataFiles (new ConsoleProgressStatus (true), recursive: true); + registry.Dispose (); + } else { + CleanAddinData (dir); + } + + engine = new AddinEngine (); + engine.Initialize (Path.Combine (dir, "Config"), Path.Combine (dir, "UserAddins"), null, Path.Combine (dir, "App")); + registry = engine.Registry; + registry.Update (); + + addins = registry.GetAddins ().Select (a => a.Id).ToArray (); + Assert.AreEqual (4, addins.Length); + Assert.Contains ("SimpleApp.Ext1,0.0.1", addins); + Assert.Contains ("SimpleApp.Ext2,0.0.1", addins); + Assert.Contains ("SimpleApp.Ext3,0.0.1", addins); + Assert.Contains ("SimpleApp.Ext4,0.0.1", addins); + engine.Shutdown (); + } + + void SetAddinVersions (string path, string oldVersion, string newVersion) + { + foreach (var file in Directory.GetFiles (path, "*.xml")) + File.WriteAllText (file, File.ReadAllText (file).Replace (oldVersion, newVersion)); + foreach (var dir in Directory.GetDirectories (path)) + SetAddinVersions (dir, oldVersion, newVersion); + } + + void CleanAddinData (string path) + { + foreach (var file in Directory.GetFiles (path, "*.addindata")) + File.Delete (file); + foreach (var dir in Directory.GetDirectories (path)) + CleanAddinData (dir); + } } class FileList: MarshalByRefObject -- cgit v1.2.3 From 315a78929024c25babd0a8add955d48789d0e8f1 Mon Sep 17 00:00:00 2001 From: Lluis Sanchez Date: Fri, 16 Sep 2022 15:38:56 +0200 Subject: Try to make code a bit more clear --- Mono.Addins/Mono.Addins.Database/AddinRegistryUpdater.cs | 13 ++++++++----- Test/UnitTests/TestScanDataFileGeneration.cs | 8 ++++---- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/Mono.Addins/Mono.Addins.Database/AddinRegistryUpdater.cs b/Mono.Addins/Mono.Addins.Database/AddinRegistryUpdater.cs index 760c4e8..5d3b91d 100644 --- a/Mono.Addins/Mono.Addins.Database/AddinRegistryUpdater.cs +++ b/Mono.Addins/Mono.Addins.Database/AddinRegistryUpdater.cs @@ -50,6 +50,12 @@ namespace Mono.Addins.Database { AddinScanFolderInfo folderInfo; + AddinScanFolderInfo previousOldFolderInfo = oldFolderInfo; + + // Don't reset oldFolderInfo here. When scanning a folder that had scan index and now it doesn't, + // we need to keep the old folder data since the root folder info had the info for all folders + // in the domain. + if (!database.GetFolderInfoForPath (monitor, path, out folderInfo)) { // folderInfo file was corrupt. // Just in case, we are going to regenerate all relation data. @@ -76,7 +82,6 @@ namespace Mono.Addins.Database bool sharedFolder = domain == AddinDatabase.GlobalDomain; bool isNewFolder = folderInfo == null; bool folderHasIndex = dirScanDataIndex != null; - bool oldFolderInfoWasSet = false; if (isNewFolder) { // No folder info. It is the first time this folder is scanned. @@ -90,11 +95,10 @@ namespace Mono.Addins.Database // Keep a copy of the old folder info, to be used to retrieve the old status of the add-ins. oldFolderInfo = folderInfo; - oldFolderInfoWasSet = true; folderInfo = new AddinScanFolderInfo (oldFolderInfo); + scanResult.RegenerateRelationData = true; folderInfo.Reset (); - scanResult.RegisterModifiedFolderInfo (folderInfo); folderInfo.FolderHasScanDataIndex = folderHasIndex; } @@ -186,8 +190,7 @@ namespace Mono.Addins.Database UpdateDeletedAddins (monitor, oldFolderInfo ?? currentFolderInfo); - if (oldFolderInfoWasSet) - oldFolderInfo = null; + oldFolderInfo = previousOldFolderInfo; } protected override void OnVisitAddinManifestFile (IProgressStatus monitor, string file) diff --git a/Test/UnitTests/TestScanDataFileGeneration.cs b/Test/UnitTests/TestScanDataFileGeneration.cs index 8fb64a7..3c68ced 100644 --- a/Test/UnitTests/TestScanDataFileGeneration.cs +++ b/Test/UnitTests/TestScanDataFileGeneration.cs @@ -233,10 +233,10 @@ namespace UnitTests } [Test] - [TestCase (true, true, TestName = "DowngradeAddins - with scan data")] - [TestCase (true, false, TestName = "DowngradeAddins - from scan to no scan data")] - [TestCase (false, true, TestName = "DowngradeAddins - from no scan to scan data")] - [TestCase (false, false, TestName = "DowngradeAddins - with no scan data")] + [TestCase (true, true, TestName = "DowngradeAddins - with scan index")] + [TestCase (true, false, TestName = "DowngradeAddins - from scan to no scan index")] + [TestCase (false, true, TestName = "DowngradeAddins - from no scan to scan index")] + [TestCase (false, false, TestName = "DowngradeAddins - with no scan index")] public void DowngradeAddins (bool hasScaIndexBefore, bool hasScanIndexAfter) { // Tests that the database is properly updated when add-ins are downgraded. -- cgit v1.2.3