Adds Default Managed Identity, adds missing SAS authentication and major tests refactoring.#117
Adds Default Managed Identity, adds missing SAS authentication and major tests refactoring.#117k-ljung wants to merge 2 commits intoFrendsPlatform:mainfrom
Conversation
Major breaking changes and enhancements: - Unified authentication handling with new AuthenticationMethod enum (incl. DefaultManagedIdentity, SAS Token) and refactored Connection parameter structure across all tasks. - Standardized and renamed parameters for clarity and consistency (e.g., ApplicationId, SasToken, Uri). - Migrated all projects and tests to .NET 8; updated dependencies. - Modernized and reorganized test projects: switched to MSTest, split tests by auth method, introduced shared UnitTestsBase, and improved test coverage for all scenarios. - Improved error handling, parameter validation, and support for tags, encoding, and metadata. - Updated documentation and CHANGELOGs with migration instructions. - Removed obsolete code and test files. **Note:** This is a breaking change. All users must update their Frends processes to use the new parameter structure and authentication options. See CHANGELOG for migration details.
Updated all project versions (removed -beta, incremented as needed). Added Pack-AllProjects.ps1 to automate NuGet packaging and optional version bumping. Updated README with instructions for building packages using the new script.
WalkthroughThis PR comprehensively refactors authentication and connection handling across the Frends.AzureBlobStorage package suite. It reorganizes parameter models from property-centric patterns (Source, Destination) to dedicated Connection and Input classes, renames ConnectionMethod to AuthenticationMethod enum, introduces DefaultManagedIdentity and SASToken authentication options, refactors ConnectionHandler signatures to accept separate Input/Connection parameters, migrates test frameworks from NUnit to MSTest with shared base test classes, upgrades all projects to .NET 8.0, and bumps package versions (including major versions for breaking changes). Changes
Sequence Diagram(s)sequenceDiagram
participant Client as AzureBlobStorage Task
participant Handler as ConnectionHandler
participant AuthFactory as AuthenticationMethod<br/>Branch
participant Azure as Azure SDK Client<br/>(Blob/Container/Service)
participant Cred as Credential Type<br/>(DefaultAzureCredential/<br/>ClientSecretCredential/etc)
Client->>Handler: GetBlobClient(Input, Connection, CancellationToken)
Handler->>AuthFactory: Switch on Connection.AuthenticationMethod
alt ConnectionString
AuthFactory->>Azure: BlobClient(connString, containerName)
else OAuth2
AuthFactory->>Cred: ClientSecretCredential(tenantId, appId, secret)
Cred->>Azure: BlobServiceClient(uri, credential)
Azure->>Client: Return BlobClient
else SASToken
AuthFactory->>Azure: BlobClient(uri with SAS appended)
else DefaultManagedIdentity
AuthFactory->>Cred: DefaultAzureCredential()
Cred->>Azure: BlobServiceClient(uri, credential)
Azure->>Client: Return BlobClient
else ArcManagedIdentity
AuthFactory->>Cred: ManagedIdentityCredential(options)
Cred->>Azure: BlobServiceClient(uri, credential)
Azure->>Client: Return BlobClient
else ArcManagedIdentityCrossTenant
AuthFactory->>Cred: ManagedIdentityCredential + ClientAssertion flow
Cred->>Azure: BlobServiceClient(uri, credential)
Azure->>Client: Return BlobClient
end
Handler->>Client: Return configured BlobClient
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
Frends.AzureBlobStorage.WriteBlob/Frends.AzureBlobStorage.WriteBlob/Definitions/Enums.cs (1)
40-43:⚠️ Potential issue | 🟡 MinorTypo in XML documentation.
"connectiong string" should be "connection string".
📝 Proposed fix
/// <summary> - /// Authenticate with connectiong string. + /// Authenticate with connection string. /// </summary>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frends.AzureBlobStorage.WriteBlob/Frends.AzureBlobStorage.WriteBlob/Definitions/Enums.cs` around lines 40 - 43, Fix the typo in the XML doc comment for the enum member ConnectionString: update the <summary> text from "Authenticate with connectiong string." to "Authenticate with connection string." so the documentation on the ConnectionString enum entry is correct.Frends.AzureBlobStorage.ListBlobsInContainer/Frends.AzureBlobStorage.ListBlobsInContainer/Definitions/Enums.cs (1)
9-9:⚠️ Potential issue | 🟡 MinorTypo in XML documentation: "connectiong" should be "connection".
The XML doc comment has a typo that will appear in IntelliSense and generated documentation.
📝 Proposed fix
/// <summary> - /// Authenticate with connectiong string. + /// Authenticate with connection string. /// </summary>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frends.AzureBlobStorage.ListBlobsInContainer/Frends.AzureBlobStorage.ListBlobsInContainer/Definitions/Enums.cs` at line 9, Fix the typo in the XML documentation comment that currently reads "Authenticate with connectiong string." — change "connectiong" to "connection" so the comment reads "Authenticate with connection string." Locate the XML doc on the enum/enum member declared in Enums.cs (the comment above the authentication option) and update the text to correct spelling for IntelliSense and generated docs.Frends.AzureBlobStorage.DeleteContainer/Frends.AzureBlobStorage.DeleteContainer/DeleteContainer.cs (1)
33-36:⚠️ Potential issue | 🟡 MinorAvoid duplicate
ExistsAsynccalls.
container.ExistsAsync()is called twice in sequence (lines 33 and 35), resulting in two network round-trips. Cache the result to improve performance.⚡ Proposed fix
var container = ConnectionHandler.GetBlobContainerClient(input, connection, cancellationToken); - if (!await container.ExistsAsync(cancellationToken) && !options.ThrowErrorIfContainerDoesNotExists) - return new Result(false, "Container not found."); - if (!await container.ExistsAsync(cancellationToken) && options.ThrowErrorIfContainerDoesNotExists) - throw new Exception("DeleteContainer error: Container not found."); + var exists = await container.ExistsAsync(cancellationToken); + if (!exists) + { + if (options.ThrowErrorIfContainerDoesNotExists) + throw new Exception("DeleteContainer error: Container not found."); + return new Result(false, "Container not found."); + } var result = await container.DeleteIfExistsAsync(null, cancellationToken);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frends.AzureBlobStorage.DeleteContainer/Frends.AzureBlobStorage.DeleteContainer/DeleteContainer.cs` around lines 33 - 36, The code calls container.ExistsAsync(cancellationToken) twice causing two network calls; call it once, store the boolean (e.g., var exists = await container.ExistsAsync(cancellationToken)) and then use that variable in the two branches that currently check ExistsAsync, keeping the existing behavior with options.ThrowErrorIfContainerDoesNotExists and returning Result(false, "Container not found.") or throwing the exception as before.Frends.AzureBlobStorage.DeleteBlob/Frends.AzureBlobStorage.DeleteBlob/Helpers/ConnectionHandler.cs (1)
35-35:⚠️ Potential issue | 🟡 MinorError message references wrong method name.
The error message says "GetBlobContainerClient error" but the method is named
GetBlobClient. This is misleading for debugging.🐛 Proposed fix
- throw new ArgumentException($"GetBlobContainerClient error: {ex.Message}", ex); + throw new ArgumentException($"GetBlobClient error: {ex.Message}", ex);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frends.AzureBlobStorage.DeleteBlob/Frends.AzureBlobStorage.DeleteBlob/Helpers/ConnectionHandler.cs` at line 35, The exception message in the GetBlobClient method incorrectly references "GetBlobContainerClient"; update the thrown ArgumentException so the message names the correct method (e.g., "GetBlobClient error") and still includes the original exception (ex) so stack/details are preserved; locate the throw in GetBlobClient and change the message text accordingly.Frends.AzureBlobStorage.ReadBlob/Frends.AzureBlobStorage.ReadBlob/Definitions/Enums.cs (1)
21-24:⚠️ Potential issue | 🟡 MinorTypo in XML documentation.
Line 22 has a typo: "connectiong string" should be "connection string".
📝 Suggested fix
/// <summary> - /// Authenticate with connectiong string. + /// Authenticate with connection string. /// </summary>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frends.AzureBlobStorage.ReadBlob/Frends.AzureBlobStorage.ReadBlob/Definitions/Enums.cs` around lines 21 - 24, Fix the typo in the XML documentation for the enum member ConnectionString in Enums.cs: change the summary text from "Authenticate with connectiong string." to "Authenticate with connection string." so the XML comment correctly describes the ConnectionString enum value.Frends.AzureBlobStorage.ReadBlob/Frends.AzureBlobStorage.ReadBlob/ReadBlob.cs (1)
37-49:⚠️ Potential issue | 🟠 MajorThe encoding conversion logic incorrectly reinterprets bytes instead of transcoding.
The
SetStringEncodingmethod converts the input text to UTF-8 bytes, then reinterprets those same bytes as a different encoding format. This doesn't perform proper encoding conversion—it will produce garbled output for any content with multi-byte character sequences.For example, with
Unicode(UTF-16) encoding, UTF-8 bytes likeC3 A9(representing "é") will be incorrectly interpreted as UTF-16, yielding wrong characters. The existing test passes only because it uses pure ASCII text, where UTF-8 and ASCII byte values are identical. Non-ASCII content will fail.To fix this properly, determine the blob's original encoding before byte reinterpretation, or use the raw binary data and decode with the appropriate
Encodingobject directly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frends.AzureBlobStorage.ReadBlob/Frends.AzureBlobStorage.ReadBlob/ReadBlob.cs` around lines 37 - 49, SetStringEncoding currently encodes the input string to UTF8 bytes and then misinterprets those bytes as other encodings; instead determine the target Encoding based on the Encode enum (e.g. map Encode.UTF8 → Encoding.UTF8, Encode.Unicode → Encoding.Unicode, etc.) and use that Encoding to do the proper conversion: if you only have a .NET string (UTF‑16) get the target bytes via selectedEncoding.GetBytes(text) and then decode them back with selectedEncoding.GetString(bytes) (or better, if you actually have the raw blob bytes, remove the UTF8.GetBytes step and simply call selectedEncoding.GetString(blobBytes)); update SetStringEncoding and any callers accordingly so bytes are decoded using the correct Encoding instance rather than always starting from UTF8 bytes.
🟠 Major comments (27)
Frends.AzureBlobStorage.UploadBlob/CHANGELOG.md-20-23 (1)
20-23:⚠️ Potential issue | 🟠 MajorAdd explicit upgrade notes for the breaking change.
Line 22 marks a breaking change, but it doesn’t tell users what action is required to upgrade safely.
Proposed changelog patch
### Changed - [Breaking] Updated dotnet SDK to version 8. + - Upgrade notes: + - Build and run this task with .NET 8 SDK/runtime. + - Ensure your CI agents and local environments are updated accordingly.As per coding guidelines, "Include all functional changes and indicate breaking changes with upgrade notes."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frends.AzureBlobStorage.UploadBlob/CHANGELOG.md` around lines 20 - 23, The changelog entry "- [Breaking] Updated dotnet SDK to version 8." is marked as breaking but lacks upgrade guidance; update CHANGELOG.md to include explicit upgrade notes describing required actions (e.g., update project TargetFramework/SDK to net8.0, bump any dependent NuGet package minimum versions, handle obsolete/removed APIs or runtime behavior changes, and any config/build adjustments) and provide a brief migration example or links to Microsoft .NET 8 migration docs; locate the entry string "- [Breaking] Updated dotnet SDK to version 8." and append a short "Upgrade notes" section with concrete steps and references.Frends.AzureBlobStorage.WriteBlob/Frends.AzureBlobStorage.WriteBlob.Tests/UnitTestsBase.cs-278-395 (1)
278-395:⚠️ Potential issue | 🟠 MajorRemove unused helper methods from UnitTestsBase.cs.
CreateWorkingDirectory,DeleteWorkingDirectory,CreateAndUploadTestFiles,CreateFiles, andUploadTestFilesare defined but never invoked anywhere in this test project. Remove them.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frends.AzureBlobStorage.WriteBlob/Frends.AzureBlobStorage.WriteBlob.Tests/UnitTestsBase.cs` around lines 278 - 395, Remove the unused helper methods from UnitTestsBase.cs: delete the entire implementations of CreateWorkingDirectory(), DeleteWorkingDirectory(), CreateAndUploadTestFiles(bool), CreateFiles(bool), and UploadTestFiles(FileInfo, string). Ensure you remove only these method definitions (including their bodies and any private fields that become unused as a result), and run tests/compile to confirm no references remain to the methods CreateWorkingDirectory, DeleteWorkingDirectory, CreateAndUploadTestFiles, CreateFiles, or UploadTestFiles.Pack-AllProjects.ps1-98-114 (1)
98-114:⚠️ Potential issue | 🟠 MajorDon't build a bumped version after the regex misses.
The
elsebranch only warns, but Line 109 still constructs$packedVersion. WithSet-StrictMode -Version Latest, the first non-matching version throws; after one successful iteration, later misses can also reuse stale$major/$minor/$patchvalues and write the wrong version back to disk.🐛 Proposed fix
if ($BumpVersion) { # --- Parse version (supports optional pre-release suffix, e.g. "1.2.3-beta") --- if ($currentVersion -match '^(\d+)\.(\d+)\.(\d+)(.*)$') { $major = [int] $Matches[1] $minor = [int] $Matches[2] $patch = [int] $Matches[3] $prerelease = $Matches[4] # e.g. "-beta" or "" + $packedVersion = "$major.$minor.$($patch + 1)$prerelease" + Write-Host " Bumped : $packedVersion" -ForegroundColor Green + + # --- Write bumped version back to project file --- + $versionNode.InnerText = $packedVersion + $xml.Save($proj.FullName) } else { - Write-Warning " Version '$currentVersion' does not match expected format (MAJOR.MINOR.PATCH[suffix]) � skipping version bump." + Write-Warning " Version '$currentVersion' does not match expected format (MAJOR.MINOR.PATCH[suffix]) - packing without a bump." } - - $packedVersion = "$major.$minor.$($patch + 1)$prerelease" - Write-Host " Bumped : $packedVersion" -ForegroundColor Green - - # --- Write bumped version back to project file --- - $versionNode.InnerText = $packedVersion - $xml.Save($proj.FullName) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Pack-AllProjects.ps1` around lines 98 - 114, When bumping versions (controlled by $BumpVersion) ensure you do not compute or write $packedVersion when the regex match fails; move the construction of $packedVersion, the Write-Host bump log, and the update to $versionNode.InnerText / $xml.Save($proj.FullName) inside the successful if ($currentVersion -match ...) block or add an explicit continue/skip in the else branch so stale $major/$minor/$patch values are never reused — update code around the regex match that sets $major, $minor, $patch, $prerelease and only compute and persist $packedVersion when those match variables were newly assigned.Pack-AllProjects.ps1-117-125 (1)
117-125:⚠️ Potential issue | 🟠 MajorRemove the unconditional
--no-restoreflag.The script is documented as a standalone tool for producing Release NuGet packages but does not perform any dependency restoration before packing. On a clean checkout,
dotnet pack --no-restorewill fail because dependencies are not available. Removing--no-restoreallows dotnet to restore dependencies automatically during the pack operation.Minimal fix
$packArgs = @( "pack" $proj.FullName "--configuration", "Release" "--output", $OutputDir - "--no-restore" )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Pack-AllProjects.ps1` around lines 117 - 125, The pack invocation is forcing --no-restore which causes dotnet pack to fail on clean checkouts; remove the "--no-restore" element from the $packArgs array so that the pack step can perform an implicit restore (i.e., update the $packArgs construction around "pack" / $proj.FullName / "--configuration", "Release", "--output", $OutputDir to omit "--no-restore").Pack-AllProjects.ps1-37-39 (1)
37-39:⚠️ Potential issue | 🟠 MajorUse a platform-neutral default output directory.
Line 38 hardcodes a Windows drive, so the script fails on PowerShell 7/macOS/Linux even though the documentation states "PowerShell 5.1 or later". Defaulting to the system temp directory preserves current Windows behavior while avoiding immediate startup failures on other platforms.
💡 Minimal fix
- [string] $OutputDir = "C:\Temp\AzureBlob", + [string] $OutputDir = (Join-Path ([System.IO.Path]::GetTempPath()) "AzureBlob"),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Pack-AllProjects.ps1` around lines 37 - 39, The default value for the parameter $OutputDir is hardcoded to "C:\Temp\AzureBlob", which breaks on non-Windows platforms; change the default to a platform-neutral temp path by replacing that literal with a runtime call such as [System.IO.Path]::Combine([System.IO.Path]::GetTempPath(), 'AzureBlob') (or using Join-Path with [System.IO.Path]::GetTempPath()) so $OutputDir uses the system temp directory across OSes while preserving the "AzureBlob" subfolder.Frends.AzureBlobStorage.CreateContainer/Frends.AzureBlobStorage.CreateContainer.Tests/DefaultManagedIDentityTests.cs-11-22 (1)
11-22:⚠️ Potential issue | 🟠 MajorMove the URI read below
await InitBase()and fix class name typo.Line 11 captures
Frends_AzureBlobStorage_Uribefore base setup runs, making tests sensitive to initialization order and masking missing-secret failures as blob-client errors. Additionally, the class name contains a typo:DefaultManagedIDentityTestsshould beDefaultManagedIdentityTests(matching Microsoft C# naming conventions for abbreviations).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frends.AzureBlobStorage.CreateContainer/Frends.AzureBlobStorage.CreateContainer.Tests/DefaultManagedIDentityTests.cs` around lines 11 - 22, The test reads the environment variable into the field _uri before base setup, and the class name has a typo; move the Environment.GetEnvironmentVariable("Frends_AzureBlobStorage_Uri") call out of the field initializer and into the Init() method after the await InitBase() call (assign to _uri there), and rename the test class from DefaultManagedIDentityTests to DefaultManagedIdentityTests to fix the casing/typo; update any references to the class name accordingly.Frends.AzureBlobStorage.DeleteBlob/Frends.AzureBlobStorage.DeleteBlob.Tests/DefaultManagedIDentityTests.cs-15-27 (1)
15-27:⚠️ Potential issue | 🟠 MajorMove the URI lookup into
Init().Field initializers execute before
[TestInitialize]in MSTest. The_urifield is captured beforeInit()runs, so if environment variables are not set at test start, the value will be null and errors will surface later during blob operations rather than at setup.Move the environment variable lookup into the
Init()method and add an assertion to fail fast with a clear message if the variable is not set. This is also required by the coding guidelines to load secrets via dotenv (which should be implemented before test initialization runs).Suggested fix
- private readonly string _uri = Environment.GetEnvironmentVariable("Frends_AzureBlobStorage_Uri"); - [TestInitialize] public async Task Init() { await InitBase(); + string uri = Environment.GetEnvironmentVariable("Frends_AzureBlobStorage_Uri"); + Assert.IsFalse(string.IsNullOrWhiteSpace(uri), "Set Frends_AzureBlobStorage_Uri before running this suite."); _connection = new Connection { AuthenticationMethod = AuthenticationMethod.DefaultManagedIdentity, - Uri = _uri, + Uri = uri, ContainerName = _containerName }; }This pattern affects other test classes (OAuthUnitTests, SASUnitTests) and should be corrected consistently.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frends.AzureBlobStorage.DeleteBlob/Frends.AzureBlobStorage.DeleteBlob.Tests/DefaultManagedIDentityTests.cs` around lines 15 - 27, The _uri field is currently initialized from Environment.GetEnvironmentVariable too early; move the environment variable lookup into the Init() method so the value is read after dotenv/setup runs, then set _connection.Uri from that local value; inside Init() assert/throw with a clear message if the environment variable is null or empty to fail fast. Update the Init() method that constructs the Connection (AuthenticationMethod.DefaultManagedIdentity, ContainerName, etc.) to use the newly-read uri and apply the same change to the other test classes (OAuthUnitTests, SASUnitTests) that currently initialize _uri as a field.Frends.AzureBlobStorage.ListBlobsInContainer/Frends.AzureBlobStorage.ListBlobsInContainer/ListBlobsInContainer.cs-56-63 (1)
56-63:⚠️ Potential issue | 🟠 MajorEscape blob names before returning
URL.Interpolating
/{blobName}returns invalid URLs for legal blob names that contain spaces or other reserved characters. Please build the blob URI through an API/helper that encodes the name correctly, and apply the same fix in the hierarchical blob path as well.Also applies to: 82-86
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frends.AzureBlobStorage.ListBlobsInContainer/Frends.AzureBlobStorage.ListBlobsInContainer/ListBlobsInContainer.cs` around lines 56 - 63, The URL property is built by string-interpolating blobContainerClient.Uri and blobItem.Name which produces invalid URLs for blob names with spaces or reserved characters; instead construct the blob's URI via the client so the name is encoded (for example use blobContainerClient.GetBlobClient(blobItem.Name).Uri or an equivalent helper) and replace the string interpolation in the BlobData creation (the URL assignment) and the analogous hierarchical-blob-path construction later in the file to use the encoded Uri from the blob client rather than concatenating the raw blobItem.Name.Frends.AzureBlobStorage.DeleteContainer/Frends.AzureBlobStorage.DeleteContainer/Helpers/ConnectionHandler.cs-53-60 (1)
53-60:⚠️ Potential issue | 🟠 MajorFail fast on missing OAuth/managed-identity inputs.
Only the connection-string, SAS, and default-managed-identity branches validate their required fields. These branches still assume
connection.Uriand the auth-specific identifiers/scopes are present, so invalid configuration now fails later with generic URI or credential errors instead of a clear task-level validation message.Also applies to: 63-69, 74-94
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frends.AzureBlobStorage.DeleteContainer/Frends.AzureBlobStorage.DeleteContainer/Helpers/ConnectionHandler.cs` around lines 53 - 60, Add explicit precondition checks that fail fast with clear ArgumentException messages for missing required fields before constructing credentials or URIs: in GetClientWithOAuth2 validate connection.Uri, connection.TenantId, connection.ApplicationId and connection.ClientSecret and throw a descriptive error if any are null/empty; do the same for the OAuth/managed-identity branches (e.g., the methods handling managed identity and default managed identity) by validating connection.Uri and whichever identity-specific fields or scopes they require (for user-assigned managed identity validate the clientId/identityId, for default managed identity validate any required scopes) and throw clear exceptions instead of letting URI/credential construction raise generic errors. Ensure checks occur at the top of each method (GetClientWithOAuth2 and the managed-identity client factory methods) so callers get task-level validation messages.Frends.AzureBlobStorage.CreateContainer/Frends.AzureBlobStorage.CreateContainer/Helpers/ConnectionHandler.cs-53-60 (1)
53-60:⚠️ Potential issue | 🟠 MajorFail fast on missing OAuth/managed-identity inputs.
Only the connection-string, SAS, and default-managed-identity branches validate their required fields. These three branches dereference
connection.Uri/ tenant / client / scopes directly, so a misconfigured task now fails later with generic URI or credential errors instead of a clear task-level validation message.Also applies to: 63-69, 72-90
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frends.AzureBlobStorage.CreateContainer/Frends.AzureBlobStorage.CreateContainer/Helpers/ConnectionHandler.cs` around lines 53 - 60, Validate required connection fields at the start of each client-creation method so misconfiguration fails fast: in GetClientWithOAuth2 verify connection.Uri, connection.TenantId, connection.ApplicationId and connection.ClientSecret are non-empty and throw a clear ArgumentException; likewise add preflight checks in GetClientWithManagedIdentity (validate connection.Uri and connection.ClientId or managed-identity indicator), GetClientWithSas (validate connection.Uri and connection.SasToken), and GetClientWithConnectionString (validate connection.ConnectionString) and throw descriptive exceptions before constructing Uri/credentials so errors surface as task-level validation messages.Frends.AzureBlobStorage.ListBlobsInContainer/Frends.AzureBlobStorage.ListBlobsInContainer/ListBlobsInContainer.cs-70-72 (1)
70-72:⚠️ Potential issue | 🟠 MajorDon't silently ignore
options.Prefixin the tag-query path.The default flat branch honors
options.Prefix, but this branch drops it entirely and can return blobs outside the caller's requested subtree. If prefix filtering is unsupported with tag queries, fail fast instead of broadening the result set.Suggested guard
else { + if (!string.IsNullOrWhiteSpace(options.Prefix)) + throw new ArgumentException("Prefix is not supported when listing blobs by tags."); + var flatEnumerator = blobContainerClient.FindBlobsByTagsAsync(options.TagQuery).AsPages().GetAsyncEnumerator(cancellationToken);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frends.AzureBlobStorage.ListBlobsInContainer/Frends.AzureBlobStorage.ListBlobsInContainer/ListBlobsInContainer.cs` around lines 70 - 72, The tag-query branch currently constructs flatEnumerator via blobContainerClient.FindBlobsByTagsAsync(options.TagQuery) while ignoring options.Prefix; add a guard in the same method (the ListBlobsInContainer logic around the flatEnumerator creation) to fail fast when a prefix is supplied: check if !string.IsNullOrEmpty(options.Prefix) and throw a clear ArgumentException/InvalidOperationException indicating that prefix filtering is not supported with FindBlobsByTagsAsync (or alternatively apply post-enumeration filtering if you prefer to support it), referencing options.Prefix, FindBlobsByTagsAsync and the flatEnumerator setup so reviewers can locate and update the correct branch.Frends.AzureBlobStorage.CreateContainer/Frends.AzureBlobStorage.CreateContainer/Helpers/ConnectionHandler.cs-93-100 (1)
93-100:⚠️ Potential issue | 🟠 MajorUse
ManagedIdentityCredentialor rename the method to reflectDefaultAzureCredentialusage.The method name
GetClientWithDefaultManagedIdentityimplies managed-identity-only authentication, butDefaultAzureCredentialchains multiple credential sources (environment variables, CLI, IDE tools, etc.) before falling back to managed identity. This can succeed with development credentials when run outside Azure, potentially allowing authentication under the wrong principal. Either replace withManagedIdentityCredentialfor true managed-identity-only authentication, or rename the method and its enum variant to reflect thatDefaultAzureCredentialis being used.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frends.AzureBlobStorage.CreateContainer/Frends.AzureBlobStorage.CreateContainer/Helpers/ConnectionHandler.cs` around lines 93 - 100, The method GetClientWithDefaultManagedIdentity currently constructs a BlobContainerClient with DefaultAzureCredential which is not managed-identity-only; either replace DefaultAzureCredential with ManagedIdentityCredential to enforce managed identity authentication in GetClientWithDefaultManagedIdentity (and adjust any enum/value that selects this path), or rename the method and its related enum/variant to reflect that it uses DefaultAzureCredential (e.g., GetClientWithDefaultAzureCredential) and update all call sites and tests accordingly so the method name and chosen credential type match.Frends.AzureBlobStorage.DeleteContainer/Frends.AzureBlobStorage.DeleteContainer/Helpers/ConnectionHandler.cs-97-104 (1)
97-104:⚠️ Potential issue | 🟠 MajorRename the enum and documentation or use
ManagedIdentityCredentialif identity-only authentication is required.The enum value
DefaultManagedIdentityand its documentation ("Default Managed Identity") suggest managed-identity-only authentication, but the implementation usesDefaultAzureCredential(), which attempts multiple credential sources (environment variables, Azure CLI, IDE credentials, etc.) before managed identity. This mismatch can cause authentication to succeed under an unintended principal. If the intent is managed-identity-only, useManagedIdentityCredential()instead; otherwise, rename the enum value and update its documentation to reflect the actualDefaultAzureCredentialbehavior.This pattern appears across multiple tasks in the codebase.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frends.AzureBlobStorage.DeleteContainer/Frends.AzureBlobStorage.DeleteContainer/Helpers/ConnectionHandler.cs` around lines 97 - 104, The enum value DefaultManagedIdentity and its docs imply managed-identity-only auth but GetClientWithDefaultManagedIdentity uses DefaultAzureCredential (which tries many credentials); either change the code to use ManagedIdentityCredential for managed-identity-only behavior or rename the enum and update its XML/docs to reflect DefaultAzureCredential semantics; update all occurrences (e.g., GetClientWithDefaultManagedIdentity and any enum definitions referencing DefaultManagedIdentity) so the name, documentation, and actual credential type are consistent across tasks.Frends.AzureBlobStorage.ReadBlob/Frends.AzureBlobStorage.ReadBlob/Helpers/ConnectionHandler.cs-17-35 (1)
17-35:⚠️ Potential issue | 🟠 MajorDon't map every runtime failure to
ArgumentException.This catch turns cancellation, auth, and storage errors into bad-input errors, which is misleading for callers and breaks normal cancellation flow. Let Azure/runtime exceptions bubble, and throw
ArgumentExceptiondirectly from the validation branches.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frends.AzureBlobStorage.ReadBlob/Frends.AzureBlobStorage.ReadBlob/Helpers/ConnectionHandler.cs` around lines 17 - 35, The current GetBlobClient method catches all exceptions and wraps them into an ArgumentException, which masks cancellations and runtime/auth errors; remove the broad try/catch (or at least stop catching Exception) and instead perform input validation inside the individual authentication branches (e.g., GetClientWithConnectionString, GetClientWithSasToken, GetClientWithOAuth2, GetClientWithArcManagedIdentity, GetClientWithArcManagedIdentityCrossTenant, GetClientWithDefaultManagedIdentity) and throw ArgumentException directly from those validation checks; allow OperationCanceledException and Azure/runtime exceptions to bubble up unchanged so cancellation and auth/storage errors propagate normally.Frends.AzureBlobStorage.ReadBlob/Frends.AzureBlobStorage.ReadBlob.Tests/UnitTestsBase.cs-16-17 (1)
16-17:⚠️ Potential issue | 🟠 MajorDon't force live Azure setup for every derived test.
InitBase()always provisions a container and uploads files, and_connstringis read directly from process env at field initialization. That makes the new auth-validation tests fail before they hit the code under test when secrets/Azure aren't available, and there's no dotenv bootstrap here for local runs.As per coding guidelines,
Frends.*/Frends.*.Tests/*: "Tests should: Load secrets via dotenv" and "Use mocking where real systems can't be simulated".Also applies to: 36-46
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frends.AzureBlobStorage.ReadBlob/Frends.AzureBlobStorage.ReadBlob.Tests/UnitTestsBase.cs` around lines 16 - 17, InitBase() and the field initializers (_connstring and _containerName) force live Azure access at test class construction time; change _connstring to be loaded lazily in the test setup (not at field initialization) using dotenv (or Environment.GetEnvironmentVariable inside InitBase/Setup) and make InitBase() skip provisioning/uploading when the connection string is null/empty so tests that don't have Azure secrets won't attempt to create containers; for tests that require Azure, explicitly require a test fixture or an environment marker and for others use mocking (replace real Azure client creation in InitBase()/any methods calling BlobContainerClient/Upload with a mocked client) so derived tests no longer depend on a live Azure setup.Frends.AzureBlobStorage.ReadBlob/Frends.AzureBlobStorage.ReadBlob/Helpers/ConnectionHandler.cs-59-69 (1)
59-69:⚠️ Potential issue | 🟠 MajorValidate
connection.Uriin the OAuth2 branch.Every non-connection-string branch needs the base URI, but this one only checks the client credentials and then calls
GetUri(connection.Uri, ...). With an empty URI the user gets a generic URI-format failure instead of the intended validation message.💡 Possible fix
- if (string.IsNullOrWhiteSpace(connection.ApplicationId) || string.IsNullOrWhiteSpace(connection.TenantId) || string.IsNullOrWhiteSpace(connection.ClientSecret)) + if (string.IsNullOrWhiteSpace(connection.Uri) || + string.IsNullOrWhiteSpace(connection.ApplicationId) || + string.IsNullOrWhiteSpace(connection.TenantId) || + string.IsNullOrWhiteSpace(connection.ClientSecret)) { - throw new Exception("Application ID, Tenant ID and Client Secret required."); + throw new ArgumentException("URI, Application ID, Tenant ID and Client Secret required."); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frends.AzureBlobStorage.ReadBlob/Frends.AzureBlobStorage.ReadBlob/Helpers/ConnectionHandler.cs` around lines 59 - 69, In GetClientWithOAuth2 validate connection.Uri (and container/name if desired) before creating ClientSecretCredential: check string.IsNullOrWhiteSpace(connection.Uri) and throw a clear Exception (e.g. "Base URI is required for OAuth2 authentication.") if missing, then call GetUri(...) and construct BlobClient as before; ensure the validation happens alongside the existing ApplicationId/TenantId/ClientSecret checks in GetClientWithOAuth2 to surface a helpful error instead of a generic URI-format failure.Frends.AzureBlobStorage.ReadBlob/Frends.AzureBlobStorage.ReadBlob/Helpers/ConnectionHandler.cs-14-15 (1)
14-15:⚠️ Potential issue | 🟠 MajorEscape blob path segments in URI construction.
GetUriconcatenatesuri,containerName, andblobNamewithout encoding. Blob names containing reserved URL characters (such as#,?,%, or spaces) must be percent-encoded in the URI to prevent incorrect blob targeting or request failures. UseUri.EscapeDataString()to encode the blob name before constructing the URI, or refactor to use theBlobClient(string connectionString, string containerName, string blobName)constructor (as used inGetClientWithConnectionString) which handles encoding automatically. This issue exists across multiple task files.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frends.AzureBlobStorage.ReadBlob/Frends.AzureBlobStorage.ReadBlob/Helpers/ConnectionHandler.cs` around lines 14 - 15, GetUri currently concatenates uri, containerName, and blobName without encoding causing failures for reserved characters; update GetUri (or callers) to either percent-encode path segments using Uri.EscapeDataString for containerName and blobName before constructing the Uri, or refactor to use the BlobClient(string connectionString, string containerName, string blobName) constructor (as in GetClientWithConnectionString) which handles encoding automatically; ensure the chosen fix is applied consistently wherever GetUri or manual URI construction is used.Frends.AzureBlobStorage.ReadBlob/Frends.AzureBlobStorage.ReadBlob/Helpers/ConnectionHandler.cs-110-119 (1)
110-119:⚠️ Potential issue | 🟠 MajorUse
ManagedIdentityCredentialinstead ofDefaultAzureCredentialto ensure the method authenticates exclusively via managed identity.
DefaultAzureCredentialchains multiple authentication sources (environment variables, Azure CLI, Visual Studio, Azure PowerShell, Azure Developer CLI, etc.), which contradicts the method's intent. According to Azure best practices, when you specifically want managed identity authentication to avoid unexpected fallback credentials, useManagedIdentityCredentialdirectly. This is inconsistent with other managed identity methods in this file (GetClientWithArcManagedIdentityandGetClientWithArcManagedIdentityCrossTenant), which already useManagedIdentityCredential.Suggested fix
- return new BlobClient(GetUri(connection.Uri, connection.ContainerName, input.BlobName), new DefaultAzureCredential()); + return new BlobClient( + GetUri(connection.Uri, connection.ContainerName, input.BlobName), + new ManagedIdentityCredential());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frends.AzureBlobStorage.ReadBlob/Frends.AzureBlobStorage.ReadBlob/Helpers/ConnectionHandler.cs` around lines 110 - 119, The GetClientWithDefaultManagedIdentity method currently constructs the BlobClient with DefaultAzureCredential; change it to use ManagedIdentityCredential so authentication is strictly via managed identity: replace the new DefaultAzureCredential() argument with a new ManagedIdentityCredential() (and add/import Azure.Identity if not already present) so the BlobClient returned by GetClientWithDefaultManagedIdentity uses ManagedIdentityCredential like GetClientWithArcManagedIdentity and GetClientWithArcManagedIdentityCrossTenant.Frends.AzureBlobStorage.ReadBlob/Frends.AzureBlobStorage.ReadBlob.Tests/UnitTestsBase.cs-20-28 (1)
20-28:⚠️ Potential issue | 🟠 MajorNormalize blob names to forward slash (/) for Azure Blob Storage hierarchical paths.
Azure Blob Storage requires
/as the path delimiter for hierarchical semantics. Currently,Path.GetRelativePath()produces platform-specific separators (backslashes on Windows, forward slashes on Unix), and hard-coded\\entries in_testFileswon't work on Unix systems. The blob names must be normalized before upload, and the folder detection check at line 208 will fail on non-Windows platforms.Suggested fix
private readonly List<string> _testFiles = [ "TestFile1.txt", "TestFile2.txt", - "Temp\\SubFolderFile1.txt", - "Temp\\SubFolderFile2.txt", - "512byte\\TestFile1.txt", - "512byte\\TestFile2.txt" + Path.Combine("Temp", "SubFolderFile1.txt"), + Path.Combine("Temp", "SubFolderFile2.txt"), + Path.Combine("512byte", "TestFile1.txt"), + Path.Combine("512byte", "TestFile2.txt") ]; ... - var relativePath = Path.GetRelativePath(_testFileDir, file); + var relativePath = Path.GetRelativePath(_testFileDir, file) + .Replace(Path.DirectorySeparatorChar, '/') + .Replace(Path.AltDirectorySeparatorChar, '/'); await UploadTestFiles(new FileInfo(file), relativePath); ... - if (blobName.Contains("Temp\\")) + if (blobName.StartsWith("Temp/", StringComparison.Ordinal)) { tags["folder"] = "temp"; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frends.AzureBlobStorage.ReadBlob/Frends.AzureBlobStorage.ReadBlob.Tests/UnitTestsBase.cs` around lines 20 - 28, The _testFiles entries and any blob name derivation using Path.GetRelativePath() in UnitTestsBase need normalization to use forward slashes; update the code that builds blob names (including the _testFiles list usage and the logic that detects folders) to replace OS-specific path separators with '/' before uploading or asserting folder detection, ensuring methods like the blob upload helper and the folder-detection check use normalizedBlobName = rawPath.Replace(Path.DirectorySeparatorChar, '/'). Replace hard-coded backslashes in the _testFiles definitions or normalize them at construction so tests run correctly on non-Windows platforms.Frends.AzureBlobStorage.UploadBlob/Frends.AzureBlobStorage.UploadBlob.Tests/UnitTestsBase.cs-421-440 (1)
421-440:⚠️ Potential issue | 🟠 MajorThis encoding matrix stays green even if encoding selection breaks.
CreateFiles()writes this fixture withCreateText()and only ASCII content. With that input, UTF-8, ASCII, Windows-1252, andwindows-1251all round-trip identically, so this loop does not actually validate the new encoding paths. Use non-ASCII fixtures or explicit byte payloads per encoding.Also applies to: 761-763
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frends.AzureBlobStorage.UploadBlob/Frends.AzureBlobStorage.UploadBlob.Tests/UnitTestsBase.cs` around lines 421 - 440, The test loop over encodings is ineffective because CreateFiles()/CreateText() writes only ASCII so different encodings round-trip the same; update the test to produce non-ASCII test data per encoding instead of relying on CreateText(): for each FileEncoding in encodings (the list using FileEncoding.UTF8, Default, ASCII, Windows1252, Other) generate the input file bytes using the corresponding System.Text.Encoding (or explicit byte arrays for windows-1251/1252) and write those bytes to input.SourceFile before calling AzureBlobStorage.UploadBlob, then compute the expected payload as the same byte sequence (not File.ReadAllText) and use BlobExists/Assert to compare the uploaded bytes against that expected payload; alternatively change CreateFiles() to write a non-ASCII fixture (e.g., accented/Cyrillic characters) so the matrix actually validates different encodings.Frends.AzureBlobStorage.UploadBlob/Frends.AzureBlobStorage.UploadBlob.Tests/UnitTestsBase.cs-20-24 (1)
20-24:⚠️ Potential issue | 🟠 MajorLoad
.envbefore these readonly fields capture the test secrets.These values are read when the test instance is constructed, and the new auth-specific test classes follow the same pattern for
_uri/_sasToken. That makes the suite depend on machine-level environment setup and means a later dotenv load would not affect the captured values.As per coding guidelines, 'Tests should: Load secrets via dotenv'.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frends.AzureBlobStorage.UploadBlob/Frends.AzureBlobStorage.UploadBlob.Tests/UnitTestsBase.cs` around lines 20 - 24, The readonly fields _connstring, _containerName and _workingDir in UnitTestsBase are initialized at construction time before dotenv is loaded, so move dotenv loading earlier and ensure these fields capture env values after dotenv; specifically, load the .env (e.g., call your dotenv loader) in a static constructor or at top of UnitTestsBase before any field initializers, or change the fields to be initialized inside the instance constructor after dotenv is loaded (so _connstring and any _uri/_sasToken siblings pick up values from dotenv). Ensure the unique symbols affected are _connstring, _containerName, _workingDir (and any auth-specific _uri/_sasToken fields) so tests use dotenv-provided secrets rather than machine-level env.Frends.AzureBlobStorage.UploadBlob/Frends.AzureBlobStorage.UploadBlob.Tests/SASUnitTests.cs-37-48 (1)
37-48:⚠️ Potential issue | 🟠 MajorUse the real fixture path in these SAS validation tests.
_testFiles[0]is only the relative name, not the file created under_testFileDir. If source validation runs before auth validation, both cases fail for the missing file instead of the empty URI / SAS token branch.Suggested change
- SourceFile = _testFiles[0], + SourceFile = Path.Combine(_testFileDir, _testFiles[0]),Also applies to: 60-71
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frends.AzureBlobStorage.UploadBlob/Frends.AzureBlobStorage.UploadBlob.Tests/SASUnitTests.cs` around lines 37 - 48, The tests are using the relative filename in SourceFile (e.g. SourceFile = _testFiles[0]) which doesn't point to the actual file in the fixture directory, causing source validation to fail before SAS/auth validation; update the Input construction(s) (where SourceFile is set) to use the real fixture path by combining the test directory with the filename (for example build the absolute path from the existing _testFileDir and _testFiles entries) so SourceFile points to the created file for both SAS validation test cases (also update the other occurrence that mirrors this pattern).Frends.AzureBlobStorage.UploadBlob/Frends.AzureBlobStorage.UploadBlob.Tests/ConnectionStringUnitTests.cs-36-45 (1)
36-45:⚠️ Potential issue | 🟠 MajorKeep the source file valid when testing an empty connection string.
This case currently uses the relative name from
_testFiles[0], so it can fail before the connection-string path is reached. Use the temp fixture path here as well.Suggested change
- SourceFile = _testFiles[0], + SourceFile = Path.Combine(_testFileDir, _testFiles[0]),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frends.AzureBlobStorage.UploadBlob/Frends.AzureBlobStorage.UploadBlob.Tests/ConnectionStringUnitTests.cs` around lines 36 - 45, The test uses a relative path _testFiles[0] for Input.SourceFile which can cause the test to fail before the connection-string validation; change Input.SourceFile to use the temp fixture path variable used by other tests (the temp fixture path variable instead of _testFiles[0]) so the source file is always valid when calling AzureBlobStorage.UploadBlob and the empty connection string path is exercised.Frends.AzureBlobStorage.UploadBlob/Frends.AzureBlobStorage.UploadBlob/Helpers/ConnectionHandler.cs-40-47 (1)
40-47:⚠️ Potential issue | 🟠 MajorFix inconsistent container name normalization across authentication branches.
The SAS Token path does not normalize the container name at all, while all other authentication methods use culture-sensitive
ToLower(). This creates auth-dependent behavior and inconsistent resource identification. UseToLowerInvariant()across all branches:
- Line 40 (ConnectionString)
- Line 53 (SASToken) – add lowercasing
- Line 70 (OAuth2)
- Line 86 (ArcManagedIdentity)
- Line 115 (ArcManagedIdentityCrossTenant)
- Line 135 (DefaultManagedIdentity)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frends.AzureBlobStorage.UploadBlob/Frends.AzureBlobStorage.UploadBlob/Helpers/ConnectionHandler.cs` around lines 40 - 47, The container name is being normalized with culture-sensitive ToLower() for the ConnectionString branch but not for the SASToken branch (and other branches); update all branches that construct a BlobContainerClient (ConnectionString, SASToken, OAuth2, ArcManagedIdentity, ArcManagedIdentityCrossTenant, DefaultManagedIdentity) to call ToLowerInvariant() on the container name when creating the BlobContainerClient (the variable/containerClient construction), keeping the existing options.BlobType switch (AzureBlobType.Append => GetAppendBlobClient, AzureBlobType.Block => GetBlobClient, AzureBlobType.Page => GetPageBlobClient) intact.Frends.AzureBlobStorage.UploadBlob/Frends.AzureBlobStorage.UploadBlob/Definitions/Enums.cs-66-69 (1)
66-69:⚠️ Potential issue | 🟠 MajorRename
SASTokentoSasTokenbefore this breaks the public API.The enum member
SASTokencontradicts the existing property namingConnection.SasTokenand violates the C# naming standard for acronyms (Csv, Url, Api). Fixing this now is far simpler than propagating the inconsistency through UI hints, documentation, and downstream code.Suggested changes
Update the enum definition:
- SASToken, + SasToken,Update enum references in
ConnectionHandler.csand test files (e.g.,SASUnitTests.cs) fromAuthenticationMethod.SASTokentoAuthenticationMethod.SasToken.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frends.AzureBlobStorage.UploadBlob/Frends.AzureBlobStorage.UploadBlob/Definitions/Enums.cs` around lines 66 - 69, Rename the enum member AuthenticationMethod.SASToken to AuthenticationMethod.SasToken to match the existing property Connection.SasToken and C# acronym casing; update every reference to the old symbol (e.g., in ConnectionHandler where AuthenticationMethod.SASToken is used and in tests such as SASUnitTests.cs) to AuthenticationMethod.SasToken so compilation and public API remain consistent.Frends.AzureBlobStorage.UploadBlob/Frends.AzureBlobStorage.UploadBlob.Tests/UnitTestsBase.cs-25-33 (1)
25-33:⚠️ Potential issue | 🟠 MajorBuild the fixture paths platform-independently.
Hard-coded
"\\"separators cause issues on Linux/macOS. When_testFilesentries are combined or whenrelativePathis computed viaPath.GetRelativePath(), the paths will use forward slashes on non-Windows platforms, breaking the folder-detection logic at line 815.Affected locations
- Lines 25-33:
_testFileslist with hard-coded backslashes- Lines 749-750:
relativePathfromPath.GetRelativePath()not normalized- Lines 815-817: Hard-coded
"Temp\\"check incompatible with forward-slash pathsUse
Path.Combine()for file paths, normalizerelativePathto forward slashes, and update the folder check to use forward slash withStringComparison.Ordinal.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frends.AzureBlobStorage.UploadBlob/Frends.AzureBlobStorage.UploadBlob.Tests/UnitTestsBase.cs` around lines 25 - 33, The test fixture paths in the _testFiles list are using hard-coded backslashes which break on non-Windows platforms; replace those literal strings by constructing entries with Path.Combine (e.g., Path.Combine("Temp","SubFolderFile1.txt") and Path.Combine("512byte","TestFile1.txt")) so paths are platform-independent, then normalize the computed relativePath (from Path.GetRelativePath()) to use forward slashes (e.g., relativePath = relativePath.Replace('\\','/')) and update the folder detection check that currently compares with "Temp\\" to instead compare with "Temp/" using StringComparison.Ordinal (or otherwise compare normalized paths) so the folder-detection logic works on all platforms.Frends.AzureBlobStorage.UploadBlob/Frends.AzureBlobStorage.UploadBlob/UploadBlob.cs-621-630 (1)
621-630:⚠️ Potential issue | 🟠 MajorTemp files created during compression/re-encoding are not cleaned up.
The helper methods
GetCompressedStream,GetReencodedStream,CreateCompressedBinaryStream, andCreateBinaryCopyStreamcreate temp files that are never deleted after the stream is disposed. This causes temp file accumulation.Consider using
FileOptions.DeleteOnCloseto automatically clean up temp files when the stream is closed.🧹 Proposed fix
private static Stream CreateUploadStream(string tempFile) { return new FileStream( tempFile, FileMode.Open, FileAccess.Read, FileShare.Read, StreamBufferSize, - FileOptions.Asynchronous | FileOptions.SequentialScan); + FileOptions.Asynchronous | FileOptions.SequentialScan | FileOptions.DeleteOnClose); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Frends.AzureBlobStorage.UploadBlob/Frends.AzureBlobStorage.UploadBlob/UploadBlob.cs` around lines 621 - 630, The temp files created by GetCompressedStream, GetReencodedStream, CreateCompressedBinaryStream, and CreateBinaryCopyStream are never deleted; update CreateUploadStream (and any other FileStream instantiations that open those temp files) to open the FileStream with FileOptions.DeleteOnClose (e.g., include FileOptions.DeleteOnClose alongside FileOptions.Asynchronous | FileOptions.SequentialScan) so the OS removes the temp file when the stream is closed, or alternatively ensure each helper disposes the stream and explicitly deletes the temp file in a finally/dispose path if DeleteOnClose is not available.
|
|
||
| Assert.IsNotNull(result); | ||
| Assert.IsTrue(result.Success); | ||
| Assert.Contains(_containerName, result.Uri); |
There was a problem hiding this comment.
Assert.Contains is not a standard MSTest assertion method.
MSTest does not provide Assert.Contains for string containment checks. This will cause a compilation error. Use StringAssert.Contains instead.
🐛 Proposed fix
- Assert.Contains(_containerName, result.Uri);
+ StringAssert.Contains(result.Uri, _containerName);Note: StringAssert.Contains(string, substring) takes the full string first, then the substring to find.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Assert.Contains(_containerName, result.Uri); | |
| StringAssert.Contains(result.Uri, _containerName); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Frends.AzureBlobStorage.CreateContainer/Frends.AzureBlobStorage.CreateContainer.Tests/UnitTestsBase.cs`
at line 48, The test uses Assert.Contains which isn't available in MSTest;
update the assertion in
Frends.AzureBlobStorage.CreateContainer.Tests.UnitTestsBase (where
Assert.Contains(_containerName, result.Uri) appears) to use
StringAssert.Contains with the full string first and the substring second (i.e.,
pass result.Uri as the first argument and _containerName as the second) so the
test compiles and correctly checks that the container name appears in the
returned URI.
| Assert.IsFalse(result.Success); | ||
| Assert.AreEqual(string.Empty, result.Uri); | ||
| Assert.IsNotNull(result.Error); | ||
| Assert.Contains("Custom error message", result.Error.Message); |
There was a problem hiding this comment.
Assert.Contains is not a standard MSTest assertion method.
Same issue as above - use StringAssert.Contains for string containment checks.
🐛 Proposed fix
- Assert.Contains("Custom error message", result.Error.Message);
+ StringAssert.Contains(result.Error.Message, "Custom error message");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Assert.Contains("Custom error message", result.Error.Message); | |
| StringAssert.Contains(result.Error.Message, "Custom error message"); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Frends.AzureBlobStorage.CreateContainer/Frends.AzureBlobStorage.CreateContainer.Tests/UnitTestsBase.cs`
at line 91, Replace the invalid Assert.Contains call with the MSTest string
assertion: use StringAssert.Contains and pass the actual string first and the
expected substring second (i.e. change Assert.Contains("Custom error message",
result.Error.Message) to StringAssert.Contains(result.Error.Message, "Custom
error message") in the test that asserts on result.Error.Message).
|
|
||
| var result = await AzureBlobStorage.DeleteBlob(input, _connection, options, default); | ||
| Assert.IsFalse(result.Success); | ||
| Assert.Contains("doesn't exist in container", result.Info); |
There was a problem hiding this comment.
Assert.Contains is not a standard MSTest assertion method.
Use StringAssert.Contains for string containment checks.
🐛 Proposed fix
- Assert.Contains("doesn't exist in container", result.Info);
+ StringAssert.Contains(result.Info, "doesn't exist in container");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Assert.Contains("doesn't exist in container", result.Info); | |
| StringAssert.Contains(result.Info, "doesn't exist in container"); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Frends.AzureBlobStorage.DeleteBlob/Frends.AzureBlobStorage.DeleteBlob.Tests/UnitTestsBase.cs`
at line 56, The test uses Assert.Contains for a string containment check which
isn't an MSTest method; replace that call with StringAssert.Contains and keep
the same operands (e.g. StringAssert.Contains(result.Info, "doesn't exist in
container")) in Frends.AzureBlobStorage.DeleteBlob.Tests.UnitTestsBase (where
Assert.Contains is used) and ensure the MSTest namespace
(Microsoft.VisualStudio.TestTools.UnitTesting) is referenced so StringAssert is
available.
|
|
||
| var result = await AzureBlobStorage.DeleteBlob(input, _connection, new Options(), default); | ||
| Assert.IsFalse(result.Success); | ||
| Assert.Contains("doesn't exist in container", result.Info); |
There was a problem hiding this comment.
Assert.Contains is not a standard MSTest assertion method.
Use StringAssert.Contains for string containment checks.
🐛 Proposed fix
- Assert.Contains("doesn't exist in container", result.Info);
+ StringAssert.Contains(result.Info, "doesn't exist in container");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Assert.Contains("doesn't exist in container", result.Info); | |
| StringAssert.Contains(result.Info, "doesn't exist in container"); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Frends.AzureBlobStorage.DeleteBlob/Frends.AzureBlobStorage.DeleteBlob.Tests/UnitTestsBase.cs`
at line 134, The test uses Assert.Contains which isn't part of MSTest; replace
that call with MSTest's StringAssert.Contains to check that result.Info contains
the expected substring. Locate the assertion that references
Assert.Contains("doesn't exist in container", result.Info) in the test class
(UnitTestsBase or where result.Info is asserted) and change it to
StringAssert.Contains(result.Info, "doesn't exist in container") so the MSTest
framework assertion signature is used correctly.
|
|
||
| var result = await AzureBlobStorage.DeleteBlob(input, _connection, new Options(), default); | ||
| Assert.IsFalse(result.Success); | ||
| Assert.Contains("doesn't exist in container", result.Info); |
There was a problem hiding this comment.
Assert.Contains is not a standard MSTest assertion method.
Use StringAssert.Contains for string containment checks.
🐛 Proposed fix
- Assert.Contains("doesn't exist in container", result.Info);
+ StringAssert.Contains(result.Info, "doesn't exist in container");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Assert.Contains("doesn't exist in container", result.Info); | |
| StringAssert.Contains(result.Info, "doesn't exist in container"); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Frends.AzureBlobStorage.DeleteBlob/Frends.AzureBlobStorage.DeleteBlob.Tests/UnitTestsBase.cs`
at line 147, The test uses NUnit-style Assert.Contains which MSTest doesn't
provide; replace the call in UnitTestsBase (the assertion against result.Info)
with MSTest's StringAssert.Contains, i.e. call
StringAssert.Contains(result.Info, "doesn't exist in container") (ensure the
test class uses Microsoft.VisualStudio.TestTools.UnitTesting).
| // Assert | ||
| Assert.IsTrue(result.Success); | ||
| Assert.IsNotNull(result.Containers); | ||
| Assert.IsGreaterThan(0, result.Containers.Count); |
There was a problem hiding this comment.
Assert.IsGreaterThan is not a standard MSTest assertion method.
MSTest does not have Assert.IsGreaterThan. Use Assert.IsTrue with a comparison expression instead.
🐛 Proposed fix
- Assert.IsGreaterThan(0, result.Containers.Count);
+ Assert.IsTrue(result.Containers.Count > 0);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Assert.IsGreaterThan(0, result.Containers.Count); | |
| Assert.IsTrue(result.Containers.Count > 0); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Frends.AzureBlobStorage.ListContainers/Frends.AzureBlobStorage.ListContainers.Tests/UnitTestsBase.cs`
at line 68, The test uses a non-existent MSTest assertion Assert.IsGreaterThan;
update the assertion in the test that checks result.Containers.Count to use a
valid MSTest assertion such as Assert.IsTrue with an explicit comparison (e.g.,
Assert.IsTrue(result.Containers.Count > 0, "...") ) so the test compiles and
correctly verifies that result.Containers.Count is greater than zero; locate the
failing assertion (Assert.IsGreaterThan) in the UnitTestsBase test and replace
it accordingly.
| // Assert | ||
| Assert.IsTrue(result.Success); | ||
| Assert.IsNotNull(result.Containers); | ||
| Assert.HasCount(1, result.Containers); |
There was a problem hiding this comment.
Assert.HasCount is not a standard MSTest assertion method.
MSTest does not provide Assert.HasCount. Use Assert.AreEqual instead.
🐛 Proposed fix
- Assert.HasCount(1, result.Containers);
+ Assert.AreEqual(1, result.Containers.Count);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Assert.HasCount(1, result.Containers); | |
| Assert.AreEqual(1, result.Containers.Count); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Frends.AzureBlobStorage.ListContainers/Frends.AzureBlobStorage.ListContainers.Tests/UnitTestsBase.cs`
at line 89, The test uses a non-existent MSTest method Assert.HasCount on
result.Containers; replace that call with Assert.AreEqual to check the expected
count (e.g. Assert.AreEqual(expectedCount, result.Containers.Count)) so the test
uses a valid MSTest assertion and verifies the container count correctly; update
the assertion referencing result.Containers and the expected count value
accordingly.
| // Assert | ||
| Assert.IsTrue(result.Success); | ||
| Assert.IsNotNull(result.Containers); | ||
| Assert.IsNotEmpty(result.Containers); |
There was a problem hiding this comment.
Assert.IsNotEmpty is not a standard MSTest assertion method.
MSTest does not have Assert.IsNotEmpty. Use a count comparison or CollectionAssert.
🐛 Proposed fix
- Assert.IsNotEmpty(result.Containers);
+ Assert.IsTrue(result.Containers.Count > 0);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Assert.IsNotEmpty(result.Containers); | |
| Assert.IsTrue(result.Containers.Count > 0); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Frends.AzureBlobStorage.ListContainers/Frends.AzureBlobStorage.ListContainers.Tests/UnitTestsBase.cs`
at line 111, Replace the non-existent MSTest assertion Assert.IsNotEmpty on
result.Containers with a valid MSTest check: use
Assert.IsTrue(result.Containers.Count > 0, "...") or an appropriate
CollectionAssert pattern to verify result.Containers is not empty; update the
assertion in Frends.AzureBlobStorage.ListContainers.Tests/UnitTestsBase.cs where
Assert.IsNotEmpty(result.Containers) is used.
| var ex = await Assert.ThrowsAsync<Exception>(async () => | ||
| await AzureBlobStorage.WriteBlob(input, _connection, _options, TestContext.CancellationToken)); | ||
| Assert.Contains("ClientSecretCredential authentication failed", ex.Message); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find and examine the OAuthUnitTests.cs file
fd -t f "OAuthUnitTests.cs"Repository: FrendsPlatform/Frends.AzureBlobStorage
Length of output: 973
🏁 Script executed:
# Once found, let's examine the file structure and using statements
cat -n "Frends.AzureBlobStorage.WriteBlob/Frends.AzureBlobStorage.WriteBlob.Tests/OAuthUnitTests.cs" | head -60Repository: FrendsPlatform/Frends.AzureBlobStorage
Length of output: 2177
🏁 Script executed:
# Check what test framework packages are referenced in the project
fd -t f "*.csproj" | grep -i test | xargs cat | head -100Repository: FrendsPlatform/Frends.AzureBlobStorage
Length of output: 369
🏁 Script executed:
# Check the csproj file to confirm test framework version
fd -t f "*.csproj" -path "*WriteBlob*Tests*"Repository: FrendsPlatform/Frends.AzureBlobStorage
Length of output: 250
🏁 Script executed:
# Let's get the test project file content
cat "Frends.AzureBlobStorage.WriteBlob/Frends.AzureBlobStorage.WriteBlob.Tests/Frends.AzureBlobStorage.WriteBlob.Tests.csproj" 2>/dev/null || echo "File not found"Repository: FrendsPlatform/Frends.AzureBlobStorage
Length of output: 940
Incorrect MSTest assertion methods used.
Assert.ThrowsAsync and Assert.Contains are xUnit/NUnit methods, not MSTest. MSTest uses Assert.ThrowsExceptionAsync<T> and StringAssert.Contains (with reversed parameter order).
🐛 Proposed fix
- var ex = await Assert.ThrowsAsync<Exception>(async () =>
+ var ex = await Assert.ThrowsExceptionAsync<Exception>(async () =>
await AzureBlobStorage.WriteBlob(input, _connection, _options, TestContext.CancellationToken));
- Assert.Contains("ClientSecretCredential authentication failed", ex.Message);
+ StringAssert.Contains(ex.Message, "ClientSecretCredential authentication failed");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| var ex = await Assert.ThrowsAsync<Exception>(async () => | |
| await AzureBlobStorage.WriteBlob(input, _connection, _options, TestContext.CancellationToken)); | |
| Assert.Contains("ClientSecretCredential authentication failed", ex.Message); | |
| var ex = await Assert.ThrowsExceptionAsync<Exception>(async () => | |
| await AzureBlobStorage.WriteBlob(input, _connection, _options, TestContext.CancellationToken)); | |
| StringAssert.Contains(ex.Message, "ClientSecretCredential authentication failed"); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Frends.AzureBlobStorage.WriteBlob/Frends.AzureBlobStorage.WriteBlob.Tests/OAuthUnitTests.cs`
around lines 50 - 52, Replace the xUnit/NUnit assertions in OAuthUnitTests.cs:
use MSTest's Assert.ThrowsExceptionAsync<Exception>(async () => await
AzureBlobStorage.WriteBlob(input, _connection, _options,
TestContext.CancellationToken)) instead of Assert.ThrowsAsync, capture the
returned exception, and check the message using
StringAssert.Contains("ClientSecretCredential authentication failed",
ex.Message) (note MSTest's StringAssert parameter order). Update the test around
the AzureBlobStorage.WriteBlob call and variable ex accordingly.
|
|
||
| // Assert | ||
| Assert.IsTrue(result.Success); | ||
| Assert.Contains(_connection.ContainerName + "/" + _connection.BlobName, result.Uri); |
There was a problem hiding this comment.
Assert.Contains is not a valid MSTest assertion method.
Same issue as in other test files. Use StringAssert.Contains for MSTest.
🐛 Proposed fix
- Assert.Contains(_connection.ContainerName + "/" + _connection.BlobName, result.Uri);
+ StringAssert.Contains(result.Uri, _connection.ContainerName + "/" + _connection.BlobName);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Assert.Contains(_connection.ContainerName + "/" + _connection.BlobName, result.Uri); | |
| StringAssert.Contains(result.Uri, _connection.ContainerName + "/" + _connection.BlobName); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Frends.AzureBlobStorage.WriteBlob/Frends.AzureBlobStorage.WriteBlob.Tests/UnitTestsBase.cs`
at line 225, Replace the invalid NUnit-style assertion Assert.Contains with
MSTest's StringAssert.Contains in the test that checks the blob URI: change the
assertion that currently uses Assert.Contains(_connection.ContainerName + "/" +
_connection.BlobName, result.Uri) to use StringAssert.Contains so it compiles
and correctly asserts that result.Uri contains _connection.ContainerName + "/" +
_connection.BlobName; update the assertion call in UnitTestsBase (and any other
tests using Assert.Contains) to reference StringAssert.Contains with the same
left/right arguments.
1 participant
Dear PR creator, please select one of the PR templates, then remove others and this text.
Default PR template
Please review my changes :)
Task Update PR template
Review Checklist
Task Harmonization PR template
Review Checklist
1. Frends Task Project File
Frends.*/Frends.*/*.csproj<TargetFramework>net8.0</TargetFramework><Version>x.0.0</Version><Authors>Frends</Authors><PackageLicenseExpression>MIT</PackageLicenseExpression><GenerateDocumentationFile>true</GenerateDocumentationFile><Description><RepositoryUrl>https://github.com/FrendsPlatform/Frends.SYSTEM/tree/main/Frends.SYSTEM.ACTION</RepositoryUrl><Nullable>disable</Nullable>StyleCop.Analyzers v1.2.0-beta.556FrendsTaskAnalyzers v1.*<Content Include="migration.json" PackagePath="/" Pack="true"/><Content Include="../CHANGELOG.md" PackagePath="/" Pack="true"/><AdditionalFiles Include="FrendsTaskMetadata.json" PackagePath="/" Pack="true"/>2. Frends Task Test Project File
Frends.*/Frends.*.Tests/*.Tests.csproj<TargetFramework>net8.0</TargetFramework><IsPackable>false</IsPackable><Nullable>disable</Nullable>StyleCop.Analyzers v1.2.0-beta.5563. Additional Files
LICENSEfile per repository.gitignorefile per repository.idea/foldersFrends.*/README.mdFrends.*/CHANGELOG.mdFrends.*/Frends.*/FrendsTaskMetadata.jsonFrends.System.Action.System.ActionFrends.*/Frends.*/migration.jsonFrends.*/Frends.*/GlobalSuppressions.csFrends.*/Frends.*.Tests/GlobalSuppressions.cs4. Source Code
5. GitHub Actions Workflows
.github/workflows/*.yml*_release.ymlfeed_api_key: ${{ secrets.TASKS_FEED_API_KEY }}*_test_on_main.ymlbadge_service_api_key: ${{ secrets.BADGE_SERVICE_API_KEY }}*_test_on_push.ymlbadge_service_api_key: ${{ secrets.BADGE_SERVICE_API_KEY }}test_feed_api_key: ${{ secrets.TASKS_TEST_FEED_API_KEY }}GITHUB_TOKENworkdir: Frends.SYSTEM.ACTIONstrict_analyzers: truedotnet_version: 8.0.xprebuild_command: docker-compose up -d)Summary by CodeRabbit
Release Notes
New Features
Breaking Changes
Bug Fixes