From 932cd2895ab3d44c7d42028d7c67caca58c33c7b Mon Sep 17 00:00:00 2001 From: Andriy Oblivantsev Date: Wed, 16 Sep 2026 21:49:08 +0000 Subject: [PATCH 1/2] fix(files): REST FileStore resolves folders for stat/rename/move/delete (#62) --- file_rest.go | 108 ++++++++++++++++++++++++++++++++------------------- 1 file changed, 67 insertions(+), 41 deletions(-) diff --git a/file_rest.go b/file_rest.go index ed236a2..5d9e5b3 100644 --- a/file_rest.go +++ b/file_rest.go @@ -7,12 +7,9 @@ package onlyoffice import ( "context" "encoding/json" - "fmt" "io" "os" "path/filepath" - "strconv" - "strings" ) // restStore is a FileStore over the REST Documents API. @@ -39,11 +36,25 @@ func (s *restStore) List(ctx context.Context, parentID string) ([]Entry, error) return out, err } -// Stat returns file metadata. The REST adapter resolves files only; folders -// are listed by their parent (use List). +// Stat returns file or folder metadata. Folders are resolved through the +// listing endpoint (their own id appears as the listing's Current); other ids +// fall back to the file metadata API. func (s *restStore) Stat(ctx context.Context, id string) (Entry, error) { + return s.stat(ctx, id) +} + +// stat resolves a single id to a folder or file Entry. +func (s *restStore) stat(ctx context.Context, id string) (Entry, error) { var out Entry err := retryStoreOp(ctx, func() error { + if l, err := s.c.ListDavFolder(ctx, id); err == nil { + if l != nil && l.Current.ID != "" && l.Current.ID == id { + out = DavFolderToEntry(l.Current, ProviderREST) + return nil + } + } else if Transient(err) { + return err + } f, err := s.c.GetFile(ctx, id) if err != nil { return err @@ -124,56 +135,84 @@ func (s *restStore) Download(ctx context.Context, id string, w io.Writer) (int64 return n, err } -// Move moves file ids into parentID. The REST MoveFiles endpoint handles files -// only; folder moves are not exposed by this adapter. +// Move moves folders and/or files into parentID. Ids are classified through +// stat so folder moves use folderIds and file moves use fileIds on the shared +// fileops/move endpoint. func (s *restStore) Move(ctx context.Context, ids []string, parentID string) error { - dest, err := strconv.Atoi(strings.TrimSpace(parentID)) - if err != nil { - return fmt.Errorf("onlyoffice: rest store: move: non-numeric destination folder id %q", parentID) - } - fileIDs, err := numericIDs(ids) + folders, files, err := s.split(ctx, ids) if err != nil { return err } - return retryStoreOp(ctx, func() error { - _, err := s.c.MoveFiles(ctx, dest, fileIDs) - return err - }) -} - -// Copy copies file ids into parentID. files.go has no copy method, so the -// shared REST fileops copy endpoint (CopyDavItems) is used. -func (s *restStore) Copy(ctx context.Context, ids []string, parentID string) error { - if len(ids) == 0 { + if len(folders) == 0 && len(files) == 0 { return nil } return retryStoreOp(ctx, func() error { - return s.c.CopyDavItems(ctx, nil, ids, parentID) + return s.c.MoveDavItems(ctx, folders, files, parentID) }) } -// Rename sets a new title (including extension) for a file. +// Copy copies folders and/or files into parentID. files.go has no copy method, +// so the shared REST fileops copy endpoint (CopyDavItems) is used. +func (s *restStore) Copy(ctx context.Context, ids []string, parentID string) error { + folders, files, err := s.split(ctx, ids) + if err != nil { + return err + } + if len(folders) == 0 && len(files) == 0 { + return nil + } + return retryStoreOp(ctx, func() error { + return s.c.CopyDavItems(ctx, folders, files, parentID) + }) +} + +// Rename sets a new title (including extension) for a file or folder. func (s *restStore) Rename(ctx context.Context, id, title string) error { + e, err := s.stat(ctx, id) + if err != nil { + return err + } + if e.Kind == Folder { + return retryStoreOp(ctx, func() error { + return s.c.RenameDavFolder(ctx, id, title) + }) + } return retryStoreOp(ctx, func() error { _, err := s.c.RenameFile(ctx, id, title) return err }) } -// Delete permanently deletes file ids. +// Delete permanently deletes folders and/or files. func (s *restStore) Delete(ctx context.Context, ids []string) error { - fileIDs, err := numericIDs(ids) + folders, files, err := s.split(ctx, ids) if err != nil { return err } - if len(fileIDs) == 0 { + if len(folders) == 0 && len(files) == 0 { return nil } return retryStoreOp(ctx, func() error { - return s.c.DeleteFiles(ctx, fileIDs) + return s.c.DeleteDavItems(ctx, folders, files) }) } +// split classifies ids into folder and file id lists. +func (s *restStore) split(ctx context.Context, ids []string) (folders, files []string, err error) { + for _, id := range ids { + e, err := s.stat(ctx, id) + if err != nil { + return nil, nil, err + } + if e.Kind == Folder { + folders = append(folders, id) + } else { + files = append(files, id) + } + } + return folders, files, nil +} + // entriesFromFolderMap converts a ListFolder response map into canonical // entries, reusing the DavFile/DavFolder decoders for robust size handling. func entriesFromFolderMap(m map[string]any, provider string) ([]Entry, error) { @@ -215,16 +254,3 @@ func folderEntryFromMap(m map[string]any, parentID, provider string) (Entry, err e = DavFolderToEntry(f, provider) return e, nil } - -// numericIDs parses Documents numeric ids from strings. -func numericIDs(ids []string) ([]int, error) { - out := make([]int, 0, len(ids)) - for _, id := range ids { - n, err := strconv.Atoi(strings.TrimSpace(id)) - if err != nil { - return nil, fmt.Errorf("onlyoffice: rest store: non-numeric id %q", id) - } - out = append(out, n) - } - return out, nil -} From 9cd157c59f550707f416d04e7c9a177a5bafaf1a Mon Sep 17 00:00:00 2001 From: Andriy Oblivantsev Date: Wed, 16 Sep 2026 21:49:10 +0000 Subject: [PATCH 2/2] =?UTF-8?q?test(files):=20live=20CRUD=20=D0=BF=D0=B0?= =?UTF-8?q?=D0=BF=D0=BE=D0=BA=20+=20=D1=84=D0=B0=D1=81=D0=B0=D0=B4=D0=BD?= =?UTF-8?q?=D1=8B=D0=B9=20CRUD=20(#62)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- file_facade_integration_test.go | 131 ++++++++++++++++++++++++++++++++ file_facade_test.go | 30 ++++++++ file_store_integration_test.go | 92 ++++++++++++++++++++-- 3 files changed, 245 insertions(+), 8 deletions(-) create mode 100644 file_facade_integration_test.go diff --git a/file_facade_integration_test.go b/file_facade_integration_test.go new file mode 100644 index 0000000..f2fbcbf --- /dev/null +++ b/file_facade_integration_test.go @@ -0,0 +1,131 @@ +//go:build integration + +package onlyoffice + +import ( + "bytes" + "context" + "strconv" + "testing" + "time" +) + +// TestIntegrationFacadeCRUD drives the whole operation set through the composed +// facade c.Files(): folder create, upload, stat, list, rename, move, copy, +// delete. Writes must go to REST (the default writeOrder), reads follow +// readOrder (REST when no SQL backend is registered) and every returned Entry +// must report its provider. Destructive — throwaway project, cleaned up. +func TestIntegrationFacadeCRUD(t *testing.T) { + c := liveClient(t) + t.Cleanup(func() { cleanupTestProjects(t, c) }) + ctx := context.Background() + + suffix := time.Now().UTC().Format("20060102-150405") + project, err := c.CreateProject(NewProjectRequest{ + Title: testProjectPrefix + "facade-" + suffix, + Description: "go-onlyoffice facade CRUD integration", + }) + if err != nil { + t.Fatalf("CreateProject: %v", err) + } + if project.ID == nil { + t.Fatal("created project without id") + } + root, err := c.projectFolderID(ctx, strconv.Itoa(*project.ID)) + if err != nil { + t.Fatalf("projectFolderID: %v", err) + } + + f := c.Files() + if got := f.Write().Name(); got != ProviderREST { + t.Fatalf("Write().Name() = %q, want %q", got, ProviderREST) + } + if got := f.Read().Name(); got != ProviderREST { + t.Fatalf("Read().Name() = %q, want %q (no SQL backend registered)", got, ProviderREST) + } + + src, err := f.CreateFolder(ctx, root, "facade-src-"+suffix) + if err != nil { + t.Fatalf("CreateFolder src: %v", err) + } + if src.Kind != Folder || src.ID == "" { + t.Fatalf("created src folder: %+v", src) + } + if src.Provider != ProviderREST { + t.Fatalf("CreateFolder provider = %q, want %q", src.Provider, ProviderREST) + } + dst, err := f.CreateFolder(ctx, root, "facade-dst-"+suffix) + if err != nil { + t.Fatalf("CreateFolder dst: %v", err) + } + if dst.Provider != ProviderREST { + t.Fatalf("CreateFolder dst provider = %q, want %q", dst.Provider, ProviderREST) + } + t.Cleanup(func() { + if err := c.DeleteDavItems(ctx, []string{src.ID, dst.ID}, nil); err != nil { + t.Logf("cleanup folders: %v", err) + } + }) + + content := []byte("facade crud " + suffix + "\n") + up, err := f.Upload(ctx, src.ID, "facade-doc-"+suffix+".txt", bytes.NewReader(content)) + if err != nil { + t.Fatalf("Upload: %v", err) + } + if up.Kind != File || up.ID == "" { + t.Fatalf("uploaded entry: %+v", up) + } + if up.Provider != ProviderREST { + t.Fatalf("Upload provider = %q, want %q (write order REST first)", up.Provider, ProviderREST) + } + if !waitEntry(ctx, f, src.ID, up.ID, 15*time.Second) { + t.Fatalf("uploaded %s not listed in src", up.ID) + } + + st, err := f.Stat(ctx, up.ID) + if err != nil { + t.Fatalf("Stat: %v", err) + } + if st.ID != up.ID || st.Kind != File { + t.Fatalf("Stat = %+v", st) + } + if st.Provider != ProviderREST { + t.Fatalf("Stat provider = %q, want %q (read order REST)", st.Provider, ProviderREST) + } + + list, err := f.List(ctx, src.ID) + if err != nil { + t.Fatalf("List: %v", err) + } + if e := entryByID(list, up.ID); e == nil { + t.Fatalf("uploaded %s not in List(src)", up.ID) + } else if e.Provider != ProviderREST { + t.Fatalf("List provider = %q, want %q", e.Provider, ProviderREST) + } + + renamed := "facade-renamed-" + suffix + ".txt" + renameEventually(t, ctx, f, up.ID, renamed) + + moveEventually(t, ctx, f, up.ID, dst.ID) + if !waitEntry(ctx, f, dst.ID, up.ID, 20*time.Second) { + t.Fatalf("moved file %s not in dst", up.ID) + } + + copied := copyEventually(t, ctx, f, up.ID, src.ID, 20*time.Second) + if copied == nil { + t.Fatalf("no copy found in src after Copy") + } + if copied.Provider != ProviderREST { + t.Fatalf("Copy provider = %q, want %q", copied.Provider, ProviderREST) + } + + if err := f.Delete(ctx, []string{up.ID, copied.ID}); err != nil { + t.Fatalf("Delete: %v", err) + } + if !waitNoEntry(ctx, f, dst.ID, up.ID, 20*time.Second) { + t.Fatalf("file %s still present in dst after delete", up.ID) + } + if !waitNoEntry(ctx, f, src.ID, copied.ID, 20*time.Second) { + t.Fatalf("copy %s still present in src after delete", copied.ID) + } +} diff --git a/file_facade_test.go b/file_facade_test.go index b424e97..4be4a74 100644 --- a/file_facade_test.go +++ b/file_facade_test.go @@ -250,6 +250,36 @@ func TestFileClientMySQLStoreIsPreferredForReads(t *testing.T) { } } +// TestFileClientWriteToReadOnlyStore guarantees the facade surfaces ErrReadOnly +// when the configured write backend is the read-only SQL store. +func TestFileClientWriteToReadOnlyStore(t *testing.T) { + pg := &pgStore{driver: ProviderPG} + f := newFacadeTestClient( + map[string]FileStore{ProviderREST: &fakeStore{name: ProviderREST}, ProviderPG: pg}, + []string{ProviderPG, ProviderREST}, + []string{ProviderPG, ProviderREST}, + ) + ctx := context.Background() + if _, err := f.CreateFolder(ctx, "1", "x"); !errors.Is(err, ErrReadOnly) { + t.Errorf("CreateFolder err = %v", err) + } + if _, err := f.Upload(ctx, "1", "x", strings.NewReader("x")); !errors.Is(err, ErrReadOnly) { + t.Errorf("Upload err = %v", err) + } + if err := f.Move(ctx, []string{"1"}, "2"); !errors.Is(err, ErrReadOnly) { + t.Errorf("Move err = %v", err) + } + if err := f.Copy(ctx, []string{"1"}, "2"); !errors.Is(err, ErrReadOnly) { + t.Errorf("Copy err = %v", err) + } + if err := f.Rename(ctx, "1", "x"); !errors.Is(err, ErrReadOnly) { + t.Errorf("Rename err = %v", err) + } + if err := f.Delete(ctx, []string{"1"}); !errors.Is(err, ErrReadOnly) { + t.Errorf("Delete err = %v", err) + } +} + func TestFileClientSearchSelection(t *testing.T) { f := &FileClient{searchers: map[string]Searcher{}, searchOrder: []string{ProviderES}} _, err := f.Search() diff --git a/file_store_integration_test.go b/file_store_integration_test.go index 4533cf8..2b91a57 100644 --- a/file_store_integration_test.go +++ b/file_store_integration_test.go @@ -10,10 +10,11 @@ import ( "time" ) -// TestIntegrationFileStores runs the same operation set (create folder, upload, -// list, stat, download, move, copy, rename, delete) through the REST and DAV -// FileStore adapters against a throwaway project Documents folder. Destructive -// — only run against instances you own. +// TestIntegrationFileStores runs the same operation set through the REST and +// DAV FileStore adapters against a throwaway project Documents folder: file +// create/upload/list/stat/download/move/copy/rename/delete and folder +// create/stat/list/rename/move/delete. Destructive — only run against +// instances you own. // // The Documents fileops API is asynchronous: a move/copy/delete is accepted // immediately and becomes visible a moment later, so effects are polled. @@ -102,10 +103,7 @@ func testFileStoreOps(t *testing.T, ctx context.Context, c *Client, store FileSt t.Fatalf("moved file %s not in dst", up.ID) } - if err := store.Copy(ctx, []string{up.ID}, src.ID); err != nil { - t.Fatalf("Copy: %v", err) - } - copied := waitOtherFile(ctx, store, src.ID, up.ID, 20*time.Second) + copied := copyEventually(t, ctx, store, up.ID, src.ID, 20*time.Second) if copied == nil { t.Fatalf("no copy found in src after Copy") } @@ -122,6 +120,67 @@ func testFileStoreOps(t *testing.T, ctx context.Context, c *Client, store FileSt if !waitNoEntry(ctx, store, src.ID, copied.ID, 20*time.Second) { t.Fatalf("copy %s still present in src after delete", copied.ID) } + + // --- CRUD on the folders themselves, reusing the throwaway src/dst --- + // A child file lets us prove it survives the folder rename and move. + child, err := store.Upload(ctx, src.ID, "child-"+suffix+".txt", bytes.NewReader(content)) + if err != nil { + t.Fatalf("Upload child: %v", err) + } + if !waitEntry(ctx, store, src.ID, child.ID, 15*time.Second) { + t.Fatalf("child %s not listed in src", child.ID) + } + + fst, err := store.Stat(ctx, src.ID) + if err != nil { + t.Fatalf("Stat(folder): %v", err) + } + if fst.ID != src.ID || fst.Kind != Folder { + t.Fatalf("Stat(folder) = %+v", fst) + } + + flist, err := store.List(ctx, src.ID) + if err != nil { + t.Fatalf("List(folder): %v", err) + } + if entryByID(flist, child.ID) == nil { + t.Fatalf("child %s not in List(src)", child.ID) + } + + folderTitle := "renamed-folder-" + suffix + renameEventually(t, ctx, store, src.ID, folderTitle) + if e, err := store.Stat(ctx, src.ID); err != nil { + t.Fatalf("Stat(folder) after rename: %v", err) + } else if e.Kind != Folder || e.Title != folderTitle { + t.Fatalf("folder after rename = %+v, want title %q", e, folderTitle) + } + + moveEventually(t, ctx, store, src.ID, dst.ID) + if !waitEntry(ctx, store, dst.ID, src.ID, 20*time.Second) { + t.Fatalf("moved folder %s not in dst %s", src.ID, dst.ID) + } + if !waitNoEntry(ctx, store, root, src.ID, 20*time.Second) { + t.Fatalf("folder %s still in root after move", src.ID) + } + if !waitEntry(ctx, store, src.ID, child.ID, 20*time.Second) { + t.Fatalf("child file %s lost after moving folder %s", child.ID, src.ID) + } + + if err := store.Delete(ctx, []string{child.ID}); err != nil { + t.Fatalf("Delete(child): %v", err) + } + if err := store.Delete(ctx, []string{src.ID}); err != nil { + t.Fatalf("Delete(folder): %v", err) + } + if !waitNoEntry(ctx, store, dst.ID, src.ID, 20*time.Second) { + t.Fatalf("folder %s still present in dst after delete", src.ID) + } + if err := store.Delete(ctx, []string{dst.ID}); err != nil { + t.Fatalf("Delete(dst folder): %v", err) + } + if !waitNoEntry(ctx, store, root, dst.ID, 20*time.Second) { + t.Fatalf("dst folder %s still present in root after delete", dst.ID) + } } // moveEventually issues Move and retries while the operation is not visible yet @@ -141,6 +200,23 @@ func moveEventually(t *testing.T, ctx context.Context, store FileStore, id, dstI t.Fatalf("Move %s -> %s: %v", id, dstID, lastErr) } +// copyEventually issues Copy and retries while the new copy is not visible yet +// (copy is accepted asynchronously, like move). +func copyEventually(t *testing.T, ctx context.Context, store FileStore, id, dstID string, d time.Duration) *Entry { + t.Helper() + var lastErr error + for attempt := 0; attempt < 5; attempt++ { + if lastErr = store.Copy(ctx, []string{id}, dstID); lastErr == nil { + if e := waitOtherFile(ctx, store, dstID, id, d); e != nil { + return e + } + } + time.Sleep(time.Second) + } + t.Fatalf("Copy %s -> %s: %v", id, dstID, lastErr) + return nil +} + func renameEventually(t *testing.T, ctx context.Context, store FileStore, id, title string) { t.Helper() var lastErr error