Skip to content

Commit

Permalink
Merge pull request #10403 from owncloud/fix-put-relative
Browse files Browse the repository at this point in the history 
fix: add missing urls to putRelative
  • Loading branch information
@micbar
micbar authored Oct 25, 2024
2 parents 1800a3f + 406c8ca commit 2a8d7f5
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 31 deletions.
5 changes: 5 additions & 0 deletions changelog/unreleased/fix-put-relative.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Bugfix: Fix put relative wopi operation for microsoft

We fixed a bug in the put relative wopi operation for microsoft. The response now contains the correct properties.

https://github.com/owncloud/ocis/pull/10403
8 changes: 5 additions & 3 deletions services/collaboration/pkg/connector/connector.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,12 +112,14 @@ func NewResponseSuccessBodyName(name string) *ConnectorResponse {
// contain "Name" and "Url" keys with their respective suplied values
//
// This is used in the `PutRelativeFile` methods (both suggested and relative).
func NewResponseSuccessBodyNameUrl(name, url string) *ConnectorResponse {
func NewResponseSuccessBodyNameUrl(name, url string, hostEditURL string, hostViewURL string) *ConnectorResponse {
return &ConnectorResponse{
Status: 200,
Body: map[string]interface{}{
"Name": name,
"Url": url,
"Name": name,
"Url": url,
"HostEditUrl": hostEditURL,
"HostViewUrl": hostViewURL,
},
}
}
Expand Down
48 changes: 35 additions & 13 deletions services/collaboration/pkg/connector/fileconnector.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -713,9 +713,9 @@ func (f *FileConnector) PutRelativeFileSuggested(ctx context.Context, ccs Conten
return NewResponse(500), nil
}
}

var newInfo *providerv1beta1.ResourceInfo
// adjust the wopi file reference to use only the resource id without path
if err := f.adjustWopiReference(ctx, &wopiContext, newLogger); err != nil {
if newInfo, err = f.adjustWopiReference(ctx, &wopiContext, newLogger); err != nil {
return nil, err
}

Expand All @@ -729,7 +729,16 @@ func (f *FileConnector) PutRelativeFileSuggested(ctx context.Context, ccs Conten
Str("FinalReference", wopiContext.FileReference.String()).
Msg("PutRelativeFileSuggested: success")

return NewResponseSuccessBodyNameUrl(finalTarget, wopiSrcURL.String()), nil
webURL, err := url.Parse(f.cfg.Commons.OcisURL)
if err != nil {
return nil, err
}
return NewResponseSuccessBodyNameUrl(
finalTarget,
wopiSrcURL.String(),
createHostUrl("write", webURL, strings.ToLower(f.cfg.App.Name), newInfo),
createHostUrl("view", webURL, strings.ToLower(f.cfg.App.Name), newInfo),
), nil
}

// PutRelativeFileRelative upload a file using the provided target name
Expand Down Expand Up @@ -811,10 +820,16 @@ func (f *FileConnector) PutRelativeFileRelative(ctx context.Context, ccs Content
lockID = putResponse.Headers[HeaderWopiLock]
}

var newInfo *providerv1beta1.ResourceInfo
webURL, err := url.Parse(f.cfg.Commons.OcisURL)
if err != nil {
return nil, err
}

switch putResponse.Status {
case 200: // success case, so don't do anything
case 409:
if err := f.adjustWopiReference(ctx, &wopiContext, newLogger); err != nil {
if newInfo, err = f.adjustWopiReference(ctx, &wopiContext, newLogger); err != nil {
return nil, err
}
// if conflict generate a different name and retry.
Expand Down Expand Up @@ -844,8 +859,10 @@ func (f *FileConnector) PutRelativeFileRelative(ctx context.Context, ccs Content
HeaderWopiLockFailureReason: "Lock Conflict",
},
Body: map[string]interface{}{
"Name": target,
"Url": wopiSrcURL.String(),
"Name": target,
"Url": wopiSrcURL.String(),
"HostViewUrl": createHostUrl("view", webURL, strings.ToLower(f.cfg.App.Name), newInfo),
"HostEditUrl": createHostUrl("write", webURL, strings.ToLower(f.cfg.App.Name), newInfo),
},
}, nil
default:
Expand All @@ -855,7 +872,7 @@ func (f *FileConnector) PutRelativeFileRelative(ctx context.Context, ccs Content
return nil, NewConnectorError(putResponse.Status, "put file failed with unhandled status")
}

if err := f.adjustWopiReference(ctx, &wopiContext, newLogger); err != nil {
if newInfo, err = f.adjustWopiReference(ctx, &wopiContext, newLogger); err != nil {
return nil, err
}

Expand All @@ -867,7 +884,12 @@ func (f *FileConnector) PutRelativeFileRelative(ctx context.Context, ccs Content

newLogger.Debug().Msg("PutRelativeFileRelative: success")

return NewResponseSuccessBodyNameUrl(target, wopiSrcURL.String()), nil
return NewResponseSuccessBodyNameUrl(
target,
wopiSrcURL.String(),
createHostUrl("write", webURL, strings.ToLower(f.cfg.App.Name), newInfo),
createHostUrl("view", webURL, strings.ToLower(f.cfg.App.Name), newInfo),
), nil
}

// DeleteFile will delete the requested file
Expand Down Expand Up @@ -1413,10 +1435,10 @@ func (f *FileConnector) generateWOPISrc(wopiContext middleware.WopiContext, logg
return wopiSrcURL, nil
}

func (f *FileConnector) adjustWopiReference(ctx context.Context, wopiContext *middleware.WopiContext, logger zerolog.Logger) error {
func (f *FileConnector) adjustWopiReference(ctx context.Context, wopiContext *middleware.WopiContext, logger zerolog.Logger) (*providerv1beta1.ResourceInfo, error) {
gwc, err := f.gws.Next()
if err != nil {
return err
return nil, err
}
// using resourceid + path won't do for WOPI, we need just the resource if of the new file
// the wopicontext has resourceid + path, which is good enough for the stat request
Expand All @@ -1425,20 +1447,20 @@ func (f *FileConnector) adjustWopiReference(ctx context.Context, wopiContext *mi
})
if err != nil {
logger.Error().Err(err).Msg("stat failed")
return err
return nil, err
}

if newStatRes.GetStatus().GetCode() != rpcv1beta1.Code_CODE_OK {
logger.Error().
Str("StatusCode", newStatRes.GetStatus().GetCode().String()).
Str("StatusMsg", newStatRes.GetStatus().GetMessage()).
Msg("stat failed with unexpected status")
return NewConnectorError(500, newStatRes.GetStatus().GetCode().String()+" "+newStatRes.GetStatus().GetMessage())
return nil, NewConnectorError(500, newStatRes.GetStatus().GetCode().String()+" "+newStatRes.GetStatus().GetMessage())
}
// adjust the reference in the wopi context to use only the resourceid without the path
wopiContext.FileReference = &providerv1beta1.Reference{
ResourceId: newStatRes.GetInfo().GetId(),
}

return nil
return newStatRes.GetInfo(), nil
}
46 changes: 31 additions & 15 deletions services/collaboration/pkg/connector/fileconnector_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"encoding/hex"
"errors"
"net/url"
"path"
"regexp"
"strings"
Expand All @@ -26,6 +27,7 @@ import (
"github.com/owncloud/ocis/v2/services/collaboration/pkg/middleware"
"github.com/owncloud/ocis/v2/services/graph/mocks"
"github.com/stretchr/testify/mock"
"google.golang.org/grpc"
)

var _ = Describe("FileConnector", func() {
Expand Down Expand Up @@ -952,6 +954,8 @@ var _ = Describe("FileConnector", func() {
rBody := response.Body.(map[string]interface{})
Expect(rBody["Name"]).To(Equal("newDocument.docx"))
Expect(rBody["Url"]).To(HavePrefix("https://ocis.server.prv/wopi/files/")) // skip checking the actual reference
Expect(rBody["HostEditUrl"]).To(Equal("https://ocis.example.prv/external-test/personal/path/to/newDocument.docx?fileId=storageid%24spaceid%21opaqueid_newDoc&view_mode=write"))
Expect(rBody["HostViewUrl"]).To(Equal("https://ocis.example.prv/external-test/personal/path/to/newDocument.docx?fileId=storageid%24spaceid%21opaqueid_newDoc&view_mode=view"))
})

It("PutRelativeFileSuggested success only extension", func() {
Expand Down Expand Up @@ -1005,6 +1009,8 @@ var _ = Describe("FileConnector", func() {
rBody := response.Body.(map[string]interface{})
Expect(rBody["Name"]).To(Equal("file.pdf"))
Expect(rBody["Url"]).To(HavePrefix("https://ocis.server.prv/wopi/files/")) // skip checking the actual reference
Expect(rBody["HostEditUrl"]).To(Equal("https://ocis.example.prv/external-test/personal/path/to/file.pdf?fileId=storageid%24spaceid%21opaqueid_newDoc&view_mode=write"))
Expect(rBody["HostViewUrl"]).To(Equal("https://ocis.example.prv/external-test/personal/path/to/file.pdf?fileId=storageid%24spaceid%21opaqueid_newDoc&view_mode=view"))
})

It("PutRelativeFileSuggested success conflict", func() {
Expand Down Expand Up @@ -1048,21 +1054,25 @@ var _ = Describe("FileConnector", func() {
}
return false
})
gatewayClient.On("Stat", mock.Anything, stat2ParamMatcher).Times(1).Return(&providerv1beta1.StatResponse{
Status: status.NewOK(ctx),
Info: &providerv1beta1.ResourceInfo{
Path: path.Join("/personal/path/to", *newFilePath),
Id: &providerv1beta1.ResourceId{
StorageId: "storageid",
OpaqueId: "opaqueid_newDoc",
SpaceId: "spaceid",
},
Lock: &providerv1beta1.Lock{
LockId: "zzz999",
Type: providerv1beta1.LockType_LOCK_TYPE_WRITE,
},
},
}, nil)

gatewayClient.EXPECT().Stat(mock.Anything, stat2ParamMatcher).
RunAndReturn(func(ctx context.Context, req *providerv1beta1.StatRequest, opts ...grpc.CallOption) (*providerv1beta1.StatResponse, error) {
return &providerv1beta1.StatResponse{
Status: status.NewOK(ctx),
Info: &providerv1beta1.ResourceInfo{
Path: path.Join("/personal/path/to", path.Base(req.GetRef().GetPath())),
Id: &providerv1beta1.ResourceId{
StorageId: "storageid",
OpaqueId: "opaqueid_newDoc",
SpaceId: "spaceid",
},
Lock: &providerv1beta1.Lock{
LockId: "zzz999",
Type: providerv1beta1.LockType_LOCK_TYPE_WRITE,
},
},
}, nil
})

response, err := fc.PutRelativeFileSuggested(ctx, ccs, stream, int64(stream.Len()), ".pdf")
Expect(err).ToNot(HaveOccurred())
Expand All @@ -1071,6 +1081,8 @@ var _ = Describe("FileConnector", func() {
rBody := response.Body.(map[string]interface{})
Expect(rBody["Name"]).To(MatchRegexp(`[a-zA-Z0-9_-] file\.pdf`))
Expect(rBody["Url"]).To(HavePrefix("https://ocis.server.prv/wopi/files/")) // skip checking the actual reference
Expect(rBody["HostEditUrl"]).To(Equal("https://ocis.example.prv/external-test/personal/path/to/" + url.PathEscape(path.Base(*newFilePath)) + "?fileId=storageid%24spaceid%21opaqueid_newDoc&view_mode=write"))
Expect(rBody["HostViewUrl"]).To(Equal("https://ocis.example.prv/external-test/personal/path/to/" + url.PathEscape(path.Base(*newFilePath)) + "?fileId=storageid%24spaceid%21opaqueid_newDoc&view_mode=view"))
})

It("PutRelativeFileSuggested put file fails", func() {
Expand Down Expand Up @@ -1193,6 +1205,8 @@ var _ = Describe("FileConnector", func() {
rBody := response.Body.(map[string]interface{})
Expect(rBody["Name"]).To(Equal("newDocument.docx"))
Expect(rBody["Url"]).To(HavePrefix("https://ocis.server.prv/wopi/files/")) // skip checking the actual reference
Expect(rBody["HostEditUrl"]).To(Equal("https://ocis.example.prv/external-test/personal/path/to/newDocument.docx?fileId=storageid%24spaceid%21opaqueid_newDoc&view_mode=write"))
Expect(rBody["HostViewUrl"]).To(Equal("https://ocis.example.prv/external-test/personal/path/to/newDocument.docx?fileId=storageid%24spaceid%21opaqueid_newDoc&view_mode=view"))
})

It("PutRelativeFileRelative conflict", func() {
Expand Down Expand Up @@ -1250,6 +1264,8 @@ var _ = Describe("FileConnector", func() {
rBody := response.Body.(map[string]interface{})
Expect(rBody["Name"]).To(Equal("convFile.pdf"))
Expect(rBody["Url"]).To(HavePrefix("https://ocis.server.prv/wopi/files/")) // skip checking the actual reference
Expect(rBody["HostEditUrl"]).To(Equal("https://ocis.example.prv/external-test/personal/path/to/convFile.pdf?fileId=storageid%24spaceid%21opaqueid_newDoc&view_mode=write"))
Expect(rBody["HostViewUrl"]).To(Equal("https://ocis.example.prv/external-test/personal/path/to/convFile.pdf?fileId=storageid%24spaceid%21opaqueid_newDoc&view_mode=view"))
})

It("PutRelativeFileRelative put file fails", func() {
Expand Down

0 comments on commit 2a8d7f5

Please sign in to comment.