From b1c387bdd0fdf2f0eb5dc1d628c9b16b5f02c148 Mon Sep 17 00:00:00 2001 From: Michael Barz Date: Mon, 5 Aug 2024 19:00:40 +0200 Subject: [PATCH] docs: improve code comments --- .../mocks/content_connector_service.go | 53 ++- .../mocks/file_connector_service.go | 93 ++-- .../pkg/connector/contentconnector.go | 38 +- .../pkg/connector/contentconnector_test.go | 54 ++- .../pkg/connector/fileconnector.go | 15 +- .../pkg/connector/fileconnector_test.go | 422 +++++++++++++----- .../pkg/connector/httpadapter.go | 2 +- .../pkg/connector/httpadapter_test.go | 66 ++- services/collaboration/pkg/helpers/version.go | 2 +- 9 files changed, 502 insertions(+), 243 deletions(-) diff --git a/services/collaboration/mocks/content_connector_service.go b/services/collaboration/mocks/content_connector_service.go index 9b034f5da2d..810f7e1b4cd 100644 --- a/services/collaboration/mocks/content_connector_service.go +++ b/services/collaboration/mocks/content_connector_service.go @@ -4,9 +4,13 @@ package mocks import ( context "context" + http "net/http" + io "io" mock "github.com/stretchr/testify/mock" + + typesv1beta1 "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" ) // ContentConnectorService is an autogenerated mock type for the ContentConnectorService type @@ -22,17 +26,17 @@ func (_m *ContentConnectorService) EXPECT() *ContentConnectorService_Expecter { return &ContentConnectorService_Expecter{mock: &_m.Mock} } -// GetFile provides a mock function with given fields: ctx, writer -func (_m *ContentConnectorService) GetFile(ctx context.Context, writer io.Writer) error { - ret := _m.Called(ctx, writer) +// GetFile provides a mock function with given fields: ctx, w +func (_m *ContentConnectorService) GetFile(ctx context.Context, w http.ResponseWriter) error { + ret := _m.Called(ctx, w) if len(ret) == 0 { panic("no return value specified for GetFile") } var r0 error - if rf, ok := ret.Get(0).(func(context.Context, io.Writer) error); ok { - r0 = rf(ctx, writer) + if rf, ok := ret.Get(0).(func(context.Context, http.ResponseWriter) error); ok { + r0 = rf(ctx, w) } else { r0 = ret.Error(0) } @@ -47,14 +51,14 @@ type ContentConnectorService_GetFile_Call struct { // GetFile is a helper method to define mock.On call // - ctx context.Context -// - writer io.Writer -func (_e *ContentConnectorService_Expecter) GetFile(ctx interface{}, writer interface{}) *ContentConnectorService_GetFile_Call { - return &ContentConnectorService_GetFile_Call{Call: _e.mock.On("GetFile", ctx, writer)} +// - w http.ResponseWriter +func (_e *ContentConnectorService_Expecter) GetFile(ctx interface{}, w interface{}) *ContentConnectorService_GetFile_Call { + return &ContentConnectorService_GetFile_Call{Call: _e.mock.On("GetFile", ctx, w)} } -func (_c *ContentConnectorService_GetFile_Call) Run(run func(ctx context.Context, writer io.Writer)) *ContentConnectorService_GetFile_Call { +func (_c *ContentConnectorService_GetFile_Call) Run(run func(ctx context.Context, w http.ResponseWriter)) *ContentConnectorService_GetFile_Call { _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].(io.Writer)) + run(args[0].(context.Context), args[1].(http.ResponseWriter)) }) return _c } @@ -64,13 +68,13 @@ func (_c *ContentConnectorService_GetFile_Call) Return(_a0 error) *ContentConnec return _c } -func (_c *ContentConnectorService_GetFile_Call) RunAndReturn(run func(context.Context, io.Writer) error) *ContentConnectorService_GetFile_Call { +func (_c *ContentConnectorService_GetFile_Call) RunAndReturn(run func(context.Context, http.ResponseWriter) error) *ContentConnectorService_GetFile_Call { _c.Call.Return(run) return _c } // PutFile provides a mock function with given fields: ctx, stream, streamLength, lockID -func (_m *ContentConnectorService) PutFile(ctx context.Context, stream io.Reader, streamLength int64, lockID string) (string, error) { +func (_m *ContentConnectorService) PutFile(ctx context.Context, stream io.Reader, streamLength int64, lockID string) (string, *typesv1beta1.Timestamp, error) { ret := _m.Called(ctx, stream, streamLength, lockID) if len(ret) == 0 { @@ -78,8 +82,9 @@ func (_m *ContentConnectorService) PutFile(ctx context.Context, stream io.Reader } var r0 string - var r1 error - if rf, ok := ret.Get(0).(func(context.Context, io.Reader, int64, string) (string, error)); ok { + var r1 *typesv1beta1.Timestamp + var r2 error + if rf, ok := ret.Get(0).(func(context.Context, io.Reader, int64, string) (string, *typesv1beta1.Timestamp, error)); ok { return rf(ctx, stream, streamLength, lockID) } if rf, ok := ret.Get(0).(func(context.Context, io.Reader, int64, string) string); ok { @@ -88,13 +93,21 @@ func (_m *ContentConnectorService) PutFile(ctx context.Context, stream io.Reader r0 = ret.Get(0).(string) } - if rf, ok := ret.Get(1).(func(context.Context, io.Reader, int64, string) error); ok { + if rf, ok := ret.Get(1).(func(context.Context, io.Reader, int64, string) *typesv1beta1.Timestamp); ok { r1 = rf(ctx, stream, streamLength, lockID) } else { - r1 = ret.Error(1) + if ret.Get(1) != nil { + r1 = ret.Get(1).(*typesv1beta1.Timestamp) + } + } + + if rf, ok := ret.Get(2).(func(context.Context, io.Reader, int64, string) error); ok { + r2 = rf(ctx, stream, streamLength, lockID) + } else { + r2 = ret.Error(2) } - return r0, r1 + return r0, r1, r2 } // ContentConnectorService_PutFile_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'PutFile' @@ -118,12 +131,12 @@ func (_c *ContentConnectorService_PutFile_Call) Run(run func(ctx context.Context return _c } -func (_c *ContentConnectorService_PutFile_Call) Return(_a0 string, _a1 error) *ContentConnectorService_PutFile_Call { - _c.Call.Return(_a0, _a1) +func (_c *ContentConnectorService_PutFile_Call) Return(_a0 string, _a1 *typesv1beta1.Timestamp, _a2 error) *ContentConnectorService_PutFile_Call { + _c.Call.Return(_a0, _a1, _a2) return _c } -func (_c *ContentConnectorService_PutFile_Call) RunAndReturn(run func(context.Context, io.Reader, int64, string) (string, error)) *ContentConnectorService_PutFile_Call { +func (_c *ContentConnectorService_PutFile_Call) RunAndReturn(run func(context.Context, io.Reader, int64, string) (string, *typesv1beta1.Timestamp, error)) *ContentConnectorService_PutFile_Call { _c.Call.Return(run) return _c } diff --git a/services/collaboration/mocks/file_connector_service.go b/services/collaboration/mocks/file_connector_service.go index 9518dd36ae8..9f0ed247fba 100644 --- a/services/collaboration/mocks/file_connector_service.go +++ b/services/collaboration/mocks/file_connector_service.go @@ -12,6 +12,10 @@ import ( io "io" mock "github.com/stretchr/testify/mock" + + providerv1beta1 "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" + + typesv1beta1 "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" ) // FileConnectorService is an autogenerated mock type for the FileConnectorService type @@ -199,31 +203,42 @@ func (_c *FileConnectorService_GetLock_Call) RunAndReturn(run func(context.Conte } // Lock provides a mock function with given fields: ctx, lockID, oldLockID -func (_m *FileConnectorService) Lock(ctx context.Context, lockID string, oldLockID string) (string, error) { +func (_m *FileConnectorService) Lock(ctx context.Context, lockID string, oldLockID string) (*providerv1beta1.Lock, *typesv1beta1.Timestamp, error) { ret := _m.Called(ctx, lockID, oldLockID) if len(ret) == 0 { panic("no return value specified for Lock") } - var r0 string - var r1 error - if rf, ok := ret.Get(0).(func(context.Context, string, string) (string, error)); ok { + var r0 *providerv1beta1.Lock + var r1 *typesv1beta1.Timestamp + var r2 error + if rf, ok := ret.Get(0).(func(context.Context, string, string) (*providerv1beta1.Lock, *typesv1beta1.Timestamp, error)); ok { return rf(ctx, lockID, oldLockID) } - if rf, ok := ret.Get(0).(func(context.Context, string, string) string); ok { + if rf, ok := ret.Get(0).(func(context.Context, string, string) *providerv1beta1.Lock); ok { r0 = rf(ctx, lockID, oldLockID) } else { - r0 = ret.Get(0).(string) + if ret.Get(0) != nil { + r0 = ret.Get(0).(*providerv1beta1.Lock) + } } - if rf, ok := ret.Get(1).(func(context.Context, string, string) error); ok { + if rf, ok := ret.Get(1).(func(context.Context, string, string) *typesv1beta1.Timestamp); ok { r1 = rf(ctx, lockID, oldLockID) } else { - r1 = ret.Error(1) + if ret.Get(1) != nil { + r1 = ret.Get(1).(*typesv1beta1.Timestamp) + } } - return r0, r1 + if rf, ok := ret.Get(2).(func(context.Context, string, string) error); ok { + r2 = rf(ctx, lockID, oldLockID) + } else { + r2 = ret.Error(2) + } + + return r0, r1, r2 } // FileConnectorService_Lock_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Lock' @@ -246,12 +261,12 @@ func (_c *FileConnectorService_Lock_Call) Run(run func(ctx context.Context, lock return _c } -func (_c *FileConnectorService_Lock_Call) Return(_a0 string, _a1 error) *FileConnectorService_Lock_Call { - _c.Call.Return(_a0, _a1) +func (_c *FileConnectorService_Lock_Call) Return(_a0 *providerv1beta1.Lock, _a1 *typesv1beta1.Timestamp, _a2 error) *FileConnectorService_Lock_Call { + _c.Call.Return(_a0, _a1, _a2) return _c } -func (_c *FileConnectorService_Lock_Call) RunAndReturn(run func(context.Context, string, string) (string, error)) *FileConnectorService_Lock_Call { +func (_c *FileConnectorService_Lock_Call) RunAndReturn(run func(context.Context, string, string) (*providerv1beta1.Lock, *typesv1beta1.Timestamp, error)) *FileConnectorService_Lock_Call { _c.Call.Return(run) return _c } @@ -390,7 +405,7 @@ func (_c *FileConnectorService_PutRelativeFileSuggested_Call) RunAndReturn(run f } // RefreshLock provides a mock function with given fields: ctx, lockID -func (_m *FileConnectorService) RefreshLock(ctx context.Context, lockID string) (string, error) { +func (_m *FileConnectorService) RefreshLock(ctx context.Context, lockID string) (string, *typesv1beta1.Timestamp, error) { ret := _m.Called(ctx, lockID) if len(ret) == 0 { @@ -398,8 +413,9 @@ func (_m *FileConnectorService) RefreshLock(ctx context.Context, lockID string) } var r0 string - var r1 error - if rf, ok := ret.Get(0).(func(context.Context, string) (string, error)); ok { + var r1 *typesv1beta1.Timestamp + var r2 error + if rf, ok := ret.Get(0).(func(context.Context, string) (string, *typesv1beta1.Timestamp, error)); ok { return rf(ctx, lockID) } if rf, ok := ret.Get(0).(func(context.Context, string) string); ok { @@ -408,13 +424,21 @@ func (_m *FileConnectorService) RefreshLock(ctx context.Context, lockID string) r0 = ret.Get(0).(string) } - if rf, ok := ret.Get(1).(func(context.Context, string) error); ok { + if rf, ok := ret.Get(1).(func(context.Context, string) *typesv1beta1.Timestamp); ok { r1 = rf(ctx, lockID) } else { - r1 = ret.Error(1) + if ret.Get(1) != nil { + r1 = ret.Get(1).(*typesv1beta1.Timestamp) + } } - return r0, r1 + if rf, ok := ret.Get(2).(func(context.Context, string) error); ok { + r2 = rf(ctx, lockID) + } else { + r2 = ret.Error(2) + } + + return r0, r1, r2 } // FileConnectorService_RefreshLock_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'RefreshLock' @@ -436,12 +460,12 @@ func (_c *FileConnectorService_RefreshLock_Call) Run(run func(ctx context.Contex return _c } -func (_c *FileConnectorService_RefreshLock_Call) Return(_a0 string, _a1 error) *FileConnectorService_RefreshLock_Call { - _c.Call.Return(_a0, _a1) +func (_c *FileConnectorService_RefreshLock_Call) Return(_a0 string, _a1 *typesv1beta1.Timestamp, _a2 error) *FileConnectorService_RefreshLock_Call { + _c.Call.Return(_a0, _a1, _a2) return _c } -func (_c *FileConnectorService_RefreshLock_Call) RunAndReturn(run func(context.Context, string) (string, error)) *FileConnectorService_RefreshLock_Call { +func (_c *FileConnectorService_RefreshLock_Call) RunAndReturn(run func(context.Context, string) (string, *typesv1beta1.Timestamp, error)) *FileConnectorService_RefreshLock_Call { _c.Call.Return(run) return _c } @@ -514,7 +538,7 @@ func (_c *FileConnectorService_RenameFile_Call) RunAndReturn(run func(context.Co } // UnLock provides a mock function with given fields: ctx, lockID -func (_m *FileConnectorService) UnLock(ctx context.Context, lockID string) (string, error) { +func (_m *FileConnectorService) UnLock(ctx context.Context, lockID string) (string, *typesv1beta1.Timestamp, error) { ret := _m.Called(ctx, lockID) if len(ret) == 0 { @@ -522,8 +546,9 @@ func (_m *FileConnectorService) UnLock(ctx context.Context, lockID string) (stri } var r0 string - var r1 error - if rf, ok := ret.Get(0).(func(context.Context, string) (string, error)); ok { + var r1 *typesv1beta1.Timestamp + var r2 error + if rf, ok := ret.Get(0).(func(context.Context, string) (string, *typesv1beta1.Timestamp, error)); ok { return rf(ctx, lockID) } if rf, ok := ret.Get(0).(func(context.Context, string) string); ok { @@ -532,13 +557,21 @@ func (_m *FileConnectorService) UnLock(ctx context.Context, lockID string) (stri r0 = ret.Get(0).(string) } - if rf, ok := ret.Get(1).(func(context.Context, string) error); ok { + if rf, ok := ret.Get(1).(func(context.Context, string) *typesv1beta1.Timestamp); ok { r1 = rf(ctx, lockID) } else { - r1 = ret.Error(1) + if ret.Get(1) != nil { + r1 = ret.Get(1).(*typesv1beta1.Timestamp) + } } - return r0, r1 + if rf, ok := ret.Get(2).(func(context.Context, string) error); ok { + r2 = rf(ctx, lockID) + } else { + r2 = ret.Error(2) + } + + return r0, r1, r2 } // FileConnectorService_UnLock_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'UnLock' @@ -560,12 +593,12 @@ func (_c *FileConnectorService_UnLock_Call) Run(run func(ctx context.Context, lo return _c } -func (_c *FileConnectorService_UnLock_Call) Return(_a0 string, _a1 error) *FileConnectorService_UnLock_Call { - _c.Call.Return(_a0, _a1) +func (_c *FileConnectorService_UnLock_Call) Return(_a0 string, _a1 *typesv1beta1.Timestamp, _a2 error) *FileConnectorService_UnLock_Call { + _c.Call.Return(_a0, _a1, _a2) return _c } -func (_c *FileConnectorService_UnLock_Call) RunAndReturn(run func(context.Context, string) (string, error)) *FileConnectorService_UnLock_Call { +func (_c *FileConnectorService_UnLock_Call) RunAndReturn(run func(context.Context, string) (string, *typesv1beta1.Timestamp, error)) *FileConnectorService_UnLock_Call { _c.Call.Return(run) return _c } diff --git a/services/collaboration/pkg/connector/contentconnector.go b/services/collaboration/pkg/connector/contentconnector.go index 0c9db3adba3..09efe89e36f 100644 --- a/services/collaboration/pkg/connector/contentconnector.go +++ b/services/collaboration/pkg/connector/contentconnector.go @@ -33,7 +33,7 @@ type ContentConnectorService interface { // locked beforehand, so the lockID needs to be provided. // The current lockID will be returned ONLY if a conflict happens (the file is // locked with a different lockID) - PutFile(ctx context.Context, stream io.Reader, streamLength int64, lockID string) (*types.Timestamp, string, error) + PutFile(ctx context.Context, stream io.Reader, streamLength int64, lockID string) (string, *types.Timestamp, error) } // ContentConnector implements the "File contents" endpoint. @@ -60,8 +60,10 @@ func NewContentConnector(gwc gatewayv1beta1.GatewayAPIClient, cfg *config.Config // You can pass a pre-configured zerologger instance through the context that // will be used to log messages. // -// The contents of the file will be written directly into the writer passed as +// The contents of the file will be written directly into the http Response writer passed as // parameter. +// Be aware that the body of the response will be written during the execution of this method. +// Any further modifications to the response headers or body will be ignored. func (c *ContentConnector) GetFile(ctx context.Context, w http.ResponseWriter) error { wopiContext, err := middleware.WopiContextFromCtx(ctx) if err != nil { @@ -207,10 +209,12 @@ func (c *ContentConnector) GetFile(ctx context.Context, w http.ResponseWriter) e // lock ID that should be used in the X-WOPI-Lock header. In other error // cases or if the method is successful, an empty string will be returned // (check for err != nil to know if something went wrong) -func (c *ContentConnector) PutFile(ctx context.Context, stream io.Reader, streamLength int64, lockID string) (*types.Timestamp, string, error) { +// +// On success, the method will return the new mtime of the file +func (c *ContentConnector) PutFile(ctx context.Context, stream io.Reader, streamLength int64, lockID string) (string, *types.Timestamp, error) { wopiContext, err := middleware.WopiContextFromCtx(ctx) if err != nil { - return nil, "", err + return "", nil, err } logger := zerolog.Ctx(ctx).With(). @@ -227,7 +231,7 @@ func (c *ContentConnector) PutFile(ctx context.Context, stream io.Reader, stream }) if err != nil { logger.Error().Err(err).Msg("PutFile: stat failed") - return nil, "", err + return "", nil, err } if statRes.GetStatus().GetCode() != rpcv1beta1.Code_CODE_OK && statRes.GetStatus().GetCode() != rpcv1beta1.Code_CODE_NOT_FOUND { @@ -235,7 +239,7 @@ func (c *ContentConnector) PutFile(ctx context.Context, stream io.Reader, stream Str("StatusCode", statRes.GetStatus().GetCode().String()). Str("StatusMsg", statRes.GetStatus().GetMessage()). Msg("PutFile: stat failed with unexpected status") - return nil, "", NewConnectorError(500, statRes.GetStatus().GetCode().String()+" "+statRes.GetStatus().GetMessage()) + return "", nil, NewConnectorError(500, statRes.GetStatus().GetCode().String()+" "+statRes.GetStatus().GetMessage()) } mtime := statRes.GetInfo().GetMtime() @@ -245,7 +249,7 @@ func (c *ContentConnector) PutFile(ctx context.Context, stream io.Reader, stream Str("LockID", statRes.GetInfo().GetLock().GetLockId()). Msg("PutFile: wrong lock") // onlyoffice says it's required to send the current lockId, MS doesn't say anything - return nil, statRes.GetInfo().GetLock().GetLockId(), NewConnectorError(409, "Wrong lock") + return statRes.GetInfo().GetLock().GetLockId(), nil, NewConnectorError(409, "Wrong lock") } // only unlocked uploads can go through if the target file is empty, @@ -255,7 +259,7 @@ func (c *ContentConnector) PutFile(ctx context.Context, stream io.Reader, stream if lockID == "" && statRes.GetInfo().GetLock() == nil && statRes.GetInfo().GetSize() > 0 { logger.Error().Msg("PutFile: file must be locked first") // onlyoffice says to send an empty string if the file is unlocked, MS doesn't say anything - return nil, "", NewConnectorError(409, "File must be locked first") + return "", nil, NewConnectorError(409, "File must be locked first") } // Prepare the data to initiate the upload @@ -283,7 +287,7 @@ func (c *ContentConnector) PutFile(ctx context.Context, stream io.Reader, stream resp, err := c.gwc.InitiateFileUpload(ctx, req) if err != nil { logger.Error().Err(err).Msg("UploadHelper: InitiateFileUpload failed") - return nil, "", err + return "", nil, err } if resp.GetStatus().GetCode() != rpcv1beta1.Code_CODE_OK { @@ -291,7 +295,7 @@ func (c *ContentConnector) PutFile(ctx context.Context, stream io.Reader, stream Str("StatusCode", resp.GetStatus().GetCode().String()). Str("StatusMsg", resp.GetStatus().GetMessage()). Msg("UploadHelper: InitiateFileUpload failed with wrong status") - return nil, "", NewConnectorError(500, resp.GetStatus().GetCode().String()+" "+resp.GetStatus().GetMessage()) + return "", nil, NewConnectorError(500, resp.GetStatus().GetCode().String()+" "+resp.GetStatus().GetMessage()) } // if the content length is greater than 0, we need to upload the content to the @@ -316,7 +320,7 @@ func (c *ContentConnector) PutFile(ctx context.Context, stream io.Reader, stream Str("Endpoint", uploadEndpoint). Bool("HasUploadToken", hasUploadToken). Msg("UploadHelper: Upload endpoint or token is missing") - return nil, "", NewConnectorError(500, "upload endpoint or token is missing") + return "", nil, NewConnectorError(500, "upload endpoint or token is missing") } httpClient := http.Client{ @@ -336,7 +340,7 @@ func (c *ContentConnector) PutFile(ctx context.Context, stream io.Reader, stream Str("Endpoint", uploadEndpoint). Bool("HasUploadToken", hasUploadToken). Msg("UploadHelper: Could not create the request to the endpoint") - return nil, "", err + return "", nil, err } // "stream" is an *http.body and doesn't fill the httpReq.ContentLength automatically // we need to fill the ContentLength ourselves, and must match the stream length in order @@ -362,7 +366,7 @@ func (c *ContentConnector) PutFile(ctx context.Context, stream io.Reader, stream Str("Endpoint", uploadEndpoint). Bool("HasUploadToken", hasUploadToken). Msg("UploadHelper: Put request to the upload endpoint failed") - return nil, "", err + return "", nil, err } defer httpResp.Body.Close() @@ -372,7 +376,7 @@ func (c *ContentConnector) PutFile(ctx context.Context, stream io.Reader, stream Bool("HasUploadToken", hasUploadToken). Int("HttpCode", httpResp.StatusCode). Msg("UploadHelper: Put request to the upload endpoint failed with unexpected status") - return nil, "", NewConnectorError(500, "PutFile: Uploading the file failed") + return "", nil, NewConnectorError(500, "PutFile: Uploading the file failed") } // We need a stat call on the target file after the upload to get the // new mtime @@ -381,18 +385,18 @@ func (c *ContentConnector) PutFile(ctx context.Context, stream io.Reader, stream }) if err != nil { logger.Error().Err(err).Msg("PutFile: stat after upload failed") - return nil, "", err + return "", nil, err } if statResAfter.GetStatus().GetCode() != rpcv1beta1.Code_CODE_OK { logger.Error(). Str("StatusCode", statRes.GetStatus().GetCode().String()). Str("StatusMsg", statRes.GetStatus().GetMessage()). Msg("PutFile: stat after upload failed with unexpected status") - return nil, "", NewConnectorError(500, statResAfter.GetStatus().GetCode().String()+" "+statResAfter.GetStatus().GetMessage()) + return "", nil, NewConnectorError(500, statResAfter.GetStatus().GetCode().String()+" "+statResAfter.GetStatus().GetMessage()) } mtime = statResAfter.GetInfo().GetMtime() } logger.Debug().Msg("PutFile: success") - return mtime, "", nil + return "", mtime, nil } diff --git a/services/collaboration/pkg/connector/contentconnector_test.go b/services/collaboration/pkg/connector/contentconnector_test.go index aaf9fecf7de..dce2ccdb3c2 100644 --- a/services/collaboration/pkg/connector/contentconnector_test.go +++ b/services/collaboration/pkg/connector/contentconnector_test.go @@ -13,7 +13,6 @@ import ( appproviderv1beta1 "github.com/cs3org/go-cs3apis/cs3/app/provider/v1beta1" gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" - userv1beta1 "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" providerv1beta1 "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" revactx "github.com/cs3org/reva/v2/pkg/ctx" "github.com/cs3org/reva/v2/pkg/rgrpc/status" @@ -52,10 +51,7 @@ var _ = Describe("ContentConnector", func() { }, Path: ".", }, - User: &userv1beta1.User{}, // Not used for now - ViewMode: appproviderv1beta1.ViewMode_VIEW_MODE_READ_WRITE, - EditAppUrl: "http://test.ex.prv/edit", - ViewAppUrl: "http://test.ex.prv/view", + ViewMode: appproviderv1beta1.ViewMode_VIEW_MODE_READ_WRITE, } randomContent = "This is the content of the test.txt file" @@ -80,14 +76,14 @@ var _ = Describe("ContentConnector", func() { Describe("GetFile", func() { It("No valid context", func() { - sb := &strings.Builder{} + sb := httptest.NewRecorder() ctx := context.Background() err := cc.GetFile(ctx, sb) Expect(err).To(HaveOccurred()) }) It("Initiate download failed", func() { - sb := &strings.Builder{} + sb := httptest.NewRecorder() ctx := middleware.WopiContextToCtx(context.Background(), wopiCtx) targetErr := errors.New("Something went wrong") @@ -100,7 +96,7 @@ var _ = Describe("ContentConnector", func() { }) It("Initiate download status not ok", func() { - sb := &strings.Builder{} + sb := httptest.NewRecorder() ctx := middleware.WopiContextToCtx(context.Background(), wopiCtx) gatewayClient.On("InitiateFileDownload", mock.Anything, mock.Anything).Times(1).Return(&gateway.InitiateFileDownloadResponse{ @@ -114,7 +110,7 @@ var _ = Describe("ContentConnector", func() { }) It("Missing download endpoint", func() { - sb := &strings.Builder{} + sb := httptest.NewRecorder() ctx := middleware.WopiContextToCtx(context.Background(), wopiCtx) gatewayClient.On("InitiateFileDownload", mock.Anything, mock.Anything).Times(1).Return(&gateway.InitiateFileDownloadResponse{ @@ -128,7 +124,7 @@ var _ = Describe("ContentConnector", func() { }) It("Download request failed", func() { - sb := &strings.Builder{} + sb := httptest.NewRecorder() ctx := middleware.WopiContextToCtx(context.Background(), wopiCtx) gatewayClient.On("InitiateFileDownload", mock.Anything, mock.Anything).Times(1).Return(&gateway.InitiateFileDownloadResponse{ @@ -151,7 +147,7 @@ var _ = Describe("ContentConnector", func() { }) It("Download request success", func() { - sb := &strings.Builder{} + sb := httptest.NewRecorder() ctx := middleware.WopiContextToCtx(context.Background(), wopiCtx) gatewayClient.On("InitiateFileDownload", mock.Anything, mock.Anything).Times(1).Return(&gateway.InitiateFileDownloadResponse{ @@ -169,11 +165,11 @@ var _ = Describe("ContentConnector", func() { Expect(srvReqHeader.Get("X-Access-Token")).To(Equal(wopiCtx.AccessToken)) Expect(srvReqHeader.Get("X-Reva-Transfer")).To(Equal("MyDownloadToken")) Expect(err).To(Succeed()) - Expect(sb.String()).To(Equal(randomContent)) + Expect(sb.Body.String()).To(Equal(randomContent)) }) It("ViewOnlyMode Download request success", func() { - sb := &strings.Builder{} + sb := httptest.NewRecorder() wopiCtx = middleware.WopiContext{ AccessToken: "abcdef123456", @@ -186,10 +182,7 @@ var _ = Describe("ContentConnector", func() { }, Path: ".", }, - User: &userv1beta1.User{}, // Not used for now - ViewMode: appproviderv1beta1.ViewMode_VIEW_MODE_VIEW_ONLY, - EditAppUrl: "http://test.ex.prv/edit", - ViewAppUrl: "http://test.ex.prv/view", + ViewMode: appproviderv1beta1.ViewMode_VIEW_MODE_VIEW_ONLY, } ctx := middleware.WopiContextToCtx(context.Background(), wopiCtx) @@ -212,7 +205,7 @@ var _ = Describe("ContentConnector", func() { Expect(srvReqHeader.Get("X-Access-Token")).To(Equal(wopiCtx.ViewOnlyToken)) Expect(srvReqHeader.Get("X-Reva-Transfer")).To(Equal("MyDownloadToken")) Expect(err).To(Succeed()) - Expect(sb.String()).To(Equal(randomContent)) + Expect(sb.Body.String()).To(Equal(randomContent)) }) }) @@ -220,9 +213,10 @@ var _ = Describe("ContentConnector", func() { It("No valid context", func() { reader := strings.NewReader("Content to upload is here!") ctx := context.Background() - newLockId, err := cc.PutFile(ctx, reader, reader.Size(), "notARandomLockId") + newLockId, mtime, err := cc.PutFile(ctx, reader, reader.Size(), "notARandomLockId") Expect(err).To(HaveOccurred()) Expect(newLockId).To(Equal("")) + Expect(mtime).To(Equal("")) }) It("Stat call failed", func() { @@ -234,7 +228,7 @@ var _ = Describe("ContentConnector", func() { Status: status.NewInternal(ctx, "Something failed"), }, targetErr) - newLockId, err := cc.PutFile(ctx, reader, reader.Size(), "notARandomLockId") + newLockId, _, err := cc.PutFile(ctx, reader, reader.Size(), "notARandomLockId") Expect(err).To(Equal(targetErr)) Expect(newLockId).To(Equal("")) }) @@ -247,7 +241,7 @@ var _ = Describe("ContentConnector", func() { Status: status.NewInternal(ctx, "Something failed"), }, nil) - newLockId, err := cc.PutFile(ctx, reader, reader.Size(), "notARandomLockId") + newLockId, _, err := cc.PutFile(ctx, reader, reader.Size(), "notARandomLockId") Expect(err).To(HaveOccurred()) conErr := err.(*connector.ConnectorError) Expect(conErr.HttpCodeOut).To(Equal(500)) @@ -268,7 +262,7 @@ var _ = Describe("ContentConnector", func() { }, }, nil) - newLockId, err := cc.PutFile(ctx, reader, reader.Size(), "notARandomLockId") + newLockId, _, err := cc.PutFile(ctx, reader, reader.Size(), "notARandomLockId") Expect(err).To(HaveOccurred()) conErr := err.(*connector.ConnectorError) Expect(conErr.HttpCodeOut).To(Equal(409)) @@ -287,7 +281,7 @@ var _ = Describe("ContentConnector", func() { }, }, nil) - newLockId, err := cc.PutFile(ctx, reader, reader.Size(), "") + newLockId, _, err := cc.PutFile(ctx, reader, reader.Size(), "") Expect(err).To(HaveOccurred()) conErr := err.(*connector.ConnectorError) Expect(conErr.HttpCodeOut).To(Equal(409)) @@ -314,7 +308,7 @@ var _ = Describe("ContentConnector", func() { Status: status.NewInternal(ctx, "Something failed"), }, targetErr) - newLockId, err := cc.PutFile(ctx, reader, reader.Size(), "goodAndValidLock") + newLockId, _, err := cc.PutFile(ctx, reader, reader.Size(), "goodAndValidLock") Expect(err).To(HaveOccurred()) Expect(newLockId).To(Equal("")) }) @@ -338,7 +332,7 @@ var _ = Describe("ContentConnector", func() { Status: status.NewInternal(ctx, "Something failed"), }, nil) - newLockId, err := cc.PutFile(ctx, reader, reader.Size(), "goodAndValidLock") + newLockId, _, err := cc.PutFile(ctx, reader, reader.Size(), "goodAndValidLock") Expect(err).To(HaveOccurred()) conErr := err.(*connector.ConnectorError) Expect(conErr.HttpCodeOut).To(Equal(500)) @@ -364,9 +358,10 @@ var _ = Describe("ContentConnector", func() { Status: status.NewOK(ctx), }, nil) - newLockId, err := cc.PutFile(ctx, reader, reader.Size(), "goodAndValidLock") + newLockId, mtime, err := cc.PutFile(ctx, reader, reader.Size(), "goodAndValidLock") Expect(err).To(Succeed()) Expect(newLockId).To(Equal("")) + Expect(mtime).To(Equal("")) }) It("Missing upload endpoint", func() { @@ -388,7 +383,7 @@ var _ = Describe("ContentConnector", func() { Status: status.NewOK(ctx), }, nil) - newLockId, err := cc.PutFile(ctx, reader, reader.Size(), "goodAndValidLock") + newLockId, _, err := cc.PutFile(ctx, reader, reader.Size(), "goodAndValidLock") Expect(err).To(HaveOccurred()) conErr := err.(*connector.ConnectorError) Expect(conErr.HttpCodeOut).To(Equal(500)) @@ -420,7 +415,7 @@ var _ = Describe("ContentConnector", func() { }, }, nil) - newLockId, err := cc.PutFile(ctx, reader, reader.Size(), "goodAndValidLock") + newLockId, _, err := cc.PutFile(ctx, reader, reader.Size(), "goodAndValidLock") Expect(srvReqHeader.Get("X-Access-Token")).To(Equal(wopiCtx.AccessToken)) Expect(err).To(HaveOccurred()) conErr := err.(*connector.ConnectorError) @@ -453,10 +448,11 @@ var _ = Describe("ContentConnector", func() { }, }, nil) - newLockId, err := cc.PutFile(ctx, reader, reader.Size(), "goodAndValidLock") + newLockId, mtime, err := cc.PutFile(ctx, reader, reader.Size(), "goodAndValidLock") Expect(srvReqHeader.Get("X-Access-Token")).To(Equal(wopiCtx.AccessToken)) Expect(err).To(Succeed()) Expect(newLockId).To(Equal("")) + Expect(mtime).To(Equal("")) }) }) }) diff --git a/services/collaboration/pkg/connector/fileconnector.go b/services/collaboration/pkg/connector/fileconnector.go index 00c356a4a90..e149203f9e0 100644 --- a/services/collaboration/pkg/connector/fileconnector.go +++ b/services/collaboration/pkg/connector/fileconnector.go @@ -75,15 +75,18 @@ type FileConnectorService interface { // is provided (not empty), the method will perform an unlockAndRelock // operation (unlock the file with the oldLockID and immediately relock // the file with the new lockID). - // The current lockID will be returned if a conflict happens + // The current lock will be returned if a conflict happens + // The timestamp of the file will be returned Lock(ctx context.Context, lockID, oldLockID string) (*providerv1beta1.Lock, *typesv1beta1.Timestamp, error) // RefreshLock will extend the lock time 30 minutes. The current lockID // needs to be provided. // The current lockID will be returned if a conflict happens + // The timestamp of the file will be returned RefreshLock(ctx context.Context, lockID string) (string, *typesv1beta1.Timestamp, error) - // Unlock will unlock the target file. The current lockID needs to be + // UnLock will unlock the target file. The current lockID needs to be // provided. // The current lockID will be returned if a conflict happens + // The timestamp of the file will be returned UnLock(ctx context.Context, lockID string) (string, *typesv1beta1.Timestamp, error) // CheckFileInfo will return the file information of the target file CheckFileInfo(ctx context.Context) (fileinfo.FileInfo, error) @@ -201,6 +204,8 @@ func (f *FileConnector) GetLock(ctx context.Context) (string, error) { // the method will return an empty lock id. // // For the "unlock and relock" operation, the behavior will be the same. +// +// On success, the mtime of the file will be returned. func (f *FileConnector) Lock(ctx context.Context, lockID, oldLockID string) (*providerv1beta1.Lock, *typesv1beta1.Timestamp, error) { wopiContext, err := middleware.WopiContextFromCtx(ctx) if err != nil { @@ -357,6 +362,8 @@ func (f *FileConnector) Lock(ctx context.Context, lockID, oldLockID string) (*pr // return an empty lock id. // The conflict happens if the provided lockID doesn't match the one actually // applied in the target file. +// +// On success, the mtime of the file will be returned. func (f *FileConnector) RefreshLock(ctx context.Context, lockID string) (string, *typesv1beta1.Timestamp, error) { wopiContext, err := middleware.WopiContextFromCtx(ctx) if err != nil { @@ -481,6 +488,8 @@ func (f *FileConnector) RefreshLock(ctx context.Context, lockID string) (string, // return an empty lock id. // The conflict happens if the provided lockID doesn't match the one actually // applied in the target file. +// +// On success, the mtime of the file will be returned. func (f *FileConnector) UnLock(ctx context.Context, lockID string) (string, *typesv1beta1.Timestamp, error) { wopiContext, err := middleware.WopiContextFromCtx(ctx) if err != nil { @@ -773,7 +782,7 @@ func (f *FileConnector) PutRelativeFileRelative(ctx context.Context, ccs Content var conError *ConnectorError // try to put the file - _, lockID, err := ccs.PutFile(newCtx, stream, streamLength, "") + lockID, _, err := ccs.PutFile(newCtx, stream, streamLength, "") if err != nil { // if the error isn't a connectorError, fail the request if !errors.As(err, &conError) { diff --git a/services/collaboration/pkg/connector/fileconnector_test.go b/services/collaboration/pkg/connector/fileconnector_test.go index d6f88c5ad79..25728319344 100644 --- a/services/collaboration/pkg/connector/fileconnector_test.go +++ b/services/collaboration/pkg/connector/fileconnector_test.go @@ -12,6 +12,7 @@ import ( userv1beta1 "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" providerv1beta1 "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" typesv1beta1 "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" + ctxpkg "github.com/cs3org/reva/v2/pkg/ctx" "github.com/cs3org/reva/v2/pkg/rgrpc/status" cs3mocks "github.com/cs3org/reva/v2/tests/cs3mocks/mocks" . "github.com/onsi/ginkgo/v2" @@ -63,37 +64,16 @@ var _ = Describe("FileConnector", func() { }, Path: ".", }, - User: &userv1beta1.User{ - Id: &userv1beta1.UserId{ - Idp: "inmemory", - OpaqueId: "opaqueId", - Type: userv1beta1.UserType_USER_TYPE_PRIMARY, - }, - Username: "Shaft", - DisplayName: "Pet Shaft", - Mail: "shaft@example.com", - // Opaque is here for reference, not used by default but might be needed for some tests - //Opaque: &typesv1beta1.Opaque{ - // Map: map[string]*typesv1beta1.OpaqueEntry{ - // "public-share-role": &typesv1beta1.OpaqueEntry{ - // Decoder: "plain", - // Value: []byte("viewer"), - // }, - // }, - //}, - }, - ViewMode: appproviderv1beta1.ViewMode_VIEW_MODE_READ_WRITE, - EditAppUrl: "http://test.ex.prv/edit", - ViewAppUrl: "http://test.ex.prv/view", + ViewMode: appproviderv1beta1.ViewMode_VIEW_MODE_READ_WRITE, } }) Describe("GetLock", func() { It("No valid context", func() { ctx := context.Background() - newLockId, err := fc.GetLock(ctx) + newLockID, err := fc.GetLock(ctx) Expect(err).To(HaveOccurred()) - Expect(newLockId).To(Equal("")) + Expect(newLockID).To(Equal("")) }) It("Get lock failed", func() { @@ -104,9 +84,9 @@ var _ = Describe("FileConnector", func() { Status: status.NewInternal(ctx, "Something failed"), }, targetErr) - newLockId, err := fc.GetLock(ctx) + newLockID, err := fc.GetLock(ctx) Expect(err).To(Equal(targetErr)) - Expect(newLockId).To(Equal("")) + Expect(newLockID).To(Equal("")) }) It("Get lock failed status not ok", func() { @@ -117,11 +97,11 @@ var _ = Describe("FileConnector", func() { Status: status.NewNotFound(ctx, "File is missing"), }, nil) - newLockId, err := fc.GetLock(ctx) + newLockID, err := fc.GetLock(ctx) Expect(err).To(HaveOccurred()) conErr := err.(*connector.ConnectorError) Expect(conErr.HttpCodeOut).To(Equal(404)) - Expect(newLockId).To(Equal("")) + Expect(newLockID).To(Equal("")) }) It("Get lock success", func() { @@ -146,19 +126,21 @@ var _ = Describe("FileConnector", func() { Describe("Lock", func() { It("No valid context", func() { ctx := context.Background() - newLockId, err := fc.Lock(ctx, "newLock", "") + newLock, mtime, err := fc.Lock(ctx, "newLock", "") Expect(err).To(HaveOccurred()) - Expect(newLockId).To(Equal("")) + Expect(newLock.GetLockId()).To(Equal("")) + Expect(mtime).To(BeNil()) }) It("Empty lockId", func() { ctx := middleware.WopiContextToCtx(context.Background(), wopiCtx) - newLockId, err := fc.Lock(ctx, "", "") + newLock, mtime, err := fc.Lock(ctx, "", "") Expect(err).To(HaveOccurred()) conErr := err.(*connector.ConnectorError) Expect(conErr.HttpCodeOut).To(Equal(400)) - Expect(newLockId).To(Equal("")) + Expect(newLock.GetLockId()).To(Equal("")) + Expect(mtime).To(BeNil()) }) It("Set lock failed", func() { @@ -169,10 +151,11 @@ var _ = Describe("FileConnector", func() { Status: status.NewInternal(ctx, "Something failed"), }, targetErr) - newLockId, err := fc.Lock(ctx, "abcdef123", "") + newLock, mtime, err := fc.Lock(ctx, "abcdef123", "") Expect(err).To(HaveOccurred()) Expect(err).To(Equal(targetErr)) - Expect(newLockId).To(Equal("")) + Expect(newLock.GetLockId()).To(Equal("")) + Expect(mtime).To(BeNil()) }) It("Set lock success", func() { @@ -182,9 +165,26 @@ var _ = Describe("FileConnector", func() { Status: status.NewOK(ctx), }, nil) - newLockId, err := fc.Lock(ctx, "abcdef123", "") + gatewayClient.EXPECT().Stat(mock.Anything, mock.Anything). + Return( + &providerv1beta1.StatResponse{ + Status: status.NewOK(ctx), + Info: &providerv1beta1.ResourceInfo{ + Mtime: &typesv1beta1.Timestamp{ + Seconds: 12345, + Nanos: 6789, + }, + }, + }, + nil, + ) + + newLock, mtime, err := fc.Lock(ctx, "abcdef123", "") Expect(err).To(Succeed()) - Expect(newLockId).To(Equal("")) + Expect(newLock.GetLockId()).To(Equal("")) + Expect(mtime.Seconds).To(Equal(uint64(12345))) + Expect(mtime.Nanos).To(Equal(uint32(6789))) + }) It("Set lock mismatches error getting lock", func() { @@ -199,10 +199,14 @@ var _ = Describe("FileConnector", func() { Status: status.NewInternal(ctx, "lock mismatch"), }, targetErr) - newLockId, err := fc.Lock(ctx, "abcdef123", "") + gatewayClient.EXPECT().Stat(mock.Anything, mock.Anything). + Return(&providerv1beta1.StatResponse{Status: status.NewOK(ctx)}, nil) + + newLock, mtime, err := fc.Lock(ctx, "abcdef123", "") Expect(err).To(HaveOccurred()) Expect(err).To(Equal(targetErr)) - Expect(newLockId).To(Equal("")) + Expect(newLock.GetLockId()).To(Equal("")) + Expect(mtime).To(BeNil()) }) It("Set lock mismatches", func() { @@ -220,11 +224,15 @@ var _ = Describe("FileConnector", func() { }, }, nil) - newLockId, err := fc.Lock(ctx, "abcdef123", "") + gatewayClient.EXPECT().Stat(mock.Anything, mock.Anything). + Return(&providerv1beta1.StatResponse{Status: status.NewOK(ctx)}, nil) + + newLock, mtime, err := fc.Lock(ctx, "abcdef123", "") Expect(err).To(HaveOccurred()) conErr := err.(*connector.ConnectorError) Expect(conErr.HttpCodeOut).To(Equal(409)) - Expect(newLockId).To(Equal("zzz999")) + Expect(newLock.GetLockId()).To(Equal("zzz999")) + Expect(mtime).To(BeNil()) }) It("Set lock mismatches but get lock matches", func() { @@ -242,9 +250,13 @@ var _ = Describe("FileConnector", func() { }, }, nil) - newLockId, err := fc.Lock(ctx, "abcdef123", "") + gatewayClient.EXPECT().Stat(mock.Anything, mock.Anything). + Return(&providerv1beta1.StatResponse{Status: status.NewOK(ctx)}, nil) + + newLock, mtime, err := fc.Lock(ctx, "abcdef123", "") Expect(err).To(Succeed()) - Expect(newLockId).To(Equal("abcdef123")) + Expect(newLock.GetLockId()).To(Equal("abcdef123")) + Expect(mtime).To(BeNil()) }) It("Set lock mismatches but get lock doesn't return lockId", func() { @@ -258,11 +270,15 @@ var _ = Describe("FileConnector", func() { Status: status.NewOK(ctx), }, nil) - newLockId, err := fc.Lock(ctx, "abcdef123", "") + gatewayClient.EXPECT().Stat(mock.Anything, mock.Anything). + Return(&providerv1beta1.StatResponse{Status: status.NewOK(ctx)}, nil) + + newLock, mtime, err := fc.Lock(ctx, "abcdef123", "") Expect(err).To(HaveOccurred()) conErr := err.(*connector.ConnectorError) Expect(conErr.HttpCodeOut).To(Equal(500)) - Expect(newLockId).To(Equal("")) + Expect(newLock.GetLockId()).To(Equal("")) + Expect(mtime).To(BeNil()) }) It("File not found", func() { @@ -272,11 +288,15 @@ var _ = Describe("FileConnector", func() { Status: status.NewNotFound(ctx, "file not found"), }, nil) - newLockId, err := fc.Lock(ctx, "abcdef123", "") + gatewayClient.EXPECT().Stat(mock.Anything, mock.Anything). + Return(&providerv1beta1.StatResponse{Status: status.NewOK(ctx)}, nil) + + newLock, mtime, err := fc.Lock(ctx, "abcdef123", "") Expect(err).To(HaveOccurred()) conErr := err.(*connector.ConnectorError) Expect(conErr.HttpCodeOut).To(Equal(404)) - Expect(newLockId).To(Equal("")) + Expect(newLock.GetLockId()).To(Equal("")) + Expect(mtime).To(BeNil()) }) It("Default error handling (insufficient storage)", func() { @@ -286,30 +306,36 @@ var _ = Describe("FileConnector", func() { Status: status.NewInsufficientStorage(ctx, nil, "file too big"), }, nil) - newLockId, err := fc.Lock(ctx, "abcdef123", "") + gatewayClient.EXPECT().Stat(mock.Anything, mock.Anything). + Return(&providerv1beta1.StatResponse{Status: status.NewOK(ctx)}, nil) + + newLock, mtime, err := fc.Lock(ctx, "abcdef123", "") Expect(err).To(HaveOccurred()) conErr := err.(*connector.ConnectorError) Expect(conErr.HttpCodeOut).To(Equal(500)) - Expect(newLockId).To(Equal("")) + Expect(newLock.GetLockId()).To(Equal("")) + Expect(mtime).To(BeNil()) }) }) Describe("Unlock and relock", func() { It("No valid context", func() { ctx := context.Background() - newLockId, err := fc.Lock(ctx, "newLock", "oldLock") + newLock, mtime, err := fc.Lock(ctx, "newLock", "oldLock") Expect(err).To(HaveOccurred()) - Expect(newLockId).To(Equal("")) + Expect(newLock.GetLockId()).To(Equal("")) + Expect(mtime).To(BeNil()) }) It("Empty lockId", func() { ctx := middleware.WopiContextToCtx(context.Background(), wopiCtx) - newLockId, err := fc.Lock(ctx, "", "oldLock") + newLock, mtime, err := fc.Lock(ctx, "", "oldLock") Expect(err).To(HaveOccurred()) conErr := err.(*connector.ConnectorError) Expect(conErr.HttpCodeOut).To(Equal(400)) - Expect(newLockId).To(Equal("")) + Expect(newLock.GetLockId()).To(Equal("")) + Expect(mtime).To(BeNil()) }) It("Refresh lock failed", func() { @@ -320,10 +346,11 @@ var _ = Describe("FileConnector", func() { Status: status.NewInternal(ctx, "Something failed"), }, targetErr) - newLockId, err := fc.Lock(ctx, "abcdef123", "oldLock") + newLock, mtime, err := fc.Lock(ctx, "abcdef123", "oldLock") Expect(err).To(HaveOccurred()) Expect(err).To(Equal(targetErr)) - Expect(newLockId).To(Equal("")) + Expect(newLock.GetLockId()).To(Equal("")) + Expect(mtime).To(BeNil()) }) It("Refresh lock success", func() { @@ -333,9 +360,19 @@ var _ = Describe("FileConnector", func() { Status: status.NewOK(ctx), }, nil) - newLockId, err := fc.Lock(ctx, "abcdef123", "oldLock") + gatewayClient.EXPECT().Stat(mock.Anything, mock.Anything). + Return(&providerv1beta1.StatResponse{ + Status: status.NewOK(ctx), + Info: &providerv1beta1.ResourceInfo{ + Mtime: &typesv1beta1.Timestamp{Seconds: uint64(12345), Nanos: uint32(6789)}, + }, + }, nil) + + newLock, mtime, err := fc.Lock(ctx, "abcdef123", "oldLock") Expect(err).To(Succeed()) - Expect(newLockId).To(Equal("")) + Expect(newLock.GetLockId()).To(Equal("")) + Expect(mtime.Seconds).To(Equal(uint64(12345))) + Expect(mtime.Nanos).To(Equal(uint32(6789))) }) It("Refresh lock mismatches error getting lock", func() { @@ -350,10 +387,14 @@ var _ = Describe("FileConnector", func() { Status: status.NewInternal(ctx, "lock mismatch"), }, targetErr) - newLockId, err := fc.Lock(ctx, "abcdef123", "112233") + gatewayClient.EXPECT().Stat(mock.Anything, mock.Anything). + Return(&providerv1beta1.StatResponse{Status: status.NewOK(ctx)}, nil) + + newLock, mtime, err := fc.Lock(ctx, "abcdef123", "112233") Expect(err).To(HaveOccurred()) Expect(err).To(Equal(targetErr)) - Expect(newLockId).To(Equal("")) + Expect(newLock.GetLockId()).To(Equal("")) + Expect(mtime).To(BeNil()) }) It("Refresh lock mismatches", func() { @@ -371,11 +412,15 @@ var _ = Describe("FileConnector", func() { }, }, nil) - newLockId, err := fc.Lock(ctx, "abcdef123", "112233") + gatewayClient.EXPECT().Stat(mock.Anything, mock.Anything). + Return(&providerv1beta1.StatResponse{Status: status.NewOK(ctx)}, nil) + + newLock, mtime, err := fc.Lock(ctx, "abcdef123", "112233") Expect(err).To(HaveOccurred()) conErr := err.(*connector.ConnectorError) Expect(conErr.HttpCodeOut).To(Equal(409)) - Expect(newLockId).To(Equal("zzz999")) + Expect(newLock.GetLockId()).To(Equal("zzz999")) + Expect(mtime).To(BeNil()) }) It("Refresh lock mismatches but get lock matches", func() { @@ -393,9 +438,13 @@ var _ = Describe("FileConnector", func() { }, }, nil) - newLockId, err := fc.Lock(ctx, "abcdef123", "112233") + gatewayClient.EXPECT().Stat(mock.Anything, mock.Anything). + Return(&providerv1beta1.StatResponse{Status: status.NewOK(ctx)}, nil) + + newLock, mtime, err := fc.Lock(ctx, "abcdef123", "112233") Expect(err).To(Succeed()) - Expect(newLockId).To(Equal("abcdef123")) + Expect(newLock.GetLockId()).To(Equal("abcdef123")) + Expect(mtime).To(BeNil()) }) It("Refresh lock mismatches but get lock doesn't return lockId", func() { @@ -409,11 +458,15 @@ var _ = Describe("FileConnector", func() { Status: status.NewOK(ctx), }, nil) - newLockId, err := fc.Lock(ctx, "abcdef123", "112233") + gatewayClient.EXPECT().Stat(mock.Anything, mock.Anything). + Return(&providerv1beta1.StatResponse{Status: status.NewOK(ctx)}, nil) + + newLock, mtime, err := fc.Lock(ctx, "abcdef123", "112233") Expect(err).To(HaveOccurred()) conErr := err.(*connector.ConnectorError) Expect(conErr.HttpCodeOut).To(Equal(500)) - Expect(newLockId).To(Equal("")) + Expect(newLock.GetLockId()).To(Equal("")) + Expect(mtime).To(BeNil()) }) It("File not found", func() { @@ -423,11 +476,15 @@ var _ = Describe("FileConnector", func() { Status: status.NewNotFound(ctx, "file not found"), }, nil) - newLockId, err := fc.Lock(ctx, "abcdef123", "112233") + gatewayClient.EXPECT().Stat(mock.Anything, mock.Anything). + Return(&providerv1beta1.StatResponse{Status: status.NewOK(ctx)}, nil) + + newLock, mtime, err := fc.Lock(ctx, "abcdef123", "112233") Expect(err).To(HaveOccurred()) conErr := err.(*connector.ConnectorError) Expect(conErr.HttpCodeOut).To(Equal(404)) - Expect(newLockId).To(Equal("")) + Expect(newLock.GetLockId()).To(Equal("")) + Expect(mtime).To(BeNil()) }) It("Default error handling (insufficient storage)", func() { @@ -437,11 +494,15 @@ var _ = Describe("FileConnector", func() { Status: status.NewInsufficientStorage(ctx, nil, "file too big"), }, nil) - newLockId, err := fc.Lock(ctx, "abcdef123", "112233") + gatewayClient.EXPECT().Stat(mock.Anything, mock.Anything). + Return(&providerv1beta1.StatResponse{Status: status.NewOK(ctx)}, nil) + + newLock, mtime, err := fc.Lock(ctx, "abcdef123", "112233") Expect(err).To(HaveOccurred()) conErr := err.(*connector.ConnectorError) Expect(conErr.HttpCodeOut).To(Equal(500)) - Expect(newLockId).To(Equal("")) + Expect(newLock.GetLockId()).To(Equal("")) + Expect(mtime).To(BeNil()) }) }) }) @@ -449,19 +510,21 @@ var _ = Describe("FileConnector", func() { Describe("RefreshLock", func() { It("No valid context", func() { ctx := context.Background() - newLockId, err := fc.RefreshLock(ctx, "newLock") + newLockID, mtime, err := fc.RefreshLock(ctx, "newLock") Expect(err).To(HaveOccurred()) - Expect(newLockId).To(Equal("")) + Expect(newLockID).To(Equal("")) + Expect(mtime).To(BeNil()) }) It("Empty lockId", func() { ctx := middleware.WopiContextToCtx(context.Background(), wopiCtx) - newLockId, err := fc.RefreshLock(ctx, "") + newLockID, mtime, err := fc.RefreshLock(ctx, "") Expect(err).To(HaveOccurred()) conErr := err.(*connector.ConnectorError) Expect(conErr.HttpCodeOut).To(Equal(400)) - Expect(newLockId).To(Equal("")) + Expect(newLockID).To(Equal("")) + Expect(mtime).To(BeNil()) }) It("Refresh lock fails", func() { @@ -472,10 +535,11 @@ var _ = Describe("FileConnector", func() { Status: status.NewConflict(ctx, nil, "lock mismatch"), }, targetErr) - newLockId, err := fc.RefreshLock(ctx, "abcdef123") + newLockID, mtime, err := fc.RefreshLock(ctx, "abcdef123") Expect(err).To(HaveOccurred()) Expect(err).To(Equal(targetErr)) - Expect(newLockId).To(Equal("")) + Expect(newLockID).To(Equal("")) + Expect(mtime).To(BeNil()) }) It("Refresh lock success", func() { @@ -485,9 +549,19 @@ var _ = Describe("FileConnector", func() { Status: status.NewOK(ctx), }, nil) - newLockId, err := fc.RefreshLock(ctx, "abcdef123") + gatewayClient.EXPECT().Stat(mock.Anything, mock.Anything). + Return(&providerv1beta1.StatResponse{ + Status: status.NewOK(ctx), + Info: &providerv1beta1.ResourceInfo{ + Mtime: &typesv1beta1.Timestamp{Seconds: uint64(12345), Nanos: uint32(6789)}, + }, + }, nil) + + newLockID, mtime, err := fc.RefreshLock(ctx, "abcdef123") Expect(err).To(Succeed()) - Expect(newLockId).To(Equal("")) + Expect(newLockID).To(Equal("")) + Expect(mtime.Seconds).To(Equal(uint64(12345))) + Expect(mtime.Nanos).To(Equal(uint32(6789))) }) It("Refresh lock file not found", func() { @@ -497,11 +571,15 @@ var _ = Describe("FileConnector", func() { Status: status.NewNotFound(ctx, "file not found"), }, nil) - newLockId, err := fc.RefreshLock(ctx, "abcdef123") + gatewayClient.EXPECT().Stat(mock.Anything, mock.Anything). + Return(&providerv1beta1.StatResponse{Status: status.NewOK(ctx)}, nil) + + newLockID, mtime, err := fc.RefreshLock(ctx, "abcdef123") Expect(err).To(HaveOccurred()) conErr := err.(*connector.ConnectorError) Expect(conErr.HttpCodeOut).To(Equal(404)) - Expect(newLockId).To(Equal("")) + Expect(newLockID).To(Equal("")) + Expect(mtime).To(BeNil()) }) It("Refresh lock mismatch and get lock fails", func() { @@ -516,10 +594,14 @@ var _ = Describe("FileConnector", func() { Status: status.NewConflict(ctx, nil, "lock mismatch"), }, targetErr) - newLockId, err := fc.RefreshLock(ctx, "abcdef123") + gatewayClient.EXPECT().Stat(mock.Anything, mock.Anything). + Return(&providerv1beta1.StatResponse{Status: status.NewOK(ctx)}, nil) + + newLockID, mtime, err := fc.RefreshLock(ctx, "abcdef123") Expect(err).To(HaveOccurred()) Expect(err).To(Equal(targetErr)) - Expect(newLockId).To(Equal("")) + Expect(newLockID).To(Equal("")) + Expect(mtime).To(BeNil()) }) It("Refresh lock mismatch and get lock status not ok", func() { @@ -533,11 +615,15 @@ var _ = Describe("FileConnector", func() { Status: status.NewInternal(ctx, "lock mismatch"), }, nil) - newLockId, err := fc.RefreshLock(ctx, "abcdef123") + gatewayClient.EXPECT().Stat(mock.Anything, mock.Anything). + Return(&providerv1beta1.StatResponse{Status: status.NewOK(ctx)}, nil) + + newLockID, mtime, err := fc.RefreshLock(ctx, "abcdef123") Expect(err).To(HaveOccurred()) conErr := err.(*connector.ConnectorError) Expect(conErr.HttpCodeOut).To(Equal(500)) - Expect(newLockId).To(Equal("")) + Expect(newLockID).To(Equal("")) + Expect(mtime).To(BeNil()) }) It("Refresh lock mismatch and no lock", func() { @@ -551,11 +637,15 @@ var _ = Describe("FileConnector", func() { Status: status.NewOK(ctx), }, nil) - newLockId, err := fc.RefreshLock(ctx, "abcdef123") + gatewayClient.EXPECT().Stat(mock.Anything, mock.Anything). + Return(&providerv1beta1.StatResponse{Status: status.NewOK(ctx)}, nil) + + newLockID, mtime, err := fc.RefreshLock(ctx, "abcdef123") Expect(err).To(HaveOccurred()) conErr := err.(*connector.ConnectorError) Expect(conErr.HttpCodeOut).To(Equal(409)) - Expect(newLockId).To(Equal("")) + Expect(newLockID).To(Equal("")) + Expect(mtime).To(BeNil()) }) It("Refresh lock mismatch", func() { @@ -573,11 +663,15 @@ var _ = Describe("FileConnector", func() { }, }, nil) - newLockId, err := fc.RefreshLock(ctx, "abcdef123") + gatewayClient.EXPECT().Stat(mock.Anything, mock.Anything). + Return(&providerv1beta1.StatResponse{Status: status.NewOK(ctx)}, nil) + + newLockID, mtime, err := fc.RefreshLock(ctx, "abcdef123") Expect(err).To(HaveOccurred()) conErr := err.(*connector.ConnectorError) Expect(conErr.HttpCodeOut).To(Equal(409)) - Expect(newLockId).To(Equal("zzz999")) + Expect(newLockID).To(Equal("zzz999")) + Expect(mtime).To(BeNil()) }) It("Default error handling (insufficient storage)", func() { @@ -587,30 +681,36 @@ var _ = Describe("FileConnector", func() { Status: status.NewInsufficientStorage(ctx, nil, "file too big"), }, nil) - newLockId, err := fc.RefreshLock(ctx, "abcdef123") + gatewayClient.EXPECT().Stat(mock.Anything, mock.Anything). + Return(&providerv1beta1.StatResponse{Status: status.NewOK(ctx)}, nil) + + newLockID, mtime, err := fc.RefreshLock(ctx, "abcdef123") Expect(err).To(HaveOccurred()) conErr := err.(*connector.ConnectorError) Expect(conErr.HttpCodeOut).To(Equal(500)) - Expect(newLockId).To(Equal("")) + Expect(newLockID).To(Equal("")) + Expect(mtime).To(BeNil()) }) }) Describe("Unlock", func() { It("No valid context", func() { ctx := context.Background() - newLockId, err := fc.UnLock(ctx, "newLock") + newLockID, mtime, err := fc.UnLock(ctx, "newLock") Expect(err).To(HaveOccurred()) - Expect(newLockId).To(Equal("")) + Expect(newLockID).To(Equal("")) + Expect(mtime).To(BeNil()) }) It("Empty lockId", func() { ctx := middleware.WopiContextToCtx(context.Background(), wopiCtx) - newLockId, err := fc.UnLock(ctx, "") + newLockID, mtime, err := fc.UnLock(ctx, "") Expect(err).To(HaveOccurred()) conErr := err.(*connector.ConnectorError) Expect(conErr.HttpCodeOut).To(Equal(400)) - Expect(newLockId).To(Equal("")) + Expect(newLockID).To(Equal("")) + Expect(mtime).To(BeNil()) }) It("Unlock fails", func() { @@ -621,10 +721,11 @@ var _ = Describe("FileConnector", func() { Status: status.NewInternal(ctx, "something failed"), }, targetErr) - newLockId, err := fc.UnLock(ctx, "abcdef123") + newLockID, mtime, err := fc.UnLock(ctx, "abcdef123") Expect(err).To(HaveOccurred()) Expect(err).To(Equal(targetErr)) - Expect(newLockId).To(Equal("")) + Expect(newLockID).To(Equal("")) + Expect(mtime).To(BeNil()) }) It("Unlock success", func() { @@ -634,9 +735,19 @@ var _ = Describe("FileConnector", func() { Status: status.NewOK(ctx), }, nil) - newLockId, err := fc.UnLock(ctx, "abcdef123") + gatewayClient.EXPECT().Stat(mock.Anything, mock.Anything). + Return(&providerv1beta1.StatResponse{ + Status: status.NewOK(ctx), + Info: &providerv1beta1.ResourceInfo{ + Mtime: &typesv1beta1.Timestamp{Seconds: uint64(12345), Nanos: uint32(6789)}, + }, + }, nil) + + newLockID, mtime, err := fc.UnLock(ctx, "abcdef123") Expect(err).To(Succeed()) - Expect(newLockId).To(Equal("")) + Expect(newLockID).To(Equal("")) + Expect(mtime.Seconds).To(Equal(uint64(12345))) + Expect(mtime.Nanos).To(Equal(uint32(6789))) }) It("Unlock file isn't locked", func() { @@ -646,11 +757,21 @@ var _ = Describe("FileConnector", func() { Status: status.NewConflict(ctx, nil, "lock mismatch"), }, nil) - newLockId, err := fc.UnLock(ctx, "abcdef123") + gatewayClient.EXPECT().Stat(mock.Anything, mock.Anything). + Return(&providerv1beta1.StatResponse{ + Status: status.NewOK(ctx), + Info: &providerv1beta1.ResourceInfo{ + Mtime: &typesv1beta1.Timestamp{Seconds: uint64(12345), Nanos: uint32(6789)}, + }, + }, nil) + + newLockID, mtime, err := fc.UnLock(ctx, "abcdef123") Expect(err).To(HaveOccurred()) conErr := err.(*connector.ConnectorError) Expect(conErr.HttpCodeOut).To(Equal(409)) - Expect(newLockId).To(Equal("")) + Expect(newLockID).To(Equal("")) + Expect(mtime.Seconds).To(Equal(uint64(12345))) + Expect(mtime.Nanos).To(Equal(uint32(6789))) }) It("Unlock mismatch get lock fails", func() { @@ -665,10 +786,14 @@ var _ = Describe("FileConnector", func() { Status: status.NewInternal(ctx, "something failed"), }, targetErr) - newLockId, err := fc.UnLock(ctx, "abcdef123") + gatewayClient.EXPECT().Stat(mock.Anything, mock.Anything). + Return(&providerv1beta1.StatResponse{Status: status.NewOK(ctx)}, nil) + + newLockID, mtime, err := fc.UnLock(ctx, "abcdef123") Expect(err).To(HaveOccurred()) Expect(err).To(Equal(targetErr)) - Expect(newLockId).To(Equal("")) + Expect(newLockID).To(Equal("")) + Expect(mtime).To(BeNil()) }) It("Unlock mismatch get lock status not ok", func() { @@ -682,11 +807,15 @@ var _ = Describe("FileConnector", func() { Status: status.NewInternal(ctx, "something failed"), }, nil) - newLockId, err := fc.UnLock(ctx, "abcdef123") + gatewayClient.EXPECT().Stat(mock.Anything, mock.Anything). + Return(&providerv1beta1.StatResponse{Status: status.NewOK(ctx)}, nil) + + newLockID, mtime, err := fc.UnLock(ctx, "abcdef123") Expect(err).To(HaveOccurred()) conErr := err.(*connector.ConnectorError) Expect(conErr.HttpCodeOut).To(Equal(500)) - Expect(newLockId).To(Equal("")) + Expect(newLockID).To(Equal("")) + Expect(mtime).To(BeNil()) }) It("Unlock mismatch get lock doesn't return lock", func() { @@ -700,11 +829,15 @@ var _ = Describe("FileConnector", func() { Status: status.NewOK(ctx), }, nil) - newLockId, err := fc.UnLock(ctx, "abcdef123") + gatewayClient.EXPECT().Stat(mock.Anything, mock.Anything). + Return(&providerv1beta1.StatResponse{Status: status.NewOK(ctx)}, nil) + + newLockID, mtime, err := fc.UnLock(ctx, "abcdef123") Expect(err).To(HaveOccurred()) conErr := err.(*connector.ConnectorError) Expect(conErr.HttpCodeOut).To(Equal(409)) - Expect(newLockId).To(Equal("")) + Expect(newLockID).To(Equal("")) + Expect(mtime).To(BeNil()) }) It("Unlock mismatch", func() { @@ -722,11 +855,15 @@ var _ = Describe("FileConnector", func() { }, }, nil) - newLockId, err := fc.UnLock(ctx, "abcdef123") + gatewayClient.EXPECT().Stat(mock.Anything, mock.Anything). + Return(&providerv1beta1.StatResponse{Status: status.NewOK(ctx)}, nil) + + newLockID, mtime, err := fc.UnLock(ctx, "abcdef123") Expect(err).To(HaveOccurred()) conErr := err.(*connector.ConnectorError) Expect(conErr.HttpCodeOut).To(Equal(409)) - Expect(newLockId).To(Equal("zzz999")) + Expect(newLockID).To(Equal("zzz999")) + Expect(mtime).To(BeNil()) }) It("Default error handling (insufficient storage)", func() { @@ -736,11 +873,15 @@ var _ = Describe("FileConnector", func() { Status: status.NewInsufficientStorage(ctx, nil, "file too big"), }, nil) - newLockId, err := fc.UnLock(ctx, "abcdef123") + gatewayClient.EXPECT().Stat(mock.Anything, mock.Anything). + Return(&providerv1beta1.StatResponse{Status: status.NewOK(ctx)}, nil) + + newLockID, mtime, err := fc.UnLock(ctx, "abcdef123") Expect(err).To(HaveOccurred()) conErr := err.(*connector.ConnectorError) Expect(conErr.HttpCodeOut).To(Equal(500)) - Expect(newLockId).To(Equal("")) + Expect(newLockID).To(Equal("")) + Expect(mtime).To(BeNil()) }) }) @@ -1173,9 +1314,9 @@ var _ = Describe("FileConnector", func() { Describe("DeleteFile", func() { It("No valid context", func() { ctx := context.Background() - newLockId, err := fc.DeleteFile(ctx, "lock") + newLockID, err := fc.DeleteFile(ctx, "lock") Expect(err).To(HaveOccurred()) - Expect(newLockId).To(Equal("")) + Expect(newLockID).To(Equal("")) }) It("Delete fails", func() { @@ -1529,6 +1670,14 @@ var _ = Describe("FileConnector", func() { It("Stat success", func() { ctx := middleware.WopiContextToCtx(context.Background(), wopiCtx) + u := &userv1beta1.User{ + Id: &userv1beta1.UserId{ + Idp: "customIdp", + OpaqueId: "admin", + }, + DisplayName: "Pet Shaft", + } + ctx = ctxpkg.ContextSetUser(ctx, u) gatewayClient.On("Stat", mock.Anything, mock.Anything).Times(1).Return(&providerv1beta1.StatResponse{ Status: status.NewOK(ctx), @@ -1543,19 +1692,25 @@ var _ = Describe("FileConnector", func() { Seconds: uint64(16273849), }, Path: "/path/to/test.txt", - // Other properties aren't used for now. + Id: &providerv1beta1.ResourceId{ + StorageId: "storageid", + OpaqueId: "opaqueid", + SpaceId: "spaceid", + }, }, }, nil) expectedFileInfo := &fileinfo.Microsoft{ - OwnerID: "61616262636340637573746f6d496470", // hex of aabbcc@customIdp - Size: int64(998877), - Version: "16273849.0", - BaseFileName: "test.txt", - BreadcrumbDocName: "test.txt", - UserCanNotWriteRelative: false, - HostViewURL: "http://test.ex.prv/view", - HostEditURL: "http://test.ex.prv/edit", + OwnerID: "61616262636340637573746f6d496470", // hex of aabbcc@customIdp + Size: int64(998877), + Version: "v162738490", + BaseFileName: "test.txt", + BreadcrumbDocName: "test.txt", + BreadcrumbFolderName: "/path/to", + BreadcrumbFolderURL: "https://ocis.example.prv/f/storageid$spaceid%21opaqueid", + UserCanNotWriteRelative: false, + //HostViewURL: "http://test.ex.prv/view", + //HostEditURL: "http://test.ex.prv/edit", SupportsExtendedLockLength: true, SupportsGetLock: true, SupportsLocks: true, @@ -1564,7 +1719,7 @@ var _ = Describe("FileConnector", func() { SupportsRename: true, UserCanWrite: true, UserCanRename: true, - UserID: "6f7061717565496440696e6d656d6f7279", // hex of opaqueId@inmemory + UserID: "61646d696e40637573746f6d496470", // hex of admin@customIdp UserFriendlyName: "Pet Shaft", } @@ -1575,7 +1730,8 @@ var _ = Describe("FileConnector", func() { It("Stat success guests", func() { // add user's opaque to include public-share-role - wopiCtx.User.Opaque = &typesv1beta1.Opaque{ + u := &userv1beta1.User{} + u.Opaque = &typesv1beta1.Opaque{ Map: map[string]*typesv1beta1.OpaqueEntry{ "public-share-role": &typesv1beta1.OpaqueEntry{ Decoder: "plain", @@ -1587,6 +1743,7 @@ var _ = Describe("FileConnector", func() { wopiCtx.ViewMode = appproviderv1beta1.ViewMode_VIEW_MODE_VIEW_ONLY ctx := middleware.WopiContextToCtx(context.Background(), wopiCtx) + ctx = ctxpkg.ContextSetUser(ctx, u) gatewayClient.On("Stat", mock.Anything, mock.Anything).Times(1).Return(&providerv1beta1.StatResponse{ Status: status.NewOK(ctx), @@ -1601,6 +1758,11 @@ var _ = Describe("FileConnector", func() { Seconds: uint64(16273849), }, Path: "/path/to/test.txt", + Id: &providerv1beta1.ResourceId{ + StorageId: "storageid", + OpaqueId: "opaqueid", + SpaceId: "spaceid", + }, // Other properties aren't used for now. }, }, nil) @@ -1645,6 +1807,16 @@ var _ = Describe("FileConnector", func() { wopiCtx.ViewMode = appproviderv1beta1.ViewMode_VIEW_MODE_VIEW_ONLY ctx := middleware.WopiContextToCtx(context.Background(), wopiCtx) + u := &userv1beta1.User{ + Id: &userv1beta1.UserId{ + Idp: "example.com", + OpaqueId: "aabbcc", + Type: userv1beta1.UserType_USER_TYPE_PRIMARY, + }, + DisplayName: "Pet Shaft", + Mail: "shaft@example.com", + } + ctx = ctxpkg.ContextSetUser(ctx, u) gatewayClient.On("Stat", mock.Anything, mock.Anything).Times(1).Return(&providerv1beta1.StatResponse{ Status: status.NewOK(ctx), @@ -1659,7 +1831,11 @@ var _ = Describe("FileConnector", func() { Seconds: uint64(16273849), }, Path: "/path/to/test.txt", - // Other properties aren't used for now. + Id: &providerv1beta1.ResourceId{ + StorageId: "storageid", + OpaqueId: "opaqueid", + SpaceId: "spaceid", + }, }, }, nil) @@ -1674,7 +1850,7 @@ var _ = Describe("FileConnector", func() { DisableExport: true, DisableCopy: true, DisablePrint: true, - UserID: hex.EncodeToString([]byte("opaqueId@inmemory")), + UserID: hex.EncodeToString([]byte("aabbcc@example.com")), UserFriendlyName: "Pet Shaft", EnableOwnerTermination: true, WatermarkText: "Pet Shaft shaft@example.com", diff --git a/services/collaboration/pkg/connector/httpadapter.go b/services/collaboration/pkg/connector/httpadapter.go index 427845b8401..5dabed45cef 100644 --- a/services/collaboration/pkg/connector/httpadapter.go +++ b/services/collaboration/pkg/connector/httpadapter.go @@ -246,7 +246,7 @@ func (h *HttpAdapter) PutFile(w http.ResponseWriter, r *http.Request) { lockID := h.locks.ParseLock(r.Header.Get(HeaderWopiLock)) contentCon := h.con.GetContentConnector() - mtime, newLockID, err := contentCon.PutFile(r.Context(), r.Body, r.ContentLength, lockID) + newLockID, mtime, err := contentCon.PutFile(r.Context(), r.Body, r.ContentLength, lockID) helpers.SetVersionHeader(w, mtime) if err != nil { var conError *ConnectorError diff --git a/services/collaboration/pkg/connector/httpadapter_test.go b/services/collaboration/pkg/connector/httpadapter_test.go index 390740e51fd..96ba464a64b 100644 --- a/services/collaboration/pkg/connector/httpadapter_test.go +++ b/services/collaboration/pkg/connector/httpadapter_test.go @@ -7,6 +7,8 @@ import ( "net/http/httptest" "strings" + providerv1beta1 "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" + typesv1beta1 "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/owncloud/ocis/v2/services/collaboration/mocks" @@ -105,7 +107,7 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("Lock", mock.Anything, "abc123", "").Times(1).Return("", errors.New("Something happened")) + fc.On("Lock", mock.Anything, "abc123", "").Times(1).Return(&providerv1beta1.Lock{}, &typesv1beta1.Timestamp{}, errors.New("Something happened")) httpAdapter.Lock(w, req) resp := w.Result() @@ -119,7 +121,8 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("Lock", mock.Anything, "", "").Times(1).Return("", connector.NewConnectorError(400, "No lockId")) + fc.EXPECT().Lock(mock.Anything, "", "").Times(1). + Return(&providerv1beta1.Lock{}, &typesv1beta1.Timestamp{}, connector.NewConnectorError(400, "No lockId")) httpAdapter.Lock(w, req) resp := w.Result() @@ -133,7 +136,8 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("Lock", mock.Anything, "abc123", "").Times(1).Return("zzz111", connector.NewConnectorError(409, "Lock conflict")) + fc.EXPECT().Lock(mock.Anything, "abc123", "").Times(1). + Return(&providerv1beta1.Lock{LockId: "zzz111"}, nil, connector.NewConnectorError(409, "Lock conflict")) httpAdapter.Lock(w, req) resp := w.Result() @@ -148,11 +152,13 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("Lock", mock.Anything, "abc123", "").Times(1).Return("", nil) + fc.EXPECT().Lock(mock.Anything, "abc123", "").Times(1). + Return(&providerv1beta1.Lock{LockId: "abc123"}, &typesv1beta1.Timestamp{}, nil) httpAdapter.Lock(w, req) resp := w.Result() Expect(resp.StatusCode).To(Equal(200)) + Expect(resp.Header.Get(connector.HeaderWopiLock)).To(Equal("abc123")) }) }) @@ -165,7 +171,8 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("Lock", mock.Anything, "abc123", "qwerty").Times(1).Return("", errors.New("Something happened")) + fc.EXPECT().Lock(mock.Anything, "abc123", "qwerty").Times(1). + Return(nil, nil, errors.New("Something happened")) httpAdapter.Lock(w, req) resp := w.Result() @@ -180,7 +187,8 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("Lock", mock.Anything, "", "").Times(1).Return("", connector.NewConnectorError(400, "No lockId")) + fc.EXPECT().Lock(mock.Anything, "", "").Times(1). + Return(nil, nil, connector.NewConnectorError(400, "No lockId")) httpAdapter.Lock(w, req) resp := w.Result() @@ -195,7 +203,8 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("Lock", mock.Anything, "abc123", "qwerty").Times(1).Return("zzz111", connector.NewConnectorError(409, "Lock conflict")) + fc.EXPECT().Lock(mock.Anything, "abc123", "qwerty").Times(1). + Return(&providerv1beta1.Lock{LockId: "zzz111"}, nil, connector.NewConnectorError(409, "Lock conflict")) httpAdapter.Lock(w, req) resp := w.Result() @@ -211,11 +220,14 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("Lock", mock.Anything, "abc123", "qwerty").Times(1).Return("", nil) + fc.EXPECT().Lock(mock.Anything, "abc123", "qwerty").Times(1). + Return(&providerv1beta1.Lock{LockId: "abc123"}, &typesv1beta1.Timestamp{Seconds: uint64(1234), Nanos: uint32(567)}, nil) httpAdapter.Lock(w, req) resp := w.Result() Expect(resp.StatusCode).To(Equal(200)) + Expect(resp.Header.Get(connector.HeaderWopiLock)).To(Equal("abc123")) + Expect(resp.Header.Get("X-Wopi-Itemversion")).To(Equal("v1234567")) }) }) }) @@ -228,7 +240,8 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("RefreshLock", mock.Anything, "abc123").Times(1).Return("", errors.New("Something happened")) + fc.EXPECT().RefreshLock(mock.Anything, "abc123").Times(1). + Return("", &typesv1beta1.Timestamp{}, errors.New("Something happened")) httpAdapter.RefreshLock(w, req) resp := w.Result() @@ -242,7 +255,8 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("RefreshLock", mock.Anything, "").Times(1).Return("", connector.NewConnectorError(400, "No lockId")) + fc.EXPECT().RefreshLock(mock.Anything, "").Times(1). + Return("", &typesv1beta1.Timestamp{}, connector.NewConnectorError(400, "No lockId")) httpAdapter.RefreshLock(w, req) resp := w.Result() @@ -256,7 +270,8 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("RefreshLock", mock.Anything, "abc123").Times(1).Return("zzz111", connector.NewConnectorError(409, "Lock conflict")) + fc.EXPECT().RefreshLock(mock.Anything, "abc123").Times(1). + Return("zzz111", &typesv1beta1.Timestamp{}, connector.NewConnectorError(409, "Lock conflict")) httpAdapter.RefreshLock(w, req) resp := w.Result() @@ -271,11 +286,13 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("RefreshLock", mock.Anything, "abc123").Times(1).Return("", nil) + fc.EXPECT().RefreshLock(mock.Anything, "abc123").Times(1). + Return("", &typesv1beta1.Timestamp{Seconds: uint64(12345), Nanos: uint32(678)}, nil) httpAdapter.RefreshLock(w, req) resp := w.Result() Expect(resp.StatusCode).To(Equal(200)) + Expect(resp.Header.Get("X-Wopi-Itemversion")).To(Equal("v12345678")) }) }) @@ -287,7 +304,8 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("UnLock", mock.Anything, "abc123").Times(1).Return("", errors.New("Something happened")) + fc.EXPECT().UnLock(mock.Anything, "abc123").Times(1). + Return("", &typesv1beta1.Timestamp{}, errors.New("Something happened")) httpAdapter.UnLock(w, req) resp := w.Result() @@ -301,7 +319,8 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("UnLock", mock.Anything, "").Times(1).Return("", connector.NewConnectorError(400, "No lockId")) + fc.EXPECT().UnLock(mock.Anything, "").Times(1). + Return("", &typesv1beta1.Timestamp{}, connector.NewConnectorError(400, "No lockId")) httpAdapter.UnLock(w, req) resp := w.Result() @@ -315,12 +334,14 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("UnLock", mock.Anything, "abc123").Times(1).Return("zzz111", connector.NewConnectorError(409, "Lock conflict")) + fc.EXPECT().UnLock(mock.Anything, "abc123").Times(1). + Return("zzz111", &typesv1beta1.Timestamp{Seconds: uint64(12345), Nanos: uint32(678)}, connector.NewConnectorError(409, "Lock conflict")) httpAdapter.UnLock(w, req) resp := w.Result() Expect(resp.StatusCode).To(Equal(409)) Expect(resp.Header.Get(connector.HeaderWopiLock)).To(Equal("zzz111")) + Expect(resp.Header.Get("X-Wopi-Itemversion")).To(Equal("v12345678")) }) It("Success", func() { @@ -330,11 +351,13 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("UnLock", mock.Anything, "abc123").Times(1).Return("", nil) + fc.EXPECT().UnLock(mock.Anything, "abc123").Times(1). + Return("", &typesv1beta1.Timestamp{Seconds: uint64(12345), Nanos: uint32(678)}, nil) httpAdapter.UnLock(w, req) resp := w.Result() Expect(resp.StatusCode).To(Equal(200)) + Expect(resp.Header.Get("X-Wopi-Itemversion")).To(Equal("v12345678")) }) }) @@ -444,7 +467,8 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - cc.On("PutFile", mock.Anything, mock.Anything, int64(len(contentBody)), "abc123").Times(1).Return("", errors.New("Something happened")) + cc.EXPECT().PutFile(mock.Anything, mock.Anything, int64(len(contentBody)), "abc123").Times(1). + Return("", &typesv1beta1.Timestamp{}, errors.New("Something happened")) httpAdapter.PutFile(w, req) resp := w.Result() @@ -458,12 +482,14 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - cc.On("PutFile", mock.Anything, mock.Anything, int64(len(contentBody)), "abc123").Times(1).Return("zzz111", connector.NewConnectorError(409, "Lock conflict")) + cc.EXPECT().PutFile(mock.Anything, mock.Anything, int64(len(contentBody)), "abc123").Times(1). + Return("zzz111", &typesv1beta1.Timestamp{Seconds: uint64(1234), Nanos: uint32(567)}, connector.NewConnectorError(409, "Lock conflict")) httpAdapter.PutFile(w, req) resp := w.Result() Expect(resp.StatusCode).To(Equal(409)) Expect(resp.Header.Get(connector.HeaderWopiLock)).To(Equal("zzz111")) + Expect(resp.Header.Get("X-Wopi-Itemversion")).To(Equal("v1234567")) }) It("Success", func() { @@ -473,11 +499,13 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - cc.On("PutFile", mock.Anything, mock.Anything, int64(len(contentBody)), "abc123").Times(1).Return("", nil) + cc.EXPECT().PutFile(mock.Anything, mock.Anything, int64(len(contentBody)), "abc123").Times(1). + Return("", &typesv1beta1.Timestamp{Seconds: uint64(1234), Nanos: uint32(567)}, nil) httpAdapter.PutFile(w, req) resp := w.Result() Expect(resp.StatusCode).To(Equal(200)) + Expect(resp.Header.Get("X-Wopi-Itemversion")).To(Equal("v1234567")) }) }) }) diff --git a/services/collaboration/pkg/helpers/version.go b/services/collaboration/pkg/helpers/version.go index 874ad8fe675..a9196da061d 100644 --- a/services/collaboration/pkg/helpers/version.go +++ b/services/collaboration/pkg/helpers/version.go @@ -10,7 +10,7 @@ import ( // SetVersionHeader sets a WOPI version header on the response writer func SetVersionHeader(w http.ResponseWriter, t *typesv1beta1.Timestamp) { // non-canonical headers can only be set directly on the header map - w.Header()["X-WOPI-ItemVersion"] = []string{GetVersion(t)} + w.Header().Set("X-WOPI-ItemVersion", GetVersion(t)) } // GetVersion returns a string representation of the timestamp