From 730f88205d1ef839bad9a665d8cfcd84e8a89020 Mon Sep 17 00:00:00 2001 From: DominicJosephEnriquez Date: Fri, 3 Jul 2026 07:28:27 +0200 Subject: [PATCH 1/2] fix: preserve whitespace-only cells in the XLS reader DataFrame.LoadCsv parses via Microsoft.VisualBasic TextFieldParser, whose TrimWhiteSpace is true and cannot be configured through LoadCsv, so it strips whitespace from every field (even quoted). As a result a cell containing only spaces was read back as an empty string. The XLS write path (CsvUtils.EscapeCsvValues) now wraps a whitespace-only value in a Unicode private-use guard character so the field's edges are non-whitespace and survive parsing. CsvVirtualDataTable strips the guard afterwards, and only when the guarded content is itself whitespace-only, so real values are never affected. Values without the guard (e.g. every CSV-provider value) are returned unchanged. Adds the shared Reader_ShouldPreserveWhitespaceOnlyCell test and a withSpaceCell.xlsx fixture. Co-Authored-By: Claude Opus 4.8 --- src/Data.Csv/Utils/CsvVirtualDataTable.cs | 31 +++++++++++++++- src/Data.Xls/Utils/CsvUtils.cs | 15 ++++++-- tests/Data.Tests.Common/DataReaderTests.cs | 33 +++++++++++++++++- .../Utils/DatabaseFullPaths.cs | 1 + tests/Data.Xls.Tests/Data.Xls.Tests.csproj | 3 ++ .../FileAsDatabase/XlsDataReaderTests.cs | 7 ++++ .../Data.Xls.Tests/Sources/withSpaceCell.xlsx | Bin 0 -> 8926 bytes .../Data.Xls.Tests/Utils/ConnectionStrings.cs | 5 ++- 8 files changed, 89 insertions(+), 6 deletions(-) create mode 100644 tests/Data.Xls.Tests/Sources/withSpaceCell.xlsx diff --git a/src/Data.Csv/Utils/CsvVirtualDataTable.cs b/src/Data.Csv/Utils/CsvVirtualDataTable.cs index 4b6915a..d43f8ce 100644 --- a/src/Data.Csv/Utils/CsvVirtualDataTable.cs +++ b/src/Data.Csv/Utils/CsvVirtualDataTable.cs @@ -158,6 +158,14 @@ private string ReadPageData(bool firstPage) return stringBuilder.ToString(); } + // Sentinel used to protect whitespace-only cells from DataFrame's CSV reader + // (Microsoft.VisualBasic TextFieldParser, whose TrimWhiteSpace defaults to true and cannot be + // configured through LoadCsv). A writer that must preserve such whitespace wraps the value in this + // guard on both ends so the field's edges are non-whitespace; ToDataRows strips it back off after + // parsing. Values that do not carry the guard (e.g. every CSV-provider value) are returned unchanged. + // A Unicode private-use character is used so it will not collide with real spreadsheet text. + public const char WhitespaceGuard = (char)0xE000; + private IEnumerable ToDataRows(DataFrame dataFrame) { for (int rowIndex = 0; rowIndex < dataFrame.Rows.Count; rowIndex++) @@ -165,13 +173,34 @@ private IEnumerable ToDataRows(DataFrame dataFrame) DataRow newRow = NewRow(); for (int colIndex = 0; colIndex < Columns.Count; colIndex++) { - var value = dataFrame.Columns[colIndex][rowIndex] ?? DBNull.Value; + var value = StripWhitespaceGuard(dataFrame.Columns[colIndex][rowIndex]) ?? DBNull.Value; newRow[colIndex] = value; } yield return newRow; } } + private static object StripWhitespaceGuard(object value) + { + if (value is string text + && text.Length >= 3 + && text[0] == WhitespaceGuard + && text[text.Length - 1] == WhitespaceGuard) + { + var inner = text.Substring(1, text.Length - 2); + + // Only unwrap what the encoder actually guards: a whitespace-only cell. This keeps a real + // cell that merely happened to begin and end with the guard character (with non-whitespace + // content between) from being corrupted. + if (string.IsNullOrWhiteSpace(inner)) + { + return inner; + } + } + + return value; + } + public void Dispose() { if (!_disposed) diff --git a/src/Data.Xls/Utils/CsvUtils.cs b/src/Data.Xls/Utils/CsvUtils.cs index 1a1b517..b2e4c95 100644 --- a/src/Data.Xls/Utils/CsvUtils.cs +++ b/src/Data.Xls/Utils/CsvUtils.cs @@ -1,3 +1,5 @@ +using Data.Csv.Utils; + namespace Data.Xls.Utils; public static class CsvUtils @@ -6,13 +8,20 @@ public static IEnumerable EscapeCsvValues(this IEnumerable value { foreach (var value in values) { - if (value.Contains(",") || value.Contains("\"")) + // A whitespace-only cell would be trimmed to an empty string by DataFrame's CSV reader + // (TextFieldParser.TrimWhiteSpace). Wrap it in a guard character on both ends so the field's + // edges are non-whitespace and survive parsing; CsvVirtualDataTable strips the guard afterwards. + var guarded = value.Length > 0 && string.IsNullOrWhiteSpace(value) + ? CsvVirtualDataTable.WhitespaceGuard + value + CsvVirtualDataTable.WhitespaceGuard + : value; + + if (guarded.Contains(",") || guarded.Contains("\"")) { - yield return $"\"{value.Replace("\"", "\"\"")}\""; + yield return $"\"{guarded.Replace("\"", "\"\"")}\""; } else { - yield return value; + yield return guarded; } } } diff --git a/tests/Data.Tests.Common/DataReaderTests.cs b/tests/Data.Tests.Common/DataReaderTests.cs index 12d60ab..b8fd25a 100644 --- a/tests/Data.Tests.Common/DataReaderTests.cs +++ b/tests/Data.Tests.Common/DataReaderTests.cs @@ -239,7 +239,38 @@ public static void Reader_ShouldReadEmptyCells( connection.Close(); } - + + public static void Reader_ShouldPreserveWhitespaceOnlyCell(Func> createFileConnection) + where TFileParameter : FileParameter, new() + { + // Arrange + var connection = createFileConnection(); + var command = connection.CreateCommand("SELECT * FROM [Sheet1]"); + + // Act & Assert + connection.Open(); + var found = false; + using (var reader = command.ExecuteReader()) + { + while (reader.Read()) + { + if (reader["Marker"].ToString() != "spaces") + { + continue; + } + + // A cell containing only whitespace must be read back with the whitespace intact, + // not trimmed away. + found = true; + Assert.IsType(reader["Value"]); + Assert.Equal(" ", reader["Value"]); + } + } + connection.Close(); + + Assert.True(found, "The 'spaces' row was not returned by the reader."); + } + public static void Reader_ShouldReadFormulasAsString(Func> createFileConnection) where TFileParameter : FileParameter, new() { diff --git a/tests/Data.Tests.Common/Utils/DatabaseFullPaths.cs b/tests/Data.Tests.Common/Utils/DatabaseFullPaths.cs index b27778d..6a3d9cf 100644 --- a/tests/Data.Tests.Common/Utils/DatabaseFullPaths.cs +++ b/tests/Data.Tests.Common/Utils/DatabaseFullPaths.cs @@ -20,6 +20,7 @@ public DatabaseFullPaths(string extension) public string WithFormula => Path.Combine(FileConnectionStringTestsExtensions.SourcesFolder, $"WithFormula.{extension}"); public string EmptyCells => Path.Combine(FileConnectionStringTestsExtensions.SourcesFolder, $"emptyCells.{extension}"); public string CellsWithComma => Path.Combine(FileConnectionStringTestsExtensions.SourcesFolder, $"cellsWithComma.{extension}"); + public string WithSpaceCell => Path.Combine(FileConnectionStringTestsExtensions.SourcesFolder, $"withSpaceCell.{extension}"); public string eComFileDataBase => Path.Combine(FileConnectionStringTestsExtensions.SourcesFolder, $"ecommerce.{extension}"); public string eComFolderDataBase => Path.Combine(FileConnectionStringTestsExtensions.SourcesFolder, $"eCom"); public string FolderEmptyWithTables => Path.Combine(FileConnectionStringTestsExtensions.SourcesFolder, "EmptyDatabase"); diff --git a/tests/Data.Xls.Tests/Data.Xls.Tests.csproj b/tests/Data.Xls.Tests/Data.Xls.Tests.csproj index 4eac125..ab90cfc 100644 --- a/tests/Data.Xls.Tests/Data.Xls.Tests.csproj +++ b/tests/Data.Xls.Tests/Data.Xls.Tests.csproj @@ -58,5 +58,8 @@ Always + + Always + diff --git a/tests/Data.Xls.Tests/FileAsDatabase/XlsDataReaderTests.cs b/tests/Data.Xls.Tests/FileAsDatabase/XlsDataReaderTests.cs index 30cba6d..cda027c 100644 --- a/tests/Data.Xls.Tests/FileAsDatabase/XlsDataReaderTests.cs +++ b/tests/Data.Xls.Tests/FileAsDatabase/XlsDataReaderTests.cs @@ -64,6 +64,13 @@ public void Reader_ShouldReadEmptyCells() DataReaderTests.Reader_ShouldReadEmptyCells(() => new XlsConnection(ConnectionStrings.Instance.EmptyCellsAsDB)); } + [Fact] + public void Reader_ShouldPreserveWhitespaceOnlyCell() + { + DataReaderTests.Reader_ShouldPreserveWhitespaceOnlyCell(() => + new XlsConnection(ConnectionStrings.Instance.WithSpaceCellAsDB)); + } + [Fact] public void Reader_Limit_ShouldReturnOnlyFirstRow() { diff --git a/tests/Data.Xls.Tests/Sources/withSpaceCell.xlsx b/tests/Data.Xls.Tests/Sources/withSpaceCell.xlsx new file mode 100644 index 0000000000000000000000000000000000000000..d79c434a04a719aca0bf6a3bbd71a11df520b0d3 GIT binary patch literal 8926 zcmeHtg;!k3_I0Ddf;$9)G(m$~2oeGb&^UnvZLD!3l*6;1AvTp{(skh@dy;h^eVP-;Y**%Tt8pq zkee%)z~JBY?IdJX5o>P$*imSton~%+n-+Y9FPTYVAy`H@IOooD*l$*DV_oeV)X|`d z6YA5|tgc5T$oZ{v2M8mhh_=_#+|MAUlHjHQgTqad0SR_BZK`}y%g^MBDa}Yo#k^Nj zXF4?q7CY*23bpcuyJlOK6g3tIgn9}_@mGh5#w^TP-1UxEIb=~o>D}i@o%C{)>E5WZ z7^>FUU&e#JIxyGrj_VsCFK`7!HQ~+=ETT7j6{Jz%qbx57IVsfYh_YmZdFXh!!I&dL zPn0j~1xU=m9NK9TK6Jh#QvhYl&`Q!09-F~;?cyZa^1TPG59hCd%S6=ei&Vz-X^~OJ zisbax>`OVFX;MnUf~>xtd0pMLLd{CMM${fQxx@F?P^HgriBAcB)E_rX;&(UGF}@4j zUF!NF;=VJquI;yX827`A=LQ`BxV=RIDF02CRq9*}CkU>|BUFctkR{k2V&%a3__zE2 zuh{J3SScB{7j~S?v>OTIcs3)|JnRuQ{wqp9 zl$Tyjp4UCI^H0OKy68?<_=-abiA5Nyol64ZFYVsmV`Z|Bdtq0++)m&$4j(^Fd@1M3 z?9>#=T2z~#3F=;Ccrkh?Rfa$K_%$UaQ4Vzgsc52)R)>P-g5mjRq;W~L-I9RP8o`XM zkAulxlX1B}?u!Qr%kPXNl5{y3nofWA=(3`NUr?&6nF*Vh8m8Glp?3uvTQwg`CBuk* zdT=PFbg9zw5uWo5zU%~Mz`e98`3`$iUE2jQ+lsmlx_v`w^A`{f^-qyxWDP|1q5uHN zm;eACVq{#bIGv&PmWEKMEd94w?E@VHC9So6OeL2|oly_h{UO`P;NvBY*k|-Ic)cUX*A2Kc` zzawrcjAaEfpNK0aRVUFmx|sPJpYhi=(S5D@aPXitQopwL>ZAU4n~H+E zOlG?z9$;oV`esSI1(yygS$aY4HyV}CaG?gVchLt1rW^A=)xRI9k9x~PiDVAf!lZLa zmpaCq)495~QW?K?V$%&R(Ew{2@u<&g^v4YJ$Q|1oM(VRpT=)C#XxP0TFWolS!L{DJ zez<9A9_!IkuQe#q<$fN#Ye;4l4#Z$6R)7D@l6DDt zg{K?C6HQxwFCh4|OykDSThkzGMdD5MSk2k~b5xoxTh0+n4vt!w3@4q6;11>p`UZRmqDt~uq;OMY<5hYojf6or?>Z9=mHJs-sHuLu#oo!E70xJ zR)3x{fjzgKQeQv3CTqNU$osWn=ggB_Nny01BG;iDU z_%kbTGn7f!Q6Gi@2e(!!~X6RNxW;)vX?u%v}|v5B`^~*1*wU4BEr!Y zzk_nF2*zF`iAvh>wHk%kZd?SR8NF`I=#OtxyXGdch`LXlru9Z;)npteup74 z!fa(!h{45C-lgernpiwE{)xLZ)0bQ6oum)lFeE%@uQK*;a2xEIW|e{fPcZmi<#9Sf z)&JDJk629nG2)kcggH_IFpvyOL#=iH_IvWRZ%sUiZxeh!X`G;JV-6>2$`AyMjUlZ(++1Aw+c} zvf39xgZ3ZyqhJ{BmeBaVL?a^H+1f7J#l}r?AgU9K?qW|P7TP#x22uoO=h91^)Qtr2 zSqyN!<*{%WdeDRLir&gK@5}dUJ0-rAH8l^DSWMP$>z>b^qNn@1(heU}V9Gwd?+O_H zK1RoO-XeX9anyKiqTo7U*^+&Uvrb(EqABDT^$2i$7})ia){7zKEFVe^Q2&4oEub394R4?UDr3d=*b|L{#b2j#13Qj**6Ud5a!t8TfEU*+Pgt9HwE>_g<)j3g zk`l>|uqnwFh1crJSwOY=CDIzVHu)T%Ja{I(sn6zp%@&9(m1KUpK|cFOWKy{uY-Vee z;V6gD!{n0UK7WlMQ(LsjhIiuxh57hpadnlOlyb6kt&<7`eBjN5v+jL|XNr{RfW-yr z+?$(>S2CHgDv7)FiTvE+x#9*y9nv7TzFU^CMN>-L5&nY^EHTn(aWl;pl8LK_R#UAy zb&j?1=JL0v*Xy>z)R7_!W)s^doh_BTY5l$f9s= z2!G*Sg(d5#3m(&eOL&bk`iJd1TvW(4AEkw37@qJW#-O3tb~o)aCU{404X6L|;NJi&!TE zhP|V!$~8#h=SlY&nvd#9KghGVPcNmEod$pw{*_~=`jyXW?*RY=^uKMTf9IH^8N?dG z`TPBMdhKZSM^e?0!0vV3O7!VY^nG_X5FW}CJl%6!j@4b0k&Ow^8?cp7ZD1ot!rB}I z`5>DQgM1d;m_yLbywU5{BDwV$6mEl_+P_Fz4hdg*JZbU6E1 zl#q!{&$uo-iM@ADL*E@XXkNbKro^-=H~U<5**JniNoG=;=*K(z{ng056%>*W2@j)` z){yqxKB)e9R>f+{6}J8$EgIER+m^rxt;SF@725;T%}C?^JD6rTx5CqyCGs&bE%xtW z8NnJq=4Y8{xoZ_-#!z^f1`v6VEcwp$(;`1%P}-6s5<@5*3)1S*kubGgSGA(Cy-)=H%u0Nq(~ z_ng`vV{b%)oNwTM72B$6{opT{5j4E!K{jtyHQP_!l#bgFh71Sr)0a{IjYwLoGjV?Fv(9K)d!*67qX}8!9h9RGL)Z^yp z6yqRuv^O`Mw?E;|-(j~mvv}Z}?(lfXb&=Q2&3Mh?<Bi7XB=HHy1^(!CvqqPOtMD zTJV#~Gt{)WWg<$VV8t$=Tc44Ogb`M)T- zBvBBZ=Bpx8(#Z;$V&b=b*IXeSOWd2qgZ`rcJh%=JTS=pPM6~WrtX(AcSKEHk=HT_E zVOIYWIbGI!1Z8GvJSohLjF>7iw;h9+c2DO`W3-$^b9q~UVCEAF>38y}Oo>AUBr^U( zbm1+WmSoRtt02j%dHI8h+XaDZtuI>3oBN)5N+v)$u3U0#cT|O2xY?+#F^amRkE}RM zAagRT2Pe^75icC*b3hv(yf-<7f6@iCZ#Z6YYj+B$GP~`KxV{(WWC*73YkWPyG$$Af(_{J&L@Y&M6l%2O{YI5w(FfoFsKB%vAU!exwK!DU=8x&zbXa2 z=aHS{Z(unJl4CO-?26uBc4+(>=Q1G_TsD<=f!b)}zxS%y&{5f?-DDH8gz0~TxAM(A z<|UzHjIVp8(zf~gk|xk1$hq;e|2j-i)2Q@u`n;pAFHT(<4&$j{bw4Vvr7yicQ$iNQ z>v}YT-k<(K3$MecdRS^o$*h;rnZt_R9_4m#holp+qr$FTEzKK747(=ZcTv8%`34z{ELx(u zheZ3kSM1D2rWXBOANWdWF~9I1bK@kX#hA<6TzQ+L#$USXBV*SPPjx+P8HeVF4A7@Y zdNFh9_T}`ca{}e+-dBI15cv7ZUZKWZtYpPVY;Yzu7q(`~6!V5;)f0tMXtY=Yzw57k_{^;u_#MU0}rYc=_l<`Ks|}R!0&_; zE*PN$&v{dvn$S+dmw6QFTt0c8wOU~Ya`FPpNp1xSfF!E-%ffgf`- zPyLebLqMH#*4E1Pz3CJ<^_HzaRQF5w8+X5Yewm14JMB~~GuJYgh9>V>T`oIa5EdDd zRlR}KUAYbHT17uVEp$kBIctJRl_d0K z{pR5L<09?3rF=e6+<{)xS&vJc=uUh^g4gUKyC#@(i6cCsBpo$sV&Khqd6QtKRwyn+v*C$$c5lN6UV(gm<;PDWN27H`MjjgS^cgUwd;=JmD}E z`f#P+XEK!{vS`j@RhfT5a8Uwv`3Cy1`GW)-H5$B>442icS>XJ=ao(ICVs4cXZdw}w2`!9lw@ zXZh{^%YQX`rBmjS0z@+T8d18t{~x1waCEhTIQ*7Dep0u#oaMs55u5VDzufC_T4l$q z291s>fA7h7uv28%8=S8f_UY3=$Bny^lOFYx>S51drMyvx!FdMVks=MON=S!&2@kdI zdvb2|t&k%10GK#Q3o65QeqsJl2*jn6{cDf7mt4s#=4q`-ADy?dK%3=e5(Qng4zKL4 zzfFQ~suq>(1NQ!qsSYkJO)nWN!ocMOnI4io0+aGQ~;6S{ao< z9F@;-Klu&3rI7}1Gkm7X<8aI9_)bQvt=9eR^I~XI_D2s|c43UA)b@|lUhLE>lGFv? z($IvCi1{4*@{vi+pyIofOBqY`a4y|B@vPSYds#vh)DLxApXyquwZO&r*Oz+KS}g8K z-3OQaocw|A89yUji|ndpS3eSh1(w6J=gUZm#ytF<-W+*}(@^qZrc1KEfwk=*!Gj2w ziMU+Qz0q^|Fwji&G|)grJAD;p#ru@vhmqTw^x%P?R|WFtK^kNZK^>vbHxlGrMWGjH)J#HP>m3e0vKN+;&EYxNGK zsoHq2x1DYv^|c%;`h)|ITE0gETlf+=eT^M?!R#UC$S4?sx&PcCodD7Zdk|daN9>h} z|KPNlfjz`n&C%Z6#`Je;>qM3*qHyC&xij9h%%q)55@dEcVSRFRjPhL9w%6E!FVBZA z?b}IU{OED5$WDA4I2hd@oAjxNraBo3$FA6Dr6RCqa~S%TlRl{;EwG)k(Ky zyU;-Bzv?+0d+UoLqTgPKks(0T6O5ro%JxuO2TmiXJ>-uQu>X}25JMLh4F zJ-D1vnt(aCPPvFslw;!%7?P3~GOESKJWNBKq_Js=#{1qJ8&89(RL?sUSys4JK6qk* z%D2qOGHMLilIdGJrlCq3;pBfG0(C?aQX1usTVlyeTR|sJP7A`78F2eFJynlp8<2^P z*&~tyq_{bCD;@dyWQY|dkJu9`nfH8-F1bQ+XI|uy+p1H8LUp#li>~zL_^XD_Y76#P z6>RAFtU?TZ$nY5rUJidk_WE($z9;o68jaAUie?n0iov8OUbGr8;^r@>UXiIrV6=6ndOW@#UFMiBddqUrrox}*W6t$rDW%+pMX!IhQBEXuV)dX zp;B3royNLM&FgxSphaU=9db(mBfM8$fIU=`sfmp3z0qEx7E6I*G*q*!GkI(Eg-b`; z=O|VL_90f3rpp2|@wmv{MdgigwhuA*$KrnwGv~IeE79IB$$oHtVCO84Q#@W1(^wI3 z9(S2nA#7-sf9_ej6q^zn@}pWGT%Mi%I5wrm!ZJLN|K4_o3o39CeT$~Av@+a|qH4>0 z+%G2q+tJI#`gPX1H9mTZ+zFEiQBWEB>{wWpl8mPKx9cztKxVX6LV%I**?{GlH0Mhc zvds=UePn}WYP-!Ylp6~HrcsghUg5(ou0utkSs&;VhXFyoM5W6|?hhJ_5&K~^c$>ee zhfITuC5GOltJtgA3FG_=G{tSP`(vU|o+~=z`J~do%kg6v*VCKTlQ#H~n~LY-3tdxQ z;NngB4fdb@frQL~@QQzbli<(k`m_C)_X?Cje-H5YGtWPPzqRoQP5k8q^e*u3InXa? zBO*h)b1HNf{P$w=FDL*|iS;}9|5IAN8|Q9?@mHixod5SH{!wqd8|7{d=~omG;)Dy) zmAe(Cy8-UzroRI4k^LUvSEhOwdN&351;wEJ5A<$Aa5uu=jr12D06?Vz0RCaCcj13u h0{;pRe()Fg9}A)~2n{j2000)^ "xlsx"; + public FileConnectionString WithSpaceCellAsDB => new FileConnectionString { DataSource = Database.WithSpaceCell }; + public new static ConnectionStrings Instance => new ConnectionStrings(); } \ No newline at end of file From 03b222858ae6eb73d59c8b4c71a06746766ae419 Mon Sep 17 00:00:00 2001 From: DominicJosephEnriquez Date: Fri, 3 Jul 2026 08:54:49 +0200 Subject: [PATCH 2/2] refactor: address PR review for whitespace-guard handling - Move the guard sentinel to a shared Data.Common CsvWhitespaceGuard class (was a public const on the CsvVirtualDataTable paging read class). - Gate guard stripping behind stripWhitespaceGuard, set only by the XLS reader, so other providers never strip a value that merely looks guard-shaped. This makes the "never corrupt data I did not encode" property structural rather than probabilistic. - Parameterize the shared whitespace test by table name; add a focused CSV test proving a guard-shaped value is returned unchanged when stripping is disabled. - Add a mixed numeric+whitespace fixture/test documenting that preserving a whitespace cell makes an otherwise-numeric column read as text (XLS read path only). - Document the whitespace-only scope limitation in CsvUtils. Co-Authored-By: Claude Opus 4.8 --- src/Data.Common/Utils/CsvWhitespaceGuard.cs | 17 +++++++ src/Data.Csv/Utils/CsvVirtualDataTable.cs | 43 ++++++++-------- src/Data.Xls/Utils/CsvUtils.cs | 7 ++- src/Data.Xls/XlsIO/Read/XlsReader.cs | 2 +- .../CsvVirtualDataTableTests.cs | 36 ++++++++++++++ tests/Data.Tests.Common/DataReaderTests.cs | 46 +++++++++++++++++- .../Utils/DatabaseFullPaths.cs | 1 + tests/Data.Xls.Tests/Data.Xls.Tests.csproj | 3 ++ .../FileAsDatabase/XlsDataReaderTests.cs | 9 +++- .../Sources/withSpaceCellMixed.xlsx | Bin 0 -> 1932 bytes .../Data.Xls.Tests/Utils/ConnectionStrings.cs | 2 + 11 files changed, 137 insertions(+), 29 deletions(-) create mode 100644 src/Data.Common/Utils/CsvWhitespaceGuard.cs create mode 100644 tests/Data.Csv.Tests/CsvVirtualDataTableTests.cs create mode 100644 tests/Data.Xls.Tests/Sources/withSpaceCellMixed.xlsx diff --git a/src/Data.Common/Utils/CsvWhitespaceGuard.cs b/src/Data.Common/Utils/CsvWhitespaceGuard.cs new file mode 100644 index 0000000..923fb50 --- /dev/null +++ b/src/Data.Common/Utils/CsvWhitespaceGuard.cs @@ -0,0 +1,17 @@ +namespace Data.Common.Utils; + +/// +/// Protocol constant shared between a CSV-producing provider (e.g. the XLS provider, which converts a +/// sheet to CSV) and the CSV reader. +/// +/// CSV parsing goes through DataFrame.LoadCsv, which is backed by +/// Microsoft.VisualBasic.FileIO.TextFieldParser whose TrimWhiteSpace is true and is +/// not configurable. That trims a whitespace-only cell down to an empty string. A provider that must +/// preserve such whitespace wraps the value in this guard on both ends (so the field's edges are +/// non-whitespace and survive parsing); the reader strips the guard afterwards. A Unicode private-use +/// character is used so it will not collide with real content. +/// +public static class CsvWhitespaceGuard +{ + public const char Sentinel = (char)0xE000; +} diff --git a/src/Data.Csv/Utils/CsvVirtualDataTable.cs b/src/Data.Csv/Utils/CsvVirtualDataTable.cs index d43f8ce..5278ff7 100644 --- a/src/Data.Csv/Utils/CsvVirtualDataTable.cs +++ b/src/Data.Csv/Utils/CsvVirtualDataTable.cs @@ -1,4 +1,5 @@ using Microsoft.Data.Analysis; +using Data.Common.Utils; using Data.Common.Utils.ConnectionString; using SqlBuildingBlocks.POCOs; using System.Text; @@ -17,6 +18,7 @@ public class CsvVirtualDataTable : VirtualDataTable, IDisposable private readonly int _guessTypeRows; private readonly FloatingPointDataType _preferredFloatingPointDataType; private readonly Func, Type> _guessTypeFunction; + private readonly bool _stripWhitespaceGuard; // Hold on to the underlying stream so we can keep paging through it. private readonly CsvTransformStream _transformStream; @@ -49,13 +51,15 @@ public CsvVirtualDataTable( int guessTypeRows, FloatingPointDataType preferredFloatingPointDataType, Func, Type> guessTypeFunction, - char separator + char separator, + bool stripWhitespaceGuard = false ) : base(tableName) { _pageSize = pageSize; _guessTypeRows = guessTypeRows > 0 ? guessTypeRows : throw new ArgumentOutOfRangeException(nameof(guessTypeRows), $"Guess type row must be greater than 0. GuessRows: {guessTypeRows}"); _preferredFloatingPointDataType = preferredFloatingPointDataType; + _stripWhitespaceGuard = stripWhitespaceGuard; _guessTypeFunction = guessTypeFunction; // Instead of using "using", store the reader and transform stream for later use. @@ -158,14 +162,6 @@ private string ReadPageData(bool firstPage) return stringBuilder.ToString(); } - // Sentinel used to protect whitespace-only cells from DataFrame's CSV reader - // (Microsoft.VisualBasic TextFieldParser, whose TrimWhiteSpace defaults to true and cannot be - // configured through LoadCsv). A writer that must preserve such whitespace wraps the value in this - // guard on both ends so the field's edges are non-whitespace; ToDataRows strips it back off after - // parsing. Values that do not carry the guard (e.g. every CSV-provider value) are returned unchanged. - // A Unicode private-use character is used so it will not collide with real spreadsheet text. - public const char WhitespaceGuard = (char)0xE000; - private IEnumerable ToDataRows(DataFrame dataFrame) { for (int rowIndex = 0; rowIndex < dataFrame.Rows.Count; rowIndex++) @@ -180,27 +176,28 @@ private IEnumerable ToDataRows(DataFrame dataFrame) } } - private static object StripWhitespaceGuard(object value) + // Removes the whitespace guard added by a producer (see CsvWhitespaceGuard). Only runs when this + // reader was told the source can contain guards (the XLS provider) — for other providers a value that + // merely looks guard-shaped is real data and must be returned untouched. + private object StripWhitespaceGuard(object value) { - if (value is string text - && text.Length >= 3 - && text[0] == WhitespaceGuard - && text[text.Length - 1] == WhitespaceGuard) + if (_stripWhitespaceGuard && value is string text && IsGuardedWhitespace(text)) { - var inner = text.Substring(1, text.Length - 2); - - // Only unwrap what the encoder actually guards: a whitespace-only cell. This keeps a real - // cell that merely happened to begin and end with the guard character (with non-whitespace - // content between) from being corrupted. - if (string.IsNullOrWhiteSpace(inner)) - { - return inner; - } + return text.Substring(1, text.Length - 2); } return value; } + // True when the value is exactly the guard sentinel + whitespace-only content + the guard sentinel — + // i.e. only what the producer would have emitted for a whitespace-only cell. + private static bool IsGuardedWhitespace(string value) => + value != null + && value.Length >= 3 + && value[0] == CsvWhitespaceGuard.Sentinel + && value[value.Length - 1] == CsvWhitespaceGuard.Sentinel + && string.IsNullOrWhiteSpace(value.Substring(1, value.Length - 2)); + public void Dispose() { if (!_disposed) diff --git a/src/Data.Xls/Utils/CsvUtils.cs b/src/Data.Xls/Utils/CsvUtils.cs index b2e4c95..d305b5a 100644 --- a/src/Data.Xls/Utils/CsvUtils.cs +++ b/src/Data.Xls/Utils/CsvUtils.cs @@ -1,4 +1,4 @@ -using Data.Csv.Utils; +using Data.Common.Utils; namespace Data.Xls.Utils; @@ -11,8 +11,11 @@ public static IEnumerable EscapeCsvValues(this IEnumerable value // A whitespace-only cell would be trimmed to an empty string by DataFrame's CSV reader // (TextFieldParser.TrimWhiteSpace). Wrap it in a guard character on both ends so the field's // edges are non-whitespace and survive parsing; CsvVirtualDataTable strips the guard afterwards. + // + // Scope: only *fully* whitespace-only cells are guarded. Leading/trailing whitespace on + // otherwise-non-empty text (e.g. " x ") is still trimmed by the reader and is not preserved. var guarded = value.Length > 0 && string.IsNullOrWhiteSpace(value) - ? CsvVirtualDataTable.WhitespaceGuard + value + CsvVirtualDataTable.WhitespaceGuard + ? CsvWhitespaceGuard.Sentinel + value + CsvWhitespaceGuard.Sentinel : value; if (guarded.Contains(",") || guarded.Contains("\"")) diff --git a/src/Data.Xls/XlsIO/Read/XlsReader.cs b/src/Data.Xls/XlsIO/Read/XlsReader.cs index 8e70234..6f13f6e 100644 --- a/src/Data.Xls/XlsIO/Read/XlsReader.cs +++ b/src/Data.Xls/XlsIO/Read/XlsReader.cs @@ -57,7 +57,7 @@ private VirtualDataTable PrepareDataTable(StreamReader streamReader, string tabl char separator = ','; //Since the XlsSheetStream composed the stream as comma separated, we are ensured that the separator is a comma and don't need to detect it. CsvVirtualDataTable virtualDataTable = new(streamReader, tableName, pageSize, xlsConnection.GuessTypeRows, fileConnection.PreferredFloatingPointDataType, xlsConnection.GuessTypeFunction, - separator); + separator, stripWhitespaceGuard: true); return virtualDataTable; } diff --git a/tests/Data.Csv.Tests/CsvVirtualDataTableTests.cs b/tests/Data.Csv.Tests/CsvVirtualDataTableTests.cs new file mode 100644 index 0000000..4d7aede --- /dev/null +++ b/tests/Data.Csv.Tests/CsvVirtualDataTableTests.cs @@ -0,0 +1,36 @@ +using System.IO; +using System.Linq; +using System.Text; +using Data.Common.Utils; +using Data.Common.Utils.ConnectionString; +using Data.Csv.Utils; +using Xunit; + +namespace Data.Csv.Tests; + +public class CsvVirtualDataTableTests +{ + [Fact] + public void GuardShapedValue_WhenGuardStrippingDisabled_IsReturnedUnchanged() + { + // A value that merely looks like a whitespace guard (sentinel + whitespace + sentinel) must NOT + // be altered by a reader whose source does not use guards. Guard stripping is opt-in via + // stripWhitespaceGuard (used only by the XLS provider, which writes the guards); every other + // provider must leave such data exactly as-is. + var guard = CsvWhitespaceGuard.Sentinel; + var guardShaped = $"{guard} {guard}"; + var csv = $"Value,Marker\n{guardShaped},guarded\nnormal,control\n"; + + using var stream = new MemoryStream(Encoding.UTF8.GetBytes(csv)); + using var reader = new StreamReader(stream); + using var table = new CsvVirtualDataTable( + reader, "guardShaped", pageSize: 4096, guessTypeRows: 1000, + FloatingPointDataType.Double, TypeGuesser.GuessType, separator: ',', + stripWhitespaceGuard: false); + + var rows = table.Rows!.ToList(); + var guardedRow = rows.Single(row => (string)row["Marker"] == "guarded"); + + Assert.Equal(guardShaped, (string)guardedRow["Value"]); + } +} diff --git a/tests/Data.Tests.Common/DataReaderTests.cs b/tests/Data.Tests.Common/DataReaderTests.cs index b8fd25a..8a20b46 100644 --- a/tests/Data.Tests.Common/DataReaderTests.cs +++ b/tests/Data.Tests.Common/DataReaderTests.cs @@ -240,12 +240,15 @@ public static void Reader_ShouldReadEmptyCells( connection.Close(); } - public static void Reader_ShouldPreserveWhitespaceOnlyCell(Func> createFileConnection) + // NOTE: whitespace-only preservation is currently an XLS-provider behavior (only the XLS write path + // guards whitespace cells). The table name is parameterized so the fixture/sheet is not assumed. + public static void Reader_ShouldPreserveWhitespaceOnlyCell( + Func> createFileConnection, string tableName) where TFileParameter : FileParameter, new() { // Arrange var connection = createFileConnection(); - var command = connection.CreateCommand("SELECT * FROM [Sheet1]"); + var command = connection.CreateCommand($"SELECT * FROM [{tableName}]"); // Act & Assert connection.Open(); @@ -271,6 +274,45 @@ public static void Reader_ShouldPreserveWhitespaceOnlyCell(Func< Assert.True(found, "The 'spaces' row was not returned by the reader."); } + // Preserving a whitespace-only cell as text has a consequence for a column that also contains + // numbers: that column is read as text, because the cell cannot be both whitespace and a number. + // The whitespace is preserved and the numeric cells come back in their string form. + public static void Reader_WhitespaceCellInNumericLikeColumn_IsPreservedAsText( + Func> createFileConnection, string tableName) + where TFileParameter : FileParameter, new() + { + // Arrange + var connection = createFileConnection(); + var command = connection.CreateCommand($"SELECT * FROM [{tableName}]"); + + // Act & Assert + connection.Open(); + var seenMarkers = new List(); + using (var reader = command.ExecuteReader()) + { + while (reader.Read()) + { + var marker = reader["Marker"].ToString() ?? string.Empty; + seenMarkers.Add(marker); + switch (marker) + { + case "num1": + Assert.Equal("10", reader["Amount"]); + break; + case "num2": + Assert.Equal("20", reader["Amount"]); + break; + case "spaces": + Assert.Equal(" ", reader["Amount"]); + break; + } + } + } + connection.Close(); + + Assert.Equal(new[] { "num1", "spaces", "num2" }.OrderBy(m => m), seenMarkers.OrderBy(m => m)); + } + public static void Reader_ShouldReadFormulasAsString(Func> createFileConnection) where TFileParameter : FileParameter, new() { diff --git a/tests/Data.Tests.Common/Utils/DatabaseFullPaths.cs b/tests/Data.Tests.Common/Utils/DatabaseFullPaths.cs index 6a3d9cf..d3831be 100644 --- a/tests/Data.Tests.Common/Utils/DatabaseFullPaths.cs +++ b/tests/Data.Tests.Common/Utils/DatabaseFullPaths.cs @@ -21,6 +21,7 @@ public DatabaseFullPaths(string extension) public string EmptyCells => Path.Combine(FileConnectionStringTestsExtensions.SourcesFolder, $"emptyCells.{extension}"); public string CellsWithComma => Path.Combine(FileConnectionStringTestsExtensions.SourcesFolder, $"cellsWithComma.{extension}"); public string WithSpaceCell => Path.Combine(FileConnectionStringTestsExtensions.SourcesFolder, $"withSpaceCell.{extension}"); + public string WithSpaceCellMixed => Path.Combine(FileConnectionStringTestsExtensions.SourcesFolder, $"withSpaceCellMixed.{extension}"); public string eComFileDataBase => Path.Combine(FileConnectionStringTestsExtensions.SourcesFolder, $"ecommerce.{extension}"); public string eComFolderDataBase => Path.Combine(FileConnectionStringTestsExtensions.SourcesFolder, $"eCom"); public string FolderEmptyWithTables => Path.Combine(FileConnectionStringTestsExtensions.SourcesFolder, "EmptyDatabase"); diff --git a/tests/Data.Xls.Tests/Data.Xls.Tests.csproj b/tests/Data.Xls.Tests/Data.Xls.Tests.csproj index ab90cfc..fcf5f1e 100644 --- a/tests/Data.Xls.Tests/Data.Xls.Tests.csproj +++ b/tests/Data.Xls.Tests/Data.Xls.Tests.csproj @@ -61,5 +61,8 @@ Always + + Always + diff --git a/tests/Data.Xls.Tests/FileAsDatabase/XlsDataReaderTests.cs b/tests/Data.Xls.Tests/FileAsDatabase/XlsDataReaderTests.cs index cda027c..74dcfdd 100644 --- a/tests/Data.Xls.Tests/FileAsDatabase/XlsDataReaderTests.cs +++ b/tests/Data.Xls.Tests/FileAsDatabase/XlsDataReaderTests.cs @@ -68,7 +68,14 @@ public void Reader_ShouldReadEmptyCells() public void Reader_ShouldPreserveWhitespaceOnlyCell() { DataReaderTests.Reader_ShouldPreserveWhitespaceOnlyCell(() => - new XlsConnection(ConnectionStrings.Instance.WithSpaceCellAsDB)); + new XlsConnection(ConnectionStrings.Instance.WithSpaceCellAsDB), "Sheet1"); + } + + [Fact] + public void Reader_WhitespaceCellInNumericLikeColumn_IsPreservedAsText() + { + DataReaderTests.Reader_WhitespaceCellInNumericLikeColumn_IsPreservedAsText(() => + new XlsConnection(ConnectionStrings.Instance.WithSpaceCellMixedAsDB), "Sheet1"); } [Fact] diff --git a/tests/Data.Xls.Tests/Sources/withSpaceCellMixed.xlsx b/tests/Data.Xls.Tests/Sources/withSpaceCellMixed.xlsx new file mode 100644 index 0000000000000000000000000000000000000000..0d4b426981c14cc921d572b90033fd1017a6f0f6 GIT binary patch literal 1932 zcmZ{l2~<*f6viP7H-=o$5|gwtm&tI;awc-kEGn0=j4VwJceB7fV`RvMqh+P3Sxke= zRBoAxSyD=|8kRT)3B8%y}H)^v>wekP=t+QI@gNXSJrchLKHR zos0_E%&pok`_7cgGdc{#U`(^164RE9w@1q8!E}QWQ6cKU%euV;%2%5e#41Zt!YYKm8Di|EYRd&v})2**y~} zrGRL9fPjiX^upsp2&kRQ&nqoV*l)&ANS`}4Hj5$_Jx3^LP7_JQ;}jk)n^G9AZa>JK)At4NsZ zGNGDl=S-fdj@k(KG+n!HciTcsjBPNFWl`Po%p2!ph<%Ig@g;18QSI($psrHb(xV3x zkOY6%G>yQ*W82`(;5BnoZ)yIE(J1J+;@`+^|Kx z@tm~C7 zY(xOl}@4J-gFBCLhrf0TC<&rLRR;KCKXq27JAwSWU<&rEt&6X0V5@I;R ziGOM21fNk$xRDMSaiwjO*`UbXTHn&P>!w1qm!I&iDZ=qxjrxOw6KSnJpLEG7RZn)W z=2sNHP2~XGOn`smN|y-%I9xaZ^(B}rw>z}8|4^MVRCdPQK+MNY{Fz*VhR@2+fnTeK zvFKVyy2svoOy_4@;N~o*Z`K(XO{qU}8M2K2VTuN}A<7L-IwAIb;PR0+=gWx|n3??k zq>Wb{sCJ(QDEgGW(^ytUo8)pM%f4BlQ#?cvXyiUN$eO*(a3~+dH5%#P9syIS=#3HL zi*z)1HWcrG0o?gX~SCQ!q2%p1XS< zX!^k@J0^<|aMvt}H96s7%Y3ggJmua8Crax}lxan?af;zlAH}@JxVD^X<-AeDH;%*j zgO;%drt5P?Q(ufKyA0&7kLcy`WJu#}wmvj4-C5*0Gv$;O-#3rN#aW+Qe|LkyNlrsjr2L6TY`-Tr7e{+4u{-p~@ zo8C$`8j$cC3zSZjww0AcIPmk4G8yS)>1|y}mV8bApWT(NBR%!s)RFo3BmT1k(p98~ cW~B;v-S new FileConnectionString { DataSource = Database.WithSpaceCell }; + public FileConnectionString WithSpaceCellMixedAsDB => new FileConnectionString { DataSource = Database.WithSpaceCellMixed }; + public new static ConnectionStrings Instance => new ConnectionStrings(); } \ No newline at end of file