From 7342edbc9923c38ad8b6ff05df20ba0ec284a2a3 Mon Sep 17 00:00:00 2001 From: Peter Chapman Date: Wed, 5 Feb 2025 14:31:01 +1300 Subject: [PATCH] Code review fixes --- src/SIL.Converters.Usj/Usj.cs | 4 +- src/SIL.Converters.Usj/UsjBase.cs | 6 +- src/SIL.Converters.Usj/UsjMarker.cs | 11 +- src/SIL.Converters.Usj/UsjToUsx.cs | 51 ++++- src/SIL.Converters.Usj/Usx.cs | 4 +- src/SIL.Converters.Usj/UsxToUsj.cs | 21 +- test/SIL.Converters.Usj.Tests/TestData.cs | 193 ++++++++++++++---- .../SIL.Converters.Usj.Tests/UsjToUsxTests.cs | 124 ++++++++++- .../SIL.Converters.Usj.Tests/UsxToUsjTests.cs | 145 ++++++++++++- 9 files changed, 495 insertions(+), 64 deletions(-) diff --git a/src/SIL.Converters.Usj/Usj.cs b/src/SIL.Converters.Usj/Usj.cs index 719bc7e997..42a34fd67a 100644 --- a/src/SIL.Converters.Usj/Usj.cs +++ b/src/SIL.Converters.Usj/Usj.cs @@ -9,12 +9,12 @@ public class Usj : UsjBase, IUsj /// /// The supported USJ spec type. /// - public const string UsjType = "USJ"; + public static readonly string UsjType = "USJ"; /// /// The supported USJ spec version. /// - public const string UsjVersion = "3.1"; + public static readonly string UsjVersion = "3.1"; /// /// The USJ spec version. diff --git a/src/SIL.Converters.Usj/UsjBase.cs b/src/SIL.Converters.Usj/UsjBase.cs index d5687850d7..a40c76d1d8 100644 --- a/src/SIL.Converters.Usj/UsjBase.cs +++ b/src/SIL.Converters.Usj/UsjBase.cs @@ -22,7 +22,11 @@ public abstract class UsjBase /// The JSON representation of scripture contents from USFM/USX. /// /// This will either be a or . - /// Nullable. The contents will be laid out in order. + /// + /// Nullable. + /// If there are no contents, this will be null for , or empty for . + /// The contents will be laid out in order. + /// [JsonConverter(typeof(UsjContentConverter))] public ArrayList Content { get; set; } diff --git a/src/SIL.Converters.Usj/UsjMarker.cs b/src/SIL.Converters.Usj/UsjMarker.cs index a6b6486bf0..06fd597866 100644 --- a/src/SIL.Converters.Usj/UsjMarker.cs +++ b/src/SIL.Converters.Usj/UsjMarker.cs @@ -12,13 +12,14 @@ public class UsjMarker : UsjBase public string Marker { get; set; } /// - /// Indicates the Book-chapter-verse value in the paragraph based structure. + /// The milestone start ID, which indicates the Book-chapter-verse value in the paragraph based structure. /// /// Nullable. public string Sid { get; set; } /// - /// Milestone end ID, matches start ID (not currently included in USJ spec). + /// Milestone end ID, which matches the milestone start ID . + /// is not specified in the USJ spec, but is kept for USX compatibility. /// /// Nullable. public string Eid { get; set; } @@ -42,8 +43,12 @@ public class UsjMarker : UsjBase public string AltNumber { get; set; } /// - /// Published character of chapter or verse. + /// Published character of a chapter or verse. /// + /// + /// This can be a letter (I, II, etc.), a number (1, 2, ...), or both. + /// It is only displayed in the published version of the scripture text. + /// /// Nullable. public string PubNumber { get; set; } diff --git a/src/SIL.Converters.Usj/UsjToUsx.cs b/src/SIL.Converters.Usj/UsjToUsx.cs index 52fd915c0f..2392a0348c 100644 --- a/src/SIL.Converters.Usj/UsjToUsx.cs +++ b/src/SIL.Converters.Usj/UsjToUsx.cs @@ -61,6 +61,13 @@ private static void SetAttributes(XmlElement element, UsjMarker markerContent) foreach (KeyValuePair nonStandardAttribute in markerContent.AdditionalData) { string key = nonStandardAttribute.Key; + + // Include any non-standard attributes that are strings for compatibility with USX. + // + // Notes: + // - If the non-standard attribute is not a string, discard it as it is invalid. + // - Type and marker are handled above, so do not repeat them. + // - Content is a collection handled in ConvertUsjRecurse. if (nonStandardAttribute.Value is string value && key != "type" && key != "marker" && key != "content") { element.SetAttribute(key, value); @@ -103,10 +110,14 @@ bool isLastItem throw new ArgumentOutOfRangeException(nameof(markerContent)); } + // Store the previous verse eid so we can close them in the correct place + string lastVerseEid = null; + // Create chapter and verse end elements from SID attributes. if (_verseEid != null && (type == "verse" || (parentElement.Name == "para" && isLastItem))) { eidElement = CreateVerseEndElement(usxDoc); + lastVerseEid = _verseEid; _verseEid = null; } @@ -126,15 +137,28 @@ bool isLastItem _chapterEid = usjMarker.Sid; } - // Append to parent. + // See if we are at a new verse + if (eidElement != null && isLastItem && _verseEid != null && _verseEid != lastVerseEid) + { + // Write the eid element for the previous verse + parentElement.AppendChild(eidElement); + + // Ensure that eid element for the current verse is not written + eidElement = null; + _verseEid = null; + } + + // Append to parent to close the verse or chapter before this new node if (eidElement != null && !isLastItem) { parentElement.AppendChild(eidElement); + eidElement = null; } parentElement.AppendChild(node); - if (eidElement != null && isLastItem) + // Append the eid element as this is the last element + if (eidElement != null) { parentElement.AppendChild(eidElement); } @@ -167,18 +191,35 @@ private void UsjToUsxDom(IUsj usj, XmlDocument usxDoc) } /// - /// Converts a USJ object to a USX string. + /// Converts a USJ object to a USX . /// /// The USJ object. - /// The USX as a string. - public string UsjToUsxString(IUsj usj) + /// The XML Document. + public XmlDocument UsjToUsxXmlDocument(IUsj usj) { + // Reset any instance variables + _chapterEid = null; + _verseEid = null; + + // Create the USX document XmlDocument usxDoc = new XmlDocument { PreserveWhitespace = true }; XmlElement documentElement = usxDoc.CreateElement(Usx.UsxType); documentElement.SetAttribute("version", Usx.UsxVersion); usxDoc.AppendChild(documentElement); UsjToUsxDom(usj, usxDoc); + return usxDoc; + } + + /// + /// Converts a USJ object to a USX string. + /// + /// The USJ object. + /// The USX as a string. + public string UsjToUsxString(IUsj usj) + { + XmlDocument usxDoc = UsjToUsxXmlDocument(usj); + // Output as a string using (StringWriter stringWriter = new StringWriter()) { // These settings conform to ParatextData.UsfmToUsx diff --git a/src/SIL.Converters.Usj/Usx.cs b/src/SIL.Converters.Usj/Usx.cs index ab8036a7cd..b79c9fc646 100644 --- a/src/SIL.Converters.Usj/Usx.cs +++ b/src/SIL.Converters.Usj/Usx.cs @@ -9,11 +9,11 @@ public static class Usx /// /// The USX spec type. /// - public const string UsxType = "usx"; + public static readonly string UsxType = "usx"; /// /// The USX spec version. /// - public const string UsxVersion = "3.0"; + public static readonly string UsxVersion = "3.0"; } } diff --git a/src/SIL.Converters.Usj/UsxToUsj.cs b/src/SIL.Converters.Usj/UsxToUsj.cs index 58628942cd..ced7db931f 100644 --- a/src/SIL.Converters.Usj/UsxToUsj.cs +++ b/src/SIL.Converters.Usj/UsxToUsj.cs @@ -1,3 +1,4 @@ +using System; using System.Collections; using System.Collections.Generic; using System.Linq; @@ -33,7 +34,7 @@ private static (T, bool) UsxDomToUsjRecurse(XmlElement usxElement) .Attributes.Cast() .ToDictionary(attrib => attrib.Name, attrib => attrib.Value); - // If style is present, make that the market + // If style is present, make that the marker if (attributes.TryGetValue("style", out string marker)) { attributes.Remove("style"); @@ -105,6 +106,9 @@ private static (T, bool) UsxDomToUsjRecurse(XmlElement usxElement) outObj.Content.Add(childDict); } + // If the next sibling is text or a user inputted space (a single space), add it to the content. + // We skip whitespace nodes with more than one space because they are padding, not formatting spaces. + // Note: Any Paratext 9.5 special whitespace characters will have a node type of XmlNodeType.Text. if ( child.NextSibling?.NodeType == XmlNodeType.Text || (child.NextSibling?.NodeType == XmlNodeType.Whitespace && child.NextSibling.Value == " ") @@ -121,7 +125,7 @@ private static (T, bool) UsxDomToUsjRecurse(XmlElement usxElement) if (outObj is UsjMarker usjMarker2 && usjMarker2.Eid != null && (type == "verse" || type == "chapter")) { - // Ignore + // Omit any verse or chapter eid elements append = false; } @@ -173,6 +177,17 @@ public static Usj UsxStringToUsj(string usx) /// /// The should have set to true. /// - public static Usj UsxXmlDocumentToUsj(XmlDocument xmlDocument) => UsxDomToUsj(xmlDocument?.DocumentElement); + public static Usj UsxXmlDocumentToUsj(XmlDocument xmlDocument) + { + if (xmlDocument?.PreserveWhitespace != true) + { + throw new ArgumentException( + "The XmlDocument should have PreserveWhitespace set to true.", + nameof(xmlDocument) + ); + } + + return UsxDomToUsj(xmlDocument.DocumentElement); + } } } diff --git a/test/SIL.Converters.Usj.Tests/TestData.cs b/test/SIL.Converters.Usj.Tests/TestData.cs index 7b196b928d..59bedc7711 100644 --- a/test/SIL.Converters.Usj.Tests/TestData.cs +++ b/test/SIL.Converters.Usj.Tests/TestData.cs @@ -32,7 +32,7 @@ public static string RemoveXmlWhiteSpace(string xml) /// /// Genesis 1:1 in USJ as a JSON string. /// - public const string JsonGen1V1 = $$""" + public static readonly string JsonGen1V1 = $$""" { type: "{{Usj.UsjType}}", version: "{{Usj.UsjVersion}}", @@ -50,6 +50,8 @@ public static string RemoveXmlWhiteSpace(string xml) { type: "verse", marker: "v", number: "15", altnumber: "3", sid: "GEN 1:15" }, "Tell the Israelites that I, the ", { type: "char", marker: "nd", content: ["Lord"] }, + " ", + { type: "char", marker: "nd", content: ["God"] }, ", the God of their ancestors, the God of Abraham, Isaac, and Jacob,", { type: "char", marker: "va", content: ["4"] }, ], @@ -88,7 +90,7 @@ public static string RemoveXmlWhiteSpace(string xml) /// /// An empty USX document. /// - public const string UsxEmpty = $""""""; + public static readonly string UsxEmpty = $""""""; /// /// An empty USJ object. @@ -107,17 +109,17 @@ public static string RemoveXmlWhiteSpace(string xml) /// - Whitespace for all self-closing element endings. /// - Reorder attributes to match UsxUsfmParserSink output. /// - public const string UsxGen1V1 = $""" + public static readonly string UsxGen1V1 = $""" Some Scripture Version - - the first verse - the second verse - Tell the Israelites that I, the Lord, the God of their ancestors, the God of Abraham, Isaac, and Jacob,4 - - - “There is no help for him in God.”3:2 The Hebrew word rendered “God” is “אֱלֹהִ֑ים” (Elohim). Selah. + + the first verse + the second verse + Tell the Israelites that I, the Lord God, the God of their ancestors, the God of Abraham, Isaac, and Jacob,4 + + + “There is no help for him in God.”3:2 The Hebrew word rendered “God” is “אֱלֹהִ֑ים” (Elohim). Selah. """; @@ -139,13 +141,13 @@ public static string RemoveXmlWhiteSpace(string xml) /// Tests a chapter with an implied paragraph. /// /// Attributes are reordered to match UsxUsfmParserSink output. - public const string UsxGen1V1ImpliedPara = $""" + public static readonly string UsxGen1V1ImpliedPara = $""" - the first verse - the second verse - Tell the Israelites that I, the Lord, the God of their ancestors, the God of Abraham, Isaac, and Jacob, + the first verse + the second verse + Tell the Israelites that I, the Lord, the God of their ancestors, the God of Abraham, Isaac, and Jacob, """; @@ -178,18 +180,18 @@ public static string RemoveXmlWhiteSpace(string xml) /// Additional test features: /// - Preserve contents of `ca` even though it seems possible `ca` should not occur as its own marker /// - Preserve non-standard contents of `b` marker that should not have contents - /// - Preserve closed attribute on character marker + /// - Preserve closed attribute on character marker. This is non-standard use of a non-standard marker. /// - Reorder attributes to match UsxUsfmParserSink output. /// - public const string UsxGen1V1Nonstandard = $""" + public static readonly string UsxGen1V1Nonstandard = $""" Some Scripture Version - - the first verse - the second verse 4 - - This should not be here + + the first verse + the second verse 4 + + This should not be here """; @@ -241,11 +243,11 @@ public static string RemoveXmlWhiteSpace(string xml) /// TODO: Especially concerning is that the editor inserts a bunch of ZWSP in many places in the editable state. /// /// Attributes are reordered to match UsxUsfmParserSink output. - public const string UsxGen1V1Whitespace = $""" + public static readonly string UsxGen1V1Whitespace = $""" - space between each{IDEOGRAPHIC_SPACE}word should{THIN_SPACE}{IDEOGRAPHIC_SPACE} stay + space between each{IDEOGRAPHIC_SPACE}word should{THIN_SPACE}{IDEOGRAPHIC_SPACE} stay """; @@ -282,17 +284,17 @@ public static string RemoveXmlWhiteSpace(string xml) /// This is a version of with attributes that will be removed. /// If round-tripping, compare the final USX to . /// - public const string UsxGen1V1WithAttributesToRemove = $""" + public static readonly string UsxGen1V1WithAttributesToRemove = $""" Some Scripture Version - - the first verse - the second verse - Tell the Israelites that I, the Lord, the God of their ancestors, the God of Abraham, Isaac, and Jacob,4 - - - “There is no help for him in God.”3:2 The Hebrew word rendered “God” is “אֱלֹהִ֑ים” (Elohim). Selah. + + the first verse + the second verse + Tell the Israelites that I, the Lord God, the God of their ancestors, the God of Abraham, Isaac, and Jacob,4 + + + “There is no help for him in God.”3:2 The Hebrew word rendered “God” is “אֱלֹהִ֑ים” (Elohim). Selah. """; @@ -300,20 +302,20 @@ public static string RemoveXmlWhiteSpace(string xml) /// /// Genesis 1:1 in USX (with a table). /// - public const string UsxGen1V1WithTable = $""" + public static readonly string UsxGen1V1WithTable = $""" Some Scripture Version - - - Tribe - Leader - Number - -
- - the first verse - + + + Tribe + Leader + Number + +
+ + the first verse + """; @@ -355,4 +357,111 @@ public static string RemoveXmlWhiteSpace(string xml) } """ )!; + + /// + /// Genesis 1:1 in USJ (with additional blank chapters). + /// + public static readonly Usj UsjGen1V1WithBlankChapters = JsonConvert.DeserializeObject( + $$""" + { + type: "{{Usj.UsjType}}", + version: "{{Usj.UsjVersion}}", + content: [ + { type: "book", marker: "id", code: "GEN" }, + { type: "chapter", marker: "c", number: "1", sid: "GEN 1" }, + { type: "verse", marker: "v", number: "1", sid: "GEN 1:1" }, + "In the beginning ", + { type: "verse", marker: "v", number: "2", sid: "GEN 1:2" }, + { type: "chapter", marker: "c", number: "2", sid: "GEN 2" }, + { type: "chapter", marker: "c", number: "3", sid: "GEN 3" }, + ], + } + """ + )!; + + /// + /// Genesis 1:1 in USX (with additional blank chapters). + /// + public static readonly string UsxGen1V1WithBlankChapters = $""" + + + + In the beginning + + + + + """; + + /// + /// Genesis 1:1 in USJ (with blank verses). + /// + public static readonly Usj UsjGen1V1WithBlankVerses = JsonConvert.DeserializeObject( + $$""" + { + type: "{{Usj.UsjType}}", + version: "{{Usj.UsjVersion}}", + content: [ + { type: "book", marker: "id", code: "GEN" }, + { type: "chapter", marker: "c", number: "1", sid: "GEN 1" }, + { type: "verse", marker: "v", number: "1", sid: "GEN 1:1" }, + "In the beginning ", + { type: "verse", marker: "v", number: "2", sid: "GEN 1:2" }, + { type: "verse", marker: "v", number: "3", sid: "GEN 1:3" }, + { type: "verse", marker: "v", number: "4", sid: "GEN 1:4" }, + ], + } + """ + )!; + + /// + /// Genesis 1:1 in USX (with blank verses). + /// + + + public static readonly string UsxGen1V1WithBlankVerses = $""" + + + + In the beginning + + + + + + """; + + /// + /// Genesis 1:1 in USJ (with no sids). + /// + public static readonly Usj UsjGen1V1WithNoSids = JsonConvert.DeserializeObject( + $$""" + { + type: "{{Usj.UsjType}}", + version: "{{Usj.UsjVersion}}", + content: [ + { type: "book", marker: "id", code: "GEN" }, + { type: "chapter", marker: "c", number: "1"}, + { type: "verse", marker: "v", number: "1" }, + "In the beginning ", + { type: "verse", marker: "v", number: "2" }, + { type: "verse", marker: "v", number: "3" }, + { type: "verse", marker: "v", number: "4" }, + ], + } + """ + )!; + + /// + /// Genesis 1:1 in USJ (with no sids). + /// + public static readonly string UsxGen1V1WithNoSids = $""" + + + + In the beginning + + + + """; } diff --git a/test/SIL.Converters.Usj.Tests/UsjToUsxTests.cs b/test/SIL.Converters.Usj.Tests/UsjToUsxTests.cs index b45f3f06e8..ecaf3b8e3e 100644 --- a/test/SIL.Converters.Usj.Tests/UsjToUsxTests.cs +++ b/test/SIL.Converters.Usj.Tests/UsjToUsxTests.cs @@ -1,4 +1,5 @@ using System; +using System.Xml; using NUnit.Framework; namespace SIL.Converters.Usj.Tests; @@ -14,6 +15,15 @@ public void ShouldConvertFromEmptyUsjToUsx() Assert.That(usx, Is.EqualTo(TestData.UsxEmpty)); } + [Test] + public void ShouldConvertFromEmptyUsjToUsx_Roundtrip() + { + var usjToUsx = new UsjToUsx(); + string usx = usjToUsx.UsjToUsxString(TestData.UsjEmpty); + Usj usj = UsxToUsj.UsxStringToUsj(usx); + Assert.That(usj, Is.EqualTo(TestData.UsjEmpty).UsingPropertiesComparer()); + } + [Test] public void ShouldConvertFromUsjToUsx() { @@ -24,7 +34,7 @@ public void ShouldConvertFromUsjToUsx() } [Test] - public void ShouldConvertFromUsjToUsxAndBack() + public void ShouldConvertFromUsjToUsx_Roundtrip() { var usjToUsx = new UsjToUsx(); string usx = usjToUsx.UsjToUsxString(TestData.UsjGen1V1); @@ -32,6 +42,64 @@ public void ShouldConvertFromUsjToUsxAndBack() Assert.That(usj, Is.EqualTo(TestData.UsjGen1V1).UsingPropertiesComparer()); } + [Test] + public void ShouldConvertFromUsjToUsxXmlDocument() + { + // Setup + var expectedUsx = new XmlDocument { PreserveWhitespace = true }; + expectedUsx.LoadXml(TestData.RemoveXmlWhiteSpace(TestData.UsxGen1V1)); + + // SUT + var usjToUsx = new UsjToUsx(); + XmlDocument usx = usjToUsx.UsjToUsxXmlDocument(TestData.UsjGen1V1); + Assert.That(usx, Is.EqualTo(expectedUsx)); + } + + [Test] + public void ShouldConvertFromUsjToUsxXmlDocument_Roundtrip() + { + var usjToUsx = new UsjToUsx(); + XmlDocument usx = usjToUsx.UsjToUsxXmlDocument(TestData.UsjGen1V1); + Usj usj = UsxToUsj.UsxXmlDocumentToUsj(usx); + Assert.That(usj, Is.EqualTo(TestData.UsjGen1V1).UsingPropertiesComparer()); + } + + [Test] + public void ShouldConvertFromUsjToUsxWithBlankChapters() + { + var usjToUsx = new UsjToUsx(); + string usx = usjToUsx.UsjToUsxString(TestData.UsjGen1V1WithBlankChapters); + usx = TestData.RemoveXmlWhiteSpace(usx); + Assert.That(usx, Is.EqualTo(TestData.RemoveXmlWhiteSpace(TestData.UsxGen1V1WithBlankChapters))); + } + + [Test] + public void ShouldConvertFromUsjToUsxWithBlankChapters_Roundtrip() + { + var usjToUsx = new UsjToUsx(); + string usx = usjToUsx.UsjToUsxString(TestData.UsjGen1V1WithBlankChapters); + Usj usj = UsxToUsj.UsxStringToUsj(usx); + Assert.That(usj, Is.EqualTo(TestData.UsjGen1V1WithBlankChapters).UsingPropertiesComparer()); + } + + [Test] + public void ShouldConvertFromUsjToUsxWithBlankVerses() + { + var usjToUsx = new UsjToUsx(); + string usx = usjToUsx.UsjToUsxString(TestData.UsjGen1V1WithBlankVerses); + usx = TestData.RemoveXmlWhiteSpace(usx); + Assert.That(usx, Is.EqualTo(TestData.RemoveXmlWhiteSpace(TestData.UsxGen1V1WithBlankVerses))); + } + + [Test] + public void ShouldConvertFromUsjToUsxWithBlankVerses_Roundtrip() + { + var usjToUsx = new UsjToUsx(); + string usx = usjToUsx.UsjToUsxString(TestData.UsjGen1V1WithBlankVerses); + Usj usj = UsxToUsj.UsxStringToUsj(usx); + Assert.That(usj, Is.EqualTo(TestData.UsjGen1V1WithBlankVerses).UsingPropertiesComparer()); + } + [Test] public void ShouldConvertFromUsjWithImpliedParagraphsToUsx() { @@ -41,6 +109,15 @@ public void ShouldConvertFromUsjWithImpliedParagraphsToUsx() Assert.That(usx, Is.EqualTo(TestData.RemoveXmlWhiteSpace(TestData.UsxGen1V1ImpliedPara))); } + [Test] + public void ShouldConvertFromUsjWithImpliedParagraphsToUsx_Roundtrip() + { + var usjToUsx = new UsjToUsx(); + string usx = usjToUsx.UsjToUsxString(TestData.UsjGen1V1ImpliedPara); + Usj usj = UsxToUsj.UsxStringToUsj(usx); + Assert.That(usj, Is.EqualTo(TestData.UsjGen1V1ImpliedPara).UsingPropertiesComparer()); + } + [Test] public void ShouldConvertFromUsjWithNonStandardFeaturesToUsx() { @@ -50,6 +127,33 @@ public void ShouldConvertFromUsjWithNonStandardFeaturesToUsx() Assert.That(usx, Is.EqualTo(TestData.RemoveXmlWhiteSpace(TestData.UsxGen1V1Nonstandard))); } + [Test] + public void ShouldConvertFromUsjWithNonStandardFeaturesToUsx_Roundtrip() + { + var usjToUsx = new UsjToUsx(); + string usx = usjToUsx.UsjToUsxString(TestData.UsjGen1V1Nonstandard); + Usj usj = UsxToUsj.UsxStringToUsj(usx); + Assert.That(usj, Is.EqualTo(TestData.UsjGen1V1Nonstandard).UsingPropertiesComparer()); + } + + [Test] + public void ShouldConvertFromUsjToUsxWithNoSids() + { + var usjToUsx = new UsjToUsx(); + string usx = usjToUsx.UsjToUsxString(TestData.UsjGen1V1WithNoSids); + usx = TestData.RemoveXmlWhiteSpace(usx); + Assert.That(usx, Is.EqualTo(TestData.RemoveXmlWhiteSpace(TestData.UsxGen1V1WithNoSids))); + } + + [Test] + public void ShouldConvertFromUsjToUsxWithNoSids_Roundtrip() + { + var usjToUsx = new UsjToUsx(); + string usx = usjToUsx.UsjToUsxString(TestData.UsjGen1V1WithNoSids); + Usj usj = UsxToUsj.UsxStringToUsj(usx); + Assert.That(usj, Is.EqualTo(TestData.UsjGen1V1WithNoSids).UsingPropertiesComparer()); + } + [Test] public void ShouldConvertFromUsjToUsxWithSpecialWhiteSpace() { @@ -59,6 +163,15 @@ public void ShouldConvertFromUsjToUsxWithSpecialWhiteSpace() Assert.That(usx, Is.EqualTo(TestData.RemoveXmlWhiteSpace(TestData.UsxGen1V1Whitespace))); } + [Test] + public void ShouldConvertFromUsjToUsxWithSpecialWhiteSpace_Roundtrip() + { + var usjToUsx = new UsjToUsx(); + string usx = usjToUsx.UsjToUsxString(TestData.UsjGen1V1Whitespace); + Usj usj = UsxToUsj.UsxStringToUsj(usx); + Assert.That(usj, Is.EqualTo(TestData.UsjGen1V1Whitespace).UsingPropertiesComparer()); + } + [Test] public void ShouldConvertFromUsjToUsxWithTable() { @@ -68,6 +181,15 @@ public void ShouldConvertFromUsjToUsxWithTable() Assert.That(usx, Is.EqualTo(TestData.RemoveXmlWhiteSpace(TestData.UsxGen1V1WithTable))); } + [Test] + public void ShouldConvertFromUsjToUsxWithTable_Roundtrip() + { + var usjToUsx = new UsjToUsx(); + string usx = usjToUsx.UsjToUsxString(TestData.UsjGen1V1WithTable); + Usj usj = UsxToUsj.UsxStringToUsj(usx); + Assert.That(usj, Is.EqualTo(TestData.UsjGen1V1WithTable).UsingPropertiesComparer()); + } + [Test] public void ShouldNotAllowInvalidContent() { diff --git a/test/SIL.Converters.Usj.Tests/UsxToUsjTests.cs b/test/SIL.Converters.Usj.Tests/UsxToUsjTests.cs index c68872ed1f..9af09acb89 100644 --- a/test/SIL.Converters.Usj.Tests/UsxToUsjTests.cs +++ b/test/SIL.Converters.Usj.Tests/UsxToUsjTests.cs @@ -1,3 +1,4 @@ +using System; using System.Xml; using NUnit.Framework; @@ -14,16 +15,19 @@ public void ShouldConvertFromEmptyUsxToUsj() } [Test] - public void ShouldConvertFromNullToUsj() + public void ShouldConvertFromEmptyUsxToUsj_Roundtrip() { - Usj usj = UsxToUsj.UsxStringToUsj(null); - Assert.That(usj, Is.EqualTo(TestData.UsjEmpty).UsingPropertiesComparer()); + Usj usj = UsxToUsj.UsxStringToUsj(TestData.UsxEmpty); + var usjToUsx = new UsjToUsx(); + string usx = usjToUsx.UsjToUsxString(usj); + usx = TestData.RemoveXmlWhiteSpace(usx); + Assert.That(usx, Is.EqualTo(TestData.RemoveXmlWhiteSpace(TestData.UsxEmpty))); } [Test] - public void ShouldConvertFromNullXmlDocumentToUsj() + public void ShouldConvertFromNullToUsj() { - Usj usj = UsxToUsj.UsxXmlDocumentToUsj(null); + Usj usj = UsxToUsj.UsxStringToUsj(null); Assert.That(usj, Is.EqualTo(TestData.UsjEmpty).UsingPropertiesComparer()); } @@ -34,6 +38,16 @@ public void ShouldConvertFromUsxToUsj() Assert.That(usj, Is.EqualTo(TestData.UsjGen1V1).UsingPropertiesComparer()); } + [Test] + public void ShouldConvertFromUsxToUsj_Roundtrip() + { + Usj usj = UsxToUsj.UsxStringToUsj(TestData.UsxGen1V1); + var usjToUsx = new UsjToUsx(); + string usx = usjToUsx.UsjToUsxString(usj); + usx = TestData.RemoveXmlWhiteSpace(usx); + Assert.That(usx, Is.EqualTo(TestData.RemoveXmlWhiteSpace(TestData.UsxGen1V1))); + } + [Test] public void ShouldConvertFromUsxToUsjAndRemoveSpecificAttributes() { @@ -41,6 +55,51 @@ public void ShouldConvertFromUsxToUsjAndRemoveSpecificAttributes() Assert.That(usj, Is.EqualTo(TestData.UsjGen1V1).UsingPropertiesComparer()); } + [Test] + public void ShouldConvertFromUsxToUsjAndRemoveSpecificAttributes_Roundtrip() + { + // NOTE: We do not compare with the original, as invalid attributes are removed + Usj usj = UsxToUsj.UsxStringToUsj(TestData.UsxGen1V1WithAttributesToRemove); + var usjToUsx = new UsjToUsx(); + string usx = usjToUsx.UsjToUsxString(usj); + usx = TestData.RemoveXmlWhiteSpace(usx); + Assert.That(usx, Is.EqualTo(TestData.RemoveXmlWhiteSpace(TestData.UsxGen1V1))); + } + + [Test] + public void ShouldConvertFromUsxToUsjWithBlankChapters() + { + Usj usj = UsxToUsj.UsxStringToUsj(TestData.UsxGen1V1WithBlankChapters); + Assert.That(usj, Is.EqualTo(TestData.UsjGen1V1WithBlankChapters).UsingPropertiesComparer()); + } + + [Test] + public void ShouldConvertFromUsxToUsjWithBlankChapters_Roundtrip() + { + Usj usj = UsxToUsj.UsxStringToUsj(TestData.UsxGen1V1WithBlankChapters); + var usjToUsx = new UsjToUsx(); + string usx = usjToUsx.UsjToUsxString(usj); + usx = TestData.RemoveXmlWhiteSpace(usx); + Assert.That(usx, Is.EqualTo(TestData.RemoveXmlWhiteSpace(TestData.UsxGen1V1WithBlankChapters))); + } + + [Test] + public void ShouldConvertFromUsxToUsjWithBlankVerses() + { + Usj usj = UsxToUsj.UsxStringToUsj(TestData.UsxGen1V1WithBlankVerses); + Assert.That(usj, Is.EqualTo(TestData.UsjGen1V1WithBlankVerses).UsingPropertiesComparer()); + } + + [Test] + public void ShouldConvertFromUsxToUsjWithBlankVerses_Roundtrip() + { + Usj usj = UsxToUsj.UsxStringToUsj(TestData.UsxGen1V1WithBlankVerses); + var usjToUsx = new UsjToUsx(); + string usx = usjToUsx.UsjToUsxString(usj); + usx = TestData.RemoveXmlWhiteSpace(usx); + Assert.That(usx, Is.EqualTo(TestData.RemoveXmlWhiteSpace(TestData.UsxGen1V1WithBlankVerses))); + } + [Test] public void ShouldConvertFromUsxToUsjWithImpliedParagraphs() { @@ -48,6 +107,16 @@ public void ShouldConvertFromUsxToUsjWithImpliedParagraphs() Assert.That(usj, Is.EqualTo(TestData.UsjGen1V1ImpliedPara).UsingPropertiesComparer()); } + [Test] + public void ShouldConvertFromUsxToUsjWithImpliedParagraphs_Roundtrip() + { + Usj usj = UsxToUsj.UsxStringToUsj(TestData.UsxGen1V1ImpliedPara); + var usjToUsx = new UsjToUsx(); + string usx = usjToUsx.UsjToUsxString(usj); + usx = TestData.RemoveXmlWhiteSpace(usx); + Assert.That(usx, Is.EqualTo(TestData.RemoveXmlWhiteSpace(TestData.UsxGen1V1ImpliedPara))); + } + [Test] public void ShouldConvertFromUsxToUsjWithSpecialWhiteSpace() { @@ -55,6 +124,16 @@ public void ShouldConvertFromUsxToUsjWithSpecialWhiteSpace() Assert.That(usj, Is.EqualTo(TestData.UsjGen1V1Whitespace).UsingPropertiesComparer()); } + [Test] + public void ShouldConvertFromUsxToUsjWithSpecialWhiteSpace_Roundtrip() + { + Usj usj = UsxToUsj.UsxStringToUsj(TestData.UsxGen1V1Whitespace); + var usjToUsx = new UsjToUsx(); + string usx = usjToUsx.UsjToUsxString(usj); + usx = TestData.RemoveXmlWhiteSpace(usx); + Assert.That(usx, Is.EqualTo(TestData.RemoveXmlWhiteSpace(TestData.UsxGen1V1Whitespace))); + } + [Test] public void ShouldConvertFromUsxToUsjWithNonStandardFeatures() { @@ -62,6 +141,33 @@ public void ShouldConvertFromUsxToUsjWithNonStandardFeatures() Assert.That(usj, Is.EqualTo(TestData.UsjGen1V1Nonstandard).UsingPropertiesComparer()); } + [Test] + public void ShouldConvertFromUsxToUsjWithNonStandardFeatures_RoundTrip() + { + Usj usj = UsxToUsj.UsxStringToUsj(TestData.UsxGen1V1Nonstandard); + var usjToUsx = new UsjToUsx(); + string usx = usjToUsx.UsjToUsxString(usj); + usx = TestData.RemoveXmlWhiteSpace(usx); + Assert.That(usx, Is.EqualTo(TestData.RemoveXmlWhiteSpace(TestData.UsxGen1V1Nonstandard))); + } + + [Test] + public void ShouldConvertFromUsxToUsjWithNoSids() + { + Usj usj = UsxToUsj.UsxStringToUsj(TestData.UsxGen1V1WithNoSids); + Assert.That(usj, Is.EqualTo(TestData.UsjGen1V1WithNoSids).UsingPropertiesComparer()); + } + + [Test] + public void ShouldConvertFromUsxToUsjWithNoSids_Roundtrip() + { + Usj usj = UsxToUsj.UsxStringToUsj(TestData.UsxGen1V1WithNoSids); + var usjToUsx = new UsjToUsx(); + string usx = usjToUsx.UsjToUsxString(usj); + usx = TestData.RemoveXmlWhiteSpace(usx); + Assert.That(usx, Is.EqualTo(TestData.RemoveXmlWhiteSpace(TestData.UsxGen1V1WithNoSids))); + } + [Test] public void ShouldConvertFromUsxToUsjWithTable() { @@ -69,6 +175,16 @@ public void ShouldConvertFromUsxToUsjWithTable() Assert.That(usj, Is.EqualTo(TestData.UsjGen1V1WithTable).UsingPropertiesComparer()); } + [Test] + public void ShouldConvertFromUsxToUsjWithTable_Roundtrip() + { + Usj usj = UsxToUsj.UsxStringToUsj(TestData.UsxGen1V1WithTable); + var usjToUsx = new UsjToUsx(); + string usx = usjToUsx.UsjToUsxString(usj); + usx = TestData.RemoveXmlWhiteSpace(usx); + Assert.That(usx, Is.EqualTo(TestData.RemoveXmlWhiteSpace(TestData.UsxGen1V1WithTable))); + } + [Test] public void ShouldConvertFromXmlDocumentToUsj() { @@ -77,4 +193,23 @@ public void ShouldConvertFromXmlDocumentToUsj() Usj usj = UsxToUsj.UsxXmlDocumentToUsj(document); Assert.That(usj, Is.EqualTo(TestData.UsjGen1V1).UsingPropertiesComparer()); } + + [Test] + public void ShouldConvertFromXmlDocumentToUsj_Roundtrip() + { + XmlDocument document = new XmlDocument { PreserveWhitespace = true }; + document.LoadXml(TestData.RemoveXmlWhiteSpace(TestData.UsxGen1V1)); + Usj usj = UsxToUsj.UsxXmlDocumentToUsj(document); + var usjToUsx = new UsjToUsx(); + XmlDocument usx = usjToUsx.UsjToUsxXmlDocument(usj); + Assert.That(usx, Is.EqualTo(document).UsingPropertiesComparer()); + } + + [Test] + public void ShouldNotConvertFromNullXmlDocumentToUsj() => + Assert.Throws(() => UsxToUsj.UsxXmlDocumentToUsj(null)); + + [Test] + public void ShouldNotConvertFromNonPreserveWhitespaceXmlDocumentToUsj() => + Assert.Throws(() => UsxToUsj.UsxXmlDocumentToUsj(new XmlDocument())); }