0

I want to overwrite or create an xml file on disk, and return the xml from the function. I figured I could do this by copying from FileStream to MemoryStream. But I end up appending a new xml document to the same file, instead of creating a new file each time. What am I doing wrong? If I remove the copying, everything works fine.

 public static string CreateAndSave(IEnumerable<OrderPage> orderPages, string filePath)
    {
        if (orderPages == null || !orderPages.Any())
        {
            return string.Empty;
        }

        var xmlBuilder = new StringBuilder();

        var writerSettings = new XmlWriterSettings
        {
            Indent = true,
            Encoding = Encoding.GetEncoding("ISO-8859-1"),
            CheckCharacters = false,
            ConformanceLevel = ConformanceLevel.Document
        };

        using (var fs = new FileStream(filePath, FileMode.OpenOrCreate, FileAccess.ReadWrite))
        {
            try
            {
                XmlWriter xmlWriter = XmlWriter.Create(fs, writerSettings);
                xmlWriter.WriteStartElement("PRINT_JOB");
                WriteXmlAttribute(xmlWriter, "TYPE", "Order Confirmations");

                foreach (var page in orderPages)
                {
                    xmlWriter.WriteStartElement("PAGE");
                    WriteXmlAttribute(xmlWriter, "FORM_TYPE", page.OrderType);

                    var outBound = page.Orders.SingleOrDefault(x => x.FlightInfo.Direction == FlightDirection.Outbound);
                    var homeBound = page.Orders.SingleOrDefault(x => x.FlightInfo.Direction == FlightDirection.Homebound);

                    WriteXmlOrder(xmlWriter, outBound, page.ContailDetails, page.UserId, page.PrintType, FlightDirection.Outbound);
                    WriteXmlOrder(xmlWriter, homeBound, page.ContailDetails, page.UserId, page.PrintType, FlightDirection.Homebound);

                    xmlWriter.WriteEndElement();
                }

                xmlWriter.WriteFullEndElement();

                MemoryStream destination = new MemoryStream();
                fs.CopyTo(destination);

                Log.Progress("Xml string length: {0}", destination.Length);

                xmlBuilder.Append(Encoding.UTF8.GetString(destination.ToArray()));

                destination.Flush();
                destination.Close();
                xmlWriter.Flush();
                xmlWriter.Close();
            }
            catch (Exception ex)
            {
                Log.Warning(ex, "Unhandled exception occured during create of xml. {0}", ex.Message);
                throw;
            }

            fs.Flush();
            fs.Close();
        }

        return xmlBuilder.ToString();
    }

Cheers Jens

M Raymaker
  • 1,059
  • 3
  • 13
  • 25

2 Answers2

2

FileMode.OpenOrCreate is causing the file contents to be overwritten without shortening, leaving any 'trailing' data from previous runs. If FileMode.Create is used the file will be truncated first. However, to read back the contents you just wrote you will need to use Seek to reset the file pointer.

Also, flush the XmlWriter before copying from the underlying stream.

See also the question Simultaneous Read Write a file in C Sharp (3817477).

The following test program seems to do what you want (less your own logging and Order details).

using System;
using System.IO;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Xml;
using System.Threading.Tasks;

namespace ReadWriteTest
{
    class Program
    {
        static void Main(string[] args)
        {
            string filePath = Path.Combine(
                Environment.GetFolderPath(Environment.SpecialFolder.Personal),
                "Test.xml");

            string result = CreateAndSave(new string[] { "Hello", "World", "!" }, filePath);

            Console.WriteLine("============== FIRST PASS ==============");
            Console.WriteLine(result);

            result = CreateAndSave(new string[] { "Hello", "World", "AGAIN", "!" }, filePath);
            Console.WriteLine("============== SECOND PASS ==============");
            Console.WriteLine(result);

            Console.ReadLine();
        }

        public static string CreateAndSave(IEnumerable<string> orderPages, string filePath)
        {
            if (orderPages == null || !orderPages.Any())
            {
                return string.Empty;
            }

            var xmlBuilder = new StringBuilder();

            var writerSettings = new XmlWriterSettings
            {
                Indent = true,
                Encoding = Encoding.GetEncoding("ISO-8859-1"),
                CheckCharacters = false,
                ConformanceLevel = ConformanceLevel.Document
            };

            using (var fs = new FileStream(filePath, FileMode.Create, FileAccess.ReadWrite))
            {
                try
                {
                    XmlWriter xmlWriter = XmlWriter.Create(fs, writerSettings);
                    xmlWriter.WriteStartElement("PRINT_JOB");

                    foreach (var page in orderPages)
                    {
                        xmlWriter.WriteElementString("PAGE", page);
                    }

                    xmlWriter.WriteFullEndElement();

                    xmlWriter.Flush();  // Flush from xmlWriter to fs
                    xmlWriter.Close();

                    fs.Seek(0, SeekOrigin.Begin); // Go back to read from the begining

                    MemoryStream destination = new MemoryStream();
                    fs.CopyTo(destination);

                    xmlBuilder.Append(Encoding.UTF8.GetString(destination.ToArray()));

                    destination.Flush();
                    destination.Close();
                }
                catch (Exception ex)
                {
                    throw;
                }

                fs.Flush();
                fs.Close();
            }

            return xmlBuilder.ToString();
        }
    }
}

For the optimizers out there, the StringBuilder was unnecessary because the string is formed whole and the MemoryStream can be avoided by just wrapping fs in a StreamReader. This would make the code as follows.

        public static string CreateAndSave(IEnumerable<string> orderPages, string filePath)
        {
            if (orderPages == null || !orderPages.Any())
            {
                return string.Empty;
            }

            string result;

            var writerSettings = new XmlWriterSettings
            {
                Indent = true,
                Encoding = Encoding.GetEncoding("ISO-8859-1"),
                CheckCharacters = false,
                ConformanceLevel = ConformanceLevel.Document
            };

            using (var fs = new FileStream(filePath, FileMode.Create, FileAccess.ReadWrite))
            {
                try
                {
                    XmlWriter xmlWriter = XmlWriter.Create(fs, writerSettings);
                    xmlWriter.WriteStartElement("PRINT_JOB");

                    foreach (var page in orderPages)
                    {
                        xmlWriter.WriteElementString("PAGE", page);
                    }

                    xmlWriter.WriteFullEndElement();

                    xmlWriter.Close();  // Flush from xmlWriter to fs

                    fs.Seek(0, SeekOrigin.Begin); // Go back to read from the begining

                    var reader = new StreamReader(fs, writerSettings.Encoding);
                    result = reader.ReadToEnd();
                    // reader.Close();  // This would just flush/close fs early(which would be OK)
                }
                catch (Exception ex)
                {
                    throw;
                }
            }

            return result;
        }
Community
  • 1
  • 1
SensorSmith
  • 963
  • 10
  • 21
1

I know I'm late, but there seems to be a simpler solution. You want your function to generate xml, write it to a file and return the generated xml. Apparently allocating a string cannot be avoided (because you want it to be returned), same for writing to a file. But reading from a file (as in your and SensorSmith's solutions) can easily be avoided by simply "swapping" the operations - generate xml string and write it to a file. Like this:

var output = new StringBuilder();
var writerSettings = new XmlWriterSettings { /* your settings ... */ };
using (var xmlWriter = XmlWriter.Create(output, writerSettings))
{
    // Your xml generation code using the writer
    // ...
    // You don't need to flush the writer, it will be done automatically
}
// Here the output variable contains the xml, let's take it...
var xml = output.ToString();
// write it to a file...
File.WriteAllText(filePath, xml);
// and we are done :-)
return xml;

IMPORTANT UPDATE: It turns out that the XmlWriter.Create(StringBuider, XmlWriterSettings) overload ignores the Encoding from the settings and always uses "utf-16", so don't use this method if you need other encoding.

Ivan Stoev
  • 159,890
  • 9
  • 211
  • 258
  • Very nice and clean code. I'll give it a try and compare performance. Thanks. – M Raymaker Sep 06 '15 at 11:25
  • Ok, I ran a comparison check dumping 10000 orders to xml. Using SensorSmiths code it took 1198ms. Using File.WriteAllText took 1108ms. So not a huge deal. Also the xml somehow got encoded in UTF-16, even when specifying ISO-8859-1. Thanks anyway for the input. – M Raymaker Sep 06 '15 at 16:32
  • Regarding performance, nothing surprising with all these caches nowadays :-) But the encoding issue is really annoying, thanks for reporting, I'll update the answer in case someone else reads it. Take care. – Ivan Stoev Sep 06 '15 at 17:17