Remove record converter#116
Merged
Merged
Conversation
将 ColumnNameResolver、RecordMappingContext、RecordMappingOptions 等与 DTO/RecordTable 映射相关的核心类型从 LuYao.Data.Meta 命名空间迁移到 LuYao.Data.Mapping，并同步调整所有相关 using 路径。移除 RecordColumnStorageTarget 中未使用的 String、Bytes 枚举值，仅保留 Skip。实现代码和注释未做实质性更改，保持英文注释规范。
本次提交将 Attribute、Binary、Json 相关类分别迁移到 LuYao.Data.Attributes、LuYao.Data.Binary、LuYao.Data.Json 命名空间，并同步调整相关文件的 using 引用。此举提升了代码的模块化和可维护性。
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the Record mapping infrastructure by removing custom converter registration and restructuring namespaces to better separate mapping, attributes, binary payload, and JSON concerns.
Changes:
- Moved mapping-related attributes into
LuYao.Data.Attributes, and binary/JSON helpers intoLuYao.Data.Binary/LuYao.Data.Json. - Simplified mapping by removing
RecordConverter/DefaultRecordConverterand related converter resolution paths. - Updated unit tests and removed tests tied to deleted converter/copying utilities.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/LuYao.Common.UnitTests/Data/RecordMappingOptionsTests.cs | Removes converter-related tests and updates usings for new namespaces. |
| tests/LuYao.Common.UnitTests/Data/RecordJsonConverterTests.cs | Updates usings to new Binary/Json namespaces. |
| tests/LuYao.Common.UnitTests/Data/Meta/XCopyGenericTests.cs | Removes tests for deleted XCopy<TSource,TTarget>. |
| src/LuYao.Common/Data/UnsupportedTypeHandling.cs | Removes converter-based handling modes, leaving Skip/Throw. |
| src/LuYao.Common/Data/RecordTable/Binary.cs | Updates references to binary payload types via LuYao.Data.Binary. |
| src/LuYao.Common/Data/RecordSet/Binary.cs | Updates references to binary payload types via LuYao.Data.Binary. |
| src/LuYao.Common/Data/RecordRow/Merge.cs | Updates mapping references to LuYao.Data.Mapping. |
| src/LuYao.Common/Data/RecordConverter.cs | Deletes converter base types. |
| src/LuYao.Common/Data/RecordColumnStorageTarget.cs | Removes non-skip storage targets. |
| src/LuYao.Common/Data/RecordColumnCollection.cs | Updates mapping references to LuYao.Data.Mapping. |
| src/LuYao.Common/Data/Meta/DefaultRecordConverter.cs | Deletes the built-in fallback converter. |
| src/LuYao.Common/Data/Mapping/XCopy.cs | Removes XCopy<TSource,TTarget> and keeps mapping centered around RecordRow/DTO mapping. |
| src/LuYao.Common/Data/Mapping/XConverter.cs | Deletes unused converter abstraction. |
| src/LuYao.Common/Data/Mapping/RecordMappingOptions.cs | Moves options into LuYao.Data.Mapping and removes converter registration API. |
| src/LuYao.Common/Data/Mapping/RecordMappingContext.cs | Removes converter resolution and enforces stricter mapping behavior. |
| src/LuYao.Common/Data/Mapping/Converters/StringToInt32Converter.cs | Deletes an unfinished converter stub. |
| src/LuYao.Common/Data/Mapping/ColumnNameResolver.cs | Moves resolver into LuYao.Data.Mapping and updates attribute usage. |
| src/LuYao.Common/Data/Json/RecordTableJsonConverter.cs | Moves JSON converter into LuYao.Data.Json and updates binary dependencies. |
| src/LuYao.Common/Data/Json/RecordSetJsonConverter.cs | Moves JSON converter into LuYao.Data.Json and updates binary dependencies. |
| src/LuYao.Common/Data/Binary/RecordPayloadCompression.cs | Moves into LuYao.Data.Binary. |
| src/LuYao.Common/Data/Binary/RecordBinaryPayloadCodec.cs | Moves into LuYao.Data.Binary. |
| src/LuYao.Common/Data/Binary/BinaryPayloadHeader.cs | Moves into LuYao.Data.Binary. |
| src/LuYao.Common/Data/Attributes/RecordColumnStorageAttribute.cs | Moves into LuYao.Data.Attributes. |
| src/LuYao.Common/Data/Attributes/RecordColumnNameAttribute.cs | Moves into LuYao.Data.Attributes. |
| src/LuYao.Common/Data/Attributes/RecordAttribute.cs | Moves into LuYao.Data.Attributes. |
Comments suppressed due to low confidence (3)
src/LuYao.Common/Data/Mapping/RecordMappingContext.cs:53
- MapDtoToRow writes into an existing column whenever the property type is a supported column type, but it does not verify that the existing column type matches the property type. Since RecordColumn.Set can coerce values via Valid.To, this can silently write converted values into a mismatched column schema and later make round-tripping back to DTO fail (MapRowToDto requires exact type match). Consider requiring col.Type == prop.Type here as well, or explicitly handling type mismatches according to the mapping options.
This issue also appears on line 110 of the same file.
src/LuYao.Common/Data/Mapping/RecordMappingContext.cs:112
- This NotSupportedException message is used both when the property type is unsupported and when the column/property types don't match. In the mismatch case, saying the property type "is not supported" is misleading. Consider including both col.Type and prop.Type in the message and/or using a different message for type mismatch vs unsupported type.
src/LuYao.Common/Data/Mapping/RecordMappingOptions.cs:7 - After removing custom converter support from RecordMappingOptions, the XML docs in this file still reference the removed AddConverter API (and the cref for RecordColumnNameAttribute appears to use an outdated namespace). Please update the documentation so it matches the current public surface and resolves cleanly when generating XML docs.
Comment on lines
9
to
13
| /// <summary> | ||
| /// 跳过该属性，不在 RecordTable 中创建对应的列。 | ||
| /// </summary> | ||
| Skip = 0, | ||
|
|
||
| /// <summary> | ||
| /// 将属性值转换为 <see cref="string"/> 后存储。 | ||
| /// 必须在 <see cref="RecordMappingOptions"/> 中注册该属性类型对应的 <see cref="RecordConverter"/>，否则将抛出异常。 | ||
| /// </summary> | ||
| String = 1, | ||
|
|
||
| /// <summary> | ||
| /// 将属性值转换为 <see cref="byte"/>[] 后存储。 | ||
| /// 必须在 <see cref="RecordMappingOptions"/> 中注册该属性类型对应的 <see cref="RecordConverter"/>，否则将抛出异常。 | ||
| /// </summary> | ||
| Bytes = 2, | ||
| Skip = 0 | ||
| } |
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
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.
This pull request primarily restructures the namespaces and refactors the mapping infrastructure of the codebase, with some deletions of unused or redundant classes. The changes aim to improve code organization, clarify responsibilities, and simplify the mapping logic by removing custom converter registration and related complexity.
Namespace and File Organization Improvements:
RecordAttribute,RecordColumnNameAttribute,RecordColumnStorageAttribute) into theLuYao.Data.Attributesnamespace for better separation of concerns. [1] [2] [3]BinaryPayloadHeader,RecordBinaryPayloadCodec,RecordPayloadCompression) into theLuYao.Data.Binarynamespace, and JSON converters intoLuYao.Data.Json, improving modularity. [1] [2] [3] [4] [5]Mapping Infrastructure Refactoring:
Metanamespace toMapping, includingColumnNameResolverandRecordMappingContext, and movedRecordMappingOptionstoLuYao.Data.Mapping. [1] [2] [3]RecordMappingOptionsand related converter resolution logic inRecordMappingContext. Now, unsupported types directly trigger error handling, simplifying the mapping process. [1] [2] [3] [4] [5]Code Cleanup:
XConverterandStringToInt32Converter, to reduce maintenance overhead. [1] [2]XCopy<TSource, TTarget>class, streamlining the codebase by eliminating unused copying utilities.These changes collectively improve code clarity, maintainability, and modularity by enforcing stricter boundaries between concerns and simplifying the mapping logic.