Skip to content

Add ShouldDispose property to StreamFile for optional direct stream access#103

Merged
Arlodotexe merged 6 commits intomainfrom
copilot/fix-102
Aug 18, 2025
Merged

Add ShouldDispose property to StreamFile for optional direct stream access#103
Arlodotexe merged 6 commits intomainfrom
copilot/fix-102

Conversation

Copy link
Contributor

Copilot AI commented Aug 18, 2025

This PR adds a ShouldDispose property to the StreamFile class to enable optional direct stream access with dispose behavior intact, addressing scenarios where single-use stream access is preferred over the default multi-use behavior.

Background

Previously, StreamFile always wrapped the underlying stream in a NonDisposableStreamWrapper, which is ideal for most use cases where the file might be opened multiple times. However, some scenarios require returning the underlying stream directly with dispose behavior intact, even if that means the file can only be opened once.

Changes

  • Added ShouldDispose property: A boolean get/set property that controls whether the underlying stream is returned directly or wrapped
  • Simplified constructors: Removed constructor overloads with shouldDispose parameter to avoid cluttering
  • Updated OpenStreamAsync logic: Conditionally wraps the stream based on the ShouldDispose value
  • Comprehensive testing: Updated tests to use the property-based approach

Usage

// Default behavior (existing) - multiple opens supported, non-disposable wrapper
var file1 = new StreamFile(stream); // ShouldDispose = false (default)

// New behavior - single use, direct access with dispose behavior
var file2 = new StreamFile(stream);
file2.ShouldDispose = true;

// When ShouldDispose = false (default)
var wrappedStream = await file1.OpenStreamAsync(); // Returns NonDisposableStreamWrapper
wrappedStream.Dispose(); // Underlying stream remains usable

// When ShouldDispose = true  
var directStream = await file2.OpenStreamAsync(); // Returns underlying stream directly
directStream.Dispose(); // Underlying stream is properly disposed

Backward Compatibility

All existing code continues to work unchanged. The default value of ShouldDispose is false, preserving the original behavior of wrapping streams in NonDisposableStreamWrapper.

Fixes #102.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copy link
Contributor Author

Copilot AI commented Aug 18, 2025

@Arlodotexe 👋 This repository doesn't have Copilot instructions. With Copilot instructions, I can understand the repository better, work faster and produce higher quality PRs.

I can generate a .github/copilot-instructions.md file for you automatically. Click here to open a pre-filled issue and assign it to me. I'll write the instructions, and then tag you for review.

Copilot AI and others added 2 commits August 18, 2025 19:50
Co-authored-by: Arlodotexe <9384894+Arlodotexe@users.noreply.github.com>
Co-authored-by: Arlodotexe <9384894+Arlodotexe@users.noreply.github.com>
Copilot AI changed the title [WIP] StreamFile should expose a ShouldDispose property Add ShouldDispose property to StreamFile for optional direct stream access Aug 18, 2025
Copilot AI requested a review from Arlodotexe August 18, 2025 19:55
@Arlodotexe

This comment was marked as resolved.

This comment was marked as resolved.

Co-authored-by: Arlodotexe <9384894+Arlodotexe@users.noreply.github.com>
@Arlodotexe

This comment was marked as resolved.

Copy link
Owner

@Arlodotexe Arlodotexe left a comment

Choose a reason for hiding this comment

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

@copilot The new property should be a getter/setter, shouldn't clutter the constructor parameters.

…or parameter

Co-authored-by: Arlodotexe <9384894+Arlodotexe@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Aug 18, 2025

@copilot Please add the ShouldDispose property as per issue body

The ShouldDispose property has been added to StreamFile as requested. It's now a settable property (get/set) that controls whether the underlying stream is disposed when the returned stream is disposed. Commit 6edbb45.

@copilot The new property should be a getter/setter, shouldn't clutter the constructor parameters.

Changed ShouldDispose to be a simple get/set property and removed the constructor parameter overloads to avoid cluttering the constructor. The property defaults to false for backward compatibility. Commit 6edbb45.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • aka.ms
    • Triggering command: curl -I -sSL --retry 5 --retry-delay 2 --connect-timeout 15 REDACTED (dns block)
  • httpbin.org
    • Triggering command: /home/REDACTED/.dotnet/dotnet exec --runtimeconfig /home/REDACTED/work/OwlCore.Storage/OwlCore.Storage/tests/OwlCore.Storage.Tests/bin/Debug/net9.0/OwlCore.Storage.Tests.runtimeconfig.json --depsfile /home/REDACTED/work/OwlCore.Storage/OwlCore.Storage/tests/OwlCore.Storage.Tests/bin/Debug/net9.0/OwlCore.Storage.Tests.deps.json /home/REDACTED/work/OwlCore.Storage/OwlCore.Storage/tests/OwlCore.Storage.Tests/bin/Debug/net9.0/testhost.dll --port 42851 --endpoint 127.0.0.1:042851 --role client --parentprocessid 4029 --telemetryoptedin false (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

@Arlodotexe Arlodotexe marked this pull request as ready for review August 18, 2025 20:35
@Arlodotexe Arlodotexe merged commit 0b8ca3e into main Aug 18, 2025
1 check passed
@Arlodotexe Arlodotexe deleted the copilot/fix-102 branch August 18, 2025 20:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

StreamFile should expose a ShouldDispose property

2 participants