Open
Conversation
feat: Refactored the platform's module loading architecture using the static module system classes, the `IPlatformStartup` extension point, and deployment scenarios for Docker and CI/CD pipelines.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 6 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for all 6 issues found in the latest run.
- ✅ Fixed: ModuleCopier silently swallows all IOException during copy
ModuleCopier.CopyFilenow rethrowsIOExceptionfor new file/version/architecture updates and only logs a warning for date-only refresh conflicts.
- ✅ Fixed: Module installer lost transactional file operation safety
ModuleInstallernow usesITransactionFileManager.SafeDelete/CreateDirectoryandIZipFileWrapper.Extractso module file operations again use the transaction-aware infrastructure.
- ✅ Fixed: ExternalModuleCatalog sets OnDemand on installed-only modules
ExternalModuleCatalog.InnerLoadnow setsInitializationMode.OnDemandonly for modules coming from external manifests, preserving installed-only module initialization modes.
- ✅ Fixed: JSON deserialization switched from Newtonsoft to System.Text.Json
ModuleDiscovery.ParseExternalManifestnow deserializes withPropertyNameCaseInsensitive = trueto preserve manifest binding behavior for camelCase payloads.
- ✅ Fixed: ModuleCopier duplicates IsCopyRequired logic in CopyFile
CopyFilenow delegates copy-decision logic through a sharedIsCopyRequiredpath instead of duplicating comparison rules.
- ✅ Fixed: Unused private method GetFileAbsoluteUri in ModuleAssemblyLoader
- Removed the unused private
GetFileAbsoluteUrimethod fromModuleAssemblyLoaderto eliminate dead code.
- Removed the unused private
Or push these changes by commenting:
@cursor push 8bb1f16e82
Preview (8bb1f16e82)
diff --git a/src/VirtoCommerce.Platform.Modules/External/ExternalModuleCatalog.cs b/src/VirtoCommerce.Platform.Modules/External/ExternalModuleCatalog.cs
--- a/src/VirtoCommerce.Platform.Modules/External/ExternalModuleCatalog.cs
+++ b/src/VirtoCommerce.Platform.Modules/External/ExternalModuleCatalog.cs
@@ -66,7 +66,11 @@
{
if (!Modules.OfType<ManifestModuleInfo>().Contains(module))
{
- module.InitializationMode = InitializationMode.OnDemand;
+ if (externalModuleInfos.Contains(module))
+ {
+ module.InitializationMode = InitializationMode.OnDemand;
+ }
+
AddModule(module);
}
}
diff --git a/src/VirtoCommerce.Platform.Modules/ModuleAssemblyLoader.cs b/src/VirtoCommerce.Platform.Modules/ModuleAssemblyLoader.cs
--- a/src/VirtoCommerce.Platform.Modules/ModuleAssemblyLoader.cs
+++ b/src/VirtoCommerce.Platform.Modules/ModuleAssemblyLoader.cs
@@ -294,17 +294,6 @@
return IntPtr.Zero;
}
- private static string GetFileAbsoluteUri(string rootPath, string relativePath)
- {
- var builder = new UriBuilder
- {
- Host = string.Empty,
- Scheme = Uri.UriSchemeFile,
- Path = Path.GetFullPath(Path.Combine(rootPath, relativePath))
- };
- return builder.Uri.ToString();
- }
-
private static Uri GetFileUri(string filePath)
{
if (string.IsNullOrEmpty(filePath))
diff --git a/src/VirtoCommerce.Platform.Modules/ModuleCopier.cs b/src/VirtoCommerce.Platform.Modules/ModuleCopier.cs
--- a/src/VirtoCommerce.Platform.Modules/ModuleCopier.cs
+++ b/src/VirtoCommerce.Platform.Modules/ModuleCopier.cs
@@ -137,51 +137,16 @@
/// </summary>
public static bool IsCopyRequired(Architecture environment, string sourceFilePath, string targetFilePath)
{
- if (!File.Exists(targetFilePath))
- {
- return IsArchitectureCompatible(sourceFilePath, environment);
- }
-
- var result = new FileCompareResult
- {
- NewFile = false,
- };
-
- CompareDates(sourceFilePath, targetFilePath, result);
- CompareVersions(sourceFilePath, targetFilePath, result);
- CompareArchitecture(sourceFilePath, targetFilePath, environment, result);
-
- return result.NewVersion && result.SameOrNewArchitecture ||
- result.NewArchitecture && result.SameOrNewVersion ||
- result.NewDate && result.SameOrNewArchitecture && result.SameOrNewVersion;
+ return IsCopyRequired(environment, sourceFilePath, targetFilePath, out _);
}
private static void CopyFile(Architecture environment, string sourceFilePath, string targetFilePath)
{
- if (!File.Exists(targetFilePath))
+ if (!IsCopyRequired(environment, sourceFilePath, targetFilePath, out var result))
{
- if (!IsArchitectureCompatible(sourceFilePath, environment))
- {
- return;
- }
+ return;
}
- else
- {
- var result = new FileCompareResult();
- CompareDates(sourceFilePath, targetFilePath, result);
- CompareVersions(sourceFilePath, targetFilePath, result);
- CompareArchitecture(sourceFilePath, targetFilePath, environment, result);
- var shouldCopy = result.NewVersion && result.SameOrNewArchitecture ||
- result.NewArchitecture && result.SameOrNewVersion ||
- result.NewDate && result.SameOrNewArchitecture && result.SameOrNewVersion;
-
- if (!shouldCopy)
- {
- return;
- }
- }
-
var targetDir = Path.GetDirectoryName(targetFilePath);
if (targetDir != null && !Directory.Exists(targetDir))
{
@@ -194,11 +159,41 @@
}
catch (IOException)
{
- // Another process may be copying the same file
- ModuleLogger.CreateLogger(typeof(ModuleCopier)).LogWarning("Could not copy {FileName} (file in use)", Path.GetFileName(sourceFilePath));
+ // Date-only refreshes are best effort. Any other copy failure (new file/version/architecture) must fail.
+ var isDateOnlyRefresh = result.NewDate && !result.NewVersion && !result.NewArchitecture && !result.NewFile;
+ if (isDateOnlyRefresh)
+ {
+ ModuleLogger.CreateLogger(typeof(ModuleCopier)).LogWarning("Could not refresh {FileName} (file in use)", Path.GetFileName(sourceFilePath));
+ }
+ else
+ {
+ throw;
+ }
}
}
+ private static bool IsCopyRequired(Architecture environment, string sourceFilePath, string targetFilePath, out FileCompareResult result)
+ {
+ result = new FileCompareResult
+ {
+ NewFile = !File.Exists(targetFilePath),
+ };
+
+ if (result.NewFile)
+ {
+ result.CompatibleArchitecture = IsArchitectureCompatible(sourceFilePath, environment);
+ return result.CompatibleArchitecture;
+ }
+
+ CompareDates(sourceFilePath, targetFilePath, result);
+ CompareVersions(sourceFilePath, targetFilePath, result);
+ CompareArchitecture(sourceFilePath, targetFilePath, environment, result);
+
+ return result.NewVersion && result.SameOrNewArchitecture ||
+ result.NewArchitecture && result.SameOrNewVersion ||
+ result.NewDate && result.SameOrNewArchitecture && result.SameOrNewVersion;
+ }
+
private static bool IsArchitectureCompatible(string filePath, Architecture environment)
{
var arch = GetArchitecture(filePath);
diff --git a/src/VirtoCommerce.Platform.Modules/ModuleDiscovery.cs b/src/VirtoCommerce.Platform.Modules/ModuleDiscovery.cs
--- a/src/VirtoCommerce.Platform.Modules/ModuleDiscovery.cs
+++ b/src/VirtoCommerce.Platform.Modules/ModuleDiscovery.cs
@@ -15,6 +15,11 @@
/// </summary>
public static class ModuleDiscovery
{
+ private static readonly JsonSerializerOptions _manifestJsonOptions = new()
+ {
+ PropertyNameCaseInsensitive = true,
+ };
+
/// <summary>
/// Parse external module manifest JSON into a list of ManifestModuleInfo.
/// Pure function - no HTTP, works on already-downloaded data.
@@ -28,7 +33,7 @@
ArgumentNullException.ThrowIfNull(platformVersion);
var result = new List<ManifestModuleInfo>();
- var manifests = JsonSerializer.Deserialize<List<ExternalModuleManifest>>(manifestJson);
+ var manifests = JsonSerializer.Deserialize<List<ExternalModuleManifest>>(manifestJson, _manifestJsonOptions);
if (manifests == null)
{
diff --git a/src/VirtoCommerce.Platform.Modules/ModuleInstaller.cs b/src/VirtoCommerce.Platform.Modules/ModuleInstaller.cs
--- a/src/VirtoCommerce.Platform.Modules/ModuleInstaller.cs
+++ b/src/VirtoCommerce.Platform.Modules/ModuleInstaller.cs
@@ -9,6 +9,8 @@
using VirtoCommerce.Platform.Core.Common;
using VirtoCommerce.Platform.Core.Modularity;
using VirtoCommerce.Platform.Core.Modularity.Exceptions;
+using VirtoCommerce.Platform.Core.TransactionFileManager;
+using VirtoCommerce.Platform.Core.ZipFile;
using VirtoCommerce.Platform.Modules.External;
#pragma warning disable VC0014 // Type is obsolete
@@ -20,15 +22,25 @@
private const string _packageFileExtension = ".zip";
private readonly LocalStorageModuleCatalogOptions _options;
private readonly IExternalModulesClient _externalClient;
+ private readonly ITransactionFileManager _fileManager;
private readonly IExternalModuleCatalog _extModuleCatalog;
private readonly IFileSystem _fileSystem;
+ private readonly IZipFileWrapper _zipFileWrapper;
- public ModuleInstaller(IExternalModuleCatalog extModuleCatalog, IExternalModulesClient externalClient, IOptions<LocalStorageModuleCatalogOptions> localOptions, IFileSystem fileSystem)
+ public ModuleInstaller(
+ IExternalModuleCatalog extModuleCatalog,
+ IExternalModulesClient externalClient,
+ ITransactionFileManager txFileManager,
+ IOptions<LocalStorageModuleCatalogOptions> localOptions,
+ IFileSystem fileSystem,
+ IZipFileWrapper zipFileWrapper)
{
_extModuleCatalog = extModuleCatalog;
_externalClient = externalClient;
_options = localOptions.Value;
+ _fileManager = txFileManager;
_fileSystem = fileSystem;
+ _zipFileWrapper = zipFileWrapper;
}
#region IModuleInstaller Members
@@ -72,7 +84,7 @@
{
var existModule = _extModuleCatalog.Modules.OfType<ManifestModuleInfo>().First(x => x.IsInstalled && x.Id == newModule.Id);
var dstModuleDir = Path.Combine(_options.DiscoveryPath, existModule.Id);
- ModulePackageInstaller.Uninstall(dstModuleDir);
+ _fileManager.SafeDelete(dstModuleDir);
Report(progress, ProgressMessageLevel.Info, "Updating '{0}' -> '{1}'", existModule, newModule);
InnerInstall(newModule, progress);
existModule.IsInstalled = false;
@@ -131,7 +143,10 @@
}
var moduleDir = Path.Combine(_options.DiscoveryPath, uninstallingModule.Id);
Report(progress, ProgressMessageLevel.Info, "Deleting module {0} folder", moduleDir);
- ModulePackageInstaller.Uninstall(moduleDir);
+ if (Directory.Exists(moduleDir))
+ {
+ _fileManager.SafeDelete(moduleDir);
+ }
Report(progress, ProgressMessageLevel.Info, "'{0}' uninstalled successfully.", uninstallingModule);
uninstallingModule.IsInstalled = false;
changedModulesLog.Add(uninstallingModule);
@@ -178,10 +193,7 @@
var dstModuleDir = Path.Combine(_options.DiscoveryPath, module.Id);
var moduleZipPath = Path.Combine(dstModuleDir, GetModuleZipFileName(module.Id, module.Version.ToString()));
- if (!Directory.Exists(dstModuleDir))
- {
- Directory.CreateDirectory(dstModuleDir);
- }
+ _fileManager.CreateDirectory(dstModuleDir);
// Download module archive from web
if (Uri.IsWellFormedUriString(module.Ref, UriKind.Absolute))
@@ -200,10 +212,10 @@
moduleZipPath = module.Ref;
}
- // Extract the downloaded/local package using ModulePackageInstaller
+ // Extract the downloaded/local package
if (File.Exists(moduleZipPath))
{
- ModulePackageInstaller.Install(moduleZipPath, dstModuleDir);
+ _zipFileWrapper.Extract(moduleZipPath, dstModuleDir);
}
Report(progress, ProgressMessageLevel.Info, "Successfully installed '{0}'.", module);This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.
src/VirtoCommerce.Platform.Modules/LocalModuleCatalogAdapter.cs
Outdated
Show resolved
Hide resolved
vc-ci
reviewed
Mar 11, 2026
Contributor
vc-ci
left a comment
vc-ci
left a comment
There was a problem hiding this comment.
Test Suite: Test Suites/Modules/module_Assets
Tests: 13
Failures: 0
Errors: 0
Time: 7.897
Timestamp: 11-03-2026T12:21:58
… dependencies and refactoring.
vc-ci
reviewed
Mar 11, 2026
Contributor
vc-ci
left a comment
vc-ci
left a comment
There was a problem hiding this comment.
Test Suite: Test Suites/Modules/module_Assets
Tests: 13
Failures: 0
Errors: 0
Time: 7.752
Timestamp: 11-03-2026T13:33:27
vc-ci
reviewed
Mar 11, 2026
Contributor
vc-ci
left a comment
vc-ci
left a comment
There was a problem hiding this comment.
Test Suite: Test Suites/Modules/module_Assets
Tests: 13
Failures: 0
Errors: 0
Time: 7.752
Timestamp: 11-03-2026T15:42:55
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
vc-ci
reviewed
Mar 11, 2026
Contributor
vc-ci
left a comment
vc-ci
left a comment
There was a problem hiding this comment.
Test Suite: Test Suites/Modules/module_Assets
Tests: 13
Failures: 0
Errors: 0
Time: 7.665
Timestamp: 11-03-2026T18:32:27
vc-ci
reviewed
Mar 11, 2026
Contributor
vc-ci
left a comment
vc-ci
left a comment
There was a problem hiding this comment.
Test Suite: Test Suites/Modules/module_Assets
Tests: 13
Failures: 0
Errors: 0
Time: 7.639
Timestamp: 11-03-2026T19:32:44
|
vc-ci
reviewed
Mar 12, 2026
Contributor
vc-ci
left a comment
vc-ci
left a comment
There was a problem hiding this comment.
Test Suite: Test Suites/Modules/module_Assets
Tests: 13
Failures: 0
Errors: 0
Time: 7.835
Timestamp: 12-03-2026T10:41:28
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Refactored the platform's module loading architecture using the static module system classes, the
IPlatformStartupextension point, and deployment scenarios for Docker and CI/CD pipelines.You can find more information in modularity.md.
References
QA-test:
Jira-link:
https://virtocommerce.atlassian.net/browse/VCST-4696
Artifact URL:
Note
High Risk
High risk because it rewires the platform boot sequence (Program/Startup) and module discovery/loading/initialization flow, which can impact configuration sources, DI registration order, and middleware execution across all deployments.
Overview
Refactors the platform’s modularity system to a static, pre-host module loading pipeline executed in
Program.Main(), including manifest discovery, optional probing-folder copy, assembly loading, and a globalModuleRegistryused later inStartup.Introduces a new module extension point
IPlatformStartup(withPipelinePhaseandStartupPriority) discovered frommodule.manifestviastartupType, allowing modules to add configuration sources, host-level services, application services, and middleware at defined phases.Replaces legacy DI-driven module infrastructure with new static helpers (
ModuleManifestReader,ModuleCopier,ModuleAssemblyLoader,ModuleRunner,PlatformStartupDiscovery,ModuleDiscovery,ModulePackageInstaller) plus a read-onlyLocalModuleCatalogAdapter, while marking older modularity interfaces/classes as obsolete and updating external module catalog/installer logic to reuse the new discovery/validation routines. Also removes built-in Azure App Configuration wiring (now module-driven), updates logging/config, adds documentation, and expands test coverage + bumpscoverlet.collectorto8.0.0.Written by Cursor Bugbot for commit 0896bda. This will update automatically on new commits. Configure here.
Image tag:
ghcr.io/VirtoCommerce/platform:3.1008.0-pr-2989-0896-vcst-4696-clean-modularity-0896bda0