Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix TempFile usage while temp dir was deleted in same process #1132

Merged
merged 29 commits into from
Apr 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
9535672
Fix TempFile usage while temp dir was deleted in same process
Bykiev Jul 27, 2023
8f86d2e
Update TempFile.cs
Bykiev Jul 27, 2023
77fd8d8
Fix CI test execution
Bykiev Jul 27, 2023
1c8a56a
Trying to fix tests2
Bykiev Jul 27, 2023
b293c63
trying to fix test 3
Bykiev Jul 27, 2023
2aea9f9
Merge remote-tracking branch 'upstream/master' into FixBug926
Bykiev Sep 1, 2023
6c8fc86
Update TempFile.cs
Bykiev Sep 1, 2023
1daa483
Update TempFile.cs
Bykiev Sep 1, 2023
928fa26
Another one try
Bykiev Sep 1, 2023
e7faafb
Remove RunSerialyAndSweepTmpFilesAttribute
Bykiev Sep 14, 2023
949d395
Merge remote-tracking branch 'upstream/master' into FixBug926
Bykiev Sep 14, 2023
a35231b
Update SXSSFWorkbookTests.cs
Bykiev Sep 14, 2023
99c5605
Update SXSSFWorkbookTests.cs
Bykiev Sep 14, 2023
23060c1
Call dispose on SheeDataWriter.Close()
Bykiev Sep 14, 2023
366130a
Comment all SXSSFWorkbookTests
Bykiev Sep 14, 2023
114434f
test
Bykiev Sep 14, 2023
ec9bb51
Update TempFile.cs
Bykiev Sep 14, 2023
874dee8
Comment TestSXSSFWorkbook
Bykiev Sep 14, 2023
e5d0d20
Update TestSXSSFWorkbook.cs
Bykiev Sep 14, 2023
f03e78d
Merge remote-tracking branch 'upstream/master' into FixBug926
Bykiev Jan 3, 2024
4e11700
Update TestTempFile.cs
Bykiev Jan 3, 2024
ede309e
Update TestTempFile.cs
Bykiev Jan 3, 2024
d061292
Update TestTempFile.cs
Bykiev Jan 3, 2024
358e23c
Update TestTempFile.cs
Bykiev Jan 3, 2024
8e5a924
Update TestXSSFWorkbook.cs
Bykiev Jan 3, 2024
9f69b61
More tries
Bykiev Jan 3, 2024
ae31cf0
Merge remote-tracking branch 'upstream/master' into FixBug926
Bykiev Mar 12, 2024
9c02753
Update TestSXSSFWorkbook.cs
Bykiev Mar 12, 2024
dedde39
Merge remote-tracking branch 'upstream/master' into FixBug926
Bykiev Apr 26, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 15 additions & 8 deletions main/Util/TempFile.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,31 +18,38 @@ public class TempFile
*/
public static FileInfo CreateTempFile(String prefix, String suffix)
{

if (dir == null)
if (string.IsNullOrWhiteSpace(dir))
{
dir = Directory.CreateDirectory(Path.Combine(Path.GetTempPath(), "poifiles")).FullName;
string tempDir = Path.Combine(Path.GetTempPath(), "poifiles");
dir = Directory.CreateDirectory(tempDir).FullName;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CreateDirectory always called so shouldn't actually be dir be assigned tempdirs value?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't change much code, but dir variable doesn't needed at all

}

if (!Directory.Exists(dir))
Directory.CreateDirectory(dir);

// Generate a unique new filename
string file = Path.Combine(dir, prefix + Guid.NewGuid().ToString() + suffix);
while (File.Exists(file))
{
file = Path.Combine(dir, prefix + Guid.NewGuid().ToString() + suffix);
Thread.Sleep(1);
}
FileStream newFile = new FileStream(file, FileMode.CreateNew, FileAccess.ReadWrite);
newFile.Close();

using (FileStream newFile = new FileStream(file, FileMode.CreateNew, FileAccess.ReadWrite, FileShare.ReadWrite)) { };

return new FileInfo(file);
}

public static string GetTempFilePath(String prefix, String suffix)
{
if (dir == null)
if (string.IsNullOrWhiteSpace(dir))
{
dir = Directory.CreateDirectory(Path.Combine(Path.GetTempPath(), "poifiles")).FullName;
string tempDir = Path.Combine(Path.GetTempPath(), "poifiles");
dir = Directory.CreateDirectory(tempDir).FullName;
}

if (!Directory.Exists(dir))
Directory.CreateDirectory(dir);

Random rnd = new Random(DateTime.Now.Millisecond);
rnd.Next();
Thread.Sleep(10);
Copy link
Collaborator Author

@Bykiev Bykiev Jul 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the purpose of Thread.Sleep?

Expand Down
15 changes: 3 additions & 12 deletions ooxml/XSSF/Streaming/SheetDataWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -116,23 +116,13 @@ public void Close()
{
try
{
_outputWriter.Flush();
OutputStream.Flush();
_outputWriter.Dispose();
OutputStream.Dispose();
}
catch
{

}
try
{
OutputStream.Close();
}
catch
{

}


}

public FileInfo TempFileInfo
Expand Down Expand Up @@ -554,6 +544,7 @@ public bool Dispose()
bool ret;
try
{
_outputWriter.Close();
OutputStream.Close();
}
finally
Expand Down
53 changes: 0 additions & 53 deletions testcases/main/RunSerialyAndSweepTmpFilesAttribute.cs

This file was deleted.

80 changes: 80 additions & 0 deletions testcases/main/Util/TestTempFile.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
using NPOI.Util;
using NUnit.Framework;
using System.IO;
using System.Threading;

namespace TestCases.Util
{
/// <summary>
/// Tests of creating temp files
/// </summary>
[TestFixture]
internal class TestTempFile
{
[Test]
public void TestCreateTempFile()
{
FileInfo fileInfo = null;
Assert.DoesNotThrow(() => fileInfo = TempFile.CreateTempFile("test", ".xls"));

Assert.IsTrue(fileInfo!=null && fileInfo.Exists);

string tempDirPath = Path.GetDirectoryName(fileInfo.FullName);

while(Directory.Exists(tempDirPath))
{
try
{
Directory.Delete(tempDirPath, true);
}
catch
{
Thread.Sleep(5);
}
}

Assert.IsFalse(Directory.Exists(tempDirPath));

if(fileInfo!=null)
{
fileInfo.Refresh();
Assert.IsFalse(fileInfo.Exists);
}

FileInfo file = null;
Assert.DoesNotThrow(() => file = TempFile.CreateTempFile("test2", ".xls"));
Assert.IsTrue(Directory.Exists(tempDirPath));

if(file !=null && file.Exists)
file.Delete();
}

[Test]
public void TestGetTempFilePath()
{
string path = "";
Assert.DoesNotThrow(() => path = TempFile.GetTempFilePath("test", ".xls"));

Assert.IsTrue(!string.IsNullOrWhiteSpace(path));

string tempDirPath = Path.GetDirectoryName(path);

while(Directory.Exists(tempDirPath))
{
try
{
Directory.Delete(tempDirPath, true);
}
catch
{
Thread.Sleep(10);
}
}

Assert.IsFalse(Directory.Exists(tempDirPath));

Assert.DoesNotThrow(() => TempFile.GetTempFilePath("test", ".xls"));
Assert.IsTrue(Directory.Exists(tempDirPath));
}
}
}
5 changes: 2 additions & 3 deletions testcases/ooxml/XSSF/Streaming/GZIPSheetDataWriterTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,10 @@ public void CleanUp()
{
if (_objectToTest != null)
{
_objectToTest.Dispose();

if (File.Exists(_objectToTest.TemporaryFilePath()))
{
_objectToTest.Dispose();
File.Delete(_objectToTest.TemporaryFilePath());
}
}
}
[Test]
Expand Down
26 changes: 13 additions & 13 deletions testcases/ooxml/XSSF/Streaming/SXSSFWorkbookTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,12 @@ the License. You may obtain a copy of the License at
See the License for the specific language governing permissions and
limitations under the License.
==================================================================== */
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Text;
using NPOI.SS.UserModel;
using NPOI.XSSF.Streaming;
using NPOI.XSSF.UserModel;
using NUnit.Framework;
using System;
using System.IO;

namespace TestCases.XSSF.Streaming
{
Expand All @@ -31,6 +28,13 @@ class SXSSFWorkbookTests
{
private SXSSFWorkbook _objectToTest { get; set; }

[TearDown]
public void CleanUp()
{
if (_objectToTest != null)
_objectToTest.Dispose();
}

[Test]
public void
CallingEmptyConstructorShouldInstanstiateNewXssfWorkbookDefaultRowAccessWindowSizeCompressTempFilesAsFalseAndUseSharedStringsTableFalse
Expand Down Expand Up @@ -76,6 +80,8 @@ public void IfCompressTmpFilesIsSetToTrueShouldReturnGZIPSheetDataWriter()

Assert.IsTrue(result is GZIPSheetDataWriter);

if (result != null)
result.Close();
}

[Test]
Expand All @@ -86,6 +92,8 @@ public void IfCompressTmpFilesIsSetToFalseShouldReturnSheetDataWriter()

Assert.IsTrue(result is SheetDataWriter);

if (result != null)
result.Close();
}

[Test]
Expand All @@ -111,7 +119,6 @@ public void IfSettingSelectedTabShouldSetSelectedTabOfXssfWorkbook()
_objectToTest.SetSelectedTab(0);

Assert.IsTrue(_objectToTest.GetSheetAt(0).IsSelected);

}

[Test]
Expand All @@ -124,7 +131,6 @@ public void IfSheetNameByIndexShouldGetSheetNameFromXssfWorkbook()
_objectToTest.SetSelectedTab(0);

Assert.IsTrue(_objectToTest.GetSheetAt(0).IsSelected);

}

[Test]
Expand All @@ -135,7 +141,6 @@ public void IfSettingSheetNameShouldChangeTheSheetNameAtTheSpecifiedIndex()
_objectToTest.SetSheetName(0, "renamed");

Assert.AreEqual("renamed", _objectToTest.GetSheetAt(0).SheetName);

}

[Test]
Expand Down Expand Up @@ -199,10 +204,8 @@ public void IfGivenTheNameOfAnExistingSheetShouldReturnTheSheet()

Assert.AreEqual("1", sheet1.SheetName);
Assert.AreEqual("2", sheet2.SheetName);

}


[Test]
public void IfGivenTheIndexOfAnExistingSheetShouldReturnTheSheet()
{
Expand All @@ -215,7 +218,6 @@ public void IfGivenTheIndexOfAnExistingSheetShouldReturnTheSheet()

Assert.AreEqual("1", sheet1.SheetName);
Assert.AreEqual("2", sheet2.SheetName);

}

[Test]
Expand Down Expand Up @@ -451,7 +453,6 @@ public void IfWriting20WorksheetsWith10000x100CellsUsingGzipShouldNotThrowOutOfM
File.Delete(savePath);
}


private void AddCells(IWorkbook wb, int sheets, int rows, int columns, CellType type)
{
for (int j = 0; j < sheets; j++)
Expand All @@ -468,7 +469,6 @@ private void AddCells(IWorkbook wb, int sheets, int rows, int columns, CellType
}
}


private void WriteFile(string saveAsPath, SXSSFWorkbook wb)
{
//Passing SXSSFWorkbook because IWorkbook does not implement .Dispose which cleans ups temporary files.
Expand Down
5 changes: 2 additions & 3 deletions testcases/ooxml/XSSF/Streaming/SheetDataWriterTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,10 @@ public void CleanUp()
{
if (_objectToTest != null)
{
_objectToTest.Dispose();

if (File.Exists(_objectToTest.TemporaryFilePath()))
{
_objectToTest.Dispose();
File.Delete(_objectToTest.TemporaryFilePath());
}
}

}
Expand Down
Loading
Loading