fix(gift): receive response is idempotent and echoes persisted state
Three coupled correctness fixes to /tutorial/gift_receive's response: - received_ids / total_receive_count_list / reward_list are now built from `toClaim` (the gifts THIS call granted), not from `requestedIds`. Echoing the client's request meant idempotent re-claims re-fired the "+N received" popup and direct-assigned the same post-state totals again, breaking the documented idempotency contract. - is_unreceived_present is now `unclaimedPresents.Count > 0`. The hardcoded false hid the inbox badge after partial claims even when present_list still carried unclaimed gifts. - tutorial_step echoes the persisted (max-preserved) state instead of a hardcoded 41. A replay against a state>=41 viewer used to surface 41 on the wire and regress the client's tutorial state machine. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -136,13 +136,32 @@ public class GiftController : SVSimController
|
|||||||
.ToListAsync();
|
.ToListAsync();
|
||||||
var allClaimed = new HashSet<string>(allClaimedList);
|
var allClaimed = new HashSet<string>(allClaimedList);
|
||||||
|
|
||||||
|
// Derive presentList/historyList up front so IsUnreceivedPresent can read the count
|
||||||
|
// without re-filtering. unclaimedPresents are the gifts still on offer after this call;
|
||||||
|
// claimedPresents are everything the viewer has ever received (this call + prior calls).
|
||||||
|
var unclaimedPresents = TutorialGifts
|
||||||
|
.Where(p => !allClaimed.Contains(p.PresentId))
|
||||||
|
.Select(p => Clone(p, nowString))
|
||||||
|
.ToList();
|
||||||
|
var claimedPresents = TutorialGifts
|
||||||
|
.Where(p => allClaimed.Contains(p.PresentId))
|
||||||
|
.Select(p => Clone(p, nowString))
|
||||||
|
.ToList();
|
||||||
|
|
||||||
return new GiftReceiveResponse
|
return new GiftReceiveResponse
|
||||||
{
|
{
|
||||||
CardList = new(),
|
CardList = new(),
|
||||||
// Capture orders received_ids ascending — match.
|
// Echo only the ids actually granted by THIS call. Building this from `requestedIds`
|
||||||
ReceivedIds = requestedIds.OrderBy(x => x).ToList(),
|
// would falsely confirm a re-grant on idempotent retries: the client would re-show
|
||||||
TotalReceiveCountList = TutorialGifts
|
// the "received N gifts" popup and direct-assign the same post-state totals it already
|
||||||
.Where(p => requestedIds.Contains(p.PresentId))
|
// applied, double-toasting the user. Sort ascending to match the prod-capture order.
|
||||||
|
ReceivedIds = toClaim
|
||||||
|
.Select(p => p.PresentId)
|
||||||
|
.OrderBy(x => x)
|
||||||
|
.ToList(),
|
||||||
|
// Same idempotency contract: only the gifts granted in THIS call belong in the
|
||||||
|
// per-reward summary list. The client uses this to drive the +N popups.
|
||||||
|
TotalReceiveCountList = toClaim
|
||||||
.Select(p => new TotalReceiveCountDto
|
.Select(p => new TotalReceiveCountDto
|
||||||
{
|
{
|
||||||
RewardType = int.Parse(p.RewardType),
|
RewardType = int.Parse(p.RewardType),
|
||||||
@@ -151,22 +170,22 @@ public class GiftController : SVSimController
|
|||||||
ItemType = p.ItemType ?? 0,
|
ItemType = p.ItemType ?? 0,
|
||||||
IsUsable = true,
|
IsUsable = true,
|
||||||
}).ToList(),
|
}).ToList(),
|
||||||
PresentList = TutorialGifts
|
PresentList = unclaimedPresents,
|
||||||
.Where(p => !allClaimed.Contains(p.PresentId))
|
PresentHistoryList = claimedPresents,
|
||||||
.Select(p => Clone(p, nowString))
|
// True when there are still unclaimed gifts on offer — drives the inbox badge state.
|
||||||
.ToList(),
|
// Hardcoding false hid the badge after partial claims even though present_list still
|
||||||
PresentHistoryList = TutorialGifts
|
// carried unclaimed entries.
|
||||||
.Where(p => allClaimed.Contains(p.PresentId))
|
IsUnreceivedPresent = unclaimedPresents.Count > 0,
|
||||||
.Select(p => Clone(p, nowString))
|
|
||||||
.ToList(),
|
|
||||||
IsUnreceivedPresent = false,
|
|
||||||
// reward_list entries must carry POST-STATE TOTALS, not gift deltas.
|
// reward_list entries must carry POST-STATE TOTALS, not gift deltas.
|
||||||
// The client's PlayerStaticData.UpdateHaveUserGoodsNumByJsonData does direct
|
// The client's PlayerStaticData.UpdateHaveUserGoodsNumByJsonData does direct
|
||||||
// assignment on each entry's reward_num — emitting the delta would clobber
|
// assignment on each entry's reward_num — emitting the delta would clobber
|
||||||
// the client-side cached balance down to the gift amount until the next /load/index.
|
// the client-side cached balance down to the gift amount until the next /load/index.
|
||||||
// See project memory: project_wire_reward_list_post_state.
|
// See project memory: project_wire_reward_list_post_state.
|
||||||
RewardList = TutorialGifts
|
//
|
||||||
.Where(p => requestedIds.Contains(p.PresentId))
|
// Iterate `toClaim` so idempotent re-receive doesn't re-emit post-state entries
|
||||||
|
// the client would direct-assign again (no-op on currency, but redundant traffic
|
||||||
|
// and risk of misinterpretation on item counts).
|
||||||
|
RewardList = toClaim
|
||||||
.Select(p => new GiftRewardListEntry
|
.Select(p => new GiftRewardListEntry
|
||||||
{
|
{
|
||||||
RewardType = p.RewardType,
|
RewardType = p.RewardType,
|
||||||
@@ -174,7 +193,11 @@ public class GiftController : SVSimController
|
|||||||
RewardNum = ResolvePostStateRewardNum(p, viewer),
|
RewardNum = ResolvePostStateRewardNum(p, viewer),
|
||||||
})
|
})
|
||||||
.ToList(),
|
.ToList(),
|
||||||
TutorialStep = 41,
|
// Echo the persisted state, not a hardcoded 41. The state may already be past 41
|
||||||
|
// for replay/edge-case calls (the Math.Max-preserve block above keeps it stable);
|
||||||
|
// emitting 41 anyway would surface a regressed step to the client and desync the
|
||||||
|
// tutorial-state machine.
|
||||||
|
TutorialStep = viewer.MissionData.TutorialState,
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -112,13 +112,73 @@ public class GiftControllerTests
|
|||||||
|
|
||||||
// Response carries the new step inline.
|
// Response carries the new step inline.
|
||||||
Assert.That(root.GetProperty("tutorial_step").GetInt32(), Is.EqualTo(41));
|
Assert.That(root.GetProperty("tutorial_step").GetInt32(), Is.EqualTo(41));
|
||||||
Assert.That(root.GetProperty("is_unreceived_present").GetBoolean(), Is.False);
|
// Only 1 of 5 gifts claimed → 4 remain unclaimed → badge state must be "still has presents".
|
||||||
|
Assert.That(root.GetProperty("is_unreceived_present").GetBoolean(), Is.True,
|
||||||
|
"Partial claim leaves 4 gifts unclaimed in present_list — is_unreceived_present " +
|
||||||
|
"must reflect that so the client's inbox badge keeps surfacing.");
|
||||||
Assert.That(root.GetProperty("reward_list").GetArrayLength(), Is.EqualTo(1));
|
Assert.That(root.GetProperty("reward_list").GetArrayLength(), Is.EqualTo(1));
|
||||||
|
Assert.That(root.GetProperty("present_list").GetArrayLength(), Is.EqualTo(4));
|
||||||
|
|
||||||
// Side effect: viewer state advanced to 41.
|
// Side effect: viewer state advanced to 41.
|
||||||
Assert.That(await factory.GetViewerTutorialStateAsync(viewerId), Is.EqualTo(41));
|
Assert.That(await factory.GetViewerTutorialStateAsync(viewerId), Is.EqualTo(41));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Test]
|
||||||
|
public async Task GiftReceive_returns_empty_received_ids_on_idempotent_replay()
|
||||||
|
{
|
||||||
|
using var factory = new SVSimTestFactory();
|
||||||
|
await factory.SeedGlobalsAsync();
|
||||||
|
long viewerId = await factory.SeedViewerAsync(tutorialState: 31);
|
||||||
|
using var client = factory.CreateAuthenticatedClient(viewerId);
|
||||||
|
|
||||||
|
var json = $$"""{"present_id_array":["71478626","71478627"],"state":1,{{BaseAuthBlock}}}""";
|
||||||
|
|
||||||
|
// First call grants both gifts.
|
||||||
|
await client.PostAsync("/tutorial/gift_receive",
|
||||||
|
new StringContent(json, Encoding.UTF8, "application/json"));
|
||||||
|
|
||||||
|
// Second call (replay) must return empty received_ids / total_receive_count_list /
|
||||||
|
// reward_list — these lists describe what THIS call granted, not what the client
|
||||||
|
// asked for. Echoing requested ids would re-fire the client's "received N gifts"
|
||||||
|
// popup and direct-assign the same post-state totals again.
|
||||||
|
var second = await client.PostAsync("/tutorial/gift_receive",
|
||||||
|
new StringContent(json, Encoding.UTF8, "application/json"));
|
||||||
|
Assert.That(second.StatusCode, Is.EqualTo(HttpStatusCode.OK));
|
||||||
|
using var doc = JsonDocument.Parse(await second.Content.ReadAsStringAsync());
|
||||||
|
var root = doc.RootElement;
|
||||||
|
|
||||||
|
Assert.That(root.GetProperty("received_ids").GetArrayLength(), Is.EqualTo(0),
|
||||||
|
"Idempotent re-claim grants nothing → received_ids empty.");
|
||||||
|
Assert.That(root.GetProperty("total_receive_count_list").GetArrayLength(), Is.EqualTo(0));
|
||||||
|
Assert.That(root.GetProperty("reward_list").GetArrayLength(), Is.EqualTo(0));
|
||||||
|
|
||||||
|
// present_history_list still includes the originally-claimed gifts.
|
||||||
|
Assert.That(root.GetProperty("present_history_list").GetArrayLength(), Is.EqualTo(2));
|
||||||
|
}
|
||||||
|
|
||||||
|
[Test]
|
||||||
|
public async Task GiftReceive_echoes_persisted_tutorial_step_not_hardcoded_41()
|
||||||
|
{
|
||||||
|
using var factory = new SVSimTestFactory();
|
||||||
|
await factory.SeedGlobalsAsync();
|
||||||
|
// Viewer is past the tutorial entirely (state=100). The gift_receive endpoint is
|
||||||
|
// still reachable via /tutorial/gift_receive — a stale client retry, for instance.
|
||||||
|
// The persistence side max-preserves (keeps state at 100); the response must echo
|
||||||
|
// 100, not the hardcoded 41 the endpoint used to emit, or the client's tutorial
|
||||||
|
// state machine regresses on a no-op retry.
|
||||||
|
long viewerId = await factory.SeedViewerAsync(tutorialState: 100);
|
||||||
|
using var client = factory.CreateAuthenticatedClient(viewerId);
|
||||||
|
|
||||||
|
var json = $$"""{"present_id_array":["71478626"],"state":1,{{BaseAuthBlock}}}""";
|
||||||
|
var response = await client.PostAsync("/tutorial/gift_receive",
|
||||||
|
new StringContent(json, Encoding.UTF8, "application/json"));
|
||||||
|
|
||||||
|
Assert.That(response.StatusCode, Is.EqualTo(HttpStatusCode.OK));
|
||||||
|
using var doc = JsonDocument.Parse(await response.Content.ReadAsStringAsync());
|
||||||
|
Assert.That(doc.RootElement.GetProperty("tutorial_step").GetInt32(), Is.EqualTo(100));
|
||||||
|
Assert.That(await factory.GetViewerTutorialStateAsync(viewerId), Is.EqualTo(100));
|
||||||
|
}
|
||||||
|
|
||||||
[Test]
|
[Test]
|
||||||
public async Task GiftReceive_with_pre_owned_item_increments_existing_row()
|
public async Task GiftReceive_with_pre_owned_item_increments_existing_row()
|
||||||
{
|
{
|
||||||
|
|||||||
Reference in New Issue
Block a user