eSlider/go-onlyoffice
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 3 Wiki Activity

fix(dav): re-upload converted ext instead of UpdateFile (#84) #85

Merged
eSlider merged 1 commits from fix/dav-updatefile-convert#84 into main 2026-09-27 15:12:36 +01:00
Conversation 0 Commits 1 Files Changed 2
+117 -39
2 changed files with 117 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files
files_replace_test.go
+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)
}
}
files_stem.go
+42 -24
View File
@@ -158,18 +158,24 @@ func (c *Client) UploadProjectFileNoClobber(ctx context.Context, projectID, loca
// uploadReplacementPlan is how a replacing upload reconciles with a folder.
type uploadReplacementPlan struct {
// UpdateID is the existing file to overwrite in place; empty when no file
// matches the upload stem and (converted) extension.
// UpdateID is the existing file to overwrite in place; set only when the
// stored extension equals the local one. UpdateFile keeps the stored name,
// so overwriting across extensions would leave an unconverted body under a
// mismatched name.
UpdateID string
// DeleteIDs are redundant duplicate ids to remove after the update.
// DeleteIDs are the ids to remove before uploading. They are the redundant
// duplicates of an in-place update, or every conversion-equivalent
// counterpart when the body must be converted again by a fresh upload.
DeleteIDs []int
}
// planUploadReplacement matches an incoming local upload (stem + ext) against
// the files already in a folder and decides between a fresh upload, an in-place
// update and duplicate cleanup. Matching tolerates the legacy→OOXML conversion
// update and delete + reupload. Matching tolerates the legacy→OOXML conversion
// OnlyOffice performs on upload, so a repeated upload of f.xls finds the saved
// f.xlsx instead of creating a second file.
// f.xlsx instead of creating a second file. Because UpdateFile replaces the body
// without re-running that conversion, an equivalent-but-different extension is
// deleted and re-uploaded rather than updated in place (#84).
func planUploadReplacement(files []*FileEntry, stem, ext string) uploadReplacementPlan {
matches := FindFilesByStemExt(files, stem, ext)
if len(matches) == 0 {
@@ -177,8 +183,12 @@ func planUploadReplacement(files []*FileEntry, stem, ext string) uploadReplaceme
}
keep, remove := pickDuplicateKeeper(matches, false)
plan := uploadReplacementPlan{}
if id := FileEntryNumericID(keep); id != 0 {
plan.UpdateID = strconv.FormatInt(id, 10)
if FileEntryExt(keep) == normalizeExt(ext) {
if id := FileEntryNumericID(keep); id != 0 {
plan.UpdateID = strconv.FormatInt(id, 10)
}
} else if id := int(FileEntryNumericID(keep)); id != 0 {
plan.DeleteIDs = append(plan.DeleteIDs, id)
}
for _, f := range remove {
if id := int(FileEntryNumericID(f)); id != 0 {
@@ -190,11 +200,11 @@ func planUploadReplacement(files []*FileEntry, stem, ext string) uploadReplaceme
// UploadToFolderReplacing upserts localPath into folderID by logical name.
// Matching is stem + extension with OnlyOffice's server-side conversion
// accounted for: a local .xls is stored as .xlsx, so a repeated upload
// overwrites the saved document instead of appending a duplicate. When a
// counterpart exists it is updated in place (stable id, no window without the
// file) and any extra duplicates are removed; portals that reject an in-place
// update fall back to delete + fresh upload, which still leaves one file.
// accounted for: a local .xls is stored as .xlsx, so a repeated upload replaces
// the saved document instead of appending a duplicate. An exact-extension
// counterpart is updated in place (stable id, no window without the file); a
// conversion-equivalent one is deleted and uploaded afresh so the server
// converts the body again. Extra duplicates are collapsed either way.
func (c *Client) UploadToFolderReplacing(ctx context.Context, folderID, localPath string) (*FileEntry, []int, error) {
stem := UploadStemFromLocal(localPath)
ext := UploadExtFromLocal(localPath)
@@ -203,12 +213,15 @@ func (c *Client) UploadToFolderReplacing(ctx context.Context, folderID, localPat
return nil, nil, err
}
plan := planUploadReplacement(files, stem, ext)
if plan.UpdateID == "" {
ent, err := c.UploadToFolder(ctx, folderID, localPath)
return ent, nil, err
}
ent, err := c.UpdateFile(ctx, plan.UpdateID, localPath)
if err != nil {
if plan.UpdateID != "" {
ent, err := c.UpdateFile(ctx, plan.UpdateID, localPath)
if err == nil {
deleted, derr := c.deleteReplacementStale(ctx, plan.DeleteIDs)
return ent, deleted, derr
}
// Portal rejected the in-place update: delete + fresh upload still
// leaves one file (server-converted).
deleted, derr := c.DeleteFilesByStemExt(ctx, folderID, stem, ext)
if derr != nil {
return nil, deleted, derr
@@ -216,16 +229,21 @@ func (c *Client) UploadToFolderReplacing(ctx context.Context, folderID, localPat
ent, uerr := c.UploadToFolder(ctx, folderID, localPath)
return ent, deleted, uerr
}
deleted, derr := c.deleteReplacementExtras(ctx, plan.DeleteIDs)
// Conversion-equivalent (or no counterpart): remove the stale files first so
// the fresh upload is converted and exactly one file remains.
deleted, derr := c.deleteReplacementStale(ctx, plan.DeleteIDs)
if derr != nil {
return ent, deleted, derr
return nil, deleted, derr
}
return ent, deleted, nil
ent, uerr := c.UploadToFolder(ctx, folderID, localPath)
return ent, deleted, uerr
}
// deleteReplacementExtras removes redundant duplicate ids collected by
// planUploadReplacement after the surviving file was updated in place.
func (c *Client) deleteReplacementExtras(ctx context.Context, ids []int) ([]int, error) {
// deleteReplacementStale removes the ids collected by planUploadReplacement:
// duplicate files after an in-place update, or every stale counterpart before a
// converted reupload.
func (c *Client) deleteReplacementStale(ctx context.Context, ids []int) ([]int, error) {
if len(ids) == 0 {
return nil, nil
}