fix(dav): upload --replace matches server-converted ext (xls→xlsx) (#82) #83

Merged
eSlider merged 1 commits from fix/dav-replace-convert#82 into main 2026-09-27 15:06:17 +01:00
8 changed files with 290 additions and 17 deletions
Showing only changes of commit 74e72d0e06 - Show all commits
+1 -1
View File
@@ -30,7 +30,7 @@ Canonical Go client for OnlyOffice Workspace (Projects + Calendar + CRM) and the
- Prefer `ResponseObject` / `postFormObject` / `putFormObject` / `deleteObject` over hand-rolled `json.Unmarshal(responseField(...))` blocks — they exist for DRY, use them.
- Domain split is by file, **not** by subpackage. Don't introduce `internal/` or `pkg/*` subpackages inside the library — it flattens the `*Client` call surface for a reason.
- CLI commands follow **subject → verb** structure (`oo <subject> <verb>`), never `oo <verb>-<subject>`. Add new commands to the existing subject file if one fits; create a new `cmd/oo/<subject>.go` for a genuinely new domain. The subject→verb tree in `cmd/oo/main.go` and the README table are documentation — update them with the code.
- **Documents for agents:** prefer Markdown in git; OnlyOffice UI is weak for `.md`/`.txt`. Use `oo docs put-md` (md→docx) and `oo docs put-txt` (txt→docx, preserves line breaks). All upload paths default to **upsert** by `stem|ext` (`--replace`, default true); `--no-replace` fails on conflict; `--allow-duplicate` opts into raw OO append. `oo projects files dedupe PROJECT_ID` reports/removes duplicate stem|ext copies (`--apply`, `--cross`; includes project root folder).
- **Documents for agents:** prefer Markdown in git; OnlyOffice UI is weak for `.md`/`.txt`. Use `oo docs put-md` (md→docx) and `oo docs put-txt` (txt→docx, preserves line breaks). All upload paths default to **upsert** by `stem` with the extension matched conversion-aware (legacy `.xls/.doc/.ppt` ↔ OOXML `.xlsx/.docx/.pptx`, since OnlyOffice converts them on upload), so a repeated `.xls` upload updates the saved `.xlsx` instead of appending a duplicate (`--replace`, default true); `--no-replace` fails on conflict; `--allow-duplicate` opts into raw OO append. `oo projects files dedupe PROJECT_ID` reports/removes duplicate stem|ext copies (`--apply`, `--cross`; includes project root folder).
- Every table output goes through `printTable(headers, rows)`; every single-object through `printObject(v)`. Do not `fmt.Println` rows ad-hoc or the `--output json` flag breaks for that command.
- No secrets in the repo; use `.env` (gitignored). Commit `.env.example` only.
- **No host/client specifics in the tree.** Endpoints, IPs/ports, client mail
+1 -1
View File
@@ -743,7 +743,7 @@ oo dav ls @root # virtual sections (Documents, Projects,
oo dav mkdir 659 "2026 inbox"
oo dav ensure-path "Banks/Caixa" # resolve-or-create; idempotent, prints the folder id
oo dav ensure-path "Banks/Caixa" --under 659 # start under an explicit folder id
oo dav upload 659 ./historico.xlsx # multipart upload, upsert by stem|ext
oo dav upload 659 ./historico.xlsx # multipart upload, upsert by stem (legacy↔OOXML ext aware)
oo dav upload 659 ./a.xlsx ./b.pdf --replace=false # fail on name conflict instead of replacing
oo dav move 659 22881 22882 # DEST_FOLDER_ID FILE_ID…
oo dav move 659 22881 --folders 670 # move folders along with files
+5 -4
View File
@@ -253,7 +253,7 @@ func davUploadCmd() *cobra.Command {
return nil
},
}
cmd.Flags().BoolVar(&replace, "replace", true, "replace same stem|ext before upload (default); false = fail if name taken")
cmd.Flags().BoolVar(&replace, "replace", true, "replace the same logical file (stem, legacy↔OOXML ext) before upload (default); false = fail if name taken")
return cmd
}
@@ -271,9 +271,10 @@ type uploader interface {
UploadToFolder(ctx context.Context, folderID, localPath string) (*onlyoffice.FileEntry, error)
}
// uploadLocal uploads each local file into folderID. With replace a same
// stem|ext file is removed first (the UploadToFolderReplacing upsert); without
// replace a name clash fails with ErrFileExists before anything is sent.
// uploadLocal uploads each local file into folderID. With replace the existing
// same logical file is overwritten in place (the UploadToFolderReplacing
// upsert, which matches the server-converted extension too); without replace a
// name clash fails with ErrFileExists before anything is sent.
func uploadLocal(ctx context.Context, up uploader, folderID string, paths []string, replace bool) ([]uploadedFile, error) {
results := make([]uploadedFile, 0, len(paths))
for _, path := range paths {
+5 -3
View File
@@ -113,9 +113,11 @@ func prjFilesUploadCmd() *cobra.Command {
var replace, allowDuplicate bool
cmd := &cobra.Command{
Use: "upload PROJECT_ID LOCAL_PATH [LOCAL_PATH...]",
Short: "Upload file(s) into the project's Documents folder (upsert by stem|ext)",
Long: `Default: replace an existing file with the same logical name (stem|ext), like cp overwrite.
Pass --no-replace to fail when the name is taken; --allow-duplicate to always create a new file id.`,
Short: "Upload file(s) into the project's Documents folder (upsert by stem, legacy↔OOXML ext)",
Long: `Default: replace an existing file with the same logical name (stem, treating
legacy .xls/.doc/.ppt and their OOXML .xlsx/.docx/.pptx as one file), like cp
overwrite. Pass --no-replace to fail when the name is taken; --allow-duplicate
to always create a new file id.`,
Args: cobra.MinimumNArgs(2),
RunE: func(cmd *cobra.Command, args []string) error {
c, err := newOO(cmd)
+2 -1
View File
@@ -237,7 +237,8 @@ func (c *Client) UploadProjectFile(ctx context.Context, projectID, localPath str
return decodeResponseFileEntry(raw)
}
// UploadProjectFileReplacing upserts by stem|ext in the project Documents folder.
// UploadProjectFileReplacing upserts by stem and (server-converted) extension
// in the project Documents folder.
func (c *Client) UploadProjectFileReplacing(ctx context.Context, projectID, localPath string) (*FileEntry, []int, error) {
folderID, err := c.projectFolderID(ctx, projectID)
if err != nil {
+79
View File
@@ -83,6 +83,85 @@ func UploadExtFromLocal(localPath string) string {
return strings.ToLower(ext)
}
// legacyToOOXMLExt maps the legacy binary Office extensions OnlyOffice accepts
// on upload to the OOXML extension the server converts them into.
var legacyToOOXMLExt = map[string]string{
".xls": ".xlsx",
".doc": ".docx",
".ppt": ".pptx",
}
// normalizeExt lowercases an extension and ensures a leading dot.
func normalizeExt(ext string) string {
ext = strings.ToLower(strings.TrimSpace(ext))
if ext == "" {
return ""
}
if !strings.HasPrefix(ext, ".") {
ext = "." + ext
}
return ext
}
// EquivalentUploadExt reports whether two extensions designate the same
// document once the server-side conversion is taken into account: equal
// extensions, or a legacy binary Office format and its OOXML equivalent
// (.xls/.xlsx, .doc/.docx, .ppt/.pptx). Empty extensions only match each other.
func EquivalentUploadExt(a, b string) bool {
na, nb := normalizeExt(a), normalizeExt(b)
if na == nb {
return true
}
if na == "" || nb == "" {
return false
}
return legacyToOOXMLExt[na] == nb || legacyToOOXMLExt[nb] == na
}
// FindFilesByStemExt returns folder files matching stem and a same-or-converted
// extension (see EquivalentUploadExt). Unlike FindFilesByDedupKey, foo.xls and
// foo.xlsx are one logical file, so a replacing upload after OnlyOffice's
// legacy→OOXML conversion finds the saved file instead of appending a copy.
func FindFilesByStemExt(files []*FileEntry, stem, ext string) []*FileEntry {
stem = strings.TrimSpace(stem)
if stem == "" {
return nil
}
var out []*FileEntry
for _, f := range files {
if FileEntryStem(f) != stem {
continue
}
if EquivalentUploadExt(FileEntryExt(f), ext) {
out = append(out, f)
}
}
return out
}
// DeleteFilesByStemExt removes every folder file matching stem and a
// same-or-converted extension (legacy ↔ OOXML).
func (c *Client) DeleteFilesByStemExt(ctx context.Context, folderID, stem, ext string) ([]int, error) {
files, err := c.FolderFiles(ctx, folderID)
if err != nil {
return nil, err
}
matches := FindFilesByStemExt(files, stem, ext)
ids := make([]int, 0, len(matches))
for _, f := range matches {
if n := int(FileEntryNumericID(f)); n != 0 {
ids = append(ids, n)
}
}
if len(ids) == 0 {
return nil, nil
}
if err := c.DeleteFiles(ctx, ids); err != nil {
return nil, err
}
return ids, nil
}
// IsTrashFolderTitle reports staging/trash folders (e.g. _trash-md).
func IsTrashFolderTitle(title string) bool {
t := strings.ToLower(strings.TrimSpace(title))
+120
View File
@@ -0,0 +1,120 @@
package onlyoffice
import (
"testing"
"time"
)
func TestEquivalentUploadExt(t *testing.T) {
tests := []struct {
a, b string
want bool
}{
{".xls", ".xlsx", true},
{".XLS", ".xlsx", true},
{"xls", "xlsx", true},
{".doc", ".docx", true},
{".ppt", ".pptx", true},
{".pdf", ".pdf", true},
{"", "", true},
{".xls", ".docx", false},
{".xlsx", "", false},
{".csv", ".xlsx", false},
}
for _, tc := range tests {
if got := EquivalentUploadExt(tc.a, tc.b); got != tc.want {
t.Errorf("EquivalentUploadExt(%q,%q)=%v want %v", tc.a, tc.b, got, tc.want)
}
if got := EquivalentUploadExt(tc.b, tc.a); got != tc.want {
t.Errorf("EquivalentUploadExt(%q,%q)=%v want %v (symmetric)", tc.b, tc.a, got, tc.want)
}
}
}
func TestFindFilesByStemExtMatchesConvertedXLS(t *testing.T) {
xlsx := &FileEntry{ID: jsonNum("3799"), Title: strPtr("ES29-extracto.xlsx"), FileExst: strPtr(".xlsx")}
pdf := &FileEntry{ID: jsonNum("5"), Title: strPtr("ES29-extracto.pdf"), FileExst: strPtr(".pdf")}
other := &FileEntry{ID: jsonNum("3887"), Title: strPtr("ES87-extracto.xlsx"), FileExst: strPtr(".xlsx")}
files := []*FileEntry{xlsx, pdf, other}
got := FindFilesByStemExt(files, "ES29-extracto", ".xls")
if len(got) != 1 || got[0] != xlsx {
t.Fatalf("converted .xls match = %+v, want the saved .xlsx only", got)
}
}
func TestFindFilesByStemExtKeepsExactMatch(t *testing.T) {
xlsx := &FileEntry{ID: jsonNum("3799"), Title: strPtr("foo.xlsx"), FileExst: strPtr(".xlsx")}
pdf := &FileEntry{ID: jsonNum("5"), Title: strPtr("foo.pdf"), FileExst: strPtr(".pdf")}
files := []*FileEntry{xlsx, pdf}
if got := FindFilesByStemExt(files, "foo", ".xlsx"); len(got) != 1 || got[0] != xlsx {
t.Fatalf("exact .xlsx match = %+v, want only xlsx", got)
}
if got := FindFilesByStemExt(files, "foo", ".pdf"); len(got) != 1 || got[0] != pdf {
t.Fatalf("exact .pdf match = %+v, want only pdf", got)
}
if got := FindFilesByStemExt(files, "foo", ".docx"); len(got) != 0 {
t.Fatalf("unrelated ext matched %+v, want none", got)
}
}
func TestFindFilesByStemExtEmptyStem(t *testing.T) {
f := &FileEntry{ID: jsonNum("1"), Title: strPtr("foo.xlsx"), FileExst: strPtr(".xlsx")}
if got := FindFilesByStemExt([]*FileEntry{f}, "", ".xlsx"); len(got) != 0 {
t.Fatalf("empty stem matched %+v, want none", got)
}
}
func TestPlanUploadReplacementFreshUpload(t *testing.T) {
plan := planUploadReplacement(nil, "ES29-extracto", ".xls")
if plan.UpdateID != "" || len(plan.DeleteIDs) != 0 {
t.Fatalf("empty folder plan = %+v, want a fresh upload", plan)
}
}
func TestPlanUploadReplacementUpdatesConvertedFile(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 len(plan.DeleteIDs) != 0 {
t.Fatalf("DeleteIDs = %v, want none", plan.DeleteIDs)
}
}
func TestPlanUploadReplacementCollapsesDuplicates(t *testing.T) {
older := time.Date(2026, 9, 1, 10, 0, 0, 0, time.UTC)
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")
if plan.UpdateID != "3887" {
t.Fatalf("UpdateID = %q, want the newest duplicate 3887", plan.UpdateID)
}
if len(plan.DeleteIDs) != 1 || plan.DeleteIDs[0] != 3799 {
t.Fatalf("DeleteIDs = %v, want [3799]", plan.DeleteIDs)
}
}
// 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.
func TestPlanUploadReplacementRepeatedXLS(t *testing.T) {
const stem = "ES29-extracto"
// First upload: nothing in the folder.
if plan := planUploadReplacement(nil, stem, ".xls"); plan.UpdateID != "" {
t.Fatalf("first upload plan = %+v, want create", plan)
}
// OnlyOffice converts .xls -> .xlsx on upload; second upload must match 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 len(plan.DeleteIDs) != 0 {
t.Fatalf("second upload would delete %v, want none", plan.DeleteIDs)
}
}
+76 -6
View File
@@ -6,6 +6,7 @@ import (
"errors"
"fmt"
"path/filepath"
"strconv"
"strings"
)
@@ -121,7 +122,9 @@ func (c *Client) DeleteFilesByStem(ctx context.Context, folderID, stem string) (
return ids, nil
}
// AssertNoFileConflict reports ErrFileExists when localPath stem|ext is already in folderID.
// AssertNoFileConflict reports ErrFileExists when localPath logical name is
// already in folderID. Matching is conversion-aware, so a .xls upload also
// conflicts with an existing .xlsx and does not create a hidden duplicate.
func (c *Client) AssertNoFileConflict(ctx context.Context, folderID, localPath string) error {
files, err := c.FolderFiles(ctx, folderID)
if err != nil {
@@ -129,7 +132,7 @@ func (c *Client) AssertNoFileConflict(ctx context.Context, folderID, localPath s
}
stem := UploadStemFromLocal(localPath)
ext := UploadExtFromLocal(localPath)
matches := FindFilesByDedupKey(files, stem, ext)
matches := FindFilesByStemExt(files, stem, ext)
if len(matches) == 0 {
return nil
}
@@ -153,14 +156,81 @@ func (c *Client) UploadProjectFileNoClobber(ctx context.Context, projectID, loca
return c.UploadProjectFile(ctx, projectID, localPath)
}
// UploadToFolderReplacing deletes same stem+ext files then uploads localPath.
// 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 string
// DeleteIDs are redundant duplicate ids to remove after the update.
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
// OnlyOffice performs on upload, so a repeated upload of f.xls finds the saved
// f.xlsx instead of creating a second file.
func planUploadReplacement(files []*FileEntry, stem, ext string) uploadReplacementPlan {
matches := FindFilesByStemExt(files, stem, ext)
if len(matches) == 0 {
return uploadReplacementPlan{}
}
keep, remove := pickDuplicateKeeper(matches, false)
plan := uploadReplacementPlan{}
if id := FileEntryNumericID(keep); id != 0 {
plan.UpdateID = strconv.FormatInt(id, 10)
}
for _, f := range remove {
if id := int(FileEntryNumericID(f)); id != 0 {
plan.DeleteIDs = append(plan.DeleteIDs, id)
}
}
return plan
}
// 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.
func (c *Client) UploadToFolderReplacing(ctx context.Context, folderID, localPath string) (*FileEntry, []int, error) {
stem := UploadStemFromLocal(localPath)
ext := UploadExtFromLocal(localPath)
deleted, err := c.DeleteFilesByDedupKey(ctx, folderID, stem, ext)
files, err := c.FolderFiles(ctx, folderID)
if err != nil {
return nil, deleted, err
return nil, nil, err
}
plan := planUploadReplacement(files, stem, ext)
if plan.UpdateID == "" {
ent, err := c.UploadToFolder(ctx, folderID, localPath)
return ent, deleted, err
return ent, nil, err
}
ent, err := c.UpdateFile(ctx, plan.UpdateID, localPath)
if err != nil {
deleted, derr := c.DeleteFilesByStemExt(ctx, folderID, stem, ext)
if derr != nil {
return nil, deleted, derr
}
ent, uerr := c.UploadToFolder(ctx, folderID, localPath)
return ent, deleted, uerr
}
deleted, derr := c.deleteReplacementExtras(ctx, plan.DeleteIDs)
if derr != nil {
return ent, deleted, derr
}
return ent, deleted, nil
}
// 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) {
if len(ids) == 0 {
return nil, nil
}
if err := c.DeleteFiles(ctx, ids); err != nil {
return ids, err
}
return ids, nil
}