fix(steam): populate RawData from Steam player payload (#518)#644
Open
yabanci wants to merge 1 commit into
Open
fix(steam): populate RawData from Steam player payload (#518)#644yabanci wants to merge 1 commit into
yabanci wants to merge 1 commit into
Conversation
The steam provider's buildUserObject never assigned anything to u.RawData, so callers had no way to read Steam-specific fields without a slot on goth.User (timecreated, primaryclanid, profileurl, communityvisibilitystate, profilestate, personastate, avatar, avatarmedium). Every other provider exposes the upstream response via RawData; steam was the outlier. Continues @leobrines's work in markbates#518 with two adjustments so the patch reflects the spirit of RawData in other providers: 1. Decode the body once via io.ReadAll, then unmarshal the player into both the typed struct and u.RawData. The original PR marshalled the already-typed struct back to JSON and decoded that into the map, which made RawData a verbatim duplicate of the six typed fields and dropped everything else Steam returned. 2. Expanded the test fixture beyond the six fields to a realistic player payload and locked the no-slot fields in with explicit RawData assertions, so a future refactor can't silently shrink the response again. Verified the new assertions fail on master and pass with this change.
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.
What
Revives and extends @leobrines's #518, which has been sitting MERGEABLE for ~2 years. The gap it identified is still real on master:
providers/steam/steam.go:buildUserObjectnever assigns anything tou.RawData, so the steam provider is the only one in goth that ships a fully emptyRawDatamap. Callers have no way to reach Steam-specific fields without a slot ongoth.User—timecreated,primaryclanid,profileurl,communityvisibilitystate,profilestate,personastate,avatar,avatarmedium.Change vs. master
Read the body once, decode the envelope into a typed struct that keeps the player payload as
json.RawMessage, then unmarshal that payload twice — into the typed struct (forgoth.Userfields) and intou.RawData(for everything else). This mirrors what auth0/discord/etc. already do.Change vs. #518
I credited @leobrines in the commit body and on his PR. Two deltas vs. his patch, both intended as response to the latent quality concern that probably kept this PR from merging:
json.Marshal(&player)→json.Unmarshal(b, &u.RawData). Worked, butRawDatabecame a verbatim copy of the six already-typed fields (steamid, personaname, realname, avatarfull, loccountrycode, locstatecode). Steam-only fields stayed unreachable, which is exactly the gap his issue described.u.RawDatadirectly. Full Steam player payload reaches callers. Updated the test fixture to a realistic Steam response and locked the no-slot fields in with explicit RawData assertions so a future refactor can't silently shrink it again.His
httpTestTransportindirection and overall test layout are preserved — those were the right design.Backward compatibility
Strictly additive. Callers that previously read
user.UserID/user.Name/etc. see no change; callers that previously found an emptyuser.RawDatanow find a populated map with the full Steam player JSON. No field semantics moved.Tests
Added
Test_FetchUser(extending @leobrines's). Asserts:RawData(matches consumer expectations from other providers).goth.User—communityvisibilitystate,profilestate,personastate,primaryclanid,timecreated,profileurl— are inRawData. These are the ones consumers had no way to reach before and are the actual reasonRawDatamatters here.Verified the new RawData assertions fail on master (the no-slot fields come back
<nil>) and pass with this fix applied. Fullgo test ./...is green across all 50+ providers;go vet ./...is clean.Closes #518.