Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix onlyoffice behavior #4138

Merged
merged 6 commits into from
Oct 9, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion docs/docker.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ $ docker run -it --rm --name=oodev --net=host cozy/onlyoffice-dev
and run the stack with:

```bash
$ cozy-stack serve --disable-csp --onlyoffice-url=http://localhost:8000/ --onlyoffice-inbox-secret=inbox_secret --onlyoffice-outbox-secret=outbox_secret
$ cozy-stack serve --disable-csp --onlyoffice-url=http://localhost:8000 --onlyoffice-inbox-secret=inbox_secret --onlyoffice-outbox-secret=outbox_secret
$ cozy-stack features defaults '{"drive.office": {"enabled": true, "write": true}}'
```

If you need to rebuild it, you can do that with:
Expand Down
60 changes: 60 additions & 0 deletions docs/office.md
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,66 @@ Content-Type: application/vnd.api+json
}
```

### GET /office/keys/:key
taratatach marked this conversation as resolved.
Show resolved Hide resolved

If a document is being edited while a new version is uploaded (via the desktop
for example), the OO webapp should call this endpoint if the user chooses to
continue editing the version on which they were working. A conflict file is
created, so that no work is lost.

#### Request

```http
GET /office/keys/7c7ccc2e7137ba774b7e44de HTTP/1.1
Host: bob.cozy.example
```

#### Response

```http
HTTP/1.1 200 OK
Content-Type: application/vnd.api+json
```

```json
{
"data": {
"type": "io.cozy.files",
"id": "32e07d806f9b0139c541543d7eb8149c",
"meta": {
"rev": "3-18c04daba326"
},
"attributes": {
"type": "file",
"name": "slideshow (2).pptx",
"trashed": false,
"md5sum": "ODZmYjI2OWQxOTBkMmM4NQo=",
"created_at": "2023-09-30T21:42:05Z",
"updated_at": "2023-09-30T22:38:04Z",
"tags": [],
"metadata": {},
"size": 12345,
"executable": false,
"class": "slide",
"mime": "application/vnd.ms-powerpoint",
"cozyMetadata": {
"doctypeVersion": "1",
"metadataVersion": 1,
"createdAt": "2023-09-30T21:42:05Z",
"createdByApp": "drive",
"createdOn": "https://bob.cozy.example/",
"updatedAt": "2023-09-30T22:38:04Z",
"uploadedAt": "2023-09-30T22:38:04Z",
"uploadedOn": "https://bob.cozy.example/",
"uploadedBy": {
"slug": "onlyoffice-server"
}
}
}
}
}
```

### POST /office/callback

This is the callback handler for OnlyOffice. It is called when the document
Expand Down
31 changes: 24 additions & 7 deletions model/office/callback.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ import (
"fmt"
"io"
"net/http"
"os"
"path"
"strings"
"time"

"github.com/cozy/cozy-stack/model/instance"
Expand All @@ -28,6 +31,10 @@ const (
StatusForceSaveRequested = 6
)

// OOSlug is the slug for uploadedBy field of the CozyMetadata when a file has
// been modified in the online OnlyOffice.
const OOSlug = "onlyoffice-server"

// CallbackParameters is a struct for the parameters sent by the document
// server to the stack.
// Cf https://api.onlyoffice.com/editors/callback
Expand Down Expand Up @@ -94,7 +101,7 @@ func checkToken(cfg *config.Office, params CallbackParameters) error {
func finalSaveFile(inst *instance.Instance, key, downloadURL string) error {
detector, err := GetStore().GetDoc(inst, key)
if err != nil || detector == nil || detector.ID == "" || detector.Rev == "" {
return errors.New("invalid key")
return ErrInvalidKey
}

_, err = saveFile(inst, *detector, downloadURL)
Expand All @@ -107,7 +114,7 @@ func finalSaveFile(inst *instance.Instance, key, downloadURL string) error {
func forceSaveFile(inst *instance.Instance, key, downloadURL string) error {
detector, err := GetStore().GetDoc(inst, key)
if err != nil || detector == nil || detector.ID == "" || detector.Rev == "" {
return errors.New("invalid key")
return ErrInvalidKey
}

updated, err := saveFile(inst, *detector, downloadURL)
Expand Down Expand Up @@ -148,6 +155,7 @@ func saveFile(inst *instance.Instance, detector conflictDetector, downloadURL st
newfile.UpdatedAt = time.Now()
newfile.CozyMetadata.UpdatedAt = newfile.UpdatedAt
newfile.CozyMetadata.UploadedAt = &newfile.UpdatedAt
newfile.CozyMetadata.UploadedBy = &vfs.UploadedByEntry{Slug: OOSlug}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nono I think we have a problem here. From what I understood, the uploadedBy is only changed when the file is created. So after this route is called, its value always remains at {"slug": "onlyoffice-server" }. For the front, it means that the modals are no longer displayed from this point. A more effective way would be to add an entry in updatedByApps and based the front on it. What do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the same way, CozyMetadata.UploadedAt should not be modified either. We don't modify these two attributes when uploading a new version via Drive or Desktop.

Copy link
Member Author

@nono nono Oct 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do modify these two attributes when a file is uploaded (creation or content modification). It's the stack that does the job. Is it OK for you?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I must have a bug in local, the stack does not modify the fields and it always remains in onlyoffice-server after a modification by OnlyOffice 🙁


// If the file was renamed while OO editor was opened, the revision has
// been changed, but we still should avoid creating a conflict if the
Expand All @@ -157,15 +165,24 @@ func saveFile(inst *instance.Instance, detector conflictDetector, downloadURL st
file = nil
newfile.SetID("")
newfile.SetRev("")
newfile.DocName = fmt.Sprintf("%s - conflict - %d", newfile.DocName, time.Now().Unix())
}

basename := newfile.DocName
var f vfs.File
for i := 2; i < 100; i++ {
f, err = fs.CreateFile(newfile, file)
if err == nil {
break
} else if !errors.Is(err, os.ErrExist) {
return nil, err
}
ext := path.Ext(basename)
filename := strings.TrimSuffix(path.Base(basename), ext)
newfile.DocName = fmt.Sprintf("%s (%d)%s", filename, i, ext)
newfile.ResetFullpath()
_, _ = newfile.Path(inst.VFS()) // Prefill the fullpath
}

f, err := fs.CreateFile(newfile, file)
if err != nil {
return nil, err
}
_, err = io.Copy(f, res.Body)
if cerr := f.Close(); cerr != nil && err == nil {
err = cerr
Expand Down
2 changes: 2 additions & 0 deletions model/office/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,6 @@ var (
// ErrInternalServerError is used when something goes wrong (like no
// connection to redis)
ErrInternalServerError = errors.New("Internal server error")
// ErrInvalidKey is used when the key is not found in the store
ErrInvalidKey = errors.New("invalid key")
)
44 changes: 44 additions & 0 deletions model/office/file_by_key.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package office

import (
"bytes"

"github.com/cozy/cozy-stack/model/instance"
"github.com/cozy/cozy-stack/model/vfs"
)

func GetFileByKey(inst *instance.Instance, key string) (*vfs.FileDoc, error) {
taratatach marked this conversation as resolved.
Show resolved Hide resolved
detector, err := GetStore().GetDoc(inst, key)
taratatach marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, err
}
if detector == nil || detector.ID == "" || detector.Rev == "" {
return nil, ErrInvalidKey
}

fs := inst.VFS()
file, err := fs.FileByID(detector.ID)
if err != nil {
return nil, err
}

if file.Rev() == detector.Rev || bytes.Equal(file.MD5Sum, detector.MD5Sum) {
return file, nil
}

// Manage the conflict
conflictName := vfs.ConflictName(fs, file.DirID, file.DocName, true)
newfile := vfs.CreateFileDocCopy(file, file.DirID, conflictName)
newfile.CozyMetadata = vfs.NewCozyMetadata(inst.PageURL("/", nil))
newfile.CozyMetadata.UpdatedAt = newfile.UpdatedAt
newfile.CozyMetadata.UploadedAt = &newfile.UpdatedAt
newfile.CozyMetadata.UploadedBy = &vfs.UploadedByEntry{Slug: OOSlug}
if err := fs.CopyFile(file, newfile); err != nil {
return nil, err
}

updated := conflictDetector{ID: newfile.ID(), Rev: newfile.Rev(), MD5Sum: newfile.MD5Sum}
_ = GetStore().UpdateSecret(inst, key, file.ID(), newfile.ID())
_ = GetStore().UpdateDoc(inst, key, updated)
return newfile, nil
}
38 changes: 36 additions & 2 deletions model/office/open.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package office

import (
"bytes"
"net/url"

"github.com/cozy/cozy-stack/client/request"
Expand Down Expand Up @@ -169,8 +170,27 @@ func (o *Opener) openLocalDocument(memberIndex int, readOnly bool) (*apiOfficeUR
Infof("Cannot build download URL: %s", err)
return nil, ErrInternalServerError
}
detector := conflictDetector{ID: o.File.ID(), Rev: o.File.Rev(), MD5Sum: o.File.MD5Sum}
key, err := GetStore().AddDoc(o.Inst, detector)
key, err := GetStore().GetSecretByID(o.Inst, o.File.ID())
if err != nil {
o.Inst.Logger().WithNamespace("office").
Infof("Cannot get secret from store: %s", err)
return nil, ErrInternalServerError
}
if key != "" {
doc, err := GetStore().GetDoc(o.Inst, key)
if err != nil {
o.Inst.Logger().WithNamespace("office").
Infof("Cannot get doc from store: %s", err)
return nil, ErrInternalServerError
}
if shouldOpenANewVersion(o.File, doc) {
key = ""
}
}
if key == "" {
detector := conflictDetector{ID: o.File.ID(), Rev: o.File.Rev(), MD5Sum: o.File.MD5Sum}
key, err = GetStore().AddDoc(o.Inst, detector)
}
if err != nil {
o.Inst.Logger().WithNamespace("office").
Infof("Cannot add doc to store: %s", err)
Expand Down Expand Up @@ -296,3 +316,17 @@ func getConfig(contextName string) *config.Office {
}
return nil
}

func shouldOpenANewVersion(file *vfs.FileDoc, detector *conflictDetector) bool {
if detector == nil {
return true
}
cm := file.CozyMetadata
if cm != nil && cm.UploadedBy != nil && cm.UploadedBy.Slug == OOSlug {
return false
}
if bytes.Equal(file.MD5Sum, detector.MD5Sum) {
return false
}
return true
}
Loading
Loading