From cccc548928218d3ca06fef0fedc36277c57b24ea Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Tue, 4 Jul 2023 09:46:52 -0400 Subject: [PATCH 1/2] Sort writing systems retrieved from import LDMLs --- .vscode/settings.json | 1 + Backend/Controllers/LiftController.cs | 6 ++-- Backend/Helper/GrammaticalCategory.cs | 2 +- Backend/Helper/Language.cs | 47 +++++++++++++++++++++++---- Backend/Helper/LiftHelper.cs | 2 +- Backend/Services/LiftService.cs | 8 ++--- 6 files changed, 51 insertions(+), 15 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 2c3eec3dce..d04e61b7ae 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -60,6 +60,7 @@ "reportgenerator", "sillsdev", "Sldr", + "Subtag", "targetdir", "textfile", "thecombine", diff --git a/Backend/Controllers/LiftController.cs b/Backend/Controllers/LiftController.cs index a30765e9be..498a75af87 100644 --- a/Backend/Controllers/LiftController.cs +++ b/Backend/Controllers/LiftController.cs @@ -74,7 +74,7 @@ internal async Task UploadLiftFileAndGetWritingSystems(FileUpload return BadRequest(e.Message); } - return Ok(Language.GetWritingSystems(extractedLiftRootPath).ToList()); + return Ok(Language.GetWritingSystems(extractedLiftRootPath)); } /// Adds data from a directory containing a .lift file @@ -106,7 +106,7 @@ internal async Task FinishUploadLiftFile(string projectId, string } var extractDir = _liftService.RetrieveImport(userId); - if (String.IsNullOrWhiteSpace(extractDir)) + if (string.IsNullOrWhiteSpace(extractDir)) { return BadRequest("No in-progress import to finish."); } @@ -238,7 +238,7 @@ private async Task AddImportToProject(string liftStoragePath, str // Add analysis writing systems found in the data, avoiding duplicate and empty bcp47 codes. project.AnalysisWritingSystems.AddRange(importedAnalysisWritingSystems.Where( iws => !project.AnalysisWritingSystems.Any(ws => ws.Bcp47 == iws.Bcp47))); - project.AnalysisWritingSystems.RemoveAll(ws => String.IsNullOrWhiteSpace(ws.Bcp47)); + project.AnalysisWritingSystems.RemoveAll(ws => string.IsNullOrWhiteSpace(ws.Bcp47)); if (project.AnalysisWritingSystems.Count == 0) { // The list cannot be empty. diff --git a/Backend/Helper/GrammaticalCategory.cs b/Backend/Helper/GrammaticalCategory.cs index ac83d5d345..e51c692af6 100644 --- a/Backend/Helper/GrammaticalCategory.cs +++ b/Backend/Helper/GrammaticalCategory.cs @@ -155,7 +155,7 @@ public bool Matches(string gramCat) public static GramCatGroup GetGramCatGroup(string grammaticalCategory) { var gramCat = removeAccents(grammaticalCategory); - if (String.IsNullOrWhiteSpace(Regex.Match(gramCat, @"[^\W]+").Value)) + if (string.IsNullOrWhiteSpace(Regex.Match(gramCat, @"[^\W]+").Value)) { return GramCatGroup.Unspecified; } diff --git a/Backend/Helper/Language.cs b/Backend/Helper/Language.cs index 04a50f9c55..bad130662c 100644 --- a/Backend/Helper/Language.cs +++ b/Backend/Helper/Language.cs @@ -1,3 +1,4 @@ +using System; using System.Collections.Generic; using System.IO; using System.Linq; @@ -11,15 +12,15 @@ public static class Language /// /// Extract list of distinct Language strings from all of a 's Definitions and Glosses. /// - public static IEnumerable GetSenseAnalysisLangTags(Sense sense) + public static List GetSenseAnalysisLangTags(Sense sense) { var tags = sense.Definitions.Select(d => d.Language).ToList(); tags.AddRange(sense.Glosses.Select(g => g.Language)); - return tags.Distinct(); + return tags.Distinct().ToList(); } /// Convert language tags into writing systems. - public static IEnumerable ConvertLangTagsToWritingSystems(IEnumerable langTags) + public static List ConvertLangTagsToWritingSystems(IEnumerable langTags) { // If Sldr isn't initialized, temporarily initialize it here. var isSldrInitialized = Sldr.IsInitialized; @@ -34,13 +35,13 @@ public static IEnumerable ConvertLangTagsToWritingSystems(IEnumer { Sldr.Cleanup(); } - return writingSystems; + return writingSystems.ToList(); } /// /// Extract s from .ldml files in a directory or its WritingSystems subdirectory. /// - public static IEnumerable GetWritingSystems(string dirPath) + public static List GetWritingSystems(string dirPath) { if (!Directory.GetFiles(dirPath, "*.ldml").Any()) { @@ -48,7 +49,41 @@ public static IEnumerable GetWritingSystems(string dirPath) } var wsr = LdmlInFolderWritingSystemRepository.Initialize(dirPath); - return wsr.AllWritingSystems.Select(wsd => new WritingSystem(wsd)); + var ws = wsr.AllWritingSystems.Select(wsd => new WritingSystem(wsd)).ToList(); + ws.Sort(CompareWritingSystems); + return ws; + } + + /// A comparison function for sorting a List of writing systems. + private static int CompareWritingSystems(WritingSystem x, WritingSystem y) + { + var xTag = x.Bcp47; + var yTag = y.Bcp47; + var xSubtag = xTag.Split("-").First(); + var ySubtag = yTag.Split("-").First(); + if (xSubtag.Length != ySubtag.Length) + { + // 3-letter language tags should sort before 2-letter ones. + return ySubtag.Length - xSubtag.Length; + } + if (x.Name != y.Name) + { + // Named writing systems should sort before nameless ones. + if (string.IsNullOrEmpty(x.Name)) + { + return 1; + } + if (string.IsNullOrEmpty(y.Name)) + { + return -1; + } + + // Primarily sort by writing system Name. + return String.Compare(x.Name, y.Name, StringComparison.InvariantCultureIgnoreCase); + } + + // Secondarily sort by writing system BCP47 tag. + return String.Compare(xTag, yTag, StringComparison.InvariantCultureIgnoreCase); } } } diff --git a/Backend/Helper/LiftHelper.cs b/Backend/Helper/LiftHelper.cs index 78cd6bb3b1..06c8788bf8 100644 --- a/Backend/Helper/LiftHelper.cs +++ b/Backend/Helper/LiftHelper.cs @@ -63,7 +63,7 @@ public static string GetLiftRootFromExtractedZip(string dirPath) public static bool IsProtected(LiftEntry entry) { return entry.Annotations.Count > 0 || entry.Etymologies.Count > 0 || entry.Fields.Count > 0 || - (entry.Notes.Count == 1 && !String.IsNullOrEmpty(entry.Notes.First().Type)) || + (entry.Notes.Count == 1 && !string.IsNullOrEmpty(entry.Notes.First().Type)) || entry.Notes.Count > 1 || entry.Pronunciations.Count > 0 || entry.Relations.Count > 0 || entry.Traits.Any(t => !t.Value.Equals("stem", StringComparison.OrdinalIgnoreCase)) || entry.Variants.Count > 0; diff --git a/Backend/Services/LiftService.cs b/Backend/Services/LiftService.cs index b2d9fd3523..183b5bff91 100644 --- a/Backend/Services/LiftService.cs +++ b/Backend/Services/LiftService.cs @@ -390,7 +390,7 @@ public async Task LiftExport( // Export character set to ldml. var ldmlDir = Path.Combine(zipDir, "WritingSystems"); Directory.CreateDirectory(ldmlDir); - if (!String.IsNullOrWhiteSpace(proj.VernacularWritingSystem.Bcp47)) + if (!string.IsNullOrWhiteSpace(proj.VernacularWritingSystem.Bcp47)) { var validChars = proj.ValidCharacters; LdmlExport(ldmlDir, proj.VernacularWritingSystem, validChars); @@ -531,7 +531,7 @@ private static void LdmlExport(string filePath, WritingSystem vernacularWS, List wsf.Create(vernacularWS.Bcp47, out var wsDef); // If the vernacular writing system font isn't present, add it. - if (!String.IsNullOrWhiteSpace(vernacularWS.Font) + if (!string.IsNullOrWhiteSpace(vernacularWS.Font) && !wsDef.Fonts.Any(f => f.Name == vernacularWS.Font)) { wsDef.Fonts.Add(new FontDefinition(vernacularWS.Font)); @@ -648,7 +648,7 @@ public List GetImportAnalysisWritingSystems() w => w.Senses.SelectMany(s => Language.GetSenseAnalysisLangTags(s)) ).Distinct(); - return Language.ConvertLangTagsToWritingSystems(langTags).ToList(); + return Language.ConvertLangTagsToWritingSystems(langTags); } /// @@ -771,7 +771,7 @@ public void FinishEntry(LiftEntry entry) } // Add grammatical info - if (!String.IsNullOrWhiteSpace(sense.GramInfo?.Value)) + if (!string.IsNullOrWhiteSpace(sense.GramInfo?.Value)) { newSense.GrammaticalInfo = new GrammaticalInfo(sense.GramInfo.Value); } From 1928b6c40de76664eef0fd22f532434261561863 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Tue, 4 Jul 2023 10:42:27 -0400 Subject: [PATCH 2/2] Use WritingSystem contructor; Add ComparteWritingSystems test --- Backend.Tests/Helper/LanguageTests.cs | 31 +++++++++++++++++++++------ Backend.Tests/Models/ProjectTests.cs | 21 ++++++++++-------- Backend.Tests/Util.cs | 7 +----- Backend/Controllers/LiftController.cs | 2 +- Backend/Helper/Language.cs | 8 +++---- Backend/Models/Project.cs | 27 ++++++++++------------- 6 files changed, 54 insertions(+), 42 deletions(-) diff --git a/Backend.Tests/Helper/LanguageTests.cs b/Backend.Tests/Helper/LanguageTests.cs index a182aa2997..d280586af5 100644 --- a/Backend.Tests/Helper/LanguageTests.cs +++ b/Backend.Tests/Helper/LanguageTests.cs @@ -38,13 +38,32 @@ public void TestGetSenseAnalysisLangTagsFull() public void TestConvertLangTagToWritingSystemEn() { var tags = new List { "en", "en-US", "ajsdlfj" }; - var writingSystems = new List { - new WritingSystem {Bcp47 = "en", Name = "English"}, - new WritingSystem {Bcp47 = "en-US", Name = "English"}, - new WritingSystem {Bcp47 = "ajsdlfj", Name = ""}, - - }; + var writingSystems = + new List { new("en", "English"), new("en-US", "English"), new("ajsdlfj") }; Assert.That(ConvertLangTagsToWritingSystems(tags).ToList(), Is.EqualTo(writingSystems)); } + + [Test] + public void TestCompareWritingSystems() + { + // 3-letter codes first. + // Named before unnamed, alphabetical primarily by name. + var ws1 = new WritingSystem("pis", "Pijin"); + var ws2 = new WritingSystem("pis", "Solomons Pijin"); + // Unnamed after named, alphabetical by code. + var ws3 = new WritingSystem("pis-x-PIJ"); + var ws4 = new WritingSystem("qaa-pis"); + // 2-letter codes last. + // Named before unnamed, alphabetical secondarily by code. + var ws5 = new WritingSystem("en", "English"); + var ws6 = new WritingSystem("en-US", "English"); + // Unnamed after named. + var ws7 = new WritingSystem("en"); + + var toSort = new List { ws6, ws7, ws4, ws5, ws2, ws3, ws1 }; + toSort.Sort(CompareWritingSystems); + Assert.That(toSort, Is.EqualTo( + new List { ws1, ws2, ws3, ws4, ws5, ws6, ws7 })); + } } } diff --git a/Backend.Tests/Models/ProjectTests.cs b/Backend.Tests/Models/ProjectTests.cs index e151dde504..12b3c44187 100644 --- a/Backend.Tests/Models/ProjectTests.cs +++ b/Backend.Tests/Models/ProjectTests.cs @@ -99,7 +99,7 @@ public void TestNotEquals() [Test] public void TestClone() { - var system = new WritingSystem { Name = "WritingSystemName", Bcp47 = "en", Font = "calibri" }; + var system = new WritingSystem("en", "WritingSystemName", "calibri"); var project = new Project { Name = "ProjectName", VernacularWritingSystem = system }; var domain = new SemanticDomain { Name = "SemanticDomainName", Id = "1" }; project.SemanticDomains.Add(domain); @@ -170,35 +170,38 @@ public void TestHashCode() public class WritingSystemTests { - private const string Name = "System 1"; private const string Bcp47 = "lang-1"; + private const string Name = "System 1"; private const string Font = "calibri"; [Test] public void TestEquals() { - var system = new WritingSystem { Name = Name }; - Assert.That(system.Equals(new WritingSystem { Name = Name })); + var system = new WritingSystem(Bcp47, Name); + Assert.That(system.Equals(new WritingSystem(Bcp47, Name))); } [Test] public void TestEqualsNull() { - var system = new WritingSystem { Name = Name }; + var system = new WritingSystem(Bcp47, Name); Assert.IsFalse(system.Equals(null)); } [Test] public void TestNotEquals() { - var system = new WritingSystem { Name = Name, Bcp47 = Bcp47 }; - Assert.IsFalse(system.Equals(new WritingSystem { Name = Name })); + var system = new WritingSystem(Bcp47, Name); + Assert.IsFalse(system.Equals(new WritingSystem(Bcp47))); + + system = new WritingSystem(Bcp47, Name, Font); + Assert.IsFalse(system.Equals(new WritingSystem(Bcp47, Name))); } [Test] public void TestToString() { - var system = new WritingSystem { Name = Name, Bcp47 = Bcp47 }; + var system = new WritingSystem(Bcp47, Name); var sysString = system.ToString(); Assert.IsTrue(sysString.Contains(Name) && sysString.Contains(Bcp47)); } @@ -206,7 +209,7 @@ public void TestToString() [Test] public void TestClone() { - var system = new WritingSystem { Name = Name, Bcp47 = Bcp47, Font = Font }; + var system = new WritingSystem(Bcp47, Name, Font); var clonedSystem = system.Clone(); Assert.AreEqual(system, clonedSystem); } diff --git a/Backend.Tests/Util.cs b/Backend.Tests/Util.cs index 87449254a1..a92e72cb34 100644 --- a/Backend.Tests/Util.cs +++ b/Backend.Tests/Util.cs @@ -132,12 +132,7 @@ public static Project RandomProject() public static WritingSystem RandomWritingSystem() { - return new WritingSystem - { - Name = RandString(), - Bcp47 = RandString(), - Font = RandString() - }; + return new(RandString(), RandString(), RandString()); } } } diff --git a/Backend/Controllers/LiftController.cs b/Backend/Controllers/LiftController.cs index 498a75af87..1512690669 100644 --- a/Backend/Controllers/LiftController.cs +++ b/Backend/Controllers/LiftController.cs @@ -242,7 +242,7 @@ private async Task AddImportToProject(string liftStoragePath, str if (project.AnalysisWritingSystems.Count == 0) { // The list cannot be empty. - project.AnalysisWritingSystems.Add(new WritingSystem { Bcp47 = "en", Name = "English" }); + project.AnalysisWritingSystems.Add(new("en", "English")); } // Store whether we have imported any senses with definitions or grammatical info diff --git a/Backend/Helper/Language.cs b/Backend/Helper/Language.cs index bad130662c..fcd877c15d 100644 --- a/Backend/Helper/Language.cs +++ b/Backend/Helper/Language.cs @@ -30,7 +30,7 @@ public static List ConvertLangTagsToWritingSystems(IEnumerable - new WritingSystem { Bcp47 = tag, Name = langLookup.GetLanguageFromCode(tag)?.DesiredName ?? "" }); + new WritingSystem(tag, langLookup.GetLanguageFromCode(tag)?.DesiredName ?? "")); if (!isSldrInitialized) { Sldr.Cleanup(); @@ -55,7 +55,7 @@ public static List GetWritingSystems(string dirPath) } /// A comparison function for sorting a List of writing systems. - private static int CompareWritingSystems(WritingSystem x, WritingSystem y) + public static int CompareWritingSystems(WritingSystem x, WritingSystem y) { var xTag = x.Bcp47; var yTag = y.Bcp47; @@ -79,11 +79,11 @@ private static int CompareWritingSystems(WritingSystem x, WritingSystem y) } // Primarily sort by writing system Name. - return String.Compare(x.Name, y.Name, StringComparison.InvariantCultureIgnoreCase); + return String.Compare(x.Name, y.Name, StringComparison.OrdinalIgnoreCase); } // Secondarily sort by writing system BCP47 tag. - return String.Compare(xTag, yTag, StringComparison.InvariantCultureIgnoreCase); + return String.Compare(xTag, yTag, StringComparison.OrdinalIgnoreCase); } } } diff --git a/Backend/Models/Project.cs b/Backend/Models/Project.cs index 110f189eab..547681939f 100644 --- a/Backend/Models/Project.cs +++ b/Backend/Models/Project.cs @@ -285,35 +285,30 @@ public override int GetHashCode() public class WritingSystem { - [Required] - public string Name { get; set; } [Required] public string Bcp47 { get; set; } [Required] + public string Name { get; set; } + [Required] public string Font { get; set; } - public WritingSystem() + public WritingSystem(string bcp47 = "", string name = "", string font = "") { - Name = ""; - Bcp47 = ""; - Font = ""; + Bcp47 = bcp47; + Name = name; + Font = font; } public WritingSystem(WritingSystemDefinition wsd) { - Name = wsd.Language?.Name ?? ""; Bcp47 = wsd.LanguageTag; + Name = wsd.Language?.Name ?? ""; Font = wsd.DefaultFont?.Name ?? ""; } public WritingSystem Clone() { - return new WritingSystem - { - Name = Name, - Bcp47 = Bcp47, - Font = Font - }; + return new(Bcp47, Name, Font); } public override bool Equals(object? obj) @@ -323,14 +318,14 @@ public override bool Equals(object? obj) return false; } - return Name.Equals(ws.Name, StringComparison.Ordinal) && - Bcp47.Equals(ws.Bcp47, StringComparison.Ordinal) && + return Bcp47.Equals(ws.Bcp47, StringComparison.Ordinal) && + Name.Equals(ws.Name, StringComparison.Ordinal) && Font.Equals(ws.Font, StringComparison.Ordinal); } public override int GetHashCode() { - return HashCode.Combine(Name, Bcp47, Font); + return HashCode.Combine(Bcp47, Name, Font); } public override string ToString()