Skip to content

fix(generic): honor m tags during realization - #3505

Merged
Alanxtl merged 1 commit into
apache:developfrom
Oxidaner:fix-map-generalizer-m-tag
Aug 19, 2026
Merged

fix(generic): honor m tags during realization#3505
Alanxtl merged 1 commit into
apache:developfrom
Oxidaner:fix-map-generalizer-m-tag

Conversation

@Oxidaner

@Oxidaner Oxidaner commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Description

Fixes #3503 (issue)

Checklist

  • I confirm the target branch is develop
  • Code has passed local testing
  • I have added tests that prove my fix is effective or that my feature works

@Alanxtl Alanxtl linked an issue Jul 26, 2026 that may be closed by this pull request
1 task
@Alanxtl

Alanxtl commented Jul 26, 2026

Copy link
Copy Markdown
Member

默认的tag是mapstructure是吧
是不是把所有的m改成mapstructure好一些

用自定义的m对开发者的心智负担有点重

@Alanxtl Alanxtl left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@Alanxtl Alanxtl added ☢️ Bug 3.3.3 version 3.3.3 labels Jul 26, 2026
@Alanxtl Alanxtl self-assigned this Jul 27, 2026
@Alanxtl

Alanxtl commented Aug 4, 2026

Copy link
Copy Markdown
Member

统一成 mapstructure

@AlexStocks
AlexStocks requested a lite review from Copilot August 5, 2026 04:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Updates the generic map realization flow to respect m struct tags when decoding, adding a regression test for the m-tag round trip (fix for #3503).

Changes:

  • Configure mapstructure decoding in Realize to use TagName: "m".
  • Add a test struct with an m:"user_id" tag and a round-trip test covering Generalize + Realize.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
filter/generic/generalizer/map.go Switches Realize to a configured mapstructure decoder that honors m tags.
filter/generic/generalizer/map_test.go Adds a regression test ensuring m tags survive generalize/realize round trips.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread filter/generic/generalizer/map_test.go Outdated
Comment on lines +85 to +87
generalized, err := mockMapGeneralizer.Generalize(original)
require.NoError(t, err)
assert.Equal(t, "42", generalized.(map[string]any)["user_id"])
Comment on lines +71 to +77
decoder, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{
Result: newobj,
TagName: "m",
})
if err != nil {
return nil, perrors.Errorf("realizing map failed, %v", err)
}
@AlexStocks

Copy link
Copy Markdown
Contributor

虽然我 A 了,但是 copilot 的两个 comment 我觉得很有道理,处理完再 merge。

@Alanxtl

Alanxtl commented Aug 18, 2026

Copy link
Copy Markdown
Member

统一成 mapstructure

虽然我 A 了,但是 copilot 的两个 comment 我觉得很有道理,处理完再 merge。

@Oxidaner what about these

@Oxidaner
Oxidaner force-pushed the fix-map-generalizer-m-tag branch from 8bbc5ba to 27dcf7e Compare August 18, 2026 10:22
@Oxidaner Oxidaner changed the title fix(generic): honor m tags during realization fix(generic): honor mapstructure tags during realization Aug 18, 2026
@codecov-commenter

codecov-commenter commented Aug 18, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 71.42857% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 55.30%. Comparing base (60d1c2a) to head (9d33cf7).
⚠️ Report is 939 commits behind head on develop.

Files with missing lines Patch % Lines
filter/generic/generalizer/map.go 71.42% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3505      +/-   ##
===========================================
+ Coverage    46.76%   55.30%   +8.53%     
===========================================
  Files          295      475     +180     
  Lines        17172    36822   +19650     
===========================================
+ Hits          8031    20364   +12333     
- Misses        8287    14833    +6546     
- Partials       854     1625     +771     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment on lines +71 to +74
decoder, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{
Result: newobj,
TagName: "mapstructure",
})

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

默认不就是mapstructure吗 这里需要显式指定吗

@Alanxtl

Alanxtl commented Aug 18, 2026

Copy link
Copy Markdown
Member

update to latest develop branch to fix the ci fail

@Oxidaner
Oxidaner force-pushed the fix-map-generalizer-m-tag branch from 27dcf7e to 6943581 Compare August 18, 2026 11:25
// setInMap sets the struct into the map using the tag or the name of the struct as the key
func setInMap(m map[string]any, structField reflect.StructField, value any) (result map[string]any) {
result = m
if tagName := structField.Tag.Get("m"); tagName == "" {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我发现这里m还不能改为mapstructure 会影响下游用户 还是改回来吧 不好意思😂

@Oxidaner
Oxidaner force-pushed the fix-map-generalizer-m-tag branch from 6943581 to 9d33cf7 Compare August 19, 2026 14:05
@Oxidaner Oxidaner changed the title fix(generic): honor mapstructure tags during realization fix(generic): honor m tags during realization Aug 19, 2026
@sonarqubecloud

Copy link
Copy Markdown

@Alanxtl
Alanxtl merged commit 6dea7f5 into apache:develop Aug 19, 2026
8 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3.3.3 version 3.3.3 ☢️ Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] MapGeneralizer.Realize ignores m tags and silently drops renamed fields

5 participants