Add explicit mapping instead of reflection to manifest reconciliation.

Emby.Server.Implementations/Plugins/PluginManager.cs

@@ -432,69 +432,67 @@ namespace Emby.Server.Implementations.Plugins
                 ImagePath = imagePath
             };
 
-            var metafile = Path.Combine(Path.Combine(path, MetafileName));
-            if (File.Exists(metafile))
-            {
-                var data = File.ReadAllBytes(metafile);
-                var localManifest = JsonSerializer.Deserialize<PluginManifest>(data, _jsonOptions) ?? new PluginManifest();
-
-                // Plugin installation is the typical cause for populating a manifest. Activate.
-                localManifest.Status = status == PluginStatus.Disabled ? PluginStatus.Disabled : PluginStatus.Active;
-
-                if (!Equals(localManifest.Id, manifest.Id))
-                {
-                    _logger.LogError("The manifest ID {LocalUUID} did not match the package info ID {PackageUUID}.", localManifest.Id, manifest.Id);
-                    localManifest.Status = PluginStatus.Malfunctioned;
-                }
-
-                if (localManifest.Version != manifest.Version)
-                {
-                    _logger.LogWarning("The version of the local manifest was {LocalVersion}, but {PackageVersion} was expected. The value will be replaced.", localManifest.Version, manifest.Version);
-
-                    // Correct the local version.
-                    localManifest.Version = manifest.Version;
-                }
-
-                // Reconcile missing data against repository manifest.
-                ReconcileManifest(localManifest, manifest);
-
-                manifest = localManifest;
-            }
-            else
+            if (!ReconcileManifest(manifest, path))
             {
-                _logger.LogInformation("No local manifest exists for plugin {Plugin}. Populating from repository manifest.", manifest.Name);
+                // An error occurred during reconciliation and saving could be undesirable.
+                return false;
             }
 
             return SaveManifest(manifest, path);
         }
 
         /// <summary>
-        /// Resolve the target plugin manifest against the source. Values are mapped onto the
-        /// target only if they are default values or empty strings. ID and status fields are ignored.
+        /// Reconciles the manifest against any properties that exist locally in a pre-packaged meta.json found at the path.
+        /// If no file is found, no reconciliation occurs.
         /// </summary>
-        /// <param name="baseManifest">The base <see cref="PluginManifest"/> to be reconciled.</param>
-        /// <param name="projector">The <see cref="PluginManifest"/> to reconcile against.</param>
-        private void ReconcileManifest(PluginManifest baseManifest, PluginManifest projector)
+        /// <param name="manifest">The <see cref="PluginManifest"/> to reconcile against.</param>
+        /// <param name="path">The plugin path.</param>
+        /// <returns>The reconciled <see cref="PluginManifest"/>.</returns>
+        private bool ReconcileManifest(PluginManifest manifest, string path)
         {
-            var ignoredFields = new string[]
+            try
             {
-                nameof(baseManifest.Id),
-                nameof(baseManifest.Status)
-            };
+                var metafile = Path.Combine(path, MetafileName);
+                if (!File.Exists(metafile))
+                {
+                    _logger.LogInformation("No local manifest exists for plugin {Plugin}. Skipping manifest reconciliation.", manifest.Name);
+                    return true;
+                }
 
-            foreach (var property in baseManifest.GetType().GetProperties())
-            {
-                var localValue = property.GetValue(baseManifest);
+                var data = File.ReadAllBytes(metafile);
+                var localManifest = JsonSerializer.Deserialize<PluginManifest>(data, _jsonOptions) ?? new PluginManifest();
 
-                if (property.PropertyType == typeof(bool) || ignoredFields.Any(s => Equals(s, property.Name)))
+                if (!Equals(localManifest.Id, manifest.Id))
                 {
-                    continue;
+                    _logger.LogError("The manifest ID {LocalUUID} did not match the package info ID {PackageUUID}.", localManifest.Id, manifest.Id);
+                    manifest.Status = PluginStatus.Malfunctioned;
                 }
 
-                if (property.PropertyType.IsNullOrDefault(localValue) || (property.PropertyType == typeof(string) && (string)localValue! == string.Empty))
+                if (localManifest.Version != manifest.Version)
                 {
-                    property.SetValue(baseManifest, property.GetValue(projector));
+                    // Package information provides the version and is the source of truth. Pre-packages meta.json is assumed to be a mistake in this regard.
+                    _logger.LogWarning("The version of the local manifest was {LocalVersion}, but {PackageVersion} was expected. The value will be replaced.", localManifest.Version, manifest.Version);
                 }
+
+                // Explicitly mapping properties instead of using reflection is preferred here.
+                manifest.Category = string.IsNullOrEmpty(localManifest.Category) ? manifest.Category : localManifest.Category;
+                manifest.AutoUpdate = localManifest.AutoUpdate; // Preserve whatever is local. Package info does not have this property.
+                manifest.Changelog = string.IsNullOrEmpty(localManifest.Changelog) ? manifest.Changelog : localManifest.Changelog;
+                manifest.Description = string.IsNullOrEmpty(localManifest.Description) ? manifest.Description : localManifest.Description;
+                manifest.Name = string.IsNullOrEmpty(localManifest.Name) ? manifest.Name : localManifest.Name;
+                manifest.Overview = string.IsNullOrEmpty(localManifest.Overview) ? manifest.Overview : localManifest.Overview;
+                manifest.Owner = string.IsNullOrEmpty(localManifest.Owner) ? manifest.Owner : localManifest.Owner;
+                manifest.TargetAbi = string.IsNullOrEmpty(localManifest.TargetAbi) ? manifest.TargetAbi : localManifest.TargetAbi;
+                manifest.Timestamp = localManifest.Timestamp.IsNullOrDefault() ? manifest.Timestamp : localManifest.Timestamp;
+                manifest.ImagePath = string.IsNullOrEmpty(localManifest.ImagePath) ? manifest.ImagePath : localManifest.ImagePath;
+                manifest.Assemblies = localManifest.Assemblies;
+
+                return true;
+            }
+            catch (Exception e)
+            {
+                _logger.LogWarning(e, "Unable to reconcile plugin manifest due to an error. {Path}", path);
+                return false;
             }
         }
 

MediaBrowser.Model/Updates/VersionInfo.cs

@@ -1,5 +1,3 @@
-using System;
-using System.Collections.Generic;
 using System.Text.Json.Serialization;
 using SysVersion = System.Version;
 

tests/Jellyfin.Server.Implementations.Tests/Plugins/PluginManagerTests.cs

@@ -172,6 +172,24 @@ namespace Jellyfin.Server.Implementations.Tests.Plugins
                 Assemblies = new[] { "Jellyfin.Test.dll" }
             };
 
+            var expectedManifest = new PluginManifest
+            {
+                Id = partial.Id,
+                Name = packageInfo.Name,
+                AutoUpdate = partial.AutoUpdate,
+                Status = PluginStatus.Active,
+                Owner = packageInfo.Owner,
+                Assemblies = partial.Assemblies,
+                Category = packageInfo.Category,
+                Description = packageInfo.Description,
+                Overview = packageInfo.Overview,
+                TargetAbi = packageInfo.Versions[0].TargetAbi!,
+                Timestamp = DateTime.Parse(packageInfo.Versions[0].Timestamp!, CultureInfo.InvariantCulture),
+                Changelog = packageInfo.Versions[0].Changelog!,
+                Version = new Version(1, 0).ToString(),
+                ImagePath = string.Empty
+            };
+
             var metafilePath = Path.Combine(_pluginPath, "meta.json");
             File.WriteAllText(metafilePath, JsonSerializer.Serialize(partial, _options));
 
@@ -183,22 +201,41 @@ namespace Jellyfin.Server.Implementations.Tests.Plugins
             var result = JsonSerializer.Deserialize<PluginManifest>(resultBytes, _options);
 
             Assert.NotNull(result);
-            Assert.Equal(packageInfo.Name, result.Name);
-            Assert.Equal(packageInfo.Owner, result.Owner);
-            Assert.Equal(PluginStatus.Active, result.Status);
-            Assert.NotEmpty(result.Assemblies);
-            Assert.False(result.AutoUpdate);
-
-            // Preserved
-            Assert.Equal(packageInfo.Category, result.Category);
-            Assert.Equal(packageInfo.Description, result.Description);
-            Assert.Equal(packageInfo.Id, result.Id);
-            Assert.Equal(packageInfo.Overview, result.Overview);
-            Assert.Equal(partial.Assemblies[0], result.Assemblies[0]);
-            Assert.Equal(packageInfo.Versions[0].TargetAbi, result.TargetAbi);
-            Assert.Equal(DateTime.Parse(packageInfo.Versions[0].Timestamp!, CultureInfo.InvariantCulture), result.Timestamp);
-            Assert.Equal(packageInfo.Versions[0].Changelog, result.Changelog);
-            Assert.Equal(packageInfo.Versions[0].Version, result.Version);
+            Assert.Equivalent(expectedManifest, result);
+        }
+
+        [Fact]
+        public async Task PopulateManifest_NoMetafile_PreservesManifest()
+        {
+            var packageInfo = GenerateTestPackage();
+            var expectedManifest = new PluginManifest
+            {
+                Id = packageInfo.Id,
+                Name = packageInfo.Name,
+                AutoUpdate = true,
+                Status = PluginStatus.Active,
+                Owner = packageInfo.Owner,
+                Assemblies = Array.Empty<string>(),
+                Category = packageInfo.Category,
+                Description = packageInfo.Description,
+                Overview = packageInfo.Overview,
+                TargetAbi = packageInfo.Versions[0].TargetAbi!,
+                Timestamp = DateTime.Parse(packageInfo.Versions[0].Timestamp!, CultureInfo.InvariantCulture),
+                Changelog = packageInfo.Versions[0].Changelog!,
+                Version = packageInfo.Versions[0].Version,
+                ImagePath = string.Empty
+            };
+
+            var pluginManager = new PluginManager(new NullLogger<PluginManager>(), null!, null!, null!, new Version(1, 0));
+
+            await pluginManager.PopulateManifest(packageInfo, new Version(1, 0), _pluginPath, PluginStatus.Active);
+
+            var metafilePath = Path.Combine(_pluginPath, "meta.json");
+            var resultBytes = File.ReadAllBytes(metafilePath);
+            var result = JsonSerializer.Deserialize<PluginManifest>(resultBytes, _options);
+
+            Assert.NotNull(result);
+            Assert.Equivalent(expectedManifest, result);
         }
 
         [Fact]