fix(dav): re-upload converted ext instead of UpdateFile (#84)
Release / GoReleaser (push) Skipped
Tests / Secret scan (gitleaks) (push) Skipped
Tests / Test (Go 1.25) (push) Skipped
Tests / Test (Go stable) (push) Skipped
Tests / Secret scan (gitleaks) (pull_request) Successful in 4s
Tests / Test (Go stable) (pull_request) Successful in 1m30s
Tests / Test (Go 1.25) (pull_request) Successful in 1m31s

UploadToFolderReplacing matched legacy->OOXML counterparts (f.xls vs the
saved f.xlsx) and updated them in place. UpdateFile replaces the body without
re-running OnlyOffice's server-side conversion, so the raw OLE2 .xls landed
under the stored .xlsx name and the file would not open (#84).

UpdateFile now runs only when the stored and local extensions are equal.
A conversion-equivalent match is deleted (all duplicates collapsed) and
uploaded afresh, so the server converts again. AssertNoFileConflict stays
conversion-aware for --no-replace and never had an UpdateFile path.
This commit is contained in:
2026-09-27 15:12:13 +01:00
parent 329c0b9561
commit aad6315731
2 changed files with 117 additions and 39 deletions
+75 -15
View File
@@ -73,14 +73,58 @@ func TestPlanUploadReplacementFreshUpload(t *testing.T) {
}
}
func TestPlanUploadReplacementUpdatesConvertedFile(t *testing.T) {
// TestPlanUploadReplacementUpdatesExactExt covers the in-place path: only a
// stored file with the same extension is updated via UpdateFile. PDF over PDF
// and OOXML over OOXML keep the id and rewrite the body.
func TestPlanUploadReplacementUpdatesExactExt(t *testing.T) {
tests := []struct {
name string
file *FileEntry
stem string
ext string
id string
}{
{
name: "xlsx over xlsx",
file: &FileEntry{ID: jsonNum("3799"), Title: strPtr("ES29-extracto.xlsx"), FileExst: strPtr(".xlsx")},
stem: "ES29-extracto", ext: ".xlsx", id: "3799",
},
{
name: "pdf over pdf",
file: &FileEntry{ID: jsonNum("42"), Title: strPtr("extracto.pdf"), FileExst: strPtr(".pdf")},
stem: "extracto", ext: ".pdf", id: "42",
},
{
name: "legacy xls over xls",
file: &FileEntry{ID: jsonNum("11"), Title: strPtr("legacy.xls"), FileExst: strPtr(".xls")},
stem: "legacy", ext: ".xls", id: "11",
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
plan := planUploadReplacement([]*FileEntry{tc.file}, tc.stem, tc.ext)
if plan.UpdateID != tc.id {
t.Fatalf("UpdateID = %q, want %q (same ext updates in place)", plan.UpdateID, tc.id)
}
if len(plan.DeleteIDs) != 0 {
t.Fatalf("DeleteIDs = %v, want none", plan.DeleteIDs)
}
})
}
}
// TestPlanUploadReplacementConvertedExtDeletesThenUploads is the regression for
// #84: UpdateFile does not re-run the server-side legacy→OOXML conversion, so a
// .xls upload must not overwrite a stored .xlsx in place (raw OLE2 under an
// .xlsx name). The stale counterpart is deleted and the file uploaded afresh.
func TestPlanUploadReplacementConvertedExtDeletesThenUploads(t *testing.T) {
xlsx := &FileEntry{ID: jsonNum("3799"), Title: strPtr("ES29-extracto.xlsx"), FileExst: strPtr(".xlsx")}
plan := planUploadReplacement([]*FileEntry{xlsx}, "ES29-extracto", ".xls")
if plan.UpdateID != "3799" {
t.Fatalf("UpdateID = %q, want 3799 (update the saved .xlsx in place)", plan.UpdateID)
if plan.UpdateID != "" {
t.Fatalf("UpdateID = %q, want empty (do not update across conversion)", plan.UpdateID)
}
if len(plan.DeleteIDs) != 0 {
t.Fatalf("DeleteIDs = %v, want none", plan.DeleteIDs)
if len(plan.DeleteIDs) != 1 || plan.DeleteIDs[0] != 3799 {
t.Fatalf("DeleteIDs = %v, want [3799] (delete the stale .xlsx before upload)", plan.DeleteIDs)
}
}
@@ -89,7 +133,7 @@ func TestPlanUploadReplacementCollapsesDuplicates(t *testing.T) {
newer := older.Add(time.Hour)
first := &FileEntry{ID: jsonNum("3799"), Title: strPtr("ES29-extracto.xlsx"), FileExst: strPtr(".xlsx"), Updated: &older}
second := &FileEntry{ID: jsonNum("3887"), Title: strPtr("ES29-extracto.xlsx"), FileExst: strPtr(".xlsx"), Updated: &newer}
plan := planUploadReplacement([]*FileEntry{first, second}, "ES29-extracto", ".xls")
plan := planUploadReplacement([]*FileEntry{first, second}, "ES29-extracto", ".xlsx")
if plan.UpdateID != "3887" {
t.Fatalf("UpdateID = %q, want the newest duplicate 3887", plan.UpdateID)
}
@@ -98,23 +142,39 @@ func TestPlanUploadReplacementCollapsesDuplicates(t *testing.T) {
}
}
// TestPlanUploadReplacementRepeatedXLS is the regression for #82: the second
// upload of the same .xls must find the server-converted .xlsx and update it,
// not append a second file.
// TestPlanUploadReplacementDeletesConvertedDuplicates: with a converted match
// every stale copy is deleted (there is no keeper — the fresh upload replaces
// them all).
func TestPlanUploadReplacementDeletesConvertedDuplicates(t *testing.T) {
first := &FileEntry{ID: jsonNum("3799"), Title: strPtr("ES29-extracto.xlsx"), FileExst: strPtr(".xlsx")}
second := &FileEntry{ID: jsonNum("3887"), Title: strPtr("ES29-extracto.xlsx"), FileExst: strPtr(".xlsx")}
plan := planUploadReplacement([]*FileEntry{first, second}, "ES29-extracto", ".xls")
if plan.UpdateID != "" {
t.Fatalf("UpdateID = %q, want empty", plan.UpdateID)
}
if len(plan.DeleteIDs) != 2 {
t.Fatalf("DeleteIDs = %v, want both stale .xlsx ids", plan.DeleteIDs)
}
}
// TestPlanUploadReplacementRepeatedXLS is the regression for #84: with the
// server-converted .xlsx already present, the second .xls upload deletes it and
// uploads anew so OnlyOffice converts again — UpdateFile would corrupt it.
func TestPlanUploadReplacementRepeatedXLS(t *testing.T) {
const stem = "ES29-extracto"
// First upload: nothing in the folder.
if plan := planUploadReplacement(nil, stem, ".xls"); plan.UpdateID != "" {
if plan := planUploadReplacement(nil, stem, ".xls"); plan.UpdateID != "" || len(plan.DeleteIDs) != 0 {
t.Fatalf("first upload plan = %+v, want create", plan)
}
// OnlyOffice converts .xls -> .xlsx on upload; second upload must match it.
// OnlyOffice converts .xls -> .xlsx on upload; the second upload must
// delete it and upload fresh, never UpdateFile it.
saved := &FileEntry{ID: jsonNum("3799"), Title: strPtr(stem + ".xlsx"), FileExst: strPtr(".xlsx")}
plan := planUploadReplacement([]*FileEntry{saved}, stem, ".xls")
if plan.UpdateID != "3799" {
t.Fatalf("second upload plan = %+v, want in-place update of 3799 (no duplicate)", plan)
if plan.UpdateID != "" {
t.Fatalf("second upload plan = %+v, want delete+upload (not UpdateFile)", plan)
}
if len(plan.DeleteIDs) != 0 {
t.Fatalf("second upload would delete %v, want none", plan.DeleteIDs)
if len(plan.DeleteIDs) != 1 || plan.DeleteIDs[0] != 3799 {
t.Fatalf("second upload DeleteIDs = %v, want [3799]", plan.DeleteIDs)
}
}