fix(review-followup): 修复失败路径清理与日志契约

- 修复 Godot 模块在附加流程失败时的登记时机,确保后续销毁仍可感知半安装模块
- 更新 ConfigurableLoggerFactory 的 name 空值校验与 minLevel XML 契约,并用可观察行为替换脆弱的反射测试
- 补充 WeakTypePairCache 热路径注释,并新增 Godot 模块安装顺序回归测试
This commit is contained in:
GeWuYou 2026-04-18 20:45:37 +08:00
parent 05de6d1e15
commit 2c2df5de29
5 changed files with 120 additions and 24 deletions

View File

@ -1,7 +1,5 @@
using System.Reflection;
using GFramework.Core.Abstractions.Logging; using GFramework.Core.Abstractions.Logging;
using GFramework.Core.Logging; using GFramework.Core.Logging;
using GFramework.Core.Logging.Appenders;
namespace GFramework.Core.Tests.Logging; namespace GFramework.Core.Tests.Logging;
@ -39,7 +37,7 @@ public sealed class ConfigurableLoggerFactoryTests
} }
/// <summary> /// <summary>
/// 验证调用方传入的默认最小级别会作为配置级别的下限参与最终 logger 级别计算。 /// 验证在未命中命名空间覆盖时,调用方传入的默认最小级别会作为最终 logger 级别的下限参与计算。
/// </summary> /// </summary>
[Test] [Test]
public void GetLogger_ShouldHonorStricterCallerMinLevelWhenNoOverrideMatches() public void GetLogger_ShouldHonorStricterCallerMinLevelWhenNoOverrideMatches()
@ -69,7 +67,51 @@ public sealed class ConfigurableLoggerFactoryTests
} }
/// <summary> /// <summary>
/// 验证工厂释放时会兼容释放未实现 <see cref="IDisposable" /> 的 <see cref="AsyncLogAppender" />。 /// 验证命名空间覆盖级别会优先于调用方传入的默认最小级别,确保覆盖配置保持最高优先级。
/// </summary>
[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);
});
}
/// <summary>
/// 验证调用方传入空 logger 名称时,会得到显式的参数异常而不是后续字符串操作的空引用异常。
/// </summary>
[Test]
public void GetLogger_WithNullName_ShouldThrowArgumentNullException()
{
var factory = LoggingConfigurationLoader.CreateFactory(new LoggingConfiguration());
Assert.Throws<ArgumentNullException>(() => factory.GetLogger(null!));
}
/// <summary>
/// 验证工厂释放时会兼容释放未实现 <see cref="IDisposable" /> 的异步 appender并让既有 logger 观察到已释放状态。
/// </summary> /// </summary>
[Test] [Test]
public void Dispose_ShouldDisposeAsyncLogAppenderCreatedFromConfiguration() public void Dispose_ShouldDisposeAsyncLogAppenderCreatedFromConfiguration()
@ -93,25 +135,11 @@ public sealed class ConfigurableLoggerFactoryTests
var factory = LoggingConfigurationLoader.CreateFactory(config); var factory = LoggingConfigurationLoader.CreateFactory(config);
var logger = factory.GetLogger("AsyncLogger"); var logger = factory.GetLogger("AsyncLogger");
var asyncAppender = GetSingleAsyncAppender(factory);
logger.Info("dispose-path"); logger.Info("dispose-path");
((IDisposable)factory).Dispose(); ((IDisposable)factory).Dispose();
Assert.That(asyncAppender.IsCompleted, Is.True); Assert.Throws<ObjectDisposedException>(() => logger.Info("after-dispose"));
}
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<AsyncLogAppender>());
return (AsyncLogAppender)appenders[0];
} }
} }

View File

@ -54,14 +54,16 @@ internal sealed class ConfigurableLoggerFactory : ILoggerFactory, IDisposable
/// 为指定名称创建日志记录器,并应用最匹配的命名空间级别配置。 /// 为指定名称创建日志记录器,并应用最匹配的命名空间级别配置。
/// </summary> /// </summary>
/// <param name="name">日志记录器名称。</param> /// <param name="name">日志记录器名称。</param>
/// <param name="minLevel">调用方要求的最小日志级别下限;最终级别不会低于该值。</param> /// <param name="minLevel">调用方要求的最小日志级别下限;在未命中命名空间覆盖时生效。</param>
/// <returns>可写入日志的记录器实例。</returns> /// <returns>可写入日志的记录器实例。</returns>
/// <remarks> /// <remarks>
/// 当配置文件与调用方同时提供默认级别时,会取两者中更严格的那一个; /// 当配置文件与调用方同时提供默认级别时,会取两者中更严格的那一个;
/// 若命中更具体的命名空间级别覆盖,则以该覆盖配置为准 /// 若命中更具体的命名空间级别覆盖,则以该覆盖配置为准,即使其低于调用方传入的默认下限
/// </remarks> /// </remarks>
public ILogger GetLogger(string name, LogLevel minLevel = LogLevel.Info) public ILogger GetLogger(string name, LogLevel minLevel = LogLevel.Info)
{ {
ArgumentNullException.ThrowIfNull(name);
var effectiveLevel = _config.MinLevel > minLevel ? _config.MinLevel : minLevel; var effectiveLevel = _config.MinLevel > minLevel ? _config.MinLevel : minLevel;
var bestMatchLength = -1; var bestMatchLength = -1;

View File

@ -34,10 +34,12 @@ internal sealed class WeakTypePairCache<TValue>
ArgumentNullException.ThrowIfNull(secondaryType); ArgumentNullException.ThrowIfNull(secondaryType);
ArgumentNullException.ThrowIfNull(valueFactory); ArgumentNullException.ThrowIfNull(valueFactory);
// 第一层按 primaryType 定位或创建二级缓存,避免每次命中都重新分配容器。
var secondaryEntries = _entries.GetOrAdd(primaryType, static _ => new WeakKeyCache<Type, TValue>()); var secondaryEntries = _entries.GetOrAdd(primaryType, static _ => new WeakKeyCache<Type, TValue>());
return secondaryEntries.GetOrAdd( return secondaryEntries.GetOrAdd(
secondaryType, secondaryType,
(PrimaryType: primaryType, Factory: valueFactory), (PrimaryType: primaryType, Factory: valueFactory),
// 使用 static lambda + state 传参,避免热路径上的闭包捕获与额外分配。
static (cachedSecondaryType, state) => state.Factory(state.PrimaryType, cachedSecondaryType)); static (cachedSecondaryType, state) => state.Factory(state.PrimaryType, cachedSecondaryType));
} }

View File

@ -0,0 +1,64 @@
using GFramework.Core.Abstractions.Architectures;
using GFramework.Godot.Architectures;
namespace GFramework.Godot.Tests.Architectures;
/// <summary>
/// 验证 Godot 架构在模块安装前会先检查锚点状态,避免未绑定场景树时留下半安装副作用。
/// </summary>
[TestFixture]
public sealed class AbstractArchitectureModuleInstallationTests
{
/// <summary>
/// 验证当锚点尚未初始化时,安装流程会直接失败,且不会执行模块安装逻辑。
/// </summary>
/// <returns>表示异步断言完成的任务。</returns>
[Test]
public async Task InstallGodotModuleAsync_ShouldThrowBeforeInvokingModuleInstall_WhenAnchorIsMissing()
{
var architecture = new TestArchitecture();
var module = new RecordingGodotModule();
var exception = Assert.ThrowsAsync<InvalidOperationException>(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()
{
}
}
}

View File

@ -111,6 +111,9 @@ public abstract class AbstractArchitecture(
module.Install(this); module.Install(this);
// 在附加流程完成前先登记模块,保证后续任一步失败时仍能参与架构销毁阶段的清理。
_extensions.Add(module);
// 等待锚点准备就绪,并保持 Godot 同步上下文,以便后续附加逻辑安全访问节点 API。 // 等待锚点准备就绪,并保持 Godot 同步上下文,以便后续附加逻辑安全访问节点 API。
await anchor.WaitUntilReadyAsync(); await anchor.WaitUntilReadyAsync();
@ -119,9 +122,6 @@ public abstract class AbstractArchitecture(
// 调用扩展的附加回调方法 // 调用扩展的附加回调方法
module.OnAttach(this); module.OnAttach(this);
// 将扩展添加到扩展集合中
_extensions.Add(module);
} }