From 2c2df5de298e2eb6d6359a8fc196eab373fd5e14 Mon Sep 17 00:00:00 2001 From: GeWuYou <95328647+GeWuYou@users.noreply.github.com> Date: Sat, 18 Apr 2026 20:45:37 +0800 Subject: [PATCH] =?UTF-8?q?fix(review-followup):=20=E4=BF=AE=E5=A4=8D?= =?UTF-8?q?=E5=A4=B1=E8=B4=A5=E8=B7=AF=E5=BE=84=E6=B8=85=E7=90=86=E4=B8=8E?= =?UTF-8?q?=E6=97=A5=E5=BF=97=E5=A5=91=E7=BA=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 修复 Godot 模块在附加流程失败时的登记时机，确保后续销毁仍可感知半安装模块 - 更新 ConfigurableLoggerFactory 的 name 空值校验与 minLevel XML 契约，并用可观察行为替换脆弱的反射测试 - 补充 WeakTypePairCache 热路径注释，并新增 Godot 模块安装顺序回归测试 --- .../Logging/ConfigurableLoggerFactoryTests.cs | 66 +++++++++++++------ .../Logging/ConfigurableLoggerFactory.cs | 6 +- GFramework.Cqrs/Internal/WeakTypePairCache.cs | 2 + ...ractArchitectureModuleInstallationTests.cs | 64 ++++++++++++++++++ .../Architectures/AbstractArchitecture.cs | 6 +- 5 files changed, 120 insertions(+), 24 deletions(-) create mode 100644 GFramework.Godot.Tests/Architectures/AbstractArchitectureModuleInstallationTests.cs diff --git a/GFramework.Core.Tests/Logging/ConfigurableLoggerFactoryTests.cs b/GFramework.Core.Tests/Logging/ConfigurableLoggerFactoryTests.cs index d712cad4..074c9a02 100644 --- a/GFramework.Core.Tests/Logging/ConfigurableLoggerFactoryTests.cs +++ b/GFramework.Core.Tests/Logging/ConfigurableLoggerFactoryTests.cs @@ -1,7 +1,5 @@ -using System.Reflection; using GFramework.Core.Abstractions.Logging; using GFramework.Core.Logging; -using GFramework.Core.Logging.Appenders; namespace GFramework.Core.Tests.Logging; @@ -39,7 +37,7 @@ public sealed class ConfigurableLoggerFactoryTests } /// - /// 验证调用方传入的默认最小级别会作为配置级别的下限参与最终 logger 级别计算。 + /// 验证在未命中命名空间覆盖时，调用方传入的默认最小级别会作为最终 logger 级别的下限参与计算。 /// [Test] public void GetLogger_ShouldHonorStricterCallerMinLevelWhenNoOverrideMatches() @@ -69,7 +67,51 @@ public sealed class ConfigurableLoggerFactoryTests } /// - /// 验证工厂释放时会兼容释放未实现 的 。 + /// 验证命名空间覆盖级别会优先于调用方传入的默认最小级别，确保覆盖配置保持最高优先级。 + /// + [Test] + public void GetLogger_ShouldPreferNamespaceOverrideOverCallerMinLevel() + { + var config = LoggingConfigurationLoader.LoadFromJsonString( + """ + { + "minLevel": "Info", + "appenders": [ + { + "type": "Console", + "formatter": "Default", + "useColors": false + } + ], + "loggerLevels": { + "MyApp.Services": "Debug" + } + } + """); + + var factory = LoggingConfigurationLoader.CreateFactory(config); + var logger = factory.GetLogger("MyApp.Services.OrderService", LogLevel.Fatal); + + Assert.Multiple(() => + { + Assert.That(logger.IsDebugEnabled(), Is.True); + Assert.That(logger.IsTraceEnabled(), Is.False); + }); + } + + /// + /// 验证调用方传入空 logger 名称时，会得到显式的参数异常而不是后续字符串操作的空引用异常。 + /// + [Test] + public void GetLogger_WithNullName_ShouldThrowArgumentNullException() + { + var factory = LoggingConfigurationLoader.CreateFactory(new LoggingConfiguration()); + + Assert.Throws(() => factory.GetLogger(null!)); + } + + /// + /// 验证工厂释放时会兼容释放未实现 的异步 appender，并让既有 logger 观察到已释放状态。 /// [Test] public void Dispose_ShouldDisposeAsyncLogAppenderCreatedFromConfiguration() @@ -93,25 +135,11 @@ public sealed class ConfigurableLoggerFactoryTests var factory = LoggingConfigurationLoader.CreateFactory(config); var logger = factory.GetLogger("AsyncLogger"); - var asyncAppender = GetSingleAsyncAppender(factory); logger.Info("dispose-path"); ((IDisposable)factory).Dispose(); - Assert.That(asyncAppender.IsCompleted, Is.True); - } - - private static AsyncLogAppender GetSingleAsyncAppender(ILoggerFactory factory) - { - var appendersField = factory.GetType().GetField("_appenders", BindingFlags.Instance | BindingFlags.NonPublic); - Assert.That(appendersField, Is.Not.Null); - - var appenders = appendersField!.GetValue(factory) as ILogAppender[]; - Assert.That(appenders, Is.Not.Null); - Assert.That(appenders, Has.Length.EqualTo(1)); - Assert.That(appenders![0], Is.TypeOf()); - - return (AsyncLogAppender)appenders[0]; + Assert.Throws(() => logger.Info("after-dispose")); } } diff --git a/GFramework.Core/Logging/ConfigurableLoggerFactory.cs b/GFramework.Core/Logging/ConfigurableLoggerFactory.cs index e1c5ab6f..5e4b497e 100644 --- a/GFramework.Core/Logging/ConfigurableLoggerFactory.cs +++ b/GFramework.Core/Logging/ConfigurableLoggerFactory.cs @@ -54,14 +54,16 @@ internal sealed class ConfigurableLoggerFactory : ILoggerFactory, IDisposable /// 为指定名称创建日志记录器，并应用最匹配的命名空间级别配置。 /// /// 日志记录器名称。 - /// 调用方要求的最小日志级别下限；最终级别不会低于该值。 + /// 调用方要求的最小日志级别下限；在未命中命名空间覆盖时生效。 /// 可写入日志的记录器实例。 /// /// 当配置文件与调用方同时提供默认级别时，会取两者中更严格的那一个； - /// 若命中更具体的命名空间级别覆盖，则以该覆盖配置为准。 + /// 若命中更具体的命名空间级别覆盖，则以该覆盖配置为准，即使其低于调用方传入的默认下限。 /// public ILogger GetLogger(string name, LogLevel minLevel = LogLevel.Info) { + ArgumentNullException.ThrowIfNull(name); + var effectiveLevel = _config.MinLevel > minLevel ? _config.MinLevel : minLevel; var bestMatchLength = -1; diff --git a/GFramework.Cqrs/Internal/WeakTypePairCache.cs b/GFramework.Cqrs/Internal/WeakTypePairCache.cs index d3dd5501..f1870545 100644 --- a/GFramework.Cqrs/Internal/WeakTypePairCache.cs +++ b/GFramework.Cqrs/Internal/WeakTypePairCache.cs @@ -34,10 +34,12 @@ internal sealed class WeakTypePairCache ArgumentNullException.ThrowIfNull(secondaryType); ArgumentNullException.ThrowIfNull(valueFactory); + // 第一层按 primaryType 定位或创建二级缓存，避免每次命中都重新分配容器。 var secondaryEntries = _entries.GetOrAdd(primaryType, static _ => new WeakKeyCache()); return secondaryEntries.GetOrAdd( secondaryType, (PrimaryType: primaryType, Factory: valueFactory), + // 使用 static lambda + state 传参，避免热路径上的闭包捕获与额外分配。 static (cachedSecondaryType, state) => state.Factory(state.PrimaryType, cachedSecondaryType)); } diff --git a/GFramework.Godot.Tests/Architectures/AbstractArchitectureModuleInstallationTests.cs b/GFramework.Godot.Tests/Architectures/AbstractArchitectureModuleInstallationTests.cs new file mode 100644 index 00000000..8952a12c --- /dev/null +++ b/GFramework.Godot.Tests/Architectures/AbstractArchitectureModuleInstallationTests.cs @@ -0,0 +1,64 @@ +using GFramework.Core.Abstractions.Architectures; +using GFramework.Godot.Architectures; + +namespace GFramework.Godot.Tests.Architectures; + +/// +/// 验证 Godot 架构在模块安装前会先检查锚点状态，避免未绑定场景树时留下半安装副作用。 +/// +[TestFixture] +public sealed class AbstractArchitectureModuleInstallationTests +{ + /// + /// 验证当锚点尚未初始化时，安装流程会直接失败，且不会执行模块安装逻辑。 + /// + /// 表示异步断言完成的任务。 + [Test] + public async Task InstallGodotModuleAsync_ShouldThrowBeforeInvokingModuleInstall_WhenAnchorIsMissing() + { + var architecture = new TestArchitecture(); + var module = new RecordingGodotModule(); + + var exception = Assert.ThrowsAsync(async () => + await architecture.InstallGodotModuleForTestAsync(module)); + + Assert.Multiple(() => + { + Assert.That(exception, Is.Not.Null); + Assert.That(exception!.Message, Is.EqualTo("Anchor not initialized")); + Assert.That(module.InstallCalled, Is.False); + }); + } + + private sealed class TestArchitecture : AbstractArchitecture + { + protected override void InstallModules() + { + } + + public Task InstallGodotModuleForTestAsync(RecordingGodotModule module) + { + return InstallGodotModule(module); + } + } + + private sealed class RecordingGodotModule : IGodotModule + { + public bool InstallCalled { get; private set; } + + public global::Godot.Node Node => null!; + + public void Install(IArchitecture architecture) + { + InstallCalled = true; + } + + public void OnAttach(GFramework.Core.Architectures.Architecture architecture) + { + } + + public void OnDetach() + { + } + } +} diff --git a/GFramework.Godot/Architectures/AbstractArchitecture.cs b/GFramework.Godot/Architectures/AbstractArchitecture.cs index d92ea1ef..668c84ce 100644 --- a/GFramework.Godot/Architectures/AbstractArchitecture.cs +++ b/GFramework.Godot/Architectures/AbstractArchitecture.cs @@ -111,6 +111,9 @@ public abstract class AbstractArchitecture( module.Install(this); + // 在附加流程完成前先登记模块，保证后续任一步失败时仍能参与架构销毁阶段的清理。 + _extensions.Add(module); + // 等待锚点准备就绪，并保持 Godot 同步上下文，以便后续附加逻辑安全访问节点 API。 await anchor.WaitUntilReadyAsync(); @@ -119,9 +122,6 @@ public abstract class AbstractArchitecture( // 调用扩展的附加回调方法 module.OnAttach(this); - - // 将扩展添加到扩展集合中 - _extensions.Add(module); }