chore: added blob string normalisation and logging

[Jacob-Polane]

#4559
Extends Azure Blob Storage configuration to support multiple credential formats through a single config value,
with automatic fallback to local file storage when no Azure config is present.

Changes

AzureStoredFileService now auto-detects the authentication method from the format of the configured value — no
extra flags needed
Added CloudStorage:ConnectionString as the preferred config key (falls back to ConnectionStrings:BlobStorage
for backwards compatibility)
Fixed blob path construction to use forward slashes on all platforms (Path.Combine → string formatting)
DI factory auto-activates Azure storage when a credential is present — IsAzureEnvironment: true is no longer
required but still works
Supported credential formats

Classic connection string DefaultEndpointsProtocol=https;AccountName=…;AccountKey=…
Container SAS URL https://account.blob.core.windows.net/container?sv=…&sig=…
Account SAS URL https://account.blob.core.windows.net?sv=…&sig=…
Not configured Falls back to local StoredFileService

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • Improved file path normalization and resolution for more robust blob handling.
  • Enhanced input validation for better error reporting.

• Improvements

  • Expanded Azure storage configuration support, including SAS URLs and various connection string formats.
  • Optimized blob container client initialization for improved reliability.

[coderabbitai]

Walkthrough

The AzureStoredFileService class has been refactored to improve blob storage handling, adding logging support, implementing lazy initialization for the blob container client, enhancing storage value retrieval with fallback logic, and improving path resolution for blob operations.

Changes

Cohort / File(s)

Summary

Azure Storage Service Refactoring shesha-core/src/Shesha.Framework/Services/StoredFiles/AzureStoredFileService.cs

Added ILogger dependency to constructor; introduced Lazy with CreateBlobContainerClient() for deferred initialization supporting SAS URLs and connection strings; replaced GetConnectionString() with GetStorageValue() featuring prioritization and fallback logic; refactored GetBlobClient() for robust blob path normalization; made GetAzureFileName() static; tightened UpdateVersionContentAsync validation; reorganized initialization and access patterns via private helper methods.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 A blob in the cloud, now lazy and wise,
With logging to watch and connections that rise,
SAS URLs blend with fallback delight,
Path normalization sets storage just right,
Refactored with care, our Azure takes flight! ☁️

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The title 'chore: added blob string normalisation and logging' covers only partial aspects of the changeset. While logging and blob path normalization are mentioned in the summary, the PR introduces significant architectural changes including multi-credential format support, storage fallback logic, and container initialization refactoring—none of which are reflected in the title.	Revise the title to reflect the primary architectural changes, such as 'Refactor AzureStoredFileService to support multiple credential formats and auto-detect authentication' or similar.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@shesha-core/src/Shesha.Framework/Services/StoredFiles/AzureStoredFileService.cs`:
- Around line 106-118: GetBlobClient builds blobPath using the unnormalized
blobName which defeats the earlier normalization; change the path construction
to use normalizedBlobName instead of blobName so blobPath is either
normalizedBlobName or $"{normalizedDirectory}/{normalizedBlobName}", then return
BlobContainerClient.GetBlobClient(blobPath).
- Around line 161-162: Replace the generic ArgumentException null check with an
ArgumentNullException for the stream parameter: in AzureStoredFileService (the
method that currently has "if (stream == null) throw new
ArgumentException($"{nameof(stream)} must not be null");") change it to throw
new ArgumentNullException(nameof(stream)) so the exception type and paramName
are semantically correct.
- Around line 84-90: The warning in AzureStoredFileService.cs inside the
hasContainerInPath branch is too vague; update the _logger.Warn call to include
both the container parsed from the SAS URL and the configured ContainerName so
you can see the actual mismatch when returning new BlobContainerClient(uri).
Locate the block that checks hasContainerInPath and modify the log message to
format and include the URI-derived container name (from the parsed uri or helper
that detects the container) and the configured ContainerName property so the
warning reads like: "SAS URL container '{urlContainer}' differs from configured
ContainerName '{configuredContainer}'. Using URL container."
- Around line 98-101: Current code in AzureStoredFileService constructs a
BlobContainerClient, calls CreateIfNotExists and
SetAccessPolicy(PublicAccessType.BlobContainer) which makes blobs publicly
accessible and uses blocking sync calls; change this to accept a configurable
access policy (e.g., an injected/config value used instead of hardcoded
PublicAccessType.BlobContainer), default the config to PublicAccessType.None,
and replace CreateIfNotExists and SetAccessPolicy calls with their async
counterparts (CreateIfNotExistsAsync, SetAccessPolicyAsync) to avoid blocking;
update the BlobContainerClient creation/return logic in the method that
constructs the client so it reads the configured access policy and awaits the
async calls.
- Around line 59-64: Remove the redundant null/whitespace check after calling
GetStorageValue() in AzureStoredFileService: GetStorageValue() already throws
InvalidOperationException when the storage value is missing, so delete the if
(string.IsNullOrWhiteSpace(value)) block and its throw; simply use the returned
value (var value = GetStorageValue();) without duplicating the validation or
exception message.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: faae2f97-4c11-4518-a2df-1723df887566

📥 Commits

Reviewing files that changed from the base of the PR and between 7097309 and 802ef54.

📒 Files selected for processing (1)

• shesha-core/src/Shesha.Framework/Services/StoredFiles/AzureStoredFileService.cs

[IvanIlyichev]

Hi @Jacob-Polane. Please review and resolve all comments from Coderabbit