pm: reign in overeager profile cleanup on install

...and remove Profile::RemoveByInternalNameAsync() because it's a footgun
This commit is contained in:
goaaats 2024-03-23 15:48:54 +01:00
parent ba1c8cba45
commit b3db0e78b3
2 changed files with 30 additions and 32 deletions

View file

@ -1355,12 +1355,25 @@ internal partial class PluginManager : IInternalDisposableService
{
try
{
// We don't need to apply, it doesn't matter
await this.profileManager.DefaultProfile.RemoveByInternalNameAsync(repoManifest.InternalName, false);
// Only remove entries from the default profile that are NOT currently tied to an active LocalPlugin
var guidsToRemove = this.profileManager.DefaultProfile.Plugins
.Where(x => this.InstalledPlugins.All(y => y.Manifest.WorkingPluginId != x.WorkingPluginId))
.Select(x => x.WorkingPluginId)
.ToArray();
if (guidsToRemove.Length != 0)
{
Log.Verbose("Removing {Cnt} orphaned entries from default profile", guidsToRemove.Length);
foreach (var guid in guidsToRemove)
{
// We don't need to apply, it doesn't matter
await this.profileManager.DefaultProfile.RemoveAsync(guid, false, false);
}
}
}
catch (ProfileOperationException)
catch (ProfileOperationException ex)
{
// ignored
Log.Error(ex, "Error during default profile cleanup");
}
}
else

View file

@ -183,10 +183,13 @@ internal class Profile
});
}
}
Log.Information("Adding plugin {Plugin}({Guid}) to profile {Profile} with state {State}", internalName, workingPluginId, this.Guid, state);
// We need to remove this plugin from the default profile, if it declares it.
if (!this.IsDefaultProfile && this.manager.DefaultProfile.WantsPlugin(workingPluginId) != null)
{
Log.Information("=> Removing plugin {Plugin}({Guid}) from default profile", internalName, workingPluginId);
await this.manager.DefaultProfile.RemoveAsync(workingPluginId, false);
}
@ -202,8 +205,12 @@ internal class Profile
/// </summary>
/// <param name="workingPluginId">The ID of the plugin.</param>
/// <param name="apply">Whether or not the current state should immediately be applied.</param>
/// <param name="checkDefault">
/// Whether or not to throw when a plugin is removed from the default profile, without being in another profile.
/// Used to prevent orphan plugins, but can be ignored when cleaning up old entries.
/// </param>
/// <returns>A <see cref="Task"/> representing the asynchronous operation.</returns>
public async Task RemoveAsync(Guid workingPluginId, bool apply = true)
public async Task RemoveAsync(Guid workingPluginId, bool apply = true, bool checkDefault = true)
{
ProfileModelV1.ProfileModelV1Plugin entry;
lock (this)
@ -215,15 +222,18 @@ internal class Profile
if (!this.modelV1.Plugins.Remove(entry))
throw new Exception("Couldn't remove plugin from model collection");
}
Log.Information("Removing plugin {Plugin}({Guid}) from profile {Profile}", entry.InternalName, entry.WorkingPluginId, this.Guid);
// We need to add this plugin back to the default profile, if we were the last profile to have it.
if (!this.manager.IsInAnyProfile(workingPluginId))
{
if (!this.IsDefaultProfile)
{
Log.Information("=> Adding plugin {Plugin}({Guid}) back to default profile", entry.InternalName, entry.WorkingPluginId);
await this.manager.DefaultProfile.AddOrUpdateAsync(workingPluginId, entry.InternalName, this.IsEnabled && entry.IsEnabled, false);
}
else
else if (checkDefault)
{
throw new PluginNotInDefaultProfileException(workingPluginId.ToString());
}
@ -235,31 +245,6 @@ internal class Profile
await this.manager.ApplyAllWantStatesAsync();
}
/// <summary>
/// Remove a plugin from this profile.
/// This will block until all states have been applied.
/// </summary>
/// <param name="internalName">The internal name of the plugin.</param>
/// <param name="apply">Whether or not the current state should immediately be applied.</param>
/// <returns>A <see cref="Task"/> representing the asynchronous operation.</returns>
public async Task RemoveByInternalNameAsync(string internalName, bool apply = true)
{
Guid? pluginToRemove = null;
lock (this)
{
foreach (var plugin in this.Plugins)
{
if (plugin.InternalName.Equals(internalName, StringComparison.Ordinal))
{
pluginToRemove = plugin.WorkingPluginId;
break;
}
}
}
await this.RemoveAsync(pluginToRemove ?? throw new PluginNotFoundException(internalName), apply);
}
/// <summary>
/// This function tries to migrate all plugins with this internalName which do not have
/// a GUID to the specified GUID.