-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Changes from 5 commits
9535672
8f86d2e
77fd8d8
1c8a56a
b293c63
2aea9f9
6c8fc86
1daa483
928fa26
e7faafb
949d395
a35231b
99c5605
23060c1
366130a
114434f
ec9bb51
874dee8
e5d0d20
f03e78d
4e11700
ede309e
d061292
358e23c
8e5a924
9f69b61
ae31cf0
9c02753
dedde39
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,11 +18,15 @@ 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; | ||
} | ||
|
||
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)) | ||
|
@@ -38,10 +42,15 @@ public static FileInfo CreateTempFile(String prefix, String suffix) | |
|
||
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); | ||
Thread.Sleep(10); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the purpose of Thread.Sleep? |
||
//return prefix + rnd.Next() + suffix; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
using NPOI.Util; | ||
using NUnit.Framework; | ||
using System.IO; | ||
|
||
namespace TestCases.Util | ||
{ | ||
/// <summary> | ||
/// Tests of creating temp files | ||
/// </summary> | ||
[TestFixture] | ||
[NonParallelizable] | ||
internal class TestTempFile | ||
{ | ||
[Test] | ||
public void TestCreateTempFile() | ||
{ | ||
FileInfo fileInfo = null; | ||
Assert.DoesNotThrow(() => fileInfo = TempFile.CreateTempFile("test", ".xls")); | ||
|
||
Assert.IsTrue(File.Exists(fileInfo.FullName)); | ||
|
||
string tempDirPath = Path.GetDirectoryName(fileInfo.FullName); | ||
Check failure on line 22 in testcases/main/Util/TestTempFile.cs GitHub Actions / windows-latestTestCreateTempFile
|
||
|
||
if (Directory.Exists(tempDirPath)) | ||
Directory.Delete(tempDirPath, true); | ||
|
||
Assert.IsFalse(File.Exists(fileInfo.FullName)); | ||
Assert.IsFalse(Directory.Exists(tempDirPath)); | ||
|
||
Assert.DoesNotThrow(() => TempFile.CreateTempFile("test2", ".xls")); | ||
Assert.IsTrue(Directory.Exists(tempDirPath)); | ||
} | ||
|
||
[Test] | ||
public void TestGetTempFilePath() | ||
{ | ||
string path = ""; | ||
Assert.DoesNotThrow(() => path = TempFile.GetTempFilePath("test", ".xls")); | ||
|
||
Assert.IsTrue(!string.IsNullOrWhiteSpace(path)); | ||
|
||
string tempDirPath = Path.GetDirectoryName(path); | ||
|
||
if (Directory.Exists(tempDirPath)) | ||
Directory.Delete(tempDirPath, true); | ||
Check failure on line 45 in testcases/main/Util/TestTempFile.cs GitHub Actions / windows-latestTestGetTempFilePath
|
||
|
||
Assert.IsFalse(Directory.Exists(tempDirPath)); | ||
|
||
Assert.DoesNotThrow(() => TempFile.GetTempFilePath("test", ".xls")); | ||
Assert.IsTrue(Directory.Exists(tempDirPath)); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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