Only unload part when saving part if it is not the same element (#1828)

This is fall out from #1760 that removes the loaded part but doesn't check if we're saving an existing root element Fixes #1771
dotnet · Nov 20, 2024 · 1e6ebc9 · 1e6ebc9
1 parent 1c165bd
commit 1e6ebc9
Show file tree

Hide file tree

Showing 5 changed files with 46 additions and 18 deletions.
diff --git a/Open-XML-SDK.sln b/Open-XML-SDK.sln
@@ -217,9 +217,9 @@ Global
 		{890B74DD-6316-4D56-B42A-5D66F10F88C6} = {C5AE39F0-A152-471A-B90E-B8F4E94AA6C2}
 		{5DE9FB8F-A7C2-4038-A4A8-1622CDB6821A} = {7DAF7304-40CC-4180-88A5-9A89DD13C565}
 		{F48B7D50-16CB-4BEF-A614-2DED9960AF09} = {7DAF7304-40CC-4180-88A5-9A89DD13C565}
-		{C91489AB-FF14-4FAD-BA51-35371ADD7E1C} = {7DAF7304-40CC-4180-88A5-9A89DD13C565}
-		{BB5DF535-E849-42AC-852A-A6D4815347C0} = {3653266D-2C88-4487-8977-839CB3E78A0A}
-		{5241BCF2-331D-428E-A9C4-F8DF92C8F726} = {A4DF60EB-3AA5-48F0-B4D2-3F94B8E62F03}
+		{C91489AB-FF14-4FAD-BA51-35371ADD7E1C} = {3653266D-2C88-4487-8977-839CB3E78A0A}
+		{BB5DF535-E849-42AC-852A-A6D4815347C0} = {A4DF60EB-3AA5-48F0-B4D2-3F94B8E62F03}
+		{5241BCF2-331D-428E-A9C4-F8DF92C8F726} = {7DAF7304-40CC-4180-88A5-9A89DD13C565}
 		{3ECC7570-3501-479D-9CD9-64799C2AD149} = {7DAF7304-40CC-4180-88A5-9A89DD13C565}
 		{BADAC0CC-F3E6-440E-B322-DA2B97625F26} = {7DAF7304-40CC-4180-88A5-9A89DD13C565}
 		{8EE9F34E-EAA1-4F03-B388-8076CE44DD7B} = {7DAF7304-40CC-4180-88A5-9A89DD13C565}

diff --git a/src/DocumentFormat.OpenXml.Framework/Framework/Metadata/ElementFactory.cs b/src/DocumentFormat.OpenXml.Framework/Framework/Metadata/ElementFactory.cs
@@ -6,7 +6,7 @@
 
 namespace DocumentFormat.OpenXml.Framework.Metadata
 {
-    [DebuggerDisplay("{Namespace}:{Name}")]
+    [DebuggerDisplay("{QName,nq}")]
     internal sealed class ElementFactory
     {
         private readonly Func<OpenXmlElement> _factory;

diff --git a/src/DocumentFormat.OpenXml.Framework/OpenXmlPartRootElement.cs b/src/DocumentFormat.OpenXml.Framework/OpenXmlPartRootElement.cs
@@ -191,10 +191,10 @@ internal void SaveToPart(OpenXmlPart openXmlPart)
                 throw new ArgumentNullException(nameof(openXmlPart));
             }
 
-            using (Stream partStream = openXmlPart.GetStream(FileMode.Create))
-            {
-                Save(partStream);
-            }
+            // If we're saving the existing root to the the part, we don't need to unload the root as they're already equal
+            using var partStream = openXmlPart.GetStream(FileMode.Create, unloadRootOnChange: !ReferenceEquals(this, openXmlPart.PartRootElement));
+
+            Save(partStream);
         }
 
         /// <summary>

diff --git a/src/DocumentFormat.OpenXml.Framework/Packaging/OpenXmlPart.cs b/src/DocumentFormat.OpenXml.Framework/Packaging/OpenXmlPart.cs
@@ -236,9 +236,10 @@ public IEnumerable<OpenXmlPart> GetParentParts()
         /// <param name="mode">The I/O mode to be used to open the content stream.</param>
         /// <returns>The content stream of the part. </returns>
         public Stream GetStream(FileMode mode)
-        {
-            return GetStream(mode, Features.GetRequired<IPackageFeature>().Package.FileOpenAccess);
-        }
+            => GetStream(mode, unloadRootOnChange: true);
+
+        internal Stream GetStream(FileMode mode, bool unloadRootOnChange)
+            => GetStream(mode, Features.GetRequired<IPackageFeature>().Package.FileOpenAccess, unloadRootOnChange);
 
         /// <summary>
         /// Returns the part content stream that was opened using a specified FileMode and FileAccess.
@@ -247,20 +248,26 @@ public Stream GetStream(FileMode mode)
         /// <param name="access">The access permissions to be used to open the content stream.</param>
         /// <returns>The content stream of the part. </returns>
         public Stream GetStream(FileMode mode, FileAccess access)
+            => GetStream(mode, access, true);
+
+        internal Stream GetStream(FileMode mode, FileAccess access, bool unloadRootOnChange)
         {
             ThrowIfObjectDisposed();
 
             var stream = PackagePart.GetStream(mode, access);
 
-            if (mode is FileMode.Create || stream.Length == 0)
+            if (unloadRootOnChange)
             {
-                UnloadRootElement();
-                return new UnloadingRootElementStream(this, stream);
-            }
+                if (mode is FileMode.Create || stream.Length == 0)
+                {
+                    UnloadRootElement();
+                    return new UnloadingRootElementStream(this, stream);
+                }
 
-            if (stream.CanWrite)
-            {
-                return new UnloadingRootElementStream(this, stream);
+                if (stream.CanWrite)
+                {
+                    return new UnloadingRootElementStream(this, stream);
+                }
             }
 
             return stream;

diff --git a/test/DocumentFormat.OpenXml.Tests/ofapiTest/OpenXmlPartTest.cs b/test/DocumentFormat.OpenXml.Tests/ofapiTest/OpenXmlPartTest.cs
@@ -321,5 +321,26 @@ public void UnloadRootElementTest()
                 }
             }
         }
+
+        [Fact]
+        public void SavingPartDoesNotUnloadRoot()
+        {
+            // Arrange
+            using Stream stream = new MemoryStream();
+            using var testDocument = WordprocessingDocument.Create(stream, WordprocessingDocumentType.Document);
+
+            var mainPart = testDocument.AddMainDocumentPart();
+            mainPart.Document = new Document();
+            mainPart.Document.AppendChild(new Body());
+
+            var rootElement = mainPart.RootElement;
+            Assert.NotNull(rootElement);
+
+            // Act
+            mainPart.RootElement.Save();
+
+            // Assert
+            Assert.Same(rootElement, mainPart.RootElement);
+        }
     }
 }