Skip to content

Commit

Permalink
Fix .NET framework xlsx reading bug.
Browse files Browse the repository at this point in the history
Fix RowFieldCount for empty rows.
  • Loading branch information
MarkPflug committed Sep 5, 2023
1 parent f45f4f4 commit 4f01492
Show file tree
Hide file tree
Showing 9 changed files with 75 additions and 20 deletions.
4 changes: 4 additions & 0 deletions docs/ReleaseNotes.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Sylvan.Data.Excel Release Notes

_0.4.15_
- Fix a bug that prevented .xlsx reader from working on .NET Framework versions.
- Fix a bug where FieldRowCount would be incorrect on empty rows.

_0.4.14_
- Add `CompressionLevel` as a configuration option for ExcelDataWriter.
Changes the default compression level to `Fastest`, which produces
Expand Down
21 changes: 21 additions & 0 deletions source/Sylvan.Data.Excel.Tests/ExcelDataReaderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,27 @@ public void Gap()
Assert.False(edr.Read());
}

[Fact]
public void GapRowFieldCount()
{
var file = GetFile("Gap");
using var edr = ExcelDataReader.Create(file, noHeaders);
for (int i = 0; i < 41; i++)
{
Assert.True(edr.Read());

if (i % 10 == 0)
{
Assert.Equal(1, edr.RowFieldCount);
}
else
{
Assert.Equal(0, edr.RowFieldCount);
}
}
Assert.False(edr.Read());
}

[Fact]
public void MultiSheet()
{
Expand Down
13 changes: 9 additions & 4 deletions source/Sylvan.Data.Excel.Tests/ExcelDataWriterTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ protected override string GetFile(string name)
return string.Format(FileFormat, name);
}
}

#if NETCOREAPP1_0_OR_GREATER
public class XlsbDataWriterTests : ExcelDataWriterTests
{
const string FileFormat = "{0}.xlsb";
Expand All @@ -34,6 +34,7 @@ protected override string GetFile(string name)
return string.Format(FileFormat, name);
}
}
#endif

public abstract class ExcelDataWriterTests
{
Expand Down Expand Up @@ -434,7 +435,11 @@ public void GuidData()
i =>
{
var b = new byte[16];
Array.Fill(b, (byte)(i | i << 4));
for (int x = 0; x < b.Length; x++)
{
b[i] = (byte)(i | i << 4);

}
return new
{
Name = "Id " + i,
Expand Down Expand Up @@ -583,7 +588,7 @@ public override long GetChars(int ordinal, long dataOffset, char[] buffer, int b
var dat = col1[row];
var count = Math.Min(length, dat.Length - (int)dataOffset);

dat.AsSpan((int)dataOffset, count).CopyTo(buffer.AsSpan(bufferOffset));
Array.Copy(dat, dataOffset, buffer, bufferOffset, count);
return count;
}

Expand Down Expand Up @@ -696,4 +701,4 @@ public override bool NextResult()
}

#endregion
}
}
2 changes: 2 additions & 0 deletions source/Sylvan.Data.Excel.Tests/ExternalDataTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ public class ExternalDataTests
public ExternalDataTests(ITestOutputHelper o)
{
this.o = o;
#if NETCOREAPP1_0_OR_GREATER
Encoding.RegisterProvider(CodePagesEncodingProvider.Instance);
#endif
}

public static IEnumerable<object[]> GetInputs()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFrameworks>net6.0</TargetFrameworks>
<TargetFrameworks>net6.0;net48</TargetFrameworks>
<RootNamespace>Sylvan.Data.Excel</RootNamespace>
<IsPackable>false</IsPackable>
<DisableImplicitNamespaceImports>true</DisableImplicitNamespaceImports>
Expand Down
2 changes: 1 addition & 1 deletion source/Sylvan.Data.Excel/Sylvan.Data.Excel.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<PropertyGroup>
<TargetFrameworks>net6.0;netstandard2.1;netstandard2.0</TargetFrameworks>
<LangVersion>latest</LangVersion>
<VersionPrefix>0.4.14</VersionPrefix>
<VersionPrefix>0.4.15</VersionPrefix>
<Description>A cross-platform .NET library for reading Excel data files.</Description>
<PackageTags>excel;xls;xlsx;xlsb;datareader</PackageTags>
<Nullable>enable</Nullable>
Expand Down
17 changes: 14 additions & 3 deletions source/Sylvan.Data.Excel/Xlsb/XlsbWorkbookReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,11 @@ public override bool Read()
{
if (rowIndex <= parsedRowIndex)
{
if (curFieldCount >= 0)
if (rowIndex < parsedRowIndex)
{
this.rowFieldCount = 0;
}
else
{
this.rowFieldCount = curFieldCount;
this.curFieldCount = -1;
Expand All @@ -272,19 +276,26 @@ public override bool Read()
{
var c = ParseRowValues();
if (c < 0)
{
this.rowFieldCount = 0;
return false;
}
if (c == 0)
{
continue;
}
if (rowIndex < parsedRowIndex)
{
this.curFieldCount = c;
this.rowFieldCount = 0;
return true;
}
return true;
}
}
else
if (state == State.Initialized)
{
//this.rowFieldCount = this.curFieldCount;
//this.curFieldCount = 0;
// after initizialization, the first record would already be in the buffer
// if hasRows is true.
if (hasRows)
Expand Down
3 changes: 2 additions & 1 deletion source/Sylvan.Data.Excel/Xlsx/SheetNameTable.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ namespace Sylvan.Data.Excel.Xlsx;
// these name tables avoid having to compute hashes on the
// most common element names, and ensures that the strings
// map the the const string so that equality will be ref
// equality to the const values in our code.
// equality to the const values in our code. NOTE: cannot use
// ReferenceEquals however, because this doesn't apply in NET4.8
sealed class SheetNameTable : NameTable
{
public override string Add(char[] key, int start, int len)
Expand Down
31 changes: 21 additions & 10 deletions source/Sylvan.Data.Excel/Xlsx/XlsxWorkbookReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,11 @@ sealed class XlsxWorkbookReader : ExcelDataReader

int rowIndex;
int parsedRowIndex = -1;
// the number of fields in the parsedRowIndex.
int curFieldCount = -1;

public override ExcelWorkbookType WorkbookType => ExcelWorkbookType.ExcelXml;


const string DefaultWorkbookPartName = "xl/workbook.xml";

public XlsxWorkbookReader(Stream iStream, ExcelDataReaderOptions opts) : base(iStream, opts)
Expand Down Expand Up @@ -320,7 +320,7 @@ bool NextRow()
}
else
{
this.parsedRowIndex++;
this.parsedRowIndex = -1;
}
reader.MoveToElement();
return true;
Expand Down Expand Up @@ -377,7 +377,11 @@ public override bool Read()
{
if (rowIndex <= parsedRowIndex)
{
if (curFieldCount >= 0)
if (rowIndex < parsedRowIndex)
{
this.rowFieldCount = 0;
}
else
{
this.rowFieldCount = curFieldCount;
this.curFieldCount = -1;
Expand All @@ -389,8 +393,15 @@ public override bool Read()
var c = ParseRowValues();
if (c == 0)
{
// handles trailing empty rows.
continue;
}
if (rowIndex < parsedRowIndex)
{
this.curFieldCount = c;
this.rowFieldCount = 0;
return true;
}
return true;
}
}
Expand Down Expand Up @@ -439,7 +450,7 @@ int ParseRowValues()
while (reader.MoveToNextAttribute())
{
var n = reader.Name;
if (ReferenceEquals(n, "r"))
if (n == "r")
{
len = reader.ReadValueChunk(valueBuffer, 0, valueBuffer.Length);
if (CellPosition.TryParse(valueBuffer.AsSpan().ToParsable(0, len), out var pos))
Expand All @@ -448,13 +459,13 @@ int ParseRowValues()
}
}
else
if (ReferenceEquals(n, "t"))
if (n == "t")
{
len = reader.ReadValueChunk(valueBuffer, 0, valueBuffer.Length);
type = GetCellType(valueBuffer, len);
}
else
if (ReferenceEquals(n, "s"))
if (n == "s")
{
len = reader.ReadValueChunk(valueBuffer, 0, valueBuffer.Length);
if (!TryParse(valueBuffer.AsSpan().ToParsable(0, len), out xfIdx))
Expand Down Expand Up @@ -679,7 +690,7 @@ static bool ReadToFollowing(XmlReader reader, string localName)
{
while (reader.Read())
{
if (reader.NodeType == XmlNodeType.Element && ReferenceEquals(localName, reader.LocalName))
if (reader.NodeType == XmlNodeType.Element && localName == reader.LocalName)
{
return true;
}
Expand All @@ -692,7 +703,7 @@ static bool ReadToNextSibling(XmlReader reader, string localName)
while (SkipSubtree(reader))
{
XmlNodeType nodeType = reader.NodeType;
if (nodeType == XmlNodeType.Element && ReferenceEquals(localName, reader.LocalName))
if (nodeType == XmlNodeType.Element && localName == reader.LocalName)
{
return true;
}
Expand Down Expand Up @@ -721,7 +732,7 @@ static bool ReadToDescendant(XmlReader reader, string localName)
}
while (reader.Read() && reader.Depth > num)
{
if (reader.NodeType == XmlNodeType.Element && ReferenceEquals(localName, reader.LocalName))
if (reader.NodeType == XmlNodeType.Element && localName == reader.LocalName)
{
return true;
}
Expand Down Expand Up @@ -813,7 +824,7 @@ void LoadSharedStrings(ZipArchiveEntry? entry)

while (reader.Read())
{
if (reader.NodeType == XmlNodeType.Element && ReferenceEquals(reader.LocalName, "si"))
if (reader.NodeType == XmlNodeType.Element && reader.LocalName == "si")
{
var str = ReadString(reader);
sstList.Add(str);
Expand Down

0 comments on commit 4f01492

Please sign in to comment.