Skip to content

Commit 96baf92

Browse files
committed
Refactored MockFileStream to simplify content synchronization logic and handle unflushed writes. Split a function to reduce complexity. Fixed code sniffs (especially insecure file path generation). Updated API baseline due to Read() and Length override. This should not be a breaking change.
1 parent eb6b880 commit 96baf92

File tree

8 files changed

+141
-110
lines changed

8 files changed

+141
-110
lines changed

src/TestableIO.System.IO.Abstractions.TestingHelpers/MockFileStream.cs

Lines changed: 119 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
using System.Threading;
33
using System.Runtime.Versioning;
44
using System.Security.AccessControl;
5+
using System.Linq;
56

67
namespace System.IO.Abstractions.TestingHelpers;
78

@@ -75,6 +76,13 @@ public MockFileStream(
7576

7677
var timeAdjustments = GetTimeAdjustmentsForFileStreamWhenFileExists(mode, access);
7778
mockFileDataAccessor.AdjustTimes(fileData, timeAdjustments);
79+
80+
// For Truncate mode, clear the file contents first
81+
if (mode == FileMode.Truncate)
82+
{
83+
fileData.Contents = new byte[0];
84+
}
85+
7886
var existingContents = fileData.Contents;
7987
var keepExistingContents =
8088
existingContents?.Length > 0 &&
@@ -145,7 +153,6 @@ public override long Length
145153
{
146154
get
147155
{
148-
// Only refresh if needed to see latest file size from other streams
149156
RefreshFromSharedContentIfNeeded();
150157
return base.Length;
151158
}
@@ -374,112 +381,22 @@ private void RefreshFromSharedContent(MockFileData mockFileData = null)
374381
// This prevents unnecessary work and maintains performance
375382
if (mockFileData.ContentVersion != lastKnownContentVersion)
376383
{
377-
// If we have unflushed writes, we need to preserve them when refreshing
378-
// This handles the case where:
379-
// 1. Stream A writes data but hasn't flushed
380-
// 2. Stream B writes and flushes, updating shared content
381-
// 3. Stream A needs to refresh but preserve its unflushed changes
382-
byte[] preservedContent = null;
383-
long preservedLength = 0;
384384
long currentPosition = Position;
385385

386-
if (hasUnflushedWrites)
387-
{
388-
// Save our current stream content to preserve unflushed writes
389-
preservedLength = base.Length;
390-
preservedContent = new byte[preservedLength];
391-
var originalPosition = Position;
392-
Position = 0;
393-
var totalBytesRead = 0;
394-
while (totalBytesRead < preservedLength)
395-
{
396-
var bytesRead = base.Read(preservedContent, totalBytesRead, (int)(preservedLength - totalBytesRead));
397-
if (bytesRead == 0)
398-
{
399-
break;
400-
}
401-
totalBytesRead += bytesRead;
402-
}
403-
Position = originalPosition;
404-
}
386+
// Preserve unflushed content if necessary
387+
var (preservedContent, preservedLength) = PreserveUnflushedContent();
405388

406389
var sharedContent = mockFileData.Contents;
407390

408-
// Performance optimization: if we have no unflushed writes and the shared content
409-
// is the same length as our current content, we might not need to do expensive copying
410-
if (!hasUnflushedWrites && sharedContent?.Length == base.Length)
411-
{
412-
// Quick content comparison for common case where only metadata changed
413-
bool contentSame = true;
414-
if (sharedContent.Length > 0 && sharedContent.Length <= 4096) // Only check small files
415-
{
416-
var currentPos = Position;
417-
Position = 0;
418-
var currentContent = new byte[base.Length];
419-
var bytesRead = base.Read(currentContent, 0, (int)base.Length);
420-
Position = currentPos;
421-
422-
if (bytesRead == sharedContent.Length)
423-
{
424-
for (int i = 0; i < bytesRead; i++)
425-
{
426-
if (currentContent[i] != sharedContent[i])
427-
{
428-
contentSame = false;
429-
break;
430-
}
431-
}
432-
}
433-
else
434-
{
435-
contentSame = false;
436-
}
437-
}
438-
else
439-
{
440-
contentSame = false; // Don't compare large files
441-
}
442-
443-
if (contentSame)
444-
{
445-
// Content is identical, just update version tracking and exit
446-
lastKnownContentVersion = mockFileData.ContentVersion;
447-
return;
448-
}
449-
}
450-
451-
// Start with shared content as the base - this gives us the latest changes from other streams
452-
base.SetLength(0);
453-
Position = 0;
454-
if (sharedContent != null && sharedContent.Length > 0)
391+
// Check if content is already the same (optimization for metadata-only changes)
392+
if (IsContentIdentical(sharedContent))
455393
{
456-
base.Write(sharedContent, 0, sharedContent.Length);
394+
lastKnownContentVersion = mockFileData.ContentVersion;
395+
return;
457396
}
458397

459-
// If we had unflushed writes, we need to overlay them on the shared content
460-
// This ensures our local changes take precedence over shared content
461-
if (hasUnflushedWrites && preservedContent != null)
462-
{
463-
// Optimization: if preserved content is same length or longer, just use it directly
464-
if (preservedLength >= (sharedContent?.Length ?? 0))
465-
{
466-
base.SetLength(0);
467-
Position = 0;
468-
base.Write(preservedContent, 0, (int)preservedLength);
469-
}
470-
else
471-
{
472-
// Need to merge: ensure stream is large enough
473-
if (base.Length < preservedLength)
474-
{
475-
base.SetLength(preservedLength);
476-
}
477-
478-
// Apply our preserved content on top of the shared content
479-
Position = 0;
480-
base.Write(preservedContent, 0, (int)preservedLength);
481-
}
482-
}
398+
// Merge shared content with any preserved unflushed writes
399+
MergeWithSharedContent(sharedContent, preservedContent, preservedLength);
483400

484401
// Restore position, but ensure it's within bounds of the new content
485402
Position = Math.Min(currentPosition, base.Length);
@@ -491,6 +408,109 @@ private void RefreshFromSharedContent(MockFileData mockFileData = null)
491408
}
492409
}
493410

411+
/// <summary>
412+
/// Preserves unflushed content from the current stream before refreshing from shared content.
413+
/// </summary>
414+
/// <returns>A tuple containing the preserved content and its length.</returns>
415+
private (byte[] content, long length) PreserveUnflushedContent()
416+
{
417+
if (!hasUnflushedWrites)
418+
{
419+
return (null, 0);
420+
}
421+
422+
// Save our current stream content to preserve unflushed writes
423+
var preservedLength = base.Length;
424+
var preservedContent = new byte[preservedLength];
425+
var originalPosition = Position;
426+
Position = 0;
427+
var totalBytesRead = 0;
428+
while (totalBytesRead < preservedLength)
429+
{
430+
var bytesRead = base.Read(preservedContent, totalBytesRead, (int)(preservedLength - totalBytesRead));
431+
if (bytesRead == 0)
432+
{
433+
break;
434+
}
435+
totalBytesRead += bytesRead;
436+
}
437+
Position = originalPosition;
438+
439+
return (preservedContent, preservedLength);
440+
}
441+
442+
/// <summary>
443+
/// Checks if the current content is identical to the shared content (optimization).
444+
/// </summary>
445+
/// <param name="sharedContent">The shared content to compare against.</param>
446+
/// <returns>True if the content is identical, false otherwise.</returns>
447+
private bool IsContentIdentical(byte[] sharedContent)
448+
{
449+
// Performance optimization: if we have no unflushed writes and the shared content
450+
// is the same length as our current content, we might not need to do expensive copying
451+
if (hasUnflushedWrites || sharedContent?.Length != base.Length)
452+
{
453+
return false;
454+
}
455+
456+
// Quick content comparison for common case where only metadata changed
457+
if (sharedContent.Length > 0 && sharedContent.Length <= 4096) // Only check small files
458+
{
459+
var currentPos = Position;
460+
Position = 0;
461+
var currentContent = new byte[base.Length];
462+
var bytesRead = base.Read(currentContent, 0, (int)base.Length);
463+
Position = currentPos;
464+
465+
return bytesRead == sharedContent.Length &&
466+
currentContent.Take(bytesRead).SequenceEqual(sharedContent);
467+
}
468+
469+
return false; // Don't compare large files
470+
}
471+
472+
/// <summary>
473+
/// Merges the shared content with any preserved unflushed writes.
474+
/// </summary>
475+
/// <param name="sharedContent">The shared content from MockFileData.</param>
476+
/// <param name="preservedContent">Any preserved unflushed content.</param>
477+
/// <param name="preservedLength">The length of the preserved content.</param>
478+
private void MergeWithSharedContent(byte[] sharedContent, byte[] preservedContent, long preservedLength)
479+
{
480+
// Start with shared content as the base - this gives us the latest changes from other streams
481+
base.SetLength(0);
482+
Position = 0;
483+
if (sharedContent != null && sharedContent.Length > 0)
484+
{
485+
base.Write(sharedContent, 0, sharedContent.Length);
486+
}
487+
488+
// If we had unflushed writes, we need to overlay them on the shared content
489+
// This ensures our local changes take precedence over shared content
490+
if (hasUnflushedWrites && preservedContent != null)
491+
{
492+
// Optimization: if preserved content is same length or longer, just use it directly
493+
if (preservedLength >= (sharedContent?.Length ?? 0))
494+
{
495+
base.SetLength(0);
496+
Position = 0;
497+
base.Write(preservedContent, 0, (int)preservedLength);
498+
}
499+
else
500+
{
501+
// Need to merge: ensure stream is large enough
502+
if (base.Length < preservedLength)
503+
{
504+
base.SetLength(preservedLength);
505+
}
506+
507+
// Apply our preserved content on top of the shared content
508+
Position = 0;
509+
base.Write(preservedContent, 0, (int)preservedLength);
510+
}
511+
}
512+
}
513+
494514
/// <summary>
495515
/// Flushes this stream's content to the shared file data, implementing proper synchronization for FileShare.ReadWrite.
496516
///

tests/TestableIO.System.IO.Abstractions.Api.Tests/Expected/TestableIO.System.IO.Abstractions.TestingHelpers_net472.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,7 @@ namespace System.IO.Abstractions.TestingHelpers
300300
public MockFileStream(System.IO.Abstractions.TestingHelpers.IMockFileDataAccessor mockFileDataAccessor, string path, System.IO.FileMode mode, System.IO.FileAccess access = 3, System.IO.FileOptions options = 0) { }
301301
public override bool CanRead { get; }
302302
public override bool CanWrite { get; }
303+
public override long Length { get; }
303304
public static System.IO.Abstractions.FileSystemStream Null { get; }
304305
protected override void Dispose(bool disposing) { }
305306
public override void EndWrite(System.IAsyncResult asyncResult) { }

tests/TestableIO.System.IO.Abstractions.Api.Tests/Expected/TestableIO.System.IO.Abstractions.TestingHelpers_net6.0.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,7 @@ namespace System.IO.Abstractions.TestingHelpers
349349
public MockFileStream(System.IO.Abstractions.TestingHelpers.IMockFileDataAccessor mockFileDataAccessor, string path, System.IO.FileMode mode, System.IO.FileAccess access = 3, System.IO.FileOptions options = 0) { }
350350
public override bool CanRead { get; }
351351
public override bool CanWrite { get; }
352+
public override long Length { get; }
352353
public static System.IO.Abstractions.FileSystemStream Null { get; }
353354
protected override void Dispose(bool disposing) { }
354355
public override void EndWrite(System.IAsyncResult asyncResult) { }
@@ -359,6 +360,7 @@ namespace System.IO.Abstractions.TestingHelpers
359360
public object GetAccessControl() { }
360361
[System.Runtime.Versioning.SupportedOSPlatform("windows")]
361362
public object GetAccessControl(System.IO.Abstractions.IFileSystemAclSupport.AccessControlSections includeSections) { }
363+
public override int Read(System.Span<byte> buffer) { }
362364
public override int Read(byte[] buffer, int offset, int count) { }
363365
[System.Runtime.Versioning.SupportedOSPlatform("windows")]
364366
public void SetAccessControl(object value) { }

tests/TestableIO.System.IO.Abstractions.Api.Tests/Expected/TestableIO.System.IO.Abstractions.TestingHelpers_net8.0.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,7 @@ namespace System.IO.Abstractions.TestingHelpers
373373
public MockFileStream(System.IO.Abstractions.TestingHelpers.IMockFileDataAccessor mockFileDataAccessor, string path, System.IO.FileMode mode, System.IO.FileAccess access = 3, System.IO.FileOptions options = 0) { }
374374
public override bool CanRead { get; }
375375
public override bool CanWrite { get; }
376+
public override long Length { get; }
376377
public static System.IO.Abstractions.FileSystemStream Null { get; }
377378
protected override void Dispose(bool disposing) { }
378379
public override void EndWrite(System.IAsyncResult asyncResult) { }
@@ -383,6 +384,7 @@ namespace System.IO.Abstractions.TestingHelpers
383384
public object GetAccessControl() { }
384385
[System.Runtime.Versioning.SupportedOSPlatform("windows")]
385386
public object GetAccessControl(System.IO.Abstractions.IFileSystemAclSupport.AccessControlSections includeSections) { }
387+
public override int Read(System.Span<byte> buffer) { }
386388
public override int Read(byte[] buffer, int offset, int count) { }
387389
[System.Runtime.Versioning.SupportedOSPlatform("windows")]
388390
public void SetAccessControl(object value) { }

tests/TestableIO.System.IO.Abstractions.Api.Tests/Expected/TestableIO.System.IO.Abstractions.TestingHelpers_net9.0.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -387,6 +387,7 @@ namespace System.IO.Abstractions.TestingHelpers
387387
public MockFileStream(System.IO.Abstractions.TestingHelpers.IMockFileDataAccessor mockFileDataAccessor, string path, System.IO.FileMode mode, System.IO.FileAccess access = 3, System.IO.FileOptions options = 0) { }
388388
public override bool CanRead { get; }
389389
public override bool CanWrite { get; }
390+
public override long Length { get; }
390391
public static System.IO.Abstractions.FileSystemStream Null { get; }
391392
protected override void Dispose(bool disposing) { }
392393
public override void EndWrite(System.IAsyncResult asyncResult) { }
@@ -397,6 +398,7 @@ namespace System.IO.Abstractions.TestingHelpers
397398
public object GetAccessControl() { }
398399
[System.Runtime.Versioning.SupportedOSPlatform("windows")]
399400
public object GetAccessControl(System.IO.Abstractions.IFileSystemAclSupport.AccessControlSections includeSections) { }
401+
public override int Read(System.Span<byte> buffer) { }
400402
public override int Read(byte[] buffer, int offset, int count) { }
401403
[System.Runtime.Versioning.SupportedOSPlatform("windows")]
402404
public void SetAccessControl(object value) { }

tests/TestableIO.System.IO.Abstractions.Api.Tests/Expected/TestableIO.System.IO.Abstractions.TestingHelpers_netstandard2.0.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,7 @@ namespace System.IO.Abstractions.TestingHelpers
300300
public MockFileStream(System.IO.Abstractions.TestingHelpers.IMockFileDataAccessor mockFileDataAccessor, string path, System.IO.FileMode mode, System.IO.FileAccess access = 3, System.IO.FileOptions options = 0) { }
301301
public override bool CanRead { get; }
302302
public override bool CanWrite { get; }
303+
public override long Length { get; }
303304
public static System.IO.Abstractions.FileSystemStream Null { get; }
304305
protected override void Dispose(bool disposing) { }
305306
public override void EndWrite(System.IAsyncResult asyncResult) { }

tests/TestableIO.System.IO.Abstractions.Api.Tests/Expected/TestableIO.System.IO.Abstractions.TestingHelpers_netstandard2.1.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,7 @@ namespace System.IO.Abstractions.TestingHelpers
326326
public MockFileStream(System.IO.Abstractions.TestingHelpers.IMockFileDataAccessor mockFileDataAccessor, string path, System.IO.FileMode mode, System.IO.FileAccess access = 3, System.IO.FileOptions options = 0) { }
327327
public override bool CanRead { get; }
328328
public override bool CanWrite { get; }
329+
public override long Length { get; }
329330
public static System.IO.Abstractions.FileSystemStream Null { get; }
330331
protected override void Dispose(bool disposing) { }
331332
public override void EndWrite(System.IAsyncResult asyncResult) { }
@@ -334,6 +335,7 @@ namespace System.IO.Abstractions.TestingHelpers
334335
public override System.Threading.Tasks.Task FlushAsync(System.Threading.CancellationToken cancellationToken) { }
335336
public object GetAccessControl() { }
336337
public object GetAccessControl(System.IO.Abstractions.IFileSystemAclSupport.AccessControlSections includeSections) { }
338+
public override int Read(System.Span<byte> buffer) { }
337339
public override int Read(byte[] buffer, int offset, int count) { }
338340
public void SetAccessControl(object value) { }
339341
public override void SetLength(long value) { }

0 commit comments

Comments
 (0)