Skip to content

Commit cf249f7

Browse files
committed
Fix cross-ROM duplicate save counting
1 parent 10f1603 commit cf249f7

6 files changed

Lines changed: 291 additions & 28 deletions

File tree

backend/cmd/server/agent_api_handlers.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -484,6 +484,7 @@ func (a *app) aggregatedSaveSummaries(romSHA1, romMD5 string, systemID int) []sa
484484
saveCount int
485485
totalSizeBytes int
486486
projectionLine bool
487+
revisionKeys map[string]struct{}
487488
}
488489
aggregates := make(map[string]saveAggregate, len(filteredRecords))
489490
for _, record := range filteredRecords {
@@ -500,6 +501,9 @@ func (a *app) aggregatedSaveSummaries(romSHA1, romMD5 string, systemID int) []sa
500501
}
501502
key := canonicalListKeyForRecord(record)
502503
agg := aggregates[key]
504+
if agg.revisionKeys == nil {
505+
agg.revisionKeys = map[string]struct{}{}
506+
}
503507
if _, _, _, ok := playStationProjectionInfoFromRecord(record); ok {
504508
agg.projectionLine = true
505509
}
@@ -514,8 +518,12 @@ func (a *app) aggregatedSaveSummaries(romSHA1, romMD5 string, systemID int) []sa
514518
agg.totalSizeBytes = record.Summary.FileSize
515519
}
516520
} else {
517-
agg.saveCount++
518-
agg.totalSizeBytes += record.Summary.FileSize
521+
revisionKey := saveRecordRevisionIdentity(record)
522+
if _, seen := agg.revisionKeys[revisionKey]; !seen {
523+
agg.revisionKeys[revisionKey] = struct{}{}
524+
agg.saveCount++
525+
agg.totalSizeBytes += record.Summary.FileSize
526+
}
519527
}
520528
aggregates[key] = agg
521529
}

backend/cmd/server/save_cleanup.go

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -101,15 +101,15 @@ func planGenericSaveDuplicateCleanup(records []saveRecord, existingRemovals map[
101101
if _, _, _, ok := n64ControllerPakProjectionInfoFromRecord(record); ok {
102102
continue
103103
}
104-
key := strings.TrimSpace(canonicalVersionKeyForRecord(record))
104+
key := strings.TrimSpace(genericDuplicateCleanupKeyForRecord(record))
105105
if key == "" {
106106
continue
107107
}
108108
latest, ok := latestByKey[key]
109109
if !ok || saveRecordSortsAfter(record, latest) {
110110
latestByKey[key] = record
111111
}
112-
groupKey := key + "::" + strings.TrimSpace(record.Summary.SHA256)
112+
groupKey := key + "::sha:" + strings.TrimSpace(record.Summary.SHA256)
113113
groups[groupKey] = append(groups[groupKey], record)
114114
}
115115

@@ -120,7 +120,7 @@ func planGenericSaveDuplicateCleanup(records []saveRecord, existingRemovals map[
120120
continue
121121
}
122122
duplicateGroups++
123-
latest := latestByKey[strings.TrimSpace(canonicalVersionKeyForRecord(group[0]))]
123+
latest := latestByKey[strings.TrimSpace(genericDuplicateCleanupKeyForRecord(group[0]))]
124124
survivorID := selectSaveRecordDuplicateSurvivor(group, latest)
125125
for _, record := range group {
126126
if strings.TrimSpace(record.Summary.ID) == survivorID {
@@ -133,6 +133,13 @@ func planGenericSaveDuplicateCleanup(records []saveRecord, existingRemovals map[
133133
return duplicateGroups, duplicateRemoved
134134
}
135135

136+
func genericDuplicateCleanupKeyForRecord(record saveRecord) string {
137+
if key := strings.TrimSpace(canonicalDuplicateTrackKeyForRecord(record)); key != "" {
138+
return "track:" + key
139+
}
140+
return strings.TrimSpace(canonicalVersionKeyForRecord(record))
141+
}
142+
136143
func selectSaveRecordDuplicateSurvivor(group []saveRecord, latest saveRecord) string {
137144
latestID := strings.TrimSpace(latest.Summary.ID)
138145
for _, record := range group {

backend/cmd/server/save_dedupe.go

Lines changed: 118 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,18 @@ func payloadSHA256Hex(payload []byte) string {
2727
}
2828

2929
func latestSaveRecordForCanonicalKey(records []saveRecord, key string) (saveRecord, bool) {
30+
return latestSaveRecordForKey(records, key, canonicalVersionKeyForRecord)
31+
}
32+
33+
func latestSaveRecordForKey(records []saveRecord, key string, keyForRecord func(saveRecord) string) (saveRecord, bool) {
3034
cleanKey := strings.TrimSpace(key)
35+
if cleanKey == "" || keyForRecord == nil {
36+
return saveRecord{}, false
37+
}
3138
var latest saveRecord
3239
found := false
3340
for _, record := range records {
34-
if strings.TrimSpace(canonicalVersionKeyForRecord(record)) != cleanKey {
41+
if strings.TrimSpace(keyForRecord(record)) != cleanKey {
3542
continue
3643
}
3744
if !saveRecordPayloadExists(record) {
@@ -45,6 +52,25 @@ func latestSaveRecordForCanonicalKey(records []saveRecord, key string) (saveReco
4552
return latest, found
4653
}
4754

55+
func canonicalDuplicateTrackKeyForRecord(record saveRecord) string {
56+
track := canonicalTrackFromRecord(record)
57+
if canonicalTrackTitleKey(track.DisplayTitle) == "unknown game" {
58+
return ""
59+
}
60+
return canonicalTrackKey(track)
61+
}
62+
63+
func canonicalDuplicateTrackKeyForInput(input saveCreateInput, filename string) string {
64+
if strings.TrimSpace(input.Filename) == "" {
65+
input.Filename = filename
66+
}
67+
track := canonicalTrackFromInput(input)
68+
if canonicalTrackTitleKey(track.DisplayTitle) == "unknown game" {
69+
return ""
70+
}
71+
return canonicalTrackKey(track)
72+
}
73+
4874
func saveRecordSortsAfter(left, right saveRecord) bool {
4975
if left.Summary.Version != right.Summary.Version {
5076
return left.Summary.Version > right.Summary.Version
@@ -57,37 +83,69 @@ func saveRecordSortsAfter(left, right saveRecord) bool {
5783

5884
func checkUploadDuplicate(records []saveRecord, input saveCreateInput) uploadDuplicateCheck {
5985
filename := safeFilename(firstNonEmpty(input.Filename, "save.bin"))
60-
key := canonicalVersionKeyForInput(input, filename)
6186
shaHex := payloadSHA256Hex(input.Payload)
62-
latest, hasLatest := latestSaveRecordForCanonicalKey(records, key)
63-
if !hasLatest {
64-
return uploadDuplicateCheck{}
87+
88+
type duplicateScope struct {
89+
name string
90+
key string
91+
keyFor func(saveRecord) string
92+
}
93+
scopes := []duplicateScope{{
94+
name: "version",
95+
key: canonicalVersionKeyForInput(input, filename),
96+
keyFor: canonicalVersionKeyForRecord,
97+
}}
98+
if trackKey := canonicalDuplicateTrackKeyForInput(input, filename); trackKey != "" {
99+
scopes = append(scopes, duplicateScope{
100+
name: "track",
101+
key: trackKey,
102+
keyFor: canonicalDuplicateTrackKeyForRecord,
103+
})
65104
}
66105

67-
var matching saveRecord
68-
foundMatch := false
69-
for _, record := range records {
70-
if strings.TrimSpace(canonicalVersionKeyForRecord(record)) != strings.TrimSpace(key) {
106+
seenScopes := map[string]struct{}{}
107+
for _, scope := range scopes {
108+
scopeKey := strings.TrimSpace(scope.key)
109+
if scopeKey == "" {
71110
continue
72111
}
73-
if !saveRecordPayloadExists(record) {
112+
seenKey := scope.name + ":" + scopeKey
113+
if _, seen := seenScopes[seenKey]; seen {
74114
continue
75115
}
76-
if strings.TrimSpace(record.Summary.SHA256) != shaHex {
116+
seenScopes[seenKey] = struct{}{}
117+
118+
latest, hasLatest := latestSaveRecordForKey(records, scopeKey, scope.keyFor)
119+
if !hasLatest {
77120
continue
78121
}
79-
if !foundMatch || saveRecordSortsAfter(record, matching) {
80-
matching = record
81-
foundMatch = true
122+
123+
var matching saveRecord
124+
foundMatch := false
125+
for _, record := range records {
126+
if strings.TrimSpace(scope.keyFor(record)) != scopeKey {
127+
continue
128+
}
129+
if !saveRecordPayloadExists(record) {
130+
continue
131+
}
132+
if strings.TrimSpace(record.Summary.SHA256) != shaHex {
133+
continue
134+
}
135+
if !foundMatch || saveRecordSortsAfter(record, matching) {
136+
matching = record
137+
foundMatch = true
138+
}
82139
}
140+
if !foundMatch {
141+
continue
142+
}
143+
if strings.TrimSpace(latest.Summary.SHA256) == shaHex {
144+
return uploadDuplicateCheck{Disposition: uploadDuplicateIgnoredLatest, Latest: latest, Matching: latest, Found: true}
145+
}
146+
return uploadDuplicateCheck{Disposition: uploadDuplicateStaleHistorical, Latest: latest, Matching: matching, Found: true}
83147
}
84-
if !foundMatch {
85-
return uploadDuplicateCheck{}
86-
}
87-
if strings.TrimSpace(latest.Summary.SHA256) == shaHex {
88-
return uploadDuplicateCheck{Disposition: uploadDuplicateIgnoredLatest, Latest: latest, Matching: latest, Found: true}
89-
}
90-
return uploadDuplicateCheck{Disposition: uploadDuplicateStaleHistorical, Latest: latest, Matching: matching, Found: true}
148+
return uploadDuplicateCheck{}
91149
}
92150

93151
func duplicateUploadResponse(record saveRecord, disposition uploadDuplicateDisposition) map[string]any {
@@ -131,6 +189,45 @@ func saveRecordIsRollback(record saveRecord) bool {
131189
return metadataHasRollbackAudit(record.Summary.Metadata)
132190
}
133191

192+
func saveRecordRevisionIdentity(record saveRecord) string {
193+
id := strings.TrimSpace(record.Summary.ID)
194+
if saveRecordIsRollback(record) {
195+
return "rollback:" + id
196+
}
197+
if sha := strings.TrimSpace(record.Summary.SHA256); sha != "" {
198+
return "sha:" + sha
199+
}
200+
return "id:" + id
201+
}
202+
203+
func saveSummaryRevisionIdentity(summary saveSummary) string {
204+
id := strings.TrimSpace(summary.ID)
205+
if metadataHasRollbackAudit(summary.Metadata) {
206+
return "rollback:" + id
207+
}
208+
if sha := strings.TrimSpace(summary.SHA256); sha != "" {
209+
return "sha:" + sha
210+
}
211+
return "id:" + id
212+
}
213+
214+
func dedupeSaveSummaryRevisions(summaries []saveSummary) []saveSummary {
215+
if len(summaries) <= 1 {
216+
return summaries
217+
}
218+
seen := map[string]struct{}{}
219+
deduped := make([]saveSummary, 0, len(summaries))
220+
for _, summary := range summaries {
221+
key := saveSummaryRevisionIdentity(summary)
222+
if _, exists := seen[key]; exists {
223+
continue
224+
}
225+
seen[key] = struct{}{}
226+
deduped = append(deduped, summary)
227+
}
228+
return deduped
229+
}
230+
134231
func psLogicalRevisionIsRollback(revision psLogicalSaveRevision) bool {
135232
return strings.HasPrefix(strings.TrimSpace(revision.ImportID), "rollback:")
136233
}

backend/cmd/server/save_dedupe_test.go

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,59 @@ func TestGenericUploadHistoricalDuplicateReturnsConflict(t *testing.T) {
8282
}
8383
}
8484

85+
func TestGenericUploadDuplicateAcrossROMVariantsIsIgnored(t *testing.T) {
86+
h := newContractHarness(t)
87+
payload := buildTestN64Payload("eep", "cross-rom-duplicate")
88+
first := uploadSave(t, h, "/saves", map[string]string{
89+
"rom_sha1": "star-fox-retail-rom",
90+
"slotName": "slot-a",
91+
"system": "n64",
92+
}, "Star Fox 64 (USA).eep", payload)
93+
firstID := mustString(t, mustObject(t, first["save"], "save")["id"], "save.id")
94+
95+
second := h.multipart("/saves", map[string]string{
96+
"rom_sha1": "star-fox-rev1-rom",
97+
"slotName": "slot-a",
98+
"system": "n64",
99+
}, "file", "Star Fox 64 (USA) (Rev 1).eep", payload)
100+
assertStatus(t, second, http.StatusOK)
101+
body := decodeJSONMap(t, second.Body)
102+
if !mustBool(t, body["duplicate"], "duplicate") {
103+
t.Fatalf("expected cross-ROM duplicate response, got %s", prettyJSON(body))
104+
}
105+
if got := mustString(t, body["duplicateDisposition"], "duplicateDisposition"); got != string(uploadDuplicateIgnoredLatest) {
106+
t.Fatalf("expected ignored-latest duplicate disposition, got %q", got)
107+
}
108+
if got := mustString(t, mustObject(t, body["save"], "save")["id"], "save.id"); got != firstID {
109+
t.Fatalf("expected cross-ROM duplicate upload to return %q, got %q", firstID, got)
110+
}
111+
112+
list := h.request(http.MethodGet, "/saves?limit=50&offset=0", nil)
113+
assertStatus(t, list, http.StatusOK)
114+
saves := mustArray(t, decodeJSONMap(t, list.Body)["saves"], "saves")
115+
matches := 0
116+
for _, item := range saves {
117+
summary := mustObject(t, item, "saves[n]")
118+
if mustString(t, summary["displayTitle"], "displayTitle") != "Star Fox 64" {
119+
continue
120+
}
121+
matches++
122+
if got := mustNumber(t, summary["saveCount"], "saveCount"); got != 1 {
123+
t.Fatalf("expected one visible save after cross-ROM duplicate upload, got %s", prettyJSON(summary))
124+
}
125+
}
126+
if matches != 1 {
127+
t.Fatalf("expected one Star Fox row, got %d", matches)
128+
}
129+
130+
history := h.request(http.MethodGet, "/save?saveId="+url.QueryEscape(firstID), nil)
131+
assertStatus(t, history, http.StatusOK)
132+
versions := mustArray(t, decodeJSONMap(t, history.Body)["versions"], "versions")
133+
if len(versions) != 1 {
134+
t.Fatalf("expected one history version after cross-ROM duplicate upload, got %d", len(versions))
135+
}
136+
}
137+
85138
func TestPlayStationHistoricalDuplicateImportIsStale(t *testing.T) {
86139
h := newContractHarness(t)
87140
firstPayload := makeTestPS1Card(t, "SCUS_941.63", "Final Fantasy VII Save")
@@ -273,3 +326,74 @@ func TestRescanCleanupRemovesDuplicateVersionsButKeepsRollbackAudit(t *testing.T
273326
t.Fatalf("expected contiguous renumbered versions [3 2 1], got %v", versions)
274327
}
275328
}
329+
330+
func TestRescanCleanupRemovesDuplicateAcrossROMVariants(t *testing.T) {
331+
h := newContractHarness(t)
332+
payload := buildTestN64Payload("eep", "cleanup-cross-rom")
333+
createdAt := time.Date(2026, 4, 24, 12, 0, 0, 0, time.UTC)
334+
335+
first, err := h.app.createSave(saveCreateInput{
336+
Filename: "Star Fox 64 (USA).eep",
337+
Payload: payload,
338+
Game: fallbackGameFromFilename("Star Fox 64 (USA).eep"),
339+
Format: "eep",
340+
ROMSHA1: "cleanup-starfox-retail",
341+
SlotName: "slot-a",
342+
SystemSlug: "n64",
343+
CreatedAt: createdAt,
344+
})
345+
if err != nil {
346+
t.Fatalf("create first cross-ROM save: %v", err)
347+
}
348+
_, err = h.app.createSave(saveCreateInput{
349+
Filename: "Star Fox 64 (USA) (Rev 1).eep",
350+
Payload: payload,
351+
Game: fallbackGameFromFilename("Star Fox 64 (USA) (Rev 1).eep"),
352+
Format: "eep",
353+
ROMSHA1: "cleanup-starfox-rev1",
354+
SlotName: "slot-a",
355+
SystemSlug: "n64",
356+
CreatedAt: createdAt.Add(time.Minute),
357+
})
358+
if err != nil {
359+
t.Fatalf("create second cross-ROM save: %v", err)
360+
}
361+
if got := countRecordsByDuplicateTrackAndSHA(h.app.snapshotSaveRecords(), canonicalDuplicateTrackKeyForRecord(first), first.Summary.SHA256); got != 2 {
362+
t.Fatalf("expected 2 cross-ROM duplicate records before cleanup, got %d", got)
363+
}
364+
365+
result, err := h.app.rescanSaves(saveRescanOptions{DryRun: false, PruneUnsupported: true})
366+
if err != nil {
367+
t.Fatalf("rescan with cross-ROM duplicate cleanup: %v", err)
368+
}
369+
if result.DuplicateGroups < 1 || result.DuplicateVersionsRemoved < 1 {
370+
t.Fatalf("expected duplicate cleanup to report cross-ROM duplicate, got %+v", result)
371+
}
372+
records := h.app.snapshotSaveRecords()
373+
matching := make([]saveRecord, 0, 1)
374+
for _, record := range records {
375+
if canonicalDuplicateTrackKeyForRecord(record) == canonicalDuplicateTrackKeyForRecord(first) && record.Summary.SHA256 == first.Summary.SHA256 {
376+
matching = append(matching, record)
377+
}
378+
}
379+
if len(matching) != 1 {
380+
t.Fatalf("expected one surviving cross-ROM duplicate record after cleanup, got %d", len(matching))
381+
}
382+
if matching[0].Summary.SHA256 != first.Summary.SHA256 {
383+
t.Fatalf("expected surviving record to keep the duplicate payload")
384+
}
385+
}
386+
387+
func countRecordsByDuplicateTrackAndSHA(records []saveRecord, key, sha string) int {
388+
count := 0
389+
for _, record := range records {
390+
if canonicalDuplicateTrackKeyForRecord(record) != key {
391+
continue
392+
}
393+
if strings.TrimSpace(record.Summary.SHA256) != strings.TrimSpace(sha) {
394+
continue
395+
}
396+
count++
397+
}
398+
return count
399+
}

0 commit comments

Comments
 (0)