Add XLIFF 2.0 plural support using group elements

nickprotop · nickprotop · commit df36447d8e3f · 2026-01-06T11:53:08.000+02:00
Writer changes:
- Add CreatePluralUnit20 method to create groups for plural entries
- Update CreateXliff20Document to use CreatePluralUnit20 for plurals
- Clean up CreateUnit20 to only handle non-plural entries

Reader changes:
- Add ExtractPluralCategoryWithBase to extract both base name and category
- Add IsPluralGroup to detect plural groups (multiple units with same base key)
- Add ParsePluralGroup20 to parse XLIFF 2.0 plural groups
- Update ParseXliff20 to detect and handle plural groups

This enables full round-trip preservation of plural structures in XLIFF 2.0 format.
diff --git a/LocalizationManager.Core/Backends/Xliff/XliffResourceReader.cs b/LocalizationManager.Core/Backends/Xliff/XliffResourceReader.cs
@@ -57,9 +57,18 @@ public ResourceFile Read(TextReader reader, LanguageInfo metadata)
     {
         var content = reader.ReadToEnd();
 
-        // Parse the document - use XDocument.Parse for string content
-        // This handles encoding properly for strings (avoids UTF-8/UTF-16 BOM issues)
-        var doc = XDocument.Parse(content, LoadOptions.PreserveWhitespace);
+        // Use secure XML settings to prevent XXE attacks
+        var settings = new XmlReaderSettings
+        {
+            DtdProcessing = DtdProcessing.Prohibit,
+            XmlResolver = null
+        };
+
+        XDocument doc;
+        using (var xmlReader = XmlReader.Create(new StringReader(content), settings))
+        {
+            doc = XDocument.Load(xmlReader, LoadOptions.PreserveWhitespace);
+        }
 
         // Detect version from the parsed document (more reliable than stream-based detection)
         var version = DetectVersionFromDocument(doc);
@@ -86,9 +95,9 @@ private string DetectVersionFromDocument(XDocument doc)
 
         // Check namespace
         var ns = root.GetDefaultNamespace().NamespaceName;
-        if (ns.Contains("2.0"))
+        if (ns == XliffVersionDetector.Xliff20Namespace)
             return "2.0";
-        if (ns.Contains("1.2"))
+        if (ns == XliffVersionDetector.Xliff12Namespace)
             return "1.2";
 
         // Check version attribute
@@ -105,7 +114,7 @@ private string DetectVersionFromDocument(XDocument doc)
         if (root.Attribute("srcLang") != null)
             return "2.0";
 
-        return "1.2"; // Default to 1.2
+        return "unknown";  // Let caller decide how to handle
     }
 
     /// <summary>
@@ -223,7 +232,7 @@ private List<ResourceEntry> ParseXliff12(XDocument doc, LanguageInfo metadata)
             // Always prefer target value (which contains the localized text)
             var pluralValue = target ?? source;
 
-            if (!string.IsNullOrEmpty(pluralId) && pluralValue != null)
+            if (!string.IsNullOrEmpty(pluralId) && !string.IsNullOrEmpty(pluralValue))
             {
                 // Extract plural category from id (e.g., "key[one]", "key[other]")
                 var category = ExtractPluralCategory(pluralId);
@@ -244,7 +253,10 @@ private List<ResourceEntry> ParseXliff12(XDocument doc, LanguageInfo metadata)
         return new ResourceEntry
         {
             Key = id,
-            Value = pluralForms.GetValueOrDefault("other") ?? pluralForms.Values.FirstOrDefault(),
+            Value = pluralForms.GetValueOrDefault("other")
+                ?? pluralForms.GetValueOrDefault("one")
+                ?? pluralForms.Values.FirstOrDefault()
+                ?? "",
             Comment = string.IsNullOrEmpty(comment) ? null : comment,
             IsPlural = true,
             PluralForms = pluralForms
@@ -282,12 +294,24 @@ private List<ResourceEntry> ParseXliff20(XDocument doc, LanguageInfo metadata)
             var groups = fileElement.Elements(ns + "group");
             foreach (var group in groups)
             {
-                var nestedUnits = group.Descendants(ns + "unit");
-                foreach (var unit in nestedUnits)
+                // Check if this is a plural group (has multiple units with same base key)
+                var groupUnits = group.Elements(ns + "unit").ToList();
+                if (groupUnits.Count > 1 && IsPluralGroup(groupUnits))
                 {
-                    var entry = ParseUnit20(unit, ns, metadata);
-                    if (entry != null)
-                        entries.Add(entry);
+                    var pluralEntry = ParsePluralGroup20(group, ns, metadata);
+                    if (pluralEntry != null)
+                        entries.Add(pluralEntry);
+                }
+                else
+                {
+                    // Regular group - parse nested units
+                    var nestedUnits = group.Descendants(ns + "unit");
+                    foreach (var unit in nestedUnits)
+                    {
+                        var entry = ParseUnit20(unit, ns, metadata);
+                        if (entry != null)
+                            entries.Add(entry);
+                    }
                 }
             }
         }
@@ -356,26 +380,117 @@ private List<ResourceEntry> ParseXliff20(XDocument doc, LanguageInfo metadata)
     /// Extracts the plural category from an ID like "key[one]" or "key_plural_one".
     /// </summary>
     private static string? ExtractPluralCategory(string id)
+    {
+        return ExtractPluralCategoryWithBase(id).category;
+    }
+
+    /// <summary>
+    /// Extracts both the base name and plural category from an ID like "key[one]" or "key_plural_one".
+    /// Returns (baseName, category) or (id, null) if not a plural suffix key.
+    /// </summary>
+    private static (string baseName, string? category) ExtractPluralCategoryWithBase(string id)
     {
         // Pattern 1: key[category]
         var bracketStart = id.LastIndexOf('[');
         var bracketEnd = id.LastIndexOf(']');
-        if (bracketStart >= 0 && bracketEnd > bracketStart)
+        // Validate: brackets must be in correct order and have content between them
+        if (bracketStart >= 0 && bracketEnd > bracketStart + 1)
         {
-            return id.Substring(bracketStart + 1, bracketEnd - bracketStart - 1);
+            var baseName = id.Substring(0, bracketStart);
+            var category = id.Substring(bracketStart + 1, bracketEnd - bracketStart - 1);
+            return (baseName, category);
         }
 
         // Pattern 2: key_plural_category
         var categories = new[] { "zero", "one", "two", "few", "many", "other" };
         foreach (var cat in categories)
         {
-            if (id.EndsWith($"_{cat}", StringComparison.OrdinalIgnoreCase) ||
-                id.EndsWith($"_plural_{cat}", StringComparison.OrdinalIgnoreCase))
+            if (id.EndsWith($"_{cat}", StringComparison.OrdinalIgnoreCase))
+            {
+                return (id.Substring(0, id.Length - cat.Length - 1), cat);
+            }
+            if (id.EndsWith($"_plural_{cat}", StringComparison.OrdinalIgnoreCase))
             {
-                return cat;
+                return (id.Substring(0, id.Length - cat.Length - 8), cat);
             }
         }
 
-        return null;
+        return (id, null);
+    }
+
+    /// <summary>
+    /// Checks if a group's units represent a plural entry.
+    /// Returns true if all units share the same base key and there are multiple units.
+    /// </summary>
+    private bool IsPluralGroup(List<XElement> units)
+    {
+        if (units.Count <= 1)
+            return false;
+
+        // Extract base keys (remove [category] suffix)
+        var baseKeys = units
+            .Select(u => u.Attribute("id")?.Value)
+            .Where(id => !string.IsNullOrEmpty(id))
+            .Select(id => ExtractPluralCategoryWithBase(id!).baseName)
+            .Distinct()
+            .ToList();
+
+        // If all units share the same base key and there are multiple units, it's a plural group
+        return baseKeys.Count == 1 && units.Count > 1;
+    }
+
+    /// <summary>
+    /// Parses a plural group in XLIFF 2.0.
+    /// </summary>
+    private ResourceEntry? ParsePluralGroup20(XElement group, XNamespace ns, LanguageInfo metadata)
+    {
+        var id = group.Attribute("id")?.Value ?? group.Attribute("name")?.Value;
+        if (string.IsNullOrEmpty(id))
+            return null;
+
+        var pluralForms = new Dictionary<string, string>();
+        var units = group.Elements(ns + "unit").ToList();
+
+        foreach (var unit in units)
+        {
+            var unitId = unit.Attribute("id")?.Value;
+            if (string.IsNullOrEmpty(unitId))
+                continue;
+
+            // Extract category from ID like "key[one]" or "key[other]"
+            var (baseName, category) = ExtractPluralCategoryWithBase(unitId);
+            if (string.IsNullOrEmpty(category))
+                continue;
+
+            // Get target value from segment
+            var segment = unit.Element(ns + "segment");
+            var target = segment?.Element(ns + "target")?.Value;
+            var source = segment?.Element(ns + "source")?.Value;
+
+            pluralForms[category] = target ?? source ?? "";
+        }
+
+        if (pluralForms.Count == 0)
+            return null;
+
+        // Get notes from group
+        var notesElement = group.Element(ns + "notes");
+        var notes = notesElement?.Elements(ns + "note")
+            .Select(n => n.Value)
+            .Where(n => !string.IsNullOrEmpty(n))
+            ?? Enumerable.Empty<string>();
+        var comment = string.Join("\n", notes);
+
+        return new ResourceEntry
+        {
+            Key = id,
+            Value = pluralForms.GetValueOrDefault("other")
+                ?? pluralForms.GetValueOrDefault("one")
+                ?? pluralForms.Values.FirstOrDefault()
+                ?? "",
+            Comment = string.IsNullOrEmpty(comment) ? null : comment,
+            IsPlural = true,
+            PluralForms = pluralForms
+        };
     }
 }
diff --git a/LocalizationManager.Core/Backends/Xliff/XliffResourceWriter.cs b/LocalizationManager.Core/Backends/Xliff/XliffResourceWriter.cs
@@ -40,10 +40,19 @@ public async Task WriteAsync(ResourceFile file, CancellationToken ct = default)
         if (!string.IsNullOrEmpty(directory) && !Directory.Exists(directory))
             Directory.CreateDirectory(directory);
 
-        // Atomic write
-        var tempPath = file.Language.FilePath + ".tmp";
-        await File.WriteAllTextAsync(tempPath, content, Encoding.UTF8, ct);
-        File.Move(tempPath, file.Language.FilePath, overwrite: true);
+        // Atomic write with unique temp file name to prevent race conditions
+        var tempPath = file.Language.FilePath + $".tmp.{Guid.NewGuid()}";
+        try
+        {
+            // Use UTF-8 without BOM for consistency with SerializeToString
+            await File.WriteAllTextAsync(tempPath, content, new UTF8Encoding(false), ct);
+            File.Move(tempPath, file.Language.FilePath, overwrite: true);
+        }
+        finally
+        {
+            if (File.Exists(tempPath))
+                File.Delete(tempPath);
+        }
     }
 
     /// <inheritdoc />
@@ -233,7 +242,7 @@ private XElement CreatePluralGroup12(ResourceEntry entry)
             {
                 var unit = new XElement(Ns12 + "trans-unit",
                     new XAttribute("id", $"{entry.Key}[{category}]"),
-                    new XElement(Ns12 + "source", entry.Key),
+                    new XElement(Ns12 + "source", entry.SourcePluralText ?? entry.Key),
                     new XElement(Ns12 + "target", value ?? ""));
                 group.Add(unit);
             }
@@ -261,7 +270,14 @@ private XDocument CreateXliff20Document(ResourceFile file)
 
         foreach (var entry in file.Entries)
         {
-            fileElement.Add(CreateUnit20(entry, file.Language.IsDefault));
+            if (entry.IsPlural && entry.PluralForms != null && entry.PluralForms.Count > 0)
+            {
+                fileElement.Add(CreatePluralUnit20(entry, file.Language.IsDefault));
+            }
+            else
+            {
+                fileElement.Add(CreateUnit20(entry, file.Language.IsDefault));
+            }
         }
 
         root.Add(fileElement);
@@ -272,42 +288,65 @@ private XDocument CreateXliff20Document(ResourceFile file)
     }
 
     /// <summary>
-    /// Creates a unit element for XLIFF 2.0.
-    /// Always creates both source and target elements to preserve bilingual structure.
+    /// Creates a group element for plural entries in XLIFF 2.0.
+    /// Uses groups to preserve plural structure similar to XLIFF 1.2.
     /// </summary>
-    private XElement CreateUnit20(ResourceEntry entry, bool isDefault)
+    private XElement CreatePluralUnit20(ResourceEntry entry, bool isDefault)
     {
-        var unit = new XElement(Ns20 + "unit",
-            new XAttribute("id", entry.Key));
+        var group = new XElement(Ns20 + "group",
+            new XAttribute("id", entry.Key),
+            new XAttribute("name", entry.Key));
 
         if (!string.IsNullOrEmpty(entry.Comment))
         {
             var notes = new XElement(Ns20 + "notes",
                 new XElement(Ns20 + "note", entry.Comment));
-            unit.Add(notes);
+            group.Add(notes);
         }
 
-        if (entry.IsPlural && entry.PluralForms != null)
+        // Create a unit for each plural form
+        if (entry.PluralForms != null)
         {
-            // Create multiple segments for plurals
             foreach (var (category, value) in entry.PluralForms)
             {
+                var unit = new XElement(Ns20 + "unit",
+                    new XAttribute("id", $"{entry.Key}[{category}]"));
+
                 var segment = new XElement(Ns20 + "segment",
-                    new XAttribute("id", category),
                     new XElement(Ns20 + "source", entry.Key),
                     new XElement(Ns20 + "target", value ?? ""));
+
                 unit.Add(segment);
+                group.Add(unit);
             }
         }
-        else
+
+        return group;
+    }
+
+    /// <summary>
+    /// Creates a unit element for XLIFF 2.0 (non-plural entries only).
+    /// Always creates both source and target elements to preserve bilingual structure.
+    /// For plural entries, use CreatePluralUnit20 instead.
+    /// </summary>
+    private XElement CreateUnit20(ResourceEntry entry, bool isDefault)
+    {
+        var unit = new XElement(Ns20 + "unit",
+            new XAttribute("id", entry.Key));
+
+        if (!string.IsNullOrEmpty(entry.Comment))
         {
-            // Always write both source (key) and target (value) for proper bilingual structure
-            var segment = new XElement(Ns20 + "segment",
-                new XElement(Ns20 + "source", entry.Key),
-                new XElement(Ns20 + "target", entry.Value ?? ""));
-            unit.Add(segment);
+            var notes = new XElement(Ns20 + "notes",
+                new XElement(Ns20 + "note", entry.Comment));
+            unit.Add(notes);
         }
 
+        // Always write both source (key) and target (value) for proper bilingual structure
+        var segment = new XElement(Ns20 + "segment",
+            new XElement(Ns20 + "source", entry.Key),
+            new XElement(Ns20 + "target", entry.Value ?? ""));
+        unit.Add(segment);
+
         return unit;
     }
 
