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.

Sign up for GitHub

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: add missing urls to putRelative #10403

Merged
merged 2 commits into from
Oct 25, 2024
Merged

fix: add missing urls to putRelative #10403

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c4bd2f3
fix: add missing urls to putRelative
micbar Oct 23, 2024
406c8ca
tests: add checks to unit tests
micbar Oct 24, 2024
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
5 changes: 5 additions & 0 deletions changelog/unreleased/fix-put-relative.md
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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