Fix OMDB People handling

2 files changed, 77 insertions, 27 deletions

diff --git a/MediaBrowser.Providers/Plugins/Omdb/OmdbProvider.cs b/MediaBrowser.Providers/Plugins/Omdb/OmdbProvider.cs
index f562d64ddd..7a8e2b3ce7 100644
--- a/MediaBrowser.Providers/Plugins/Omdb/OmdbProvider.cs
+++ b/MediaBrowser.Providers/Plugins/Omdb/OmdbProvider.cs
@@ -420,41 +420,40 @@ namespace MediaBrowser.Providers.Plugins.Omdb
                 return;
             }
 
-            if (!string.IsNullOrWhiteSpace(result.Director))
-            {
-                var person = new PersonInfo
-                {
-                    Name = result.Director.Trim(),
-                    Type = PersonKind.Director
-                };
-
-                itemResult.AddPerson(person);
-            }
+            AddPeople(itemResult, result.Director, PersonKind.Director);
+            AddPeople(itemResult, result.Writer, PersonKind.Writer);
+            AddPeople(itemResult, result.Actors, PersonKind.Actor);
+        }
 
-            if (!string.IsNullOrWhiteSpace(result.Writer))
+        internal static void AddPeople<T>(MetadataResult<T> itemResult, string credits, PersonKind type)
+            where T : BaseItem
+        {
+            if (string.IsNullOrWhiteSpace(credits))
             {
-                var person = new PersonInfo
-                {
-                    Name = result.Writer.Trim(),
-                    Type = PersonKind.Writer
-                };
-
-                itemResult.AddPerson(person);
+                return;
             }
 
-            if (!string.IsNullOrWhiteSpace(result.Actors))
+            foreach (var credit in credits.Split(',', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries))
             {
-                var actorList = result.Actors.Split(',', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries);
-                foreach (var actor in actorList)
+                // OMDb annotates the credited role in parentheses, e.g. "Mari Okada (screenplay)". The same
+                // person can be credited more than once this way, so strip it and let AddPerson deduplicate.
+                var name = credit;
+                var annotation = name.IndexOf('(', StringComparison.Ordinal);
+                if (annotation >= 0)
                 {
-                    var person = new PersonInfo
-                    {
-                        Name = actor,
-                        Type = PersonKind.Actor
-                    };
+                    name = name[..annotation].TrimEnd();
+                }
 
-                    itemResult.AddPerson(person);
+                if (string.IsNullOrEmpty(name))
+                {
+                    continue;
                 }
+
+                itemResult.AddPerson(new PersonInfo
+                {
+                    Name = name,
+                    Type = type
+                });
             }
         }
 
diff --git a/tests/Jellyfin.Providers.Tests/Omdb/OmdbProviderTests.cs b/tests/Jellyfin.Providers.Tests/Omdb/OmdbProviderTests.cs
new file mode 100644
index 0000000000..d18c8c21a2
--- /dev/null
+++ b/tests/Jellyfin.Providers.Tests/Omdb/OmdbProviderTests.cs
@@ -0,0 +1,51 @@
+using System.Linq;
+using Jellyfin.Data.Enums;
+using MediaBrowser.Controller.Entities.Movies;
+using MediaBrowser.Controller.Providers;
+using MediaBrowser.Providers.Plugins.Omdb;
+using Xunit;
+
+namespace Jellyfin.Providers.Tests.Omdb
+{
+    public class OmdbProviderTests
+    {
+        [Fact]
+        public void AddPeople_CommaSeparatedList_SplitsIntoIndividualPeople()
+        {
+            var result = new MetadataResult<Movie>();
+
+            OmdbProvider.AddPeople(result, "Philip G. Epstein, Julius J. Epstein, Howard Koch", PersonKind.Writer);
+
+            Assert.Equal(
+                new[] { "Philip G. Epstein", "Julius J. Epstein", "Howard Koch" },
+                result.People!.Select(p => p.Name));
+            Assert.All(result.People!, p => Assert.Equal(PersonKind.Writer, p.Type));
+        }
+
+        [Fact]
+        public void AddPeople_RoleAnnotations_AreStrippedAndDeduplicated()
+        {
+            var result = new MetadataResult<Movie>();
+
+            OmdbProvider.AddPeople(result, "Mari Okada (screenplay), Mari Okada (story), Jun'ichi Satô (screenplay), Jun'ichi Satô (story)", PersonKind.Writer);
+
+            Assert.Equal(
+                new[] { "Mari Okada", "Jun'ichi Satô" },
+                result.People!.Select(p => p.Name));
+        }
+
+        [Theory]
+        [InlineData(null)]
+        [InlineData("")]
+        [InlineData("   ")]
+        [InlineData("(uncredited)")]
+        public void AddPeople_NoUsableName_AddsNothing(string? credits)
+        {
+            var result = new MetadataResult<Movie>();
+
+            OmdbProvider.AddPeople(result, credits!, PersonKind.Actor);
+
+            Assert.Null(result.People);
+        }
+    }
+}