From 6d9717f6d16101c9dba6a905a7bc6589666b3f3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Wed, 7 Aug 2024 18:34:58 +0200 Subject: [PATCH 1/6] refactor: simplify and homogenize connector code --- .../collaboration/pkg/connector/connector.go | 12 + .../pkg/connector/contentconnector.go | 38 +- .../pkg/connector/fileconnector.go | 381 ++++++++++-------- .../pkg/connector/httpadapter.go | 216 +++------- 4 files changed, 302 insertions(+), 345 deletions(-) diff --git a/services/collaboration/pkg/connector/connector.go b/services/collaboration/pkg/connector/connector.go index c4224ba74..200c13105 100644 --- a/services/collaboration/pkg/connector/connector.go +++ b/services/collaboration/pkg/connector/connector.go @@ -1,5 +1,17 @@ package connector +// ConnectorResponse represent a response from the FileConnectorService. +// The ConnectorResponse is oriented to HTTP, so it has the Status, Headers +// and Body that the actual HTTP response should have. This includes HTTP +// errors with status 4xx and 5xx, which will also represent some error +// conditions for the FileConnectorService. +// Note that the Body is expected to be JSON-encoded outside before sending. +type ConnectorResponse struct { + Status int + Headers map[string]string + Body interface{} +} + // ConnectorError defines an error in the connector. It contains an error code // and a message. // For convenience, the error code can be used as HTTP error code, although diff --git a/services/collaboration/pkg/connector/contentconnector.go b/services/collaboration/pkg/connector/contentconnector.go index ee7cad2d0..d816f4029 100644 --- a/services/collaboration/pkg/connector/contentconnector.go +++ b/services/collaboration/pkg/connector/contentconnector.go @@ -32,7 +32,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) (string, error) + PutFile(ctx context.Context, stream io.Reader, streamLength int64, lockID string) (*ConnectorResponse, error) } // ContentConnector implements the "File contents" endpoint. @@ -195,10 +195,10 @@ func (c *ContentConnector) GetFile(ctx context.Context, writer io.Writer) error // 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) (string, error) { +func (c *ContentConnector) PutFile(ctx context.Context, stream io.Reader, streamLength int64, lockID string) (*ConnectorResponse, error) { wopiContext, err := middleware.WopiContextFromCtx(ctx) if err != nil { - return "", err + return nil, err } logger := zerolog.Ctx(ctx).With(). @@ -215,7 +215,7 @@ func (c *ContentConnector) PutFile(ctx context.Context, stream io.Reader, stream }) if err != nil { logger.Error().Err(err).Msg("PutFile: stat failed") - return "", err + return nil, err } if statRes.GetStatus().GetCode() != rpcv1beta1.Code_CODE_OK && statRes.GetStatus().GetCode() != rpcv1beta1.Code_CODE_NOT_FOUND { @@ -223,7 +223,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 "", NewConnectorError(500, statRes.GetStatus().GetCode().String()+" "+statRes.GetStatus().GetMessage()) + return &ConnectorResponse{Status: 500}, nil } // If there is a lock and it mismatches, return 409 @@ -232,7 +232,12 @@ 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 statRes.GetInfo().GetLock().GetLockId(), NewConnectorError(409, "Wrong lock") + return &ConnectorResponse{ + Status: 409, + Headers: map[string]string{ + HeaderWopiLock: statRes.GetInfo().GetLock().GetLockId(), + }, + }, nil } // only unlocked uploads can go through if the target file is empty, @@ -242,7 +247,12 @@ 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 "", NewConnectorError(409, "File must be locked first") + return &ConnectorResponse{ + Status: 409, + Headers: map[string]string{ + HeaderWopiLock: "", + }, + }, nil } // Prepare the data to initiate the upload @@ -270,7 +280,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 "", err + return nil, err } if resp.GetStatus().GetCode() != rpcv1beta1.Code_CODE_OK { @@ -278,7 +288,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 "", NewConnectorError(500, resp.GetStatus().GetCode().String()+" "+resp.GetStatus().GetMessage()) + return &ConnectorResponse{Status: 500}, nil } // if the content length is greater than 0, we need to upload the content to the @@ -303,7 +313,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 "", NewConnectorError(500, "upload endpoint or token is missing") + return &ConnectorResponse{Status: 500}, nil } httpClient := http.Client{ @@ -323,7 +333,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 "", 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 @@ -349,7 +359,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 "", err + return nil, err } defer httpResp.Body.Close() @@ -359,10 +369,10 @@ 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 "", NewConnectorError(500, "PutFile: Uploading the file failed") + return &ConnectorResponse{Status: 500}, nil } } logger.Debug().Msg("PutFile: success") - return "", nil + return &ConnectorResponse{Status: 200}, nil } diff --git a/services/collaboration/pkg/connector/fileconnector.go b/services/collaboration/pkg/connector/fileconnector.go index 9bca8866f..df77254d3 100644 --- a/services/collaboration/pkg/connector/fileconnector.go +++ b/services/collaboration/pkg/connector/fileconnector.go @@ -7,7 +7,6 @@ import ( "encoding/base64" "encoding/binary" "encoding/hex" - "errors" "io" "net/url" "path" @@ -36,30 +35,6 @@ const ( lockDuration time.Duration = 30 * time.Minute ) -// PutRelativeHeaders contains the values for headers used in a -// "PutRelative" WOPI response -type PutRelativeHeaders struct { - ValidTarget string - LockID string -} - -// PutRelativeResponse contains the values for the body used in a -// "PutRelative" WOPI response -type PutRelativeResponse struct { - Name string - Url string - - // These are optional and not used for now - HostView string - HostEdit string -} - -// RenameResponse contains the values for the body used in a -// "RenameFile" WOPI response -type RenameResponse struct { - Name string -} - // FileConnectorService is the interface to implement the "Files" // endpoint. Basically lock operations on the file plus the CheckFileInfo. // All operations need a context containing a WOPI context and, optionally, @@ -67,23 +42,23 @@ type RenameResponse struct { // Target file is within the WOPI context type FileConnectorService interface { // GetLock will return the lockID present in the target file. - GetLock(ctx context.Context) (string, error) + GetLock(ctx context.Context) (*ConnectorResponse, error) // Lock will lock the target file with the provided lockID. If the oldLockID // 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 - Lock(ctx context.Context, lockID, oldLockID string) (string, error) + Lock(ctx context.Context, lockID, oldLockID string) (*ConnectorResponse, 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 - RefreshLock(ctx context.Context, lockID string) (string, error) + RefreshLock(ctx context.Context, lockID string) (*ConnectorResponse, error) // Unlock will unlock the target file. The current lockID needs to be // provided. // The current lockID will be returned if a conflict happens - UnLock(ctx context.Context, lockID string) (string, error) + UnLock(ctx context.Context, lockID string) (*ConnectorResponse, error) // CheckFileInfo will return the file information of the target file - CheckFileInfo(ctx context.Context) (fileinfo.FileInfo, error) + CheckFileInfo(ctx context.Context) (*ConnectorResponse, error) // PutRelativeFileSuggested will create a new file based on the contents of the // current file. Target is the filename that will be used for this // new file. @@ -91,7 +66,7 @@ type FileConnectorService interface { // Since we need to upload contents, it will be done through the provided // The target must be UTF8-encoded. // ContentConnectorService - PutRelativeFileSuggested(ctx context.Context, ccs ContentConnectorService, stream io.Reader, streamLength int64, target string) (*PutRelativeResponse, error) + PutRelativeFileSuggested(ctx context.Context, ccs ContentConnectorService, stream io.Reader, streamLength int64, target string) (*ConnectorResponse, error) // PutRelativeFileRelative will create a new file based on the contents of the // current file. Target is the filename that will be used for this // new file. @@ -101,17 +76,17 @@ type FileConnectorService interface { // The target must be UTF8-encoded. // Since we need to upload contents, it will be done through the provided // ContentConnectorService - PutRelativeFileRelative(ctx context.Context, ccs ContentConnectorService, stream io.Reader, streamLength int64, target string) (*PutRelativeResponse, *PutRelativeHeaders, error) + PutRelativeFileRelative(ctx context.Context, ccs ContentConnectorService, stream io.Reader, streamLength int64, target string) (*ConnectorResponse, error) // DeleteFile will delete the provided file in the context. Although // not documented, a lockID can be used to try to delete a locked file // assuming the lock matches. // The current lockID will be returned if the file is locked. - DeleteFile(ctx context.Context, lockID string) (string, error) + DeleteFile(ctx context.Context, lockID string) (*ConnectorResponse, error) // RenameFile will rename the provided file in the context to the requested // filename. The filename must be UTF8-encoded. // In case of conflict, this method will return the actual lockId in // the file as second return value. - RenameFile(ctx context.Context, lockID, target string) (*RenameResponse, string, error) + RenameFile(ctx context.Context, lockID, target string) (*ConnectorResponse, error) } // FileConnector implements the "File" endpoint. @@ -140,10 +115,10 @@ func NewFileConnector(gwc gatewayv1beta1.GatewayAPIClient, cfg *config.Config) * // The lock ID applied to the file reference in the context will be returned // (if any). An error will be returned if something goes wrong. The error // could be a ConnectorError -func (f *FileConnector) GetLock(ctx context.Context) (string, error) { +func (f *FileConnector) GetLock(ctx context.Context) (*ConnectorResponse, error) { wopiContext, err := middleware.WopiContextFromCtx(ctx) if err != nil { - return "", err + return nil, err } logger := zerolog.Ctx(ctx) @@ -155,7 +130,7 @@ func (f *FileConnector) GetLock(ctx context.Context) (string, error) { resp, err := f.gwc.GetLock(ctx, req) if err != nil { logger.Error().Err(err).Msg("GetLock failed") - return "", err + return nil, err } if resp.GetStatus().GetCode() != rpcv1beta1.Code_CODE_OK { @@ -164,7 +139,7 @@ func (f *FileConnector) GetLock(ctx context.Context) (string, error) { Str("StatusMsg", resp.GetStatus().GetMessage()). Msg("GetLock failed with unexpected status") // TODO: Should we be more strict? There could be more causes for the failure - return "", NewConnectorError(404, resp.GetStatus().GetCode().String()+" "+resp.GetStatus().GetMessage()) + return &ConnectorResponse{Status: 404}, nil } lockID := "" @@ -177,7 +152,12 @@ func (f *FileConnector) GetLock(ctx context.Context) (string, error) { Str("LockID", lockID). Msg("GetLock success") - return lockID, nil + return &ConnectorResponse{ + Status: 200, + Headers: map[string]string{ + HeaderWopiLock: lockID, + }, + }, nil } // Lock returns a WOPI lock or performs an unlock and relock @@ -198,10 +178,10 @@ 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. -func (f *FileConnector) Lock(ctx context.Context, lockID, oldLockID string) (string, error) { +func (f *FileConnector) Lock(ctx context.Context, lockID, oldLockID string) (*ConnectorResponse, error) { wopiContext, err := middleware.WopiContextFromCtx(ctx) if err != nil { - return "", err + return nil, err } logger := zerolog.Ctx(ctx).With(). @@ -212,7 +192,7 @@ func (f *FileConnector) Lock(ctx context.Context, lockID, oldLockID string) (str if lockID == "" { logger.Error().Msg("Lock failed due to empty lockID") - return "", NewConnectorError(400, "Requested lockID is empty") + return &ConnectorResponse{Status: 400}, nil } var setOrRefreshStatus *rpcv1beta1.Status @@ -234,7 +214,7 @@ func (f *FileConnector) Lock(ctx context.Context, lockID, oldLockID string) (str resp, err := f.gwc.SetLock(ctx, req) if err != nil { logger.Error().Err(err).Msg("SetLock failed") - return "", err + return nil, err } setOrRefreshStatus = resp.GetStatus() } else { @@ -257,7 +237,7 @@ func (f *FileConnector) Lock(ctx context.Context, lockID, oldLockID string) (str resp, err := f.gwc.RefreshLock(ctx, req) if err != nil { logger.Error().Err(err).Msg("UnlockAndRefresh failed") - return "", err + return nil, err } setOrRefreshStatus = resp.GetStatus() } @@ -266,7 +246,7 @@ func (f *FileConnector) Lock(ctx context.Context, lockID, oldLockID string) (str switch setOrRefreshStatus.GetCode() { case rpcv1beta1.Code_CODE_OK: logger.Debug().Msg("SetLock successful") - return "", nil + return &ConnectorResponse{Status: 200}, nil case rpcv1beta1.Code_CODE_FAILED_PRECONDITION, rpcv1beta1.Code_CODE_ABORTED: // Code_CODE_FAILED_PRECONDITION -> Lock operation mismatched lock @@ -280,7 +260,7 @@ func (f *FileConnector) Lock(ctx context.Context, lockID, oldLockID string) (str resp, err := f.gwc.GetLock(ctx, req) if err != nil { logger.Error().Err(err).Msg("SetLock failed, fallback to GetLock failed too") - return "", err + return nil, err } if resp.GetStatus().GetCode() != rpcv1beta1.Code_CODE_OK { @@ -296,7 +276,12 @@ func (f *FileConnector) Lock(ctx context.Context, lockID, oldLockID string) (str logger.Warn(). Str("LockID", resp.GetLock().GetLockId()). Msg("SetLock conflict") - return resp.GetLock().GetLockId(), NewConnectorError(409, "Lock conflict") + return &ConnectorResponse{ + Status: 409, + Headers: map[string]string{ + HeaderWopiLock: resp.GetLock().GetLockId(), + }, + }, nil } // TODO: according to the spec we need to treat this as a RefreshLock @@ -307,22 +292,22 @@ func (f *FileConnector) Lock(ctx context.Context, lockID, oldLockID string) (str logger.Warn(). Str("LockID", resp.GetLock().GetLockId()). Msg("SetLock lock refreshed instead") - return resp.GetLock().GetLockId(), nil + return &ConnectorResponse{Status: 200}, nil // no need to send the lockID for a 200 code } logger.Error().Msg("SetLock failed and could not refresh") - return "", NewConnectorError(500, "Could not refresh the lock") + return &ConnectorResponse{Status: 500}, nil case rpcv1beta1.Code_CODE_NOT_FOUND: logger.Error().Msg("SetLock failed, file not found") - return "", NewConnectorError(404, "File not found") + return &ConnectorResponse{Status: 404}, nil default: logger.Error(). Str("StatusCode", setOrRefreshStatus.GetCode().String()). Str("StatusMsg", setOrRefreshStatus.GetMessage()). Msg("SetLock failed with unexpected status") - return "", NewConnectorError(500, setOrRefreshStatus.GetCode().String()+" "+setOrRefreshStatus.GetMessage()) + return &ConnectorResponse{Status: 500}, nil } } @@ -339,10 +324,10 @@ func (f *FileConnector) Lock(ctx context.Context, lockID, oldLockID string) (str // return an empty lock id. // The conflict happens if the provided lockID doesn't match the one actually // applied in the target file. -func (f *FileConnector) RefreshLock(ctx context.Context, lockID string) (string, error) { +func (f *FileConnector) RefreshLock(ctx context.Context, lockID string) (*ConnectorResponse, error) { wopiContext, err := middleware.WopiContextFromCtx(ctx) if err != nil { - return "", err + return nil, err } logger := zerolog.Ctx(ctx).With(). @@ -352,7 +337,7 @@ func (f *FileConnector) RefreshLock(ctx context.Context, lockID string) (string, if lockID == "" { logger.Error().Msg("RefreshLock failed due to empty lockID") - return "", NewConnectorError(400, "Requested lockID is empty") + return &ConnectorResponse{Status: 400}, nil } req := &providerv1beta1.RefreshLockRequest{ @@ -370,20 +355,20 @@ func (f *FileConnector) RefreshLock(ctx context.Context, lockID string) (string, resp, err := f.gwc.RefreshLock(ctx, req) if err != nil { logger.Error().Err(err).Msg("RefreshLock failed") - return "", err + return nil, err } switch resp.GetStatus().GetCode() { case rpcv1beta1.Code_CODE_OK: logger.Debug().Msg("RefreshLock successful") - return "", nil + return &ConnectorResponse{Status: 200}, nil case rpcv1beta1.Code_CODE_NOT_FOUND: logger.Error(). Str("StatusCode", resp.GetStatus().GetCode().String()). Str("StatusMsg", resp.GetStatus().GetMessage()). Msg("RefreshLock failed, file reference not found") - return "", NewConnectorError(404, "File reference not found") + return &ConnectorResponse{Status: 404}, nil case rpcv1beta1.Code_CODE_ABORTED: logger.Error(). @@ -400,7 +385,7 @@ func (f *FileConnector) RefreshLock(ctx context.Context, lockID string) (string, resp, err := f.gwc.GetLock(ctx, req) if err != nil { logger.Error().Err(err).Msg("RefreshLock failed trying to get the current lock") - return "", err + return nil, err } if resp.GetStatus().GetCode() != rpcv1beta1.Code_CODE_OK { @@ -408,7 +393,7 @@ func (f *FileConnector) RefreshLock(ctx context.Context, lockID string) (string, Str("StatusCode", resp.GetStatus().GetCode().String()). Str("StatusMsg", resp.GetStatus().GetMessage()). Msg("RefreshLock failed, tried to get the current lock failed with unexpected status") - return "", NewConnectorError(500, resp.GetStatus().GetCode().String()+" "+resp.GetStatus().GetMessage()) + return &ConnectorResponse{Status: 500}, nil } if resp.GetLock() == nil { @@ -416,7 +401,12 @@ func (f *FileConnector) RefreshLock(ctx context.Context, lockID string) (string, Str("StatusCode", resp.GetStatus().GetCode().String()). Str("StatusMsg", resp.GetStatus().GetMessage()). Msg("RefreshLock failed, no lock on file") - return "", NewConnectorError(409, "No lock on file") + return &ConnectorResponse{ + Status: 409, + Headers: map[string]string{ + HeaderWopiLock: "", + }, + }, nil } else { // lock is different than the one requested, otherwise we wouldn't reached this point logger.Error(). @@ -424,14 +414,19 @@ func (f *FileConnector) RefreshLock(ctx context.Context, lockID string) (string, Str("StatusCode", resp.GetStatus().GetCode().String()). Str("StatusMsg", resp.GetStatus().GetMessage()). Msg("RefreshLock failed, lock mismatch") - return resp.GetLock().GetLockId(), NewConnectorError(409, "Lock mismatch") + return &ConnectorResponse{ + Status: 409, + Headers: map[string]string{ + HeaderWopiLock: resp.GetLock().GetLockId(), + }, + }, nil } default: logger.Error(). Str("StatusCode", resp.GetStatus().GetCode().String()). Str("StatusMsg", resp.GetStatus().GetMessage()). Msg("RefreshLock failed with unexpected status") - return "", NewConnectorError(500, resp.GetStatus().GetCode().String()+" "+resp.GetStatus().GetMessage()) + return &ConnectorResponse{Status: 500}, nil } } @@ -448,10 +443,10 @@ 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. -func (f *FileConnector) UnLock(ctx context.Context, lockID string) (string, error) { +func (f *FileConnector) UnLock(ctx context.Context, lockID string) (*ConnectorResponse, error) { wopiContext, err := middleware.WopiContextFromCtx(ctx) if err != nil { - return "", err + return nil, err } logger := zerolog.Ctx(ctx).With(). @@ -461,7 +456,7 @@ func (f *FileConnector) UnLock(ctx context.Context, lockID string) (string, erro if lockID == "" { logger.Error().Msg("Unlock failed due to empty lockID") - return "", NewConnectorError(400, "Requested lockID is empty") + return &ConnectorResponse{Status: 400}, nil } req := &providerv1beta1.UnlockRequest{ @@ -475,17 +470,22 @@ func (f *FileConnector) UnLock(ctx context.Context, lockID string) (string, erro resp, err := f.gwc.Unlock(ctx, req) if err != nil { logger.Error().Err(err).Msg("Unlock failed") - return "", err + return nil, err } switch resp.GetStatus().GetCode() { case rpcv1beta1.Code_CODE_OK: logger.Debug().Msg("Unlock successful") - return "", nil + return &ConnectorResponse{Status: 200}, nil case rpcv1beta1.Code_CODE_ABORTED: // File isn't locked. Need to return 409 with empty lock logger.Error().Err(err).Msg("Unlock failed, file isn't locked") - return "", NewConnectorError(409, "File is not locked") + return &ConnectorResponse{ + Status: 409, + Headers: map[string]string{ + HeaderWopiLock: "", + }, + }, nil case rpcv1beta1.Code_CODE_LOCKED: // We need to return 409 with the current lock req := &providerv1beta1.GetLockRequest{ @@ -495,7 +495,7 @@ func (f *FileConnector) UnLock(ctx context.Context, lockID string) (string, erro resp, err := f.gwc.GetLock(ctx, req) if err != nil { logger.Error().Err(err).Msg("Unlock failed trying to get the current lock") - return "", err + return nil, err } if resp.GetStatus().GetCode() != rpcv1beta1.Code_CODE_OK { @@ -503,7 +503,7 @@ func (f *FileConnector) UnLock(ctx context.Context, lockID string) (string, erro Str("StatusCode", resp.GetStatus().GetCode().String()). Str("StatusMsg", resp.GetStatus().GetMessage()). Msg("Unlock failed, tried to get the current lock failed with unexpected status") - return "", NewConnectorError(500, resp.GetStatus().GetCode().String()+" "+resp.GetStatus().GetMessage()) + return &ConnectorResponse{Status: 500}, nil } var outLockId string @@ -522,13 +522,18 @@ func (f *FileConnector) UnLock(ctx context.Context, lockID string) (string, erro Msg("Unlock failed, lock mismatch") outLockId = resp.GetLock().GetLockId() } - return outLockId, NewConnectorError(409, "Lock mismatch") + return &ConnectorResponse{ + Status: 409, + Headers: map[string]string{ + HeaderWopiLock: outLockId, + }, + }, nil default: logger.Error(). Str("StatusCode", resp.GetStatus().GetCode().String()). Str("StatusMsg", resp.GetStatus().GetMessage()). Msg("Unlock failed with unexpected status") - return "", NewConnectorError(500, resp.GetStatus().GetCode().String()+" "+resp.GetStatus().GetMessage()) + return &ConnectorResponse{Status: 500}, nil } } @@ -558,7 +563,7 @@ func (f *FileConnector) UnLock(ctx context.Context, lockID string) (string, erro // Since the upload won't use any lock, the upload will fail if the target file // already exists and it isn't empty. This means that, this method can only // generate new files. -func (f *FileConnector) PutRelativeFileSuggested(ctx context.Context, ccs ContentConnectorService, stream io.Reader, streamLength int64, target string) (*PutRelativeResponse, error) { +func (f *FileConnector) PutRelativeFileSuggested(ctx context.Context, ccs ContentConnectorService, stream io.Reader, streamLength int64, target string) (*ConnectorResponse, error) { // assume the target is a full name wopiContext, err := middleware.WopiContextFromCtx(ctx) if err != nil { @@ -583,7 +588,7 @@ func (f *FileConnector) PutRelativeFileSuggested(ctx context.Context, ccs Conten Str("StatusCode", oldStatRes.GetStatus().GetCode().String()). Str("StatusMsg", oldStatRes.GetStatus().GetMessage()). Msg("PutRelativeFileSuggested: stat failed with unexpected status") - return nil, NewConnectorError(500, oldStatRes.GetStatus().GetCode().String()+" "+oldStatRes.GetStatus().GetMessage()) + return &ConnectorResponse{Status: 500}, nil } if strings.HasPrefix(target, ".") { @@ -597,8 +602,6 @@ func (f *FileConnector) PutRelativeFileSuggested(ctx context.Context, ccs Conten finalTarget := target newLogger := logger for isDone := false; !isDone; { - var conError *ConnectorError - targetPath := utils.MakeRelativePath(finalTarget) // need to change the file reference of the wopicontext to point to the new path wopiContext.FileReference = &providerv1beta1.Reference{ @@ -611,29 +614,27 @@ func (f *FileConnector) PutRelativeFileSuggested(ctx context.Context, ccs Conten newCtx := middleware.WopiContextToCtx(newLogger.WithContext(ctx), wopiContext) // try to put the file. It mustn't return a 400 or 409 - _, err := ccs.PutFile(newCtx, stream, streamLength, "") + putResponse, err := ccs.PutFile(newCtx, stream, streamLength, "") if err != nil { - // if the error isn't a connectorError, fail the request - if !errors.As(err, &conError) { - newLogger.Error().Err(err).Msg("PutRelativeFileSuggested: put file failed") - return nil, err - } + newLogger.Error().Err(err).Msg("PutRelativeFileSuggested: put file failed") + return nil, err + } - if conError.HttpCodeOut == 409 { - // if conflict generate a different name and retry. - // this should happen only once - actualFilename, _ := f.extractFilenameAndPrefix(target) - finalTarget = f.generatePrefix() + " " + actualFilename - } else { - // TODO: code 400 might happen, what to do? - // in other cases, just return the error - newLogger.Error().Err(err).Msg("PutRelativeFileSuggested: put file failed with unhandled status") - return nil, err - } - } else { + switch putResponse.Status { + case 200: // if the put is successful, exit the loop and move on isDone = true logger = newLogger + case 409: + // if conflict generate a different name and retry. + // this should happen only once + actualFilename, _ := f.extractFilenameAndPrefix(target) + finalTarget = f.generatePrefix() + " " + actualFilename + default: + // TODO: code 400 might happen, what to do? + // in other cases, just return the error + newLogger.Error().Msg("PutRelativeFileSuggested: put file failed with unhandled status") + return &ConnectorResponse{Status: 500}, nil } } @@ -648,16 +649,17 @@ func (f *FileConnector) PutRelativeFileSuggested(ctx context.Context, ccs Conten return nil, err } - // send the info - result := &PutRelativeResponse{ - Name: finalTarget, - Url: wopiSrcURL.String(), - } - logger.Debug(). Str("FinalReference", wopiContext.FileReference.String()). Msg("PutRelativeFileSuggested: success") - return result, nil + + return &ConnectorResponse{ + Status: 200, + Body: map[string]interface{}{ + "Name": finalTarget, + "Url": wopiSrcURL.String(), + }, + }, nil } // PutRelativeFileRelative upload a file using the provided target name @@ -684,11 +686,11 @@ func (f *FileConnector) PutRelativeFileSuggested(ctx context.Context, ccs Conten // Since the upload won't use any lock, the upload will fail if the target file // already exists and it isn't empty. This means that, this method can only // generate new files. -func (f *FileConnector) PutRelativeFileRelative(ctx context.Context, ccs ContentConnectorService, stream io.Reader, streamLength int64, target string) (*PutRelativeResponse, *PutRelativeHeaders, error) { +func (f *FileConnector) PutRelativeFileRelative(ctx context.Context, ccs ContentConnectorService, stream io.Reader, streamLength int64, target string) (*ConnectorResponse, error) { // assume the target is a full name wopiContext, err := middleware.WopiContextFromCtx(ctx) if err != nil { - return nil, nil, err + return nil, err } logger := zerolog.Ctx(ctx).With(). @@ -701,7 +703,7 @@ func (f *FileConnector) PutRelativeFileRelative(ctx context.Context, ccs Content }) if err != nil { logger.Error().Err(err).Msg("PutRelativeFileRelative: stat failed") - return nil, nil, err + return nil, err } if oldStatRes.GetStatus().GetCode() != rpcv1beta1.Code_CODE_OK { @@ -709,7 +711,7 @@ func (f *FileConnector) PutRelativeFileRelative(ctx context.Context, ccs Content Str("StatusCode", oldStatRes.GetStatus().GetCode().String()). Str("StatusMsg", oldStatRes.GetStatus().GetMessage()). Msg("PutRelativeFileRelative: stat failed with unexpected status") - return nil, nil, NewConnectorError(500, oldStatRes.GetStatus().GetCode().String()+" "+oldStatRes.GetStatus().GetMessage()) + return &ConnectorResponse{Status: 500}, nil } targetPath := utils.MakeRelativePath(target) @@ -723,72 +725,79 @@ func (f *FileConnector) PutRelativeFileRelative(ctx context.Context, ccs Content newLogger := logger.With().Str("NewFileReference", wopiContext.FileReference.String()).Logger() newCtx := middleware.WopiContextToCtx(newLogger.WithContext(ctx), wopiContext) - var conError *ConnectorError // try to put the file - lockID, err := ccs.PutFile(newCtx, stream, streamLength, "") + putResponse, err := ccs.PutFile(newCtx, stream, streamLength, "") if err != nil { - // if the error isn't a connectorError, fail the request - if !errors.As(err, &conError) { - newLogger.Error().Err(err).Msg("PutRelativeFileRelative: put file failed") - return nil, nil, err + newLogger.Error().Err(err).Msg("PutRelativeFileRelative: put file failed") + return nil, err + } + + lockID := "" + if putResponse.Headers != nil { + lockID = putResponse.Headers[HeaderWopiLock] + } + + switch putResponse.Status { + case 200: // success case, so don't do anything + case 409: + if err := f.adjustWopiReference(ctx, &wopiContext, newLogger); err != nil { + return nil, err + } + // if conflict generate a different name and retry. + // this should happen only once + wopiSrcURL, err2 := f.generateWOPISrc(ctx, wopiContext, newLogger) + if err2 != nil { + newLogger.Error(). + Err(err2). + Str("LockID", lockID). + Msg("PutRelativeFileRelative: error generating the WOPISrc parameter for conflict response") + return nil, err2 } - if conError.HttpCodeOut == 409 { - if err := f.adjustWopiReference(ctx, &wopiContext, newLogger); err != nil { - return nil, nil, err - } - // if conflict generate a different name and retry. - // this should happen only once - wopiSrcURL, err2 := f.generateWOPISrc(ctx, wopiContext, newLogger) - if err2 != nil { - newLogger.Error(). - Err(err2). - Str("LockID", lockID). - Msg("PutRelativeFileRelative: error generating the WOPISrc parameter for conflict response") - return nil, nil, err - } + actualFilename, _ := f.extractFilenameAndPrefix(target) + finalTarget := f.generatePrefix() + " " + actualFilename - actualFilename, _ := f.extractFilenameAndPrefix(target) - finalTarget := f.generatePrefix() + " " + actualFilename - headers := &PutRelativeHeaders{ - ValidTarget: finalTarget, - LockID: lockID, - } - response := &PutRelativeResponse{ - Name: target, - Url: wopiSrcURL.String(), - } - newLogger.Error(). - Err(err). - Str("LockID", lockID). - Msg("PutRelativeFileRelative: error conflict") - return response, headers, err - } else { - newLogger.Error(). - Err(err). - Str("LockID", lockID). - Msg("PutRelativeFileRelative: put file failed with unhandled status") - return nil, nil, err - } + newLogger.Error(). + Str("LockID", lockID). + Msg("PutRelativeFileRelative: error conflict") + + return &ConnectorResponse{ + Status: 409, + Headers: map[string]string{ + HeaderWopiValidRT: finalTarget, + HeaderWopiLock: lockID, + }, + Body: map[string]interface{}{ + "Name": target, + "Url": wopiSrcURL.String(), + }, + }, nil + default: + newLogger.Error(). + Str("LockID", lockID). + Msg("PutRelativeFileRelative: put file failed with unhandled status") + return &ConnectorResponse{Status: 500}, nil } if err := f.adjustWopiReference(ctx, &wopiContext, newLogger); err != nil { - return nil, nil, err + return nil, err } wopiSrcURL, err := f.generateWOPISrc(ctx, wopiContext, newLogger) if err != nil { newLogger.Error().Err(err).Msg("PutRelativeFileRelative: error generating the WOPISrc parameter") - return nil, nil, err - } - // send the info - result := &PutRelativeResponse{ - Name: target, - Url: wopiSrcURL.String(), + return nil, err } newLogger.Debug().Msg("PutRelativeFileRelative: success") - return result, nil, nil + + return &ConnectorResponse{ + Status: 200, + Body: map[string]interface{}{ + "Name": target, + "Url": wopiSrcURL.String(), + }, + }, nil } // DeleteFile will delete the requested file @@ -804,10 +813,10 @@ func (f *FileConnector) PutRelativeFileRelative(ctx context.Context, ccs Content // // Note that this method isn't required and it's likely used just for the // WOPI validator -func (f *FileConnector) DeleteFile(ctx context.Context, lockID string) (string, error) { +func (f *FileConnector) DeleteFile(ctx context.Context, lockID string) (*ConnectorResponse, error) { wopiContext, err := middleware.WopiContextFromCtx(ctx) if err != nil { - return "", err + return nil, err } logger := zerolog.Ctx(ctx).With(). @@ -825,7 +834,7 @@ func (f *FileConnector) DeleteFile(ctx context.Context, lockID string) (string, deleteRes, err = f.gwc.Delete(ctx, deleteReq) if err != nil { logger.Error().Err(err).Msg("DeleteFile: stat failed") - return "", err + return nil, err } if deleteRes.GetStatus().GetCode() == rpcv1beta1.Code_CODE_TOO_EARLY { @@ -856,7 +865,7 @@ func (f *FileConnector) DeleteFile(ctx context.Context, lockID string) (string, if deleteRes.GetStatus().GetCode() == rpcv1beta1.Code_CODE_NOT_FOUND { // don't bother to check for locks of a missing file logger.Error().Msg("DeleteFile: tried to delete a missing file") - return "", NewConnectorError(404, deleteRes.GetStatus().GetCode().String()+" "+deleteRes.GetStatus().GetMessage()) + return &ConnectorResponse{Status: 404}, nil } // check if the file is locked to return a proper lockID @@ -867,7 +876,7 @@ func (f *FileConnector) DeleteFile(ctx context.Context, lockID string) (string, resp, err2 := f.gwc.GetLock(ctx, req) if err2 != nil { logger.Error().Err(err2).Msg("DeleteFile: GetLock failed") - return "", err2 + return nil, err2 } if resp.GetStatus().GetCode() != rpcv1beta1.Code_CODE_OK { @@ -875,22 +884,27 @@ func (f *FileConnector) DeleteFile(ctx context.Context, lockID string) (string, Str("StatusCode", resp.GetStatus().GetCode().String()). Str("StatusMsg", resp.GetStatus().GetMessage()). Msg("DeleteFile: GetLock failed with unexpected status") - return "", NewConnectorError(500, resp.GetStatus().GetCode().String()+" "+resp.GetStatus().GetMessage()) + return &ConnectorResponse{Status: 500}, nil } if resp.GetLock() != nil { logger.Error(). Str("LockID", resp.GetLock().GetLockId()). Msg("DeleteFile: file is locked") - return resp.GetLock().GetLockId(), NewConnectorError(409, "file is locked") + return &ConnectorResponse{ + Status: 409, + Headers: map[string]string{ + HeaderWopiLock: resp.GetLock().GetLockId(), + }, + }, nil } else { // return the original error since the file isn't locked logger.Error().Msg("DeleteFile: delete failed on unlocked file") - return "", NewConnectorError(500, deleteRes.GetStatus().GetCode().String()+" "+deleteRes.GetStatus().GetMessage()) + return &ConnectorResponse{Status: 500}, nil } } logger.Debug().Msg("DeleteFile: success") - return "", nil + return &ConnectorResponse{Status: 200}, nil } // RenameFile will rename the requested file @@ -907,10 +921,10 @@ func (f *FileConnector) DeleteFile(ctx context.Context, lockID string) (string, // is just a suggestion, so it could have changed) and the actual lockId in // case of conflict as second return value, otherwise the returned lockId will // be empty. -func (f *FileConnector) RenameFile(ctx context.Context, lockID, target string) (*RenameResponse, string, error) { +func (f *FileConnector) RenameFile(ctx context.Context, lockID, target string) (*ConnectorResponse, error) { wopiContext, err := middleware.WopiContextFromCtx(ctx) if err != nil { - return nil, "", err + return nil, err } logger := zerolog.Ctx(ctx).With(). @@ -924,19 +938,19 @@ func (f *FileConnector) RenameFile(ctx context.Context, lockID, target string) ( }) if err != nil { logger.Error().Err(err).Msg("RenameFile: stat failed") - return nil, "", err + return nil, err } if oldStatRes.GetStatus().GetCode() != rpcv1beta1.Code_CODE_OK { if oldStatRes.GetStatus().GetCode() == rpcv1beta1.Code_CODE_NOT_FOUND { logger.Error().Msg("RenameFile: file not found") - return nil, "", NewConnectorError(404, oldStatRes.GetStatus().GetCode().String()+" "+oldStatRes.GetStatus().GetMessage()) + return &ConnectorResponse{Status: 404}, nil } else { logger.Error(). Str("StatusCode", oldStatRes.GetStatus().GetCode().String()). Str("StatusMsg", oldStatRes.GetStatus().GetMessage()). Msg("RenameFile: stat failed with unexpected status") - return nil, "", NewConnectorError(500, oldStatRes.GetStatus().GetCode().String()+" "+oldStatRes.GetStatus().GetMessage()) + return &ConnectorResponse{Status: 500}, nil } } @@ -962,7 +976,7 @@ func (f *FileConnector) RenameFile(ctx context.Context, lockID, target string) ( }) if err != nil { newLogger.Error().Err(err).Msg("RenameFile: move failed") - return nil, "", err + return nil, err } if moveRes.GetStatus().GetCode() != rpcv1beta1.Code_CODE_OK { if moveRes.GetStatus().GetCode() == rpcv1beta1.Code_CODE_LOCKED || moveRes.GetStatus().GetCode() == rpcv1beta1.Code_CODE_ABORTED { @@ -975,7 +989,12 @@ func (f *FileConnector) RenameFile(ctx context.Context, lockID, target string) ( Str("StatusCode", moveRes.GetStatus().GetCode().String()). Str("StatusMsg", moveRes.GetStatus().GetMessage()). Msg("RenameFile: conflict") - return nil, currentLockID, NewConnectorError(409, "file is locked") + return &ConnectorResponse{ + Status: 409, + Headers: map[string]string{ + HeaderWopiLock: currentLockID, + }, + }, nil } if moveRes.GetStatus().GetCode() == rpcv1beta1.Code_CODE_ALREADY_EXISTS { @@ -990,7 +1009,7 @@ func (f *FileConnector) RenameFile(ctx context.Context, lockID, target string) ( Str("StatusMsg", moveRes.GetStatus().GetMessage()). Msg("RenameFile: move failed with unexpected status") - return nil, "", NewConnectorError(500, moveRes.GetStatus().GetCode().String()+" "+moveRes.GetStatus().GetMessage()) + return &ConnectorResponse{Status: 500}, nil } } else { // if the put is successful, exit the loop and move on @@ -1000,9 +1019,12 @@ func (f *FileConnector) RenameFile(ctx context.Context, lockID, target string) ( } logger.Debug().Msg("RenameFile: success") - return &RenameResponse{ - Name: strings.TrimSuffix(path.Base(finalTarget), path.Ext(finalTarget)), // return the final filename without extension - }, "", nil + return &ConnectorResponse{ + Status: 200, + Body: map[string]interface{}{ + "Name": strings.TrimSuffix(path.Base(finalTarget), path.Ext(finalTarget)), // return the final filename without extension + }, + }, nil } @@ -1015,7 +1037,7 @@ func (f *FileConnector) RenameFile(ctx context.Context, lockID, target string) ( // // If the operation is successful, a "FileInfo" instance will be returned, // otherwise the "FileInfo" will be empty and an error will be returned. -func (f *FileConnector) CheckFileInfo(ctx context.Context) (fileinfo.FileInfo, error) { +func (f *FileConnector) CheckFileInfo(ctx context.Context) (*ConnectorResponse, error) { wopiContext, err := middleware.WopiContextFromCtx(ctx) if err != nil { return nil, err @@ -1036,7 +1058,7 @@ func (f *FileConnector) CheckFileInfo(ctx context.Context) (fileinfo.FileInfo, e Str("StatusCode", statRes.GetStatus().GetCode().String()). Str("StatusMsg", statRes.GetStatus().GetMessage()). Msg("CheckFileInfo: stat failed with unexpected status") - return nil, NewConnectorError(500, statRes.GetStatus().GetCode().String()+" "+statRes.GetStatus().GetMessage()) + return &ConnectorResponse{Status: 500}, nil } // If a not known app name is used, consider "Microsoft" as default. @@ -1124,7 +1146,10 @@ func (f *FileConnector) CheckFileInfo(ctx context.Context) (fileinfo.FileInfo, e info.SetProperties(infoMap) logger.Debug().Interface("FileInfo", info).Msg("CheckFileInfo: success") - return info, nil + return &ConnectorResponse{ + Status: 200, + Body: info, + }, nil } func (f *FileConnector) watermarkText(user *userv1beta1.User) string { diff --git a/services/collaboration/pkg/connector/httpadapter.go b/services/collaboration/pkg/connector/httpadapter.go index 2486ec768..20c820dc0 100644 --- a/services/collaboration/pkg/connector/httpadapter.go +++ b/services/collaboration/pkg/connector/httpadapter.go @@ -72,19 +72,14 @@ func NewHttpAdapterWithConnector(con ConnectorService, l locks.LockParser) *Http // the headers according to the spec func (h *HttpAdapter) GetLock(w http.ResponseWriter, r *http.Request) { fileCon := h.con.GetFileConnector() + response, err := fileCon.GetLock(r.Context()) - lockID, err := fileCon.GetLock(r.Context()) if err != nil { - var conError *ConnectorError - if errors.As(err, &conError) { - http.Error(w, http.StatusText(conError.HttpCodeOut), conError.HttpCodeOut) - } else { - http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) - } + http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) return } - w.Header().Set(HeaderWopiLock, lockID) - w.WriteHeader(http.StatusOK) + + h.writeConnectorResponse(w, r, response) } // Lock adapts the "Lock" and "UnlockAndRelock" operations for WOPI. @@ -98,20 +93,14 @@ func (h *HttpAdapter) Lock(w http.ResponseWriter, r *http.Request) { lockID := h.locks.ParseLock(r.Header.Get(HeaderWopiLock)) fileCon := h.con.GetFileConnector() - newLockID, err := fileCon.Lock(r.Context(), lockID, oldLockID) + response, err := fileCon.Lock(r.Context(), lockID, oldLockID) + if err != nil { - var conError *ConnectorError - if errors.As(err, &conError) { - if conError.HttpCodeOut == 409 { - w.Header().Set(HeaderWopiLock, newLockID) - } - http.Error(w, http.StatusText(conError.HttpCodeOut), conError.HttpCodeOut) - } else { - http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) - } + http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) return } - w.WriteHeader(http.StatusOK) + + h.writeConnectorResponse(w, r, response) } // RefreshLock adapts the "RefreshLock" operation for WOPI @@ -125,20 +114,14 @@ func (h *HttpAdapter) RefreshLock(w http.ResponseWriter, r *http.Request) { lockID := h.locks.ParseLock(r.Header.Get(HeaderWopiLock)) fileCon := h.con.GetFileConnector() - newLockID, err := fileCon.RefreshLock(r.Context(), lockID) + response, err := fileCon.RefreshLock(r.Context(), lockID) + if err != nil { - var conError *ConnectorError - if errors.As(err, &conError) { - if conError.HttpCodeOut == 409 { - w.Header().Set(HeaderWopiLock, newLockID) - } - http.Error(w, http.StatusText(conError.HttpCodeOut), conError.HttpCodeOut) - } else { - http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) - } + http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) return } - w.WriteHeader(http.StatusOK) + + h.writeConnectorResponse(w, r, response) } // UnLock adapts the "Unlock" operation for WOPI @@ -150,20 +133,14 @@ func (h *HttpAdapter) UnLock(w http.ResponseWriter, r *http.Request) { lockID := h.locks.ParseLock(r.Header.Get(HeaderWopiLock)) fileCon := h.con.GetFileConnector() - newLockID, err := fileCon.UnLock(r.Context(), lockID) + response, err := fileCon.UnLock(r.Context(), lockID) + if err != nil { - var conError *ConnectorError - if errors.As(err, &conError) { - if conError.HttpCodeOut == 409 { - w.Header().Set(HeaderWopiLock, newLockID) - } - http.Error(w, http.StatusText(conError.HttpCodeOut), conError.HttpCodeOut) - } else { - http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) - } + http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) return } - w.WriteHeader(http.StatusOK) + + h.writeConnectorResponse(w, r, response) } // CheckFileInfo will retrieve the information of the file in json format @@ -172,41 +149,14 @@ func (h *HttpAdapter) UnLock(w http.ResponseWriter, r *http.Request) { // the headers according to the spec func (h *HttpAdapter) CheckFileInfo(w http.ResponseWriter, r *http.Request) { fileCon := h.con.GetFileConnector() + response, err := fileCon.CheckFileInfo(r.Context()) - w.Header().Set(HeaderContentType, "application/json") - w.Header().Set(HeaderContentLength, "0") - - fileInfo, err := fileCon.CheckFileInfo(r.Context()) if err != nil { - var conError *ConnectorError - if errors.As(err, &conError) { - http.Error(w, http.StatusText(conError.HttpCodeOut), conError.HttpCodeOut) - } else { - http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) - } - return - } - - logger := zerolog.Ctx(r.Context()) - - jsonFileInfo, err := json.Marshal(fileInfo) - if err != nil { - logger.Error().Err(err).Msg("CheckFileInfo: failed to marshal fileinfo") http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) return } - w.Header().Set(HeaderContentLength, strconv.Itoa(len(jsonFileInfo))) - w.WriteHeader(http.StatusOK) - bytes, err := w.Write(jsonFileInfo) - - if err != nil { - logger.Error(). - Err(err). - Int("TotalBytes", len(jsonFileInfo)). - Int("WrittenBytes", bytes). - Msg("CheckFileInfo: failed to write contents in the HTTP response") - } + h.writeConnectorResponse(w, r, response) } // GetFile will download the file @@ -237,20 +187,14 @@ func (h *HttpAdapter) PutFile(w http.ResponseWriter, r *http.Request) { lockID := h.locks.ParseLock(r.Header.Get(HeaderWopiLock)) contentCon := h.con.GetContentConnector() - newLockID, err := contentCon.PutFile(r.Context(), r.Body, r.ContentLength, lockID) + response, err := contentCon.PutFile(r.Context(), r.Body, r.ContentLength, lockID) + if err != nil { - var conError *ConnectorError - if errors.As(err, &conError) { - if conError.HttpCodeOut == 409 { - w.Header().Set(HeaderWopiLock, newLockID) - } - http.Error(w, http.StatusText(conError.HttpCodeOut), conError.HttpCodeOut) - } else { - http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) - } + http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) return } - w.WriteHeader(http.StatusOK) + + h.writeConnectorResponse(w, r, response) } // PutRelativeFile will upload the file with a specific name. The name might be @@ -267,17 +211,13 @@ func (h *HttpAdapter) PutRelativeFile(w http.ResponseWriter, r *http.Request) { relativeTarget := r.Header.Get(HeaderWopiRT) suggestedTarget := r.Header.Get(HeaderWopiST) - w.Header().Set(HeaderContentType, "application/json") - w.Header().Set(HeaderContentLength, "0") - if relativeTarget != "" && suggestedTarget != "" { // headers are mutually exclusive http.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest) return } - var response *PutRelativeResponse - var headers *PutRelativeHeaders + var response *ConnectorResponse var putErr error fileCon := h.con.GetFileConnector() @@ -296,43 +236,15 @@ func (h *HttpAdapter) PutRelativeFile(w http.ResponseWriter, r *http.Request) { http.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest) return } - response, headers, putErr = fileCon.PutRelativeFileRelative(r.Context(), h.con.GetContentConnector(), r.Body, r.ContentLength, utf8Target) + response, putErr = fileCon.PutRelativeFileRelative(r.Context(), h.con.GetContentConnector(), r.Body, r.ContentLength, utf8Target) } - var conError *ConnectorError - if putErr != nil && !errors.As(putErr, &conError) { + if putErr != nil { http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) return } - logger := zerolog.Ctx(r.Context()) - - jsonFileInfo, err := json.Marshal(response) - if err != nil { - logger.Error().Err(err).Msg("PutRelativeFile: failed to marshal response") - http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) - return - } - - w.Header().Set(HeaderContentLength, strconv.Itoa(len(jsonFileInfo))) - if conError != nil { - if headers != nil { - w.Header().Set(HeaderWopiValidRT, utf7.EncodeString(headers.ValidTarget)) - w.Header().Set(HeaderWopiLock, headers.LockID) - } - w.WriteHeader(conError.HttpCodeOut) - } else { - w.WriteHeader(http.StatusOK) - } - bytes, err := w.Write(jsonFileInfo) - - if err != nil { - logger.Error(). - Err(err). - Int("TotalBytes", len(jsonFileInfo)). - Int("WrittenBytes", bytes). - Msg("PutRelativeFile: failed to write contents in the HTTP response") - } + h.writeConnectorResponse(w, r, response) } // DeleteFile will delete the provided file. If the file is locked and can't @@ -342,20 +254,14 @@ func (h *HttpAdapter) DeleteFile(w http.ResponseWriter, r *http.Request) { lockID := r.Header.Get(HeaderWopiLock) fileCon := h.con.GetFileConnector() - newLockID, err := fileCon.DeleteFile(r.Context(), lockID) + response, err := fileCon.DeleteFile(r.Context(), lockID) + if err != nil { - var conError *ConnectorError - if errors.As(err, &conError) { - if conError.HttpCodeOut == 409 { - w.Header().Set(HeaderWopiLock, newLockID) - } - http.Error(w, http.StatusText(conError.HttpCodeOut), conError.HttpCodeOut) - } else { - http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) - } + http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) return } - w.WriteHeader(http.StatusOK) + + h.writeConnectorResponse(w, r, response) } // RenameFile will rename the file. The name might be automatically adjusted. @@ -377,39 +283,43 @@ func (h *HttpAdapter) RenameFile(w http.ResponseWriter, r *http.Request) { } fileCon := h.con.GetFileConnector() - response, newLockID, err := fileCon.RenameFile(r.Context(), lockID, utf8Target) + response, err := fileCon.RenameFile(r.Context(), lockID, utf8Target) if err != nil { - var conError *ConnectorError - if errors.As(err, &conError) { - if conError.HttpCodeOut == 409 { - w.Header().Set(HeaderWopiLock, newLockID) - } - http.Error(w, http.StatusText(conError.HttpCodeOut), conError.HttpCodeOut) - } else { - http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) - } - return - } - - // need to return a JSON response with the name if this is successful - logger := zerolog.Ctx(r.Context()) - - jsonFileInfo, err := json.Marshal(response) - if err != nil { - logger.Error().Err(err).Msg("RenameFile: failed to marshal response") http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) return } - w.Header().Set(HeaderContentLength, strconv.Itoa(len(jsonFileInfo))) - w.WriteHeader(http.StatusOK) - bytes, err := w.Write(jsonFileInfo) + h.writeConnectorResponse(w, r, response) +} +func (h *HttpAdapter) writeConnectorResponse(w http.ResponseWriter, r *http.Request, response *ConnectorResponse) { + jsonBody := []byte{} + if response.Body != nil { + var err error + jsonBody, err = json.Marshal(response.Body) + if err != nil { + logger := zerolog.Ctx(r.Context()) + logger.Error().Err(err).Msg("failed to marshal response") + http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) + return + } + + w.Header().Set(HeaderContentType, "application/json") + w.Header().Set(HeaderContentLength, strconv.Itoa(len(jsonBody))) + } + + for key, value := range response.Headers { + w.Header().Set(key, value) + } + w.WriteHeader(response.Status) + + bytes, err := w.Write(jsonBody) if err != nil { + logger := zerolog.Ctx(r.Context()) logger.Error(). Err(err). - Int("TotalBytes", len(jsonFileInfo)). + Int("TotalBytes", len(jsonBody)). Int("WrittenBytes", bytes). - Msg("RenameFile: failed to write contents in the HTTP response") + Msg("failed to write contents in the HTTP response") } } From 3d7440aa4720e237d10030ecc0abac9a4320efce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Thu, 8 Aug 2024 15:33:14 +0200 Subject: [PATCH 2/6] chore: bump mockery version to 2.43 --- .bingo/Variables.mk | 6 +++--- .bingo/mockery.mod | 6 ++++-- .bingo/mockery.sum | 2 ++ .bingo/variables.env | 2 +- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/.bingo/Variables.mk b/.bingo/Variables.mk index 0cb05d325..752cac323 100644 --- a/.bingo/Variables.mk +++ b/.bingo/Variables.mk @@ -65,11 +65,11 @@ $(HUGO): $(BINGO_DIR)/hugo.mod @echo "(re)installing $(GOBIN)/hugo-v0.123.7" @cd $(BINGO_DIR) && GOWORK=off $(GO) build -mod=mod -modfile=hugo.mod -o=$(GOBIN)/hugo-v0.123.7 "github.com/gohugoio/hugo" -MOCKERY := $(GOBIN)/mockery-v2.40.2 +MOCKERY := $(GOBIN)/mockery-v2.43.2 $(MOCKERY): $(BINGO_DIR)/mockery.mod @# Install binary/ries using Go 1.14+ build command. This is using bwplotka/bingo-controlled, separate go module with pinned dependencies. - @echo "(re)installing $(GOBIN)/mockery-v2.40.2" - @cd $(BINGO_DIR) && GOWORK=off $(GO) build -mod=mod -modfile=mockery.mod -o=$(GOBIN)/mockery-v2.40.2 "github.com/vektra/mockery/v2" + @echo "(re)installing $(GOBIN)/mockery-v2.43.2" + @cd $(BINGO_DIR) && GOWORK=off $(GO) build -mod=mod -modfile=mockery.mod -o=$(GOBIN)/mockery-v2.43.2 "github.com/vektra/mockery/v2" MUTAGEN := $(GOBIN)/mutagen-v0.14.0 $(MUTAGEN): $(BINGO_DIR)/mutagen.mod diff --git a/.bingo/mockery.mod b/.bingo/mockery.mod index 6dd4aa4be..c6fa529e6 100644 --- a/.bingo/mockery.mod +++ b/.bingo/mockery.mod @@ -1,5 +1,7 @@ module _ // Auto generated by https://github.com/bwplotka/bingo. DO NOT EDIT -go 1.21.6 +go 1.22 -require github.com/vektra/mockery/v2 v2.40.2 +toolchain go1.22.1 + +require github.com/vektra/mockery/v2 v2.43.2 diff --git a/.bingo/mockery.sum b/.bingo/mockery.sum index f9acfc356..58cc9531a 100644 --- a/.bingo/mockery.sum +++ b/.bingo/mockery.sum @@ -192,6 +192,8 @@ github.com/vektra/mockery/v2 v2.40.1 h1:8D01rBqloDLDHKZGXkyUD9Yj5Z+oDXBqDZ+tRXYM github.com/vektra/mockery/v2 v2.40.1/go.mod h1:dPzGtjT0/Uu4hqpF6QNHwz+GLago7lq1bxdj9wHbGKo= github.com/vektra/mockery/v2 v2.40.2 h1:JKuQ74IXibMCGKG6F0yvH/s2sNs1CHy/nGBP0We5DJ8= github.com/vektra/mockery/v2 v2.40.2/go.mod h1:KYBZF/7sqOa86BaOZPYsoCZWEWLS90a5oBLg2pVudxY= +github.com/vektra/mockery/v2 v2.43.2 h1:OdivAsQL/uoQ55UnTt25tliRI8kaj5j6caHk9xaAUD0= +github.com/vektra/mockery/v2 v2.43.2/go.mod h1:XNTE9RIu3deGAGQRVjP1VZxGpQNm0YedZx4oDs3prr8= github.com/yuin/goldmark v1.1.25/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= diff --git a/.bingo/variables.env b/.bingo/variables.env index f2be57019..810398eb0 100644 --- a/.bingo/variables.env +++ b/.bingo/variables.env @@ -24,7 +24,7 @@ GOVULNCHECK="${GOBIN}/govulncheck-v1.0.1" HUGO="${GOBIN}/hugo-v0.123.7" -MOCKERY="${GOBIN}/mockery-v2.40.2" +MOCKERY="${GOBIN}/mockery-v2.43.2" MUTAGEN="${GOBIN}/mutagen-v0.14.0" From 132defd73855922f332ffd96c70c526acbdd2a95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Thu, 8 Aug 2024 15:35:08 +0200 Subject: [PATCH 3/6] refactor: adjust unit tests to refactored code --- .../mocks/content_connector_service.go | 19 +- .../mocks/file_connector_service.go | 174 +++--- .../pkg/connector/contentconnector_test.go | 76 ++- .../pkg/connector/fileconnector_test.go | 553 +++++++++--------- .../pkg/connector/httpadapter_test.go | 52 +- 5 files changed, 431 insertions(+), 443 deletions(-) diff --git a/services/collaboration/mocks/content_connector_service.go b/services/collaboration/mocks/content_connector_service.go index 9b034f5da..da6ff575c 100644 --- a/services/collaboration/mocks/content_connector_service.go +++ b/services/collaboration/mocks/content_connector_service.go @@ -4,6 +4,9 @@ package mocks import ( context "context" + + connector "github.com/owncloud/ocis/v2/services/collaboration/pkg/connector" + io "io" mock "github.com/stretchr/testify/mock" @@ -70,22 +73,24 @@ func (_c *ContentConnectorService_GetFile_Call) RunAndReturn(run func(context.Co } // 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) (*connector.ConnectorResponse, error) { ret := _m.Called(ctx, stream, streamLength, lockID) if len(ret) == 0 { panic("no return value specified for PutFile") } - var r0 string + var r0 *connector.ConnectorResponse var r1 error - if rf, ok := ret.Get(0).(func(context.Context, io.Reader, int64, string) (string, error)); ok { + if rf, ok := ret.Get(0).(func(context.Context, io.Reader, int64, string) (*connector.ConnectorResponse, error)); ok { return rf(ctx, stream, streamLength, lockID) } - if rf, ok := ret.Get(0).(func(context.Context, io.Reader, int64, string) string); ok { + if rf, ok := ret.Get(0).(func(context.Context, io.Reader, int64, string) *connector.ConnectorResponse); ok { r0 = rf(ctx, stream, streamLength, lockID) } else { - r0 = ret.Get(0).(string) + if ret.Get(0) != nil { + r0 = ret.Get(0).(*connector.ConnectorResponse) + } } if rf, ok := ret.Get(1).(func(context.Context, io.Reader, int64, string) error); ok { @@ -118,12 +123,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 { +func (_c *ContentConnectorService_PutFile_Call) Return(_a0 *connector.ConnectorResponse, _a1 error) *ContentConnectorService_PutFile_Call { _c.Call.Return(_a0, _a1) 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) (*connector.ConnectorResponse, 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 9518dd36a..b822ed2e2 100644 --- a/services/collaboration/mocks/file_connector_service.go +++ b/services/collaboration/mocks/file_connector_service.go @@ -7,8 +7,6 @@ import ( connector "github.com/owncloud/ocis/v2/services/collaboration/pkg/connector" - fileinfo "github.com/owncloud/ocis/v2/services/collaboration/pkg/connector/fileinfo" - io "io" mock "github.com/stretchr/testify/mock" @@ -28,23 +26,23 @@ func (_m *FileConnectorService) EXPECT() *FileConnectorService_Expecter { } // CheckFileInfo provides a mock function with given fields: ctx -func (_m *FileConnectorService) CheckFileInfo(ctx context.Context) (fileinfo.FileInfo, error) { +func (_m *FileConnectorService) CheckFileInfo(ctx context.Context) (*connector.ConnectorResponse, error) { ret := _m.Called(ctx) if len(ret) == 0 { panic("no return value specified for CheckFileInfo") } - var r0 fileinfo.FileInfo + var r0 *connector.ConnectorResponse var r1 error - if rf, ok := ret.Get(0).(func(context.Context) (fileinfo.FileInfo, error)); ok { + if rf, ok := ret.Get(0).(func(context.Context) (*connector.ConnectorResponse, error)); ok { return rf(ctx) } - if rf, ok := ret.Get(0).(func(context.Context) fileinfo.FileInfo); ok { + if rf, ok := ret.Get(0).(func(context.Context) *connector.ConnectorResponse); ok { r0 = rf(ctx) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(fileinfo.FileInfo) + r0 = ret.Get(0).(*connector.ConnectorResponse) } } @@ -75,33 +73,35 @@ func (_c *FileConnectorService_CheckFileInfo_Call) Run(run func(ctx context.Cont return _c } -func (_c *FileConnectorService_CheckFileInfo_Call) Return(_a0 fileinfo.FileInfo, _a1 error) *FileConnectorService_CheckFileInfo_Call { +func (_c *FileConnectorService_CheckFileInfo_Call) Return(_a0 *connector.ConnectorResponse, _a1 error) *FileConnectorService_CheckFileInfo_Call { _c.Call.Return(_a0, _a1) return _c } -func (_c *FileConnectorService_CheckFileInfo_Call) RunAndReturn(run func(context.Context) (fileinfo.FileInfo, error)) *FileConnectorService_CheckFileInfo_Call { +func (_c *FileConnectorService_CheckFileInfo_Call) RunAndReturn(run func(context.Context) (*connector.ConnectorResponse, error)) *FileConnectorService_CheckFileInfo_Call { _c.Call.Return(run) return _c } // DeleteFile provides a mock function with given fields: ctx, lockID -func (_m *FileConnectorService) DeleteFile(ctx context.Context, lockID string) (string, error) { +func (_m *FileConnectorService) DeleteFile(ctx context.Context, lockID string) (*connector.ConnectorResponse, error) { ret := _m.Called(ctx, lockID) if len(ret) == 0 { panic("no return value specified for DeleteFile") } - var r0 string + var r0 *connector.ConnectorResponse var r1 error - if rf, ok := ret.Get(0).(func(context.Context, string) (string, error)); ok { + if rf, ok := ret.Get(0).(func(context.Context, string) (*connector.ConnectorResponse, error)); ok { return rf(ctx, lockID) } - if rf, ok := ret.Get(0).(func(context.Context, string) string); ok { + if rf, ok := ret.Get(0).(func(context.Context, string) *connector.ConnectorResponse); ok { r0 = rf(ctx, lockID) } else { - r0 = ret.Get(0).(string) + if ret.Get(0) != nil { + r0 = ret.Get(0).(*connector.ConnectorResponse) + } } if rf, ok := ret.Get(1).(func(context.Context, string) error); ok { @@ -132,33 +132,35 @@ func (_c *FileConnectorService_DeleteFile_Call) Run(run func(ctx context.Context return _c } -func (_c *FileConnectorService_DeleteFile_Call) Return(_a0 string, _a1 error) *FileConnectorService_DeleteFile_Call { +func (_c *FileConnectorService_DeleteFile_Call) Return(_a0 *connector.ConnectorResponse, _a1 error) *FileConnectorService_DeleteFile_Call { _c.Call.Return(_a0, _a1) return _c } -func (_c *FileConnectorService_DeleteFile_Call) RunAndReturn(run func(context.Context, string) (string, error)) *FileConnectorService_DeleteFile_Call { +func (_c *FileConnectorService_DeleteFile_Call) RunAndReturn(run func(context.Context, string) (*connector.ConnectorResponse, error)) *FileConnectorService_DeleteFile_Call { _c.Call.Return(run) return _c } // GetLock provides a mock function with given fields: ctx -func (_m *FileConnectorService) GetLock(ctx context.Context) (string, error) { +func (_m *FileConnectorService) GetLock(ctx context.Context) (*connector.ConnectorResponse, error) { ret := _m.Called(ctx) if len(ret) == 0 { panic("no return value specified for GetLock") } - var r0 string + var r0 *connector.ConnectorResponse var r1 error - if rf, ok := ret.Get(0).(func(context.Context) (string, error)); ok { + if rf, ok := ret.Get(0).(func(context.Context) (*connector.ConnectorResponse, error)); ok { return rf(ctx) } - if rf, ok := ret.Get(0).(func(context.Context) string); ok { + if rf, ok := ret.Get(0).(func(context.Context) *connector.ConnectorResponse); ok { r0 = rf(ctx) } else { - r0 = ret.Get(0).(string) + if ret.Get(0) != nil { + r0 = ret.Get(0).(*connector.ConnectorResponse) + } } if rf, ok := ret.Get(1).(func(context.Context) error); ok { @@ -188,33 +190,35 @@ func (_c *FileConnectorService_GetLock_Call) Run(run func(ctx context.Context)) return _c } -func (_c *FileConnectorService_GetLock_Call) Return(_a0 string, _a1 error) *FileConnectorService_GetLock_Call { +func (_c *FileConnectorService_GetLock_Call) Return(_a0 *connector.ConnectorResponse, _a1 error) *FileConnectorService_GetLock_Call { _c.Call.Return(_a0, _a1) return _c } -func (_c *FileConnectorService_GetLock_Call) RunAndReturn(run func(context.Context) (string, error)) *FileConnectorService_GetLock_Call { +func (_c *FileConnectorService_GetLock_Call) RunAndReturn(run func(context.Context) (*connector.ConnectorResponse, error)) *FileConnectorService_GetLock_Call { _c.Call.Return(run) return _c } // 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) (*connector.ConnectorResponse, error) { ret := _m.Called(ctx, lockID, oldLockID) if len(ret) == 0 { panic("no return value specified for Lock") } - var r0 string + var r0 *connector.ConnectorResponse var r1 error - if rf, ok := ret.Get(0).(func(context.Context, string, string) (string, error)); ok { + if rf, ok := ret.Get(0).(func(context.Context, string, string) (*connector.ConnectorResponse, 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) *connector.ConnectorResponse); ok { r0 = rf(ctx, lockID, oldLockID) } else { - r0 = ret.Get(0).(string) + if ret.Get(0) != nil { + r0 = ret.Get(0).(*connector.ConnectorResponse) + } } if rf, ok := ret.Get(1).(func(context.Context, string, string) error); ok { @@ -246,53 +250,44 @@ 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 { +func (_c *FileConnectorService_Lock_Call) Return(_a0 *connector.ConnectorResponse, _a1 error) *FileConnectorService_Lock_Call { _c.Call.Return(_a0, _a1) 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) (*connector.ConnectorResponse, error)) *FileConnectorService_Lock_Call { _c.Call.Return(run) return _c } // PutRelativeFileRelative provides a mock function with given fields: ctx, ccs, stream, streamLength, target -func (_m *FileConnectorService) PutRelativeFileRelative(ctx context.Context, ccs connector.ContentConnectorService, stream io.Reader, streamLength int64, target string) (*connector.PutRelativeResponse, *connector.PutRelativeHeaders, error) { +func (_m *FileConnectorService) PutRelativeFileRelative(ctx context.Context, ccs connector.ContentConnectorService, stream io.Reader, streamLength int64, target string) (*connector.ConnectorResponse, error) { ret := _m.Called(ctx, ccs, stream, streamLength, target) if len(ret) == 0 { panic("no return value specified for PutRelativeFileRelative") } - var r0 *connector.PutRelativeResponse - var r1 *connector.PutRelativeHeaders - var r2 error - if rf, ok := ret.Get(0).(func(context.Context, connector.ContentConnectorService, io.Reader, int64, string) (*connector.PutRelativeResponse, *connector.PutRelativeHeaders, error)); ok { + var r0 *connector.ConnectorResponse + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, connector.ContentConnectorService, io.Reader, int64, string) (*connector.ConnectorResponse, error)); ok { return rf(ctx, ccs, stream, streamLength, target) } - if rf, ok := ret.Get(0).(func(context.Context, connector.ContentConnectorService, io.Reader, int64, string) *connector.PutRelativeResponse); ok { + if rf, ok := ret.Get(0).(func(context.Context, connector.ContentConnectorService, io.Reader, int64, string) *connector.ConnectorResponse); ok { r0 = rf(ctx, ccs, stream, streamLength, target) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(*connector.PutRelativeResponse) + r0 = ret.Get(0).(*connector.ConnectorResponse) } } - if rf, ok := ret.Get(1).(func(context.Context, connector.ContentConnectorService, io.Reader, int64, string) *connector.PutRelativeHeaders); ok { + if rf, ok := ret.Get(1).(func(context.Context, connector.ContentConnectorService, io.Reader, int64, string) error); ok { r1 = rf(ctx, ccs, stream, streamLength, target) } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*connector.PutRelativeHeaders) - } + r1 = ret.Error(1) } - if rf, ok := ret.Get(2).(func(context.Context, connector.ContentConnectorService, io.Reader, int64, string) error); ok { - r2 = rf(ctx, ccs, stream, streamLength, target) - } else { - r2 = ret.Error(2) - } - - return r0, r1, r2 + return r0, r1 } // FileConnectorService_PutRelativeFileRelative_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'PutRelativeFileRelative' @@ -317,34 +312,34 @@ func (_c *FileConnectorService_PutRelativeFileRelative_Call) Run(run func(ctx co return _c } -func (_c *FileConnectorService_PutRelativeFileRelative_Call) Return(_a0 *connector.PutRelativeResponse, _a1 *connector.PutRelativeHeaders, _a2 error) *FileConnectorService_PutRelativeFileRelative_Call { - _c.Call.Return(_a0, _a1, _a2) +func (_c *FileConnectorService_PutRelativeFileRelative_Call) Return(_a0 *connector.ConnectorResponse, _a1 error) *FileConnectorService_PutRelativeFileRelative_Call { + _c.Call.Return(_a0, _a1) return _c } -func (_c *FileConnectorService_PutRelativeFileRelative_Call) RunAndReturn(run func(context.Context, connector.ContentConnectorService, io.Reader, int64, string) (*connector.PutRelativeResponse, *connector.PutRelativeHeaders, error)) *FileConnectorService_PutRelativeFileRelative_Call { +func (_c *FileConnectorService_PutRelativeFileRelative_Call) RunAndReturn(run func(context.Context, connector.ContentConnectorService, io.Reader, int64, string) (*connector.ConnectorResponse, error)) *FileConnectorService_PutRelativeFileRelative_Call { _c.Call.Return(run) return _c } // PutRelativeFileSuggested provides a mock function with given fields: ctx, ccs, stream, streamLength, target -func (_m *FileConnectorService) PutRelativeFileSuggested(ctx context.Context, ccs connector.ContentConnectorService, stream io.Reader, streamLength int64, target string) (*connector.PutRelativeResponse, error) { +func (_m *FileConnectorService) PutRelativeFileSuggested(ctx context.Context, ccs connector.ContentConnectorService, stream io.Reader, streamLength int64, target string) (*connector.ConnectorResponse, error) { ret := _m.Called(ctx, ccs, stream, streamLength, target) if len(ret) == 0 { panic("no return value specified for PutRelativeFileSuggested") } - var r0 *connector.PutRelativeResponse + var r0 *connector.ConnectorResponse var r1 error - if rf, ok := ret.Get(0).(func(context.Context, connector.ContentConnectorService, io.Reader, int64, string) (*connector.PutRelativeResponse, error)); ok { + if rf, ok := ret.Get(0).(func(context.Context, connector.ContentConnectorService, io.Reader, int64, string) (*connector.ConnectorResponse, error)); ok { return rf(ctx, ccs, stream, streamLength, target) } - if rf, ok := ret.Get(0).(func(context.Context, connector.ContentConnectorService, io.Reader, int64, string) *connector.PutRelativeResponse); ok { + if rf, ok := ret.Get(0).(func(context.Context, connector.ContentConnectorService, io.Reader, int64, string) *connector.ConnectorResponse); ok { r0 = rf(ctx, ccs, stream, streamLength, target) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(*connector.PutRelativeResponse) + r0 = ret.Get(0).(*connector.ConnectorResponse) } } @@ -379,33 +374,35 @@ func (_c *FileConnectorService_PutRelativeFileSuggested_Call) Run(run func(ctx c return _c } -func (_c *FileConnectorService_PutRelativeFileSuggested_Call) Return(_a0 *connector.PutRelativeResponse, _a1 error) *FileConnectorService_PutRelativeFileSuggested_Call { +func (_c *FileConnectorService_PutRelativeFileSuggested_Call) Return(_a0 *connector.ConnectorResponse, _a1 error) *FileConnectorService_PutRelativeFileSuggested_Call { _c.Call.Return(_a0, _a1) return _c } -func (_c *FileConnectorService_PutRelativeFileSuggested_Call) RunAndReturn(run func(context.Context, connector.ContentConnectorService, io.Reader, int64, string) (*connector.PutRelativeResponse, error)) *FileConnectorService_PutRelativeFileSuggested_Call { +func (_c *FileConnectorService_PutRelativeFileSuggested_Call) RunAndReturn(run func(context.Context, connector.ContentConnectorService, io.Reader, int64, string) (*connector.ConnectorResponse, error)) *FileConnectorService_PutRelativeFileSuggested_Call { _c.Call.Return(run) return _c } // 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) (*connector.ConnectorResponse, error) { ret := _m.Called(ctx, lockID) if len(ret) == 0 { panic("no return value specified for RefreshLock") } - var r0 string + var r0 *connector.ConnectorResponse var r1 error - if rf, ok := ret.Get(0).(func(context.Context, string) (string, error)); ok { + if rf, ok := ret.Get(0).(func(context.Context, string) (*connector.ConnectorResponse, error)); ok { return rf(ctx, lockID) } - if rf, ok := ret.Get(0).(func(context.Context, string) string); ok { + if rf, ok := ret.Get(0).(func(context.Context, string) *connector.ConnectorResponse); ok { r0 = rf(ctx, lockID) } else { - r0 = ret.Get(0).(string) + if ret.Get(0) != nil { + r0 = ret.Get(0).(*connector.ConnectorResponse) + } } if rf, ok := ret.Get(1).(func(context.Context, string) error); ok { @@ -436,51 +433,44 @@ 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 { +func (_c *FileConnectorService_RefreshLock_Call) Return(_a0 *connector.ConnectorResponse, _a1 error) *FileConnectorService_RefreshLock_Call { _c.Call.Return(_a0, _a1) 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) (*connector.ConnectorResponse, error)) *FileConnectorService_RefreshLock_Call { _c.Call.Return(run) return _c } // RenameFile provides a mock function with given fields: ctx, lockID, target -func (_m *FileConnectorService) RenameFile(ctx context.Context, lockID string, target string) (*connector.RenameResponse, string, error) { +func (_m *FileConnectorService) RenameFile(ctx context.Context, lockID string, target string) (*connector.ConnectorResponse, error) { ret := _m.Called(ctx, lockID, target) if len(ret) == 0 { panic("no return value specified for RenameFile") } - var r0 *connector.RenameResponse - var r1 string - var r2 error - if rf, ok := ret.Get(0).(func(context.Context, string, string) (*connector.RenameResponse, string, error)); ok { + var r0 *connector.ConnectorResponse + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, string, string) (*connector.ConnectorResponse, error)); ok { return rf(ctx, lockID, target) } - if rf, ok := ret.Get(0).(func(context.Context, string, string) *connector.RenameResponse); ok { + if rf, ok := ret.Get(0).(func(context.Context, string, string) *connector.ConnectorResponse); ok { r0 = rf(ctx, lockID, target) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(*connector.RenameResponse) + r0 = ret.Get(0).(*connector.ConnectorResponse) } } - if rf, ok := ret.Get(1).(func(context.Context, string, string) string); ok { + if rf, ok := ret.Get(1).(func(context.Context, string, string) error); ok { r1 = rf(ctx, lockID, target) } else { - r1 = ret.Get(1).(string) + r1 = ret.Error(1) } - if rf, ok := ret.Get(2).(func(context.Context, string, string) error); ok { - r2 = rf(ctx, lockID, target) - } else { - r2 = ret.Error(2) - } - - return r0, r1, r2 + return r0, r1 } // FileConnectorService_RenameFile_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'RenameFile' @@ -503,33 +493,35 @@ func (_c *FileConnectorService_RenameFile_Call) Run(run func(ctx context.Context return _c } -func (_c *FileConnectorService_RenameFile_Call) Return(_a0 *connector.RenameResponse, _a1 string, _a2 error) *FileConnectorService_RenameFile_Call { - _c.Call.Return(_a0, _a1, _a2) +func (_c *FileConnectorService_RenameFile_Call) Return(_a0 *connector.ConnectorResponse, _a1 error) *FileConnectorService_RenameFile_Call { + _c.Call.Return(_a0, _a1) return _c } -func (_c *FileConnectorService_RenameFile_Call) RunAndReturn(run func(context.Context, string, string) (*connector.RenameResponse, string, error)) *FileConnectorService_RenameFile_Call { +func (_c *FileConnectorService_RenameFile_Call) RunAndReturn(run func(context.Context, string, string) (*connector.ConnectorResponse, error)) *FileConnectorService_RenameFile_Call { _c.Call.Return(run) return _c } // 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) (*connector.ConnectorResponse, error) { ret := _m.Called(ctx, lockID) if len(ret) == 0 { panic("no return value specified for UnLock") } - var r0 string + var r0 *connector.ConnectorResponse var r1 error - if rf, ok := ret.Get(0).(func(context.Context, string) (string, error)); ok { + if rf, ok := ret.Get(0).(func(context.Context, string) (*connector.ConnectorResponse, error)); ok { return rf(ctx, lockID) } - if rf, ok := ret.Get(0).(func(context.Context, string) string); ok { + if rf, ok := ret.Get(0).(func(context.Context, string) *connector.ConnectorResponse); ok { r0 = rf(ctx, lockID) } else { - r0 = ret.Get(0).(string) + if ret.Get(0) != nil { + r0 = ret.Get(0).(*connector.ConnectorResponse) + } } if rf, ok := ret.Get(1).(func(context.Context, string) error); ok { @@ -560,12 +552,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 { +func (_c *FileConnectorService_UnLock_Call) Return(_a0 *connector.ConnectorResponse, _a1 error) *FileConnectorService_UnLock_Call { _c.Call.Return(_a0, _a1) 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) (*connector.ConnectorResponse, error)) *FileConnectorService_UnLock_Call { _c.Call.Return(run) return _c } diff --git a/services/collaboration/pkg/connector/contentconnector_test.go b/services/collaboration/pkg/connector/contentconnector_test.go index aaf9fecf7..38f308afc 100644 --- a/services/collaboration/pkg/connector/contentconnector_test.go +++ b/services/collaboration/pkg/connector/contentconnector_test.go @@ -220,9 +220,9 @@ 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") + response, err := cc.PutFile(ctx, reader, reader.Size(), "notARandomLockId") Expect(err).To(HaveOccurred()) - Expect(newLockId).To(Equal("")) + Expect(response).To(BeNil()) }) It("Stat call failed", func() { @@ -234,9 +234,9 @@ var _ = Describe("ContentConnector", func() { Status: status.NewInternal(ctx, "Something failed"), }, targetErr) - newLockId, err := cc.PutFile(ctx, reader, reader.Size(), "notARandomLockId") + response, err := cc.PutFile(ctx, reader, reader.Size(), "notARandomLockId") Expect(err).To(Equal(targetErr)) - Expect(newLockId).To(Equal("")) + Expect(response).To(BeNil()) }) It("Stat call status not ok", func() { @@ -247,11 +247,10 @@ var _ = Describe("ContentConnector", func() { Status: status.NewInternal(ctx, "Something failed"), }, nil) - newLockId, err := cc.PutFile(ctx, reader, reader.Size(), "notARandomLockId") - Expect(err).To(HaveOccurred()) - conErr := err.(*connector.ConnectorError) - Expect(conErr.HttpCodeOut).To(Equal(500)) - Expect(newLockId).To(Equal("")) + response, err := cc.PutFile(ctx, reader, reader.Size(), "notARandomLockId") + Expect(err).To(Succeed()) + Expect(response.Status).To(Equal(500)) + Expect(response.Headers).To(BeNil()) }) It("Mismatched lockId", func() { @@ -268,11 +267,10 @@ var _ = Describe("ContentConnector", func() { }, }, nil) - newLockId, err := cc.PutFile(ctx, reader, reader.Size(), "notARandomLockId") - Expect(err).To(HaveOccurred()) - conErr := err.(*connector.ConnectorError) - Expect(conErr.HttpCodeOut).To(Equal(409)) - Expect(newLockId).To(Equal("goodAndValidLock")) + response, err := cc.PutFile(ctx, reader, reader.Size(), "notARandomLockId") + Expect(err).To(Succeed()) + Expect(response.Status).To(Equal(409)) + Expect(response.Headers[connector.HeaderWopiLock]).To(Equal("goodAndValidLock")) }) It("Upload without lockId but on a non empty file", func() { @@ -287,11 +285,10 @@ var _ = Describe("ContentConnector", func() { }, }, nil) - newLockId, err := cc.PutFile(ctx, reader, reader.Size(), "") - Expect(err).To(HaveOccurred()) - conErr := err.(*connector.ConnectorError) - Expect(conErr.HttpCodeOut).To(Equal(409)) - Expect(newLockId).To(Equal("")) + response, err := cc.PutFile(ctx, reader, reader.Size(), "") + Expect(err).To(Succeed()) + Expect(response.Status).To(Equal(409)) + Expect(response.Headers[connector.HeaderWopiLock]).To(Equal("")) }) It("Initiate upload fails", func() { @@ -314,9 +311,9 @@ var _ = Describe("ContentConnector", func() { Status: status.NewInternal(ctx, "Something failed"), }, targetErr) - newLockId, err := cc.PutFile(ctx, reader, reader.Size(), "goodAndValidLock") + response, err := cc.PutFile(ctx, reader, reader.Size(), "goodAndValidLock") Expect(err).To(HaveOccurred()) - Expect(newLockId).To(Equal("")) + Expect(response).To(BeNil()) }) It("Initiate upload status not ok", func() { @@ -338,11 +335,10 @@ var _ = Describe("ContentConnector", func() { Status: status.NewInternal(ctx, "Something failed"), }, nil) - newLockId, err := cc.PutFile(ctx, reader, reader.Size(), "goodAndValidLock") - Expect(err).To(HaveOccurred()) - conErr := err.(*connector.ConnectorError) - Expect(conErr.HttpCodeOut).To(Equal(500)) - Expect(newLockId).To(Equal("")) + response, err := cc.PutFile(ctx, reader, reader.Size(), "goodAndValidLock") + Expect(err).To(Succeed()) + Expect(response.Status).To(Equal(500)) + Expect(response.Headers).To(BeNil()) }) It("Empty upload successful", func() { @@ -364,9 +360,10 @@ var _ = Describe("ContentConnector", func() { Status: status.NewOK(ctx), }, nil) - newLockId, err := cc.PutFile(ctx, reader, reader.Size(), "goodAndValidLock") + response, err := cc.PutFile(ctx, reader, reader.Size(), "goodAndValidLock") Expect(err).To(Succeed()) - Expect(newLockId).To(Equal("")) + Expect(response.Status).To(Equal(200)) + Expect(response.Headers).To(BeNil()) }) It("Missing upload endpoint", func() { @@ -388,11 +385,10 @@ var _ = Describe("ContentConnector", func() { Status: status.NewOK(ctx), }, nil) - newLockId, err := cc.PutFile(ctx, reader, reader.Size(), "goodAndValidLock") - Expect(err).To(HaveOccurred()) - conErr := err.(*connector.ConnectorError) - Expect(conErr.HttpCodeOut).To(Equal(500)) - Expect(newLockId).To(Equal("")) + response, err := cc.PutFile(ctx, reader, reader.Size(), "goodAndValidLock") + Expect(err).To(BeNil()) + Expect(response.Status).To(Equal(500)) + Expect(response.Headers).To(BeNil()) }) It("upload request failed", func() { @@ -420,12 +416,11 @@ var _ = Describe("ContentConnector", func() { }, }, nil) - newLockId, err := cc.PutFile(ctx, reader, reader.Size(), "goodAndValidLock") + response, 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) - Expect(conErr.HttpCodeOut).To(Equal(500)) - Expect(newLockId).To(Equal("")) + Expect(err).To(Succeed()) + Expect(response.Status).To(Equal(500)) + Expect(response.Headers).To(BeNil()) }) It("upload request success", func() { @@ -453,10 +448,11 @@ var _ = Describe("ContentConnector", func() { }, }, nil) - newLockId, err := cc.PutFile(ctx, reader, reader.Size(), "goodAndValidLock") + response, 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(response.Status).To(Equal(200)) + Expect(response.Headers).To(BeNil()) }) }) }) diff --git a/services/collaboration/pkg/connector/fileconnector_test.go b/services/collaboration/pkg/connector/fileconnector_test.go index d6f88c5ad..aeb303bc1 100644 --- a/services/collaboration/pkg/connector/fileconnector_test.go +++ b/services/collaboration/pkg/connector/fileconnector_test.go @@ -91,9 +91,9 @@ var _ = Describe("FileConnector", func() { Describe("GetLock", func() { It("No valid context", func() { ctx := context.Background() - newLockId, err := fc.GetLock(ctx) + response, err := fc.GetLock(ctx) Expect(err).To(HaveOccurred()) - Expect(newLockId).To(Equal("")) + Expect(response).To(BeNil()) }) It("Get lock failed", func() { @@ -104,9 +104,9 @@ var _ = Describe("FileConnector", func() { Status: status.NewInternal(ctx, "Something failed"), }, targetErr) - newLockId, err := fc.GetLock(ctx) + response, err := fc.GetLock(ctx) Expect(err).To(Equal(targetErr)) - Expect(newLockId).To(Equal("")) + Expect(response).To(BeNil()) }) It("Get lock failed status not ok", func() { @@ -117,11 +117,10 @@ var _ = Describe("FileConnector", func() { Status: status.NewNotFound(ctx, "File is missing"), }, nil) - newLockId, err := fc.GetLock(ctx) - Expect(err).To(HaveOccurred()) - conErr := err.(*connector.ConnectorError) - Expect(conErr.HttpCodeOut).To(Equal(404)) - Expect(newLockId).To(Equal("")) + response, err := fc.GetLock(ctx) + Expect(err).To(Succeed()) + Expect(response.Status).To(Equal(404)) + Expect(response.Headers).To(BeNil()) }) It("Get lock success", func() { @@ -136,9 +135,10 @@ var _ = Describe("FileConnector", func() { }, }, nil) - newLockId, err := fc.GetLock(ctx) + response, err := fc.GetLock(ctx) Expect(err).To(Succeed()) - Expect(newLockId).To(Equal("zzz999")) + Expect(response.Status).To(Equal(200)) + Expect(response.Headers[connector.HeaderWopiLock]).To(Equal("zzz999")) }) }) @@ -146,19 +146,18 @@ var _ = Describe("FileConnector", func() { Describe("Lock", func() { It("No valid context", func() { ctx := context.Background() - newLockId, err := fc.Lock(ctx, "newLock", "") + response, err := fc.Lock(ctx, "newLock", "") Expect(err).To(HaveOccurred()) - Expect(newLockId).To(Equal("")) + Expect(response).To(BeNil()) }) It("Empty lockId", func() { ctx := middleware.WopiContextToCtx(context.Background(), wopiCtx) - newLockId, err := fc.Lock(ctx, "", "") - Expect(err).To(HaveOccurred()) - conErr := err.(*connector.ConnectorError) - Expect(conErr.HttpCodeOut).To(Equal(400)) - Expect(newLockId).To(Equal("")) + response, err := fc.Lock(ctx, "", "") + Expect(err).To(Succeed()) + Expect(response.Status).To(Equal(400)) + Expect(response.Headers).To(BeNil()) }) It("Set lock failed", func() { @@ -169,10 +168,10 @@ var _ = Describe("FileConnector", func() { Status: status.NewInternal(ctx, "Something failed"), }, targetErr) - newLockId, err := fc.Lock(ctx, "abcdef123", "") + response, err := fc.Lock(ctx, "abcdef123", "") Expect(err).To(HaveOccurred()) Expect(err).To(Equal(targetErr)) - Expect(newLockId).To(Equal("")) + Expect(response).To(BeNil()) }) It("Set lock success", func() { @@ -182,9 +181,10 @@ var _ = Describe("FileConnector", func() { Status: status.NewOK(ctx), }, nil) - newLockId, err := fc.Lock(ctx, "abcdef123", "") + response, err := fc.Lock(ctx, "abcdef123", "") Expect(err).To(Succeed()) - Expect(newLockId).To(Equal("")) + Expect(response.Status).To(Equal(200)) + Expect(response.Headers).To(BeNil()) }) It("Set lock mismatches error getting lock", func() { @@ -199,10 +199,10 @@ var _ = Describe("FileConnector", func() { Status: status.NewInternal(ctx, "lock mismatch"), }, targetErr) - newLockId, err := fc.Lock(ctx, "abcdef123", "") + response, err := fc.Lock(ctx, "abcdef123", "") Expect(err).To(HaveOccurred()) Expect(err).To(Equal(targetErr)) - Expect(newLockId).To(Equal("")) + Expect(response).To(BeNil()) }) It("Set lock mismatches", func() { @@ -220,11 +220,10 @@ var _ = Describe("FileConnector", func() { }, }, nil) - newLockId, err := fc.Lock(ctx, "abcdef123", "") - Expect(err).To(HaveOccurred()) - conErr := err.(*connector.ConnectorError) - Expect(conErr.HttpCodeOut).To(Equal(409)) - Expect(newLockId).To(Equal("zzz999")) + response, err := fc.Lock(ctx, "abcdef123", "") + Expect(err).To(Succeed()) + Expect(response.Status).To(Equal(409)) + Expect(response.Headers[connector.HeaderWopiLock]).To(Equal("zzz999")) }) It("Set lock mismatches but get lock matches", func() { @@ -242,9 +241,10 @@ var _ = Describe("FileConnector", func() { }, }, nil) - newLockId, err := fc.Lock(ctx, "abcdef123", "") + response, err := fc.Lock(ctx, "abcdef123", "") Expect(err).To(Succeed()) - Expect(newLockId).To(Equal("abcdef123")) + Expect(response.Status).To(Equal(200)) + Expect(response.Headers).To(BeNil()) }) It("Set lock mismatches but get lock doesn't return lockId", func() { @@ -258,11 +258,10 @@ var _ = Describe("FileConnector", func() { Status: status.NewOK(ctx), }, nil) - newLockId, err := fc.Lock(ctx, "abcdef123", "") - Expect(err).To(HaveOccurred()) - conErr := err.(*connector.ConnectorError) - Expect(conErr.HttpCodeOut).To(Equal(500)) - Expect(newLockId).To(Equal("")) + response, err := fc.Lock(ctx, "abcdef123", "") + Expect(err).To(Succeed()) + Expect(response.Status).To(Equal(500)) + Expect(response.Headers).To(BeNil()) }) It("File not found", func() { @@ -272,11 +271,10 @@ var _ = Describe("FileConnector", func() { Status: status.NewNotFound(ctx, "file not found"), }, nil) - newLockId, err := fc.Lock(ctx, "abcdef123", "") - Expect(err).To(HaveOccurred()) - conErr := err.(*connector.ConnectorError) - Expect(conErr.HttpCodeOut).To(Equal(404)) - Expect(newLockId).To(Equal("")) + response, err := fc.Lock(ctx, "abcdef123", "") + Expect(err).To(Succeed()) + Expect(response.Status).To(Equal(404)) + Expect(response.Headers).To(BeNil()) }) It("Default error handling (insufficient storage)", func() { @@ -286,30 +284,28 @@ var _ = Describe("FileConnector", func() { Status: status.NewInsufficientStorage(ctx, nil, "file too big"), }, nil) - newLockId, err := fc.Lock(ctx, "abcdef123", "") - Expect(err).To(HaveOccurred()) - conErr := err.(*connector.ConnectorError) - Expect(conErr.HttpCodeOut).To(Equal(500)) - Expect(newLockId).To(Equal("")) + response, err := fc.Lock(ctx, "abcdef123", "") + Expect(err).To(Succeed()) + Expect(response.Status).To(Equal(500)) + Expect(response.Headers).To(BeNil()) }) }) Describe("Unlock and relock", func() { It("No valid context", func() { ctx := context.Background() - newLockId, err := fc.Lock(ctx, "newLock", "oldLock") + response, err := fc.Lock(ctx, "newLock", "oldLock") Expect(err).To(HaveOccurred()) - Expect(newLockId).To(Equal("")) + Expect(response).To(BeNil()) }) It("Empty lockId", func() { ctx := middleware.WopiContextToCtx(context.Background(), wopiCtx) - newLockId, err := fc.Lock(ctx, "", "oldLock") - Expect(err).To(HaveOccurred()) - conErr := err.(*connector.ConnectorError) - Expect(conErr.HttpCodeOut).To(Equal(400)) - Expect(newLockId).To(Equal("")) + response, err := fc.Lock(ctx, "", "oldLock") + Expect(err).To(Succeed()) + Expect(response.Status).To(Equal(400)) + Expect(response.Headers).To(BeNil()) }) It("Refresh lock failed", func() { @@ -320,10 +316,10 @@ var _ = Describe("FileConnector", func() { Status: status.NewInternal(ctx, "Something failed"), }, targetErr) - newLockId, err := fc.Lock(ctx, "abcdef123", "oldLock") + response, err := fc.Lock(ctx, "abcdef123", "oldLock") Expect(err).To(HaveOccurred()) Expect(err).To(Equal(targetErr)) - Expect(newLockId).To(Equal("")) + Expect(response).To(BeNil()) }) It("Refresh lock success", func() { @@ -333,9 +329,10 @@ var _ = Describe("FileConnector", func() { Status: status.NewOK(ctx), }, nil) - newLockId, err := fc.Lock(ctx, "abcdef123", "oldLock") + response, err := fc.Lock(ctx, "abcdef123", "oldLock") Expect(err).To(Succeed()) - Expect(newLockId).To(Equal("")) + Expect(response.Status).To(Equal(200)) + Expect(response.Headers).To(BeNil()) }) It("Refresh lock mismatches error getting lock", func() { @@ -350,10 +347,10 @@ var _ = Describe("FileConnector", func() { Status: status.NewInternal(ctx, "lock mismatch"), }, targetErr) - newLockId, err := fc.Lock(ctx, "abcdef123", "112233") + response, err := fc.Lock(ctx, "abcdef123", "112233") Expect(err).To(HaveOccurred()) Expect(err).To(Equal(targetErr)) - Expect(newLockId).To(Equal("")) + Expect(response).To(BeNil()) }) It("Refresh lock mismatches", func() { @@ -371,11 +368,10 @@ var _ = Describe("FileConnector", func() { }, }, nil) - newLockId, 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")) + response, err := fc.Lock(ctx, "abcdef123", "112233") + Expect(err).To(Succeed()) + Expect(response.Status).To(Equal(409)) + Expect(response.Headers[connector.HeaderWopiLock]).To(Equal("zzz999")) }) It("Refresh lock mismatches but get lock matches", func() { @@ -393,9 +389,10 @@ var _ = Describe("FileConnector", func() { }, }, nil) - newLockId, err := fc.Lock(ctx, "abcdef123", "112233") + response, err := fc.Lock(ctx, "abcdef123", "112233") Expect(err).To(Succeed()) - Expect(newLockId).To(Equal("abcdef123")) + Expect(response.Status).To(Equal(200)) + Expect(response.Headers).To(BeNil()) }) It("Refresh lock mismatches but get lock doesn't return lockId", func() { @@ -409,11 +406,10 @@ var _ = Describe("FileConnector", func() { Status: status.NewOK(ctx), }, nil) - newLockId, err := fc.Lock(ctx, "abcdef123", "112233") - Expect(err).To(HaveOccurred()) - conErr := err.(*connector.ConnectorError) - Expect(conErr.HttpCodeOut).To(Equal(500)) - Expect(newLockId).To(Equal("")) + response, err := fc.Lock(ctx, "abcdef123", "112233") + Expect(err).To(Succeed()) + Expect(response.Status).To(Equal(500)) + Expect(response.Headers).To(BeNil()) }) It("File not found", func() { @@ -423,11 +419,10 @@ var _ = Describe("FileConnector", func() { Status: status.NewNotFound(ctx, "file not found"), }, nil) - newLockId, err := fc.Lock(ctx, "abcdef123", "112233") - Expect(err).To(HaveOccurred()) - conErr := err.(*connector.ConnectorError) - Expect(conErr.HttpCodeOut).To(Equal(404)) - Expect(newLockId).To(Equal("")) + response, err := fc.Lock(ctx, "abcdef123", "112233") + Expect(err).To(Succeed()) + Expect(response.Status).To(Equal(404)) + Expect(response.Headers).To(BeNil()) }) It("Default error handling (insufficient storage)", func() { @@ -437,11 +432,10 @@ var _ = Describe("FileConnector", func() { Status: status.NewInsufficientStorage(ctx, nil, "file too big"), }, nil) - newLockId, err := fc.Lock(ctx, "abcdef123", "112233") - Expect(err).To(HaveOccurred()) - conErr := err.(*connector.ConnectorError) - Expect(conErr.HttpCodeOut).To(Equal(500)) - Expect(newLockId).To(Equal("")) + response, err := fc.Lock(ctx, "abcdef123", "112233") + Expect(err).To(Succeed()) + Expect(response.Status).To(Equal(500)) + Expect(response.Headers).To(BeNil()) }) }) }) @@ -449,19 +443,18 @@ var _ = Describe("FileConnector", func() { Describe("RefreshLock", func() { It("No valid context", func() { ctx := context.Background() - newLockId, err := fc.RefreshLock(ctx, "newLock") + response, err := fc.RefreshLock(ctx, "newLock") Expect(err).To(HaveOccurred()) - Expect(newLockId).To(Equal("")) + Expect(response).To(BeNil()) }) It("Empty lockId", func() { ctx := middleware.WopiContextToCtx(context.Background(), wopiCtx) - newLockId, err := fc.RefreshLock(ctx, "") - Expect(err).To(HaveOccurred()) - conErr := err.(*connector.ConnectorError) - Expect(conErr.HttpCodeOut).To(Equal(400)) - Expect(newLockId).To(Equal("")) + response, err := fc.RefreshLock(ctx, "") + Expect(err).To(Succeed()) + Expect(response.Status).To(Equal(400)) + Expect(response.Headers).To(BeNil()) }) It("Refresh lock fails", func() { @@ -472,10 +465,10 @@ var _ = Describe("FileConnector", func() { Status: status.NewConflict(ctx, nil, "lock mismatch"), }, targetErr) - newLockId, err := fc.RefreshLock(ctx, "abcdef123") + response, err := fc.RefreshLock(ctx, "abcdef123") Expect(err).To(HaveOccurred()) Expect(err).To(Equal(targetErr)) - Expect(newLockId).To(Equal("")) + Expect(response).To(BeNil()) }) It("Refresh lock success", func() { @@ -485,9 +478,10 @@ var _ = Describe("FileConnector", func() { Status: status.NewOK(ctx), }, nil) - newLockId, err := fc.RefreshLock(ctx, "abcdef123") + response, err := fc.RefreshLock(ctx, "abcdef123") Expect(err).To(Succeed()) - Expect(newLockId).To(Equal("")) + Expect(response.Status).To(Equal(200)) + Expect(response.Headers).To(BeNil()) }) It("Refresh lock file not found", func() { @@ -497,11 +491,10 @@ var _ = Describe("FileConnector", func() { Status: status.NewNotFound(ctx, "file not found"), }, nil) - newLockId, err := fc.RefreshLock(ctx, "abcdef123") - Expect(err).To(HaveOccurred()) - conErr := err.(*connector.ConnectorError) - Expect(conErr.HttpCodeOut).To(Equal(404)) - Expect(newLockId).To(Equal("")) + response, err := fc.RefreshLock(ctx, "abcdef123") + Expect(err).To(Succeed()) + Expect(response.Status).To(Equal(404)) + Expect(response.Headers).To(BeNil()) }) It("Refresh lock mismatch and get lock fails", func() { @@ -516,10 +509,10 @@ var _ = Describe("FileConnector", func() { Status: status.NewConflict(ctx, nil, "lock mismatch"), }, targetErr) - newLockId, err := fc.RefreshLock(ctx, "abcdef123") + response, err := fc.RefreshLock(ctx, "abcdef123") Expect(err).To(HaveOccurred()) Expect(err).To(Equal(targetErr)) - Expect(newLockId).To(Equal("")) + Expect(response).To(BeNil()) }) It("Refresh lock mismatch and get lock status not ok", func() { @@ -533,11 +526,10 @@ var _ = Describe("FileConnector", func() { Status: status.NewInternal(ctx, "lock mismatch"), }, nil) - newLockId, err := fc.RefreshLock(ctx, "abcdef123") - Expect(err).To(HaveOccurred()) - conErr := err.(*connector.ConnectorError) - Expect(conErr.HttpCodeOut).To(Equal(500)) - Expect(newLockId).To(Equal("")) + response, err := fc.RefreshLock(ctx, "abcdef123") + Expect(err).To(Succeed()) + Expect(response.Status).To(Equal(500)) + Expect(response.Headers).To(BeNil()) }) It("Refresh lock mismatch and no lock", func() { @@ -551,11 +543,10 @@ var _ = Describe("FileConnector", func() { Status: status.NewOK(ctx), }, nil) - newLockId, err := fc.RefreshLock(ctx, "abcdef123") - Expect(err).To(HaveOccurred()) - conErr := err.(*connector.ConnectorError) - Expect(conErr.HttpCodeOut).To(Equal(409)) - Expect(newLockId).To(Equal("")) + response, err := fc.RefreshLock(ctx, "abcdef123") + Expect(err).To(Succeed()) + Expect(response.Status).To(Equal(409)) + Expect(response.Headers[connector.HeaderWopiLock]).To(Equal("")) }) It("Refresh lock mismatch", func() { @@ -573,11 +564,10 @@ var _ = Describe("FileConnector", func() { }, }, nil) - newLockId, err := fc.RefreshLock(ctx, "abcdef123") - Expect(err).To(HaveOccurred()) - conErr := err.(*connector.ConnectorError) - Expect(conErr.HttpCodeOut).To(Equal(409)) - Expect(newLockId).To(Equal("zzz999")) + response, err := fc.RefreshLock(ctx, "abcdef123") + Expect(err).To(Succeed()) + Expect(response.Status).To(Equal(409)) + Expect(response.Headers[connector.HeaderWopiLock]).To(Equal("zzz999")) }) It("Default error handling (insufficient storage)", func() { @@ -587,30 +577,28 @@ var _ = Describe("FileConnector", func() { Status: status.NewInsufficientStorage(ctx, nil, "file too big"), }, nil) - newLockId, err := fc.RefreshLock(ctx, "abcdef123") - Expect(err).To(HaveOccurred()) - conErr := err.(*connector.ConnectorError) - Expect(conErr.HttpCodeOut).To(Equal(500)) - Expect(newLockId).To(Equal("")) + response, err := fc.RefreshLock(ctx, "abcdef123") + Expect(err).To(Succeed()) + Expect(response.Status).To(Equal(500)) + Expect(response.Headers).To(BeNil()) }) }) Describe("Unlock", func() { It("No valid context", func() { ctx := context.Background() - newLockId, err := fc.UnLock(ctx, "newLock") + response, err := fc.UnLock(ctx, "newLock") Expect(err).To(HaveOccurred()) - Expect(newLockId).To(Equal("")) + Expect(response).To(BeNil()) }) It("Empty lockId", func() { ctx := middleware.WopiContextToCtx(context.Background(), wopiCtx) - newLockId, err := fc.UnLock(ctx, "") - Expect(err).To(HaveOccurred()) - conErr := err.(*connector.ConnectorError) - Expect(conErr.HttpCodeOut).To(Equal(400)) - Expect(newLockId).To(Equal("")) + response, err := fc.UnLock(ctx, "") + Expect(err).To(Succeed()) + Expect(response.Status).To(Equal(400)) + Expect(response.Headers).To(BeNil()) }) It("Unlock fails", func() { @@ -621,10 +609,10 @@ var _ = Describe("FileConnector", func() { Status: status.NewInternal(ctx, "something failed"), }, targetErr) - newLockId, err := fc.UnLock(ctx, "abcdef123") + response, err := fc.UnLock(ctx, "abcdef123") Expect(err).To(HaveOccurred()) Expect(err).To(Equal(targetErr)) - Expect(newLockId).To(Equal("")) + Expect(response).To(BeNil()) }) It("Unlock success", func() { @@ -634,9 +622,10 @@ var _ = Describe("FileConnector", func() { Status: status.NewOK(ctx), }, nil) - newLockId, err := fc.UnLock(ctx, "abcdef123") + response, err := fc.UnLock(ctx, "abcdef123") Expect(err).To(Succeed()) - Expect(newLockId).To(Equal("")) + Expect(response.Status).To(Equal(200)) + Expect(response.Headers).To(BeNil()) }) It("Unlock file isn't locked", func() { @@ -646,11 +635,10 @@ var _ = Describe("FileConnector", func() { Status: status.NewConflict(ctx, nil, "lock mismatch"), }, nil) - newLockId, err := fc.UnLock(ctx, "abcdef123") - Expect(err).To(HaveOccurred()) - conErr := err.(*connector.ConnectorError) - Expect(conErr.HttpCodeOut).To(Equal(409)) - Expect(newLockId).To(Equal("")) + response, err := fc.UnLock(ctx, "abcdef123") + Expect(err).To(Succeed()) + Expect(response.Status).To(Equal(409)) + Expect(response.Headers[connector.HeaderWopiLock]).To(Equal("")) }) It("Unlock mismatch get lock fails", func() { @@ -665,10 +653,10 @@ var _ = Describe("FileConnector", func() { Status: status.NewInternal(ctx, "something failed"), }, targetErr) - newLockId, err := fc.UnLock(ctx, "abcdef123") + response, err := fc.UnLock(ctx, "abcdef123") Expect(err).To(HaveOccurred()) Expect(err).To(Equal(targetErr)) - Expect(newLockId).To(Equal("")) + Expect(response).To(BeNil()) }) It("Unlock mismatch get lock status not ok", func() { @@ -682,11 +670,10 @@ var _ = Describe("FileConnector", func() { Status: status.NewInternal(ctx, "something failed"), }, nil) - newLockId, err := fc.UnLock(ctx, "abcdef123") - Expect(err).To(HaveOccurred()) - conErr := err.(*connector.ConnectorError) - Expect(conErr.HttpCodeOut).To(Equal(500)) - Expect(newLockId).To(Equal("")) + response, err := fc.UnLock(ctx, "abcdef123") + Expect(err).To(Succeed()) + Expect(response.Status).To(Equal(500)) + Expect(response.Headers).To(BeNil()) }) It("Unlock mismatch get lock doesn't return lock", func() { @@ -700,11 +687,10 @@ var _ = Describe("FileConnector", func() { Status: status.NewOK(ctx), }, nil) - newLockId, err := fc.UnLock(ctx, "abcdef123") - Expect(err).To(HaveOccurred()) - conErr := err.(*connector.ConnectorError) - Expect(conErr.HttpCodeOut).To(Equal(409)) - Expect(newLockId).To(Equal("")) + response, err := fc.UnLock(ctx, "abcdef123") + Expect(err).To(Succeed()) + Expect(response.Status).To(Equal(409)) + Expect(response.Headers[connector.HeaderWopiLock]).To(Equal("")) }) It("Unlock mismatch", func() { @@ -722,11 +708,10 @@ var _ = Describe("FileConnector", func() { }, }, nil) - newLockId, err := fc.UnLock(ctx, "abcdef123") - Expect(err).To(HaveOccurred()) - conErr := err.(*connector.ConnectorError) - Expect(conErr.HttpCodeOut).To(Equal(409)) - Expect(newLockId).To(Equal("zzz999")) + response, err := fc.UnLock(ctx, "abcdef123") + Expect(err).To(Succeed()) + Expect(response.Status).To(Equal(409)) + Expect(response.Headers[connector.HeaderWopiLock]).To(Equal("zzz999")) }) It("Default error handling (insufficient storage)", func() { @@ -736,11 +721,10 @@ var _ = Describe("FileConnector", func() { Status: status.NewInsufficientStorage(ctx, nil, "file too big"), }, nil) - newLockId, err := fc.UnLock(ctx, "abcdef123") - Expect(err).To(HaveOccurred()) - conErr := err.(*connector.ConnectorError) - Expect(conErr.HttpCodeOut).To(Equal(500)) - Expect(newLockId).To(Equal("")) + response, err := fc.UnLock(ctx, "abcdef123") + Expect(err).To(Succeed()) + Expect(response.Status).To(Equal(500)) + Expect(response.Headers).To(BeNil()) }) }) @@ -777,10 +761,10 @@ var _ = Describe("FileConnector", func() { stream := strings.NewReader("This is the content of a file") response, err := fc.PutRelativeFileSuggested(ctx, ccs, stream, int64(stream.Len()), "newFile.txt") - Expect(err).To(HaveOccurred()) - conErr := err.(*connector.ConnectorError) - Expect(conErr.HttpCodeOut).To(Equal(500)) - Expect(response).To(BeNil()) + Expect(err).To(Succeed()) + Expect(response.Status).To(Equal(500)) + Expect(response.Headers).To(BeNil()) + Expect(response.Body).To(BeNil()) }) It("PutRelativeFileSuggested success", func() { @@ -807,7 +791,7 @@ var _ = Describe("FileConnector", func() { }, }, nil) - ccs.On("PutFile", mock.Anything, stream, int64(stream.Len()), "").Times(1).Return("", nil) + ccs.On("PutFile", mock.Anything, stream, int64(stream.Len()), "").Times(1).Return(&connector.ConnectorResponse{Status: 200}, nil) stat2ParamMatcher := mock.MatchedBy(func(statReq *providerv1beta1.StatRequest) bool { if statReq.Ref.ResourceId.StorageId == "storageid" && @@ -832,8 +816,11 @@ var _ = Describe("FileConnector", func() { response, err := fc.PutRelativeFileSuggested(ctx, ccs, stream, int64(stream.Len()), "newDocument.docx") Expect(err).To(Succeed()) - Expect(response.Name).To(Equal("newDocument.docx")) - Expect(response.Url).To(HavePrefix("https://ocis.server.prv/wopi/files/")) // skip checking the actual reference + Expect(response.Status).To(Equal(200)) + Expect(response.Headers).To(BeNil()) + 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 }) It("PutRelativeFileSuggested success only extension", func() { @@ -860,7 +847,7 @@ var _ = Describe("FileConnector", func() { }, }, nil) - ccs.On("PutFile", mock.Anything, stream, int64(stream.Len()), "").Times(1).Return("", nil) + ccs.On("PutFile", mock.Anything, stream, int64(stream.Len()), "").Times(1).Return(&connector.ConnectorResponse{Status: 200}, nil) stat2ParamMatcher := mock.MatchedBy(func(statReq *providerv1beta1.StatRequest) bool { if statReq.Ref.ResourceId.StorageId == "storageid" && @@ -885,8 +872,11 @@ var _ = Describe("FileConnector", func() { response, err := fc.PutRelativeFileSuggested(ctx, ccs, stream, int64(stream.Len()), ".pdf") Expect(err).To(Succeed()) - Expect(response.Name).To(Equal("file.pdf")) - Expect(response.Url).To(HavePrefix("https://ocis.server.prv/wopi/files/")) // skip checking the actual reference + Expect(response.Status).To(Equal(200)) + Expect(response.Headers).To(BeNil()) + 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 }) It("PutRelativeFileSuggested success conflict", func() { @@ -916,9 +906,10 @@ var _ = Describe("FileConnector", func() { }, }, nil) - // first call will fail with conflict, second call succeeds - ccs.On("PutFile", mock.Anything, stream, int64(stream.Len()), "").Times(1).Return("", connector.NewConnectorError(409, "file conflict")).Once() - ccs.On("PutFile", mock.Anything, stream, int64(stream.Len()), "").Times(1).Return("", nil).Once() + // first call will fail with conflict, second call succeeds. + // we're only interested on whether the file is locked or not, the actual lockID is irrelevant + ccs.On("PutFile", mock.Anything, stream, int64(stream.Len()), "").Times(1).Return(&connector.ConnectorResponse{Status: 409}, nil).Once() + ccs.On("PutFile", mock.Anything, stream, int64(stream.Len()), "").Times(1).Return(&connector.ConnectorResponse{Status: 200}, nil).Once() newFilePath := new(string) stat2ParamMatcher := mock.MatchedBy(func(statReq *providerv1beta1.StatRequest) bool { @@ -950,8 +941,11 @@ var _ = Describe("FileConnector", func() { response, err := fc.PutRelativeFileSuggested(ctx, ccs, stream, int64(stream.Len()), ".pdf") Expect(err).To(Succeed()) - Expect(response.Name).To(MatchRegexp(`[a-zA-Z0-9_-] file\.pdf`)) - Expect(response.Url).To(HavePrefix("https://ocis.server.prv/wopi/files/")) // skip checking the actual reference + Expect(response.Status).To(Equal(200)) + Expect(response.Headers).To(BeNil()) + 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 }) It("PutRelativeFileSuggested put file fails", func() { @@ -977,11 +971,13 @@ var _ = Describe("FileConnector", func() { }, }, nil) - ccs.On("PutFile", mock.Anything, stream, int64(stream.Len()), "").Times(1).Return("", connector.NewConnectorError(500, "something bad happened")) + ccs.On("PutFile", mock.Anything, stream, int64(stream.Len()), "").Times(1).Return(&connector.ConnectorResponse{Status: 500}, nil) response, err := fc.PutRelativeFileSuggested(ctx, ccs, stream, int64(stream.Len()), ".pdf") - Expect(err).To(HaveOccurred()) - Expect(response).To(BeNil()) + Expect(err).To(Succeed()) + Expect(response.Status).To(Equal(500)) + Expect(response.Headers).To(BeNil()) + Expect(response.Body).To(BeNil()) }) }) @@ -989,10 +985,9 @@ var _ = Describe("FileConnector", func() { It("No valid context", func() { ctx := context.Background() stream := strings.NewReader("This is the content of a file") - response, headers, err := fc.PutRelativeFileRelative(ctx, ccs, stream, int64(stream.Len()), "newFile.txt") + response, err := fc.PutRelativeFileRelative(ctx, ccs, stream, int64(stream.Len()), "newFile.txt") Expect(err).To(HaveOccurred()) Expect(response).To(BeNil()) - Expect(headers).To(BeNil()) }) It("Stat fails", func() { @@ -1004,11 +999,10 @@ var _ = Describe("FileConnector", func() { }, targetErr) stream := strings.NewReader("This is the content of a file") - response, headers, err := fc.PutRelativeFileRelative(ctx, ccs, stream, int64(stream.Len()), "newFile.txt") + response, err := fc.PutRelativeFileRelative(ctx, ccs, stream, int64(stream.Len()), "newFile.txt") Expect(err).To(HaveOccurred()) Expect(err).To(Equal(targetErr)) Expect(response).To(BeNil()) - Expect(headers).To(BeNil()) }) It("Stat fails status not ok", func() { @@ -1019,12 +1013,11 @@ var _ = Describe("FileConnector", func() { }, nil) stream := strings.NewReader("This is the content of a file") - response, headers, err := fc.PutRelativeFileRelative(ctx, ccs, stream, int64(stream.Len()), "newFile.txt") - Expect(err).To(HaveOccurred()) - conErr := err.(*connector.ConnectorError) - Expect(conErr.HttpCodeOut).To(Equal(500)) - Expect(response).To(BeNil()) - Expect(headers).To(BeNil()) + response, err := fc.PutRelativeFileRelative(ctx, ccs, stream, int64(stream.Len()), "newFile.txt") + Expect(err).To(Succeed()) + Expect(response.Status).To(Equal(500)) + Expect(response.Headers).To(BeNil()) + Expect(response.Body).To(BeNil()) }) It("PutRelativeFileRelative success", func() { @@ -1050,7 +1043,7 @@ var _ = Describe("FileConnector", func() { }, }, nil) - ccs.On("PutFile", mock.Anything, stream, int64(stream.Len()), "").Times(1).Return("", nil) + ccs.On("PutFile", mock.Anything, stream, int64(stream.Len()), "").Times(1).Return(&connector.ConnectorResponse{Status: 200}, nil) stat2ParamMatcher := mock.MatchedBy(func(statReq *providerv1beta1.StatRequest) bool { if statReq.Ref.ResourceId.StorageId == "storageid" && @@ -1073,11 +1066,13 @@ var _ = Describe("FileConnector", func() { }, }, nil) - response, headers, err := fc.PutRelativeFileRelative(ctx, ccs, stream, int64(stream.Len()), "newDocument.docx") + response, err := fc.PutRelativeFileRelative(ctx, ccs, stream, int64(stream.Len()), "newDocument.docx") Expect(err).To(Succeed()) - Expect(response.Name).To(Equal("newDocument.docx")) - Expect(response.Url).To(HavePrefix("https://ocis.server.prv/wopi/files/")) // skip checking the actual reference - Expect(headers).To(BeNil()) + Expect(response.Status).To(Equal(200)) + Expect(response.Headers).To(BeNil()) + 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 }) It("PutRelativeFileRelative conflict", func() { @@ -1103,7 +1098,7 @@ var _ = Describe("FileConnector", func() { }, }, nil) - ccs.On("PutFile", mock.Anything, stream, int64(stream.Len()), "").Times(1).Return("zzz999", connector.NewConnectorError(409, "file conflict")) + ccs.On("PutFile", mock.Anything, stream, int64(stream.Len()), "").Times(1).Return(&connector.ConnectorResponse{Status: 409, Headers: map[string]string{connector.HeaderWopiLock: "zzz999"}}, nil) stat2ParamMatcher := mock.MatchedBy(func(statReq *providerv1beta1.StatRequest) bool { if statReq.Ref.ResourceId.StorageId == "storageid" && @@ -1130,12 +1125,14 @@ var _ = Describe("FileConnector", func() { }, }, nil) - response, headers, err := fc.PutRelativeFileRelative(ctx, ccs, stream, int64(stream.Len()), "convFile.pdf") - Expect(err).To(HaveOccurred()) - Expect(response.Name).To(Equal("convFile.pdf")) - Expect(response.Url).To(HavePrefix("https://ocis.server.prv/wopi/files/")) // skip checking the actual reference - Expect(headers.ValidTarget).To(MatchRegexp(`[a-zA-Z0-9_-] convFile\.pdf`)) - Expect(headers.LockID).To(Equal("zzz999")) + response, err := fc.PutRelativeFileRelative(ctx, ccs, stream, int64(stream.Len()), "convFile.pdf") + Expect(err).To(Succeed()) + Expect(response.Status).To(Equal(409)) + Expect(response.Headers[connector.HeaderWopiLock]).To(Equal("zzz999")) + Expect(response.Headers[connector.HeaderWopiValidRT]).To(MatchRegexp(`[a-zA-Z0-9_-] convFile\.pdf`)) + 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 }) It("PutRelativeFileRelative put file fails", func() { @@ -1161,21 +1158,22 @@ var _ = Describe("FileConnector", func() { }, }, nil) - ccs.On("PutFile", mock.Anything, stream, int64(stream.Len()), "").Times(1).Return("", connector.NewConnectorError(500, "something bad happened")) + ccs.On("PutFile", mock.Anything, stream, int64(stream.Len()), "").Times(1).Return(&connector.ConnectorResponse{Status: 500}, nil) - response, headers, err := fc.PutRelativeFileRelative(ctx, ccs, stream, int64(stream.Len()), "convFile.pdf") - Expect(err).To(HaveOccurred()) - Expect(response).To(BeNil()) - Expect(headers).To(BeNil()) + response, err := fc.PutRelativeFileRelative(ctx, ccs, stream, int64(stream.Len()), "convFile.pdf") + Expect(err).To(Succeed()) + Expect(response.Status).To(Equal(500)) + Expect(response.Headers).To(BeNil()) + Expect(response.Body).To(BeNil()) }) }) Describe("DeleteFile", func() { It("No valid context", func() { ctx := context.Background() - newLockId, err := fc.DeleteFile(ctx, "lock") + response, err := fc.DeleteFile(ctx, "lock") Expect(err).To(HaveOccurred()) - Expect(newLockId).To(Equal("")) + Expect(response).To(BeNil()) }) It("Delete fails", func() { @@ -1190,10 +1188,10 @@ var _ = Describe("FileConnector", func() { Status: status.NewInternal(ctx, "something failed"), }, targetErr) - lockID, err := fc.DeleteFile(ctx, "newlock") + response, err := fc.DeleteFile(ctx, "newlock") Expect(err).To(HaveOccurred()) Expect(err).To(Equal(targetErr)) - Expect(lockID).To(Equal("")) + Expect(response).To(BeNil()) }) It("Delete fails status not ok, get lock fails", func() { @@ -1208,10 +1206,10 @@ var _ = Describe("FileConnector", func() { Status: status.NewInternal(ctx, "something failed"), }, targetErr) - lockID, err := fc.DeleteFile(ctx, "newlock") + response, err := fc.DeleteFile(ctx, "newlock") Expect(err).To(HaveOccurred()) Expect(err).To(Equal(targetErr)) - Expect(lockID).To(Equal("")) + Expect(response).To(BeNil()) }) It("Delete fails file missing", func() { @@ -1226,11 +1224,10 @@ var _ = Describe("FileConnector", func() { Status: status.NewInternal(ctx, "something failed"), }, targetErr) - lockID, err := fc.DeleteFile(ctx, "newlock") - Expect(err).To(HaveOccurred()) - conErr := err.(*connector.ConnectorError) - Expect(conErr.HttpCodeOut).To(Equal(404)) - Expect(lockID).To(Equal("")) + response, err := fc.DeleteFile(ctx, "newlock") + Expect(err).To(Succeed()) + Expect(response.Status).To(Equal(404)) + Expect(response.Headers).To(BeNil()) }) It("Delete fails status not ok, get lock not ok", func() { @@ -1244,11 +1241,10 @@ var _ = Describe("FileConnector", func() { Status: status.NewInternal(ctx, "something failed"), }, nil) - lockID, err := fc.DeleteFile(ctx, "newlock") - Expect(err).To(HaveOccurred()) - conErr := err.(*connector.ConnectorError) - Expect(conErr.HttpCodeOut).To(Equal(500)) - Expect(lockID).To(Equal("")) + response, err := fc.DeleteFile(ctx, "newlock") + Expect(err).To(Succeed()) + Expect(response.Status).To(Equal(500)) + Expect(response.Headers).To(BeNil()) }) It("Delete fails, file locked", func() { @@ -1266,11 +1262,10 @@ var _ = Describe("FileConnector", func() { }, }, nil) - lockID, err := fc.DeleteFile(ctx, "newlock") - Expect(err).To(HaveOccurred()) - conErr := err.(*connector.ConnectorError) - Expect(conErr.HttpCodeOut).To(Equal(409)) - Expect(lockID).To(Equal("zzz999")) + response, err := fc.DeleteFile(ctx, "newlock") + Expect(err).To(Succeed()) + Expect(response.Status).To(Equal(409)) + Expect(response.Headers[connector.HeaderWopiLock]).To(Equal("zzz999")) }) It("Delete fails, file not locked", func() { @@ -1284,11 +1279,10 @@ var _ = Describe("FileConnector", func() { Status: status.NewOK(ctx), }, nil) - lockID, err := fc.DeleteFile(ctx, "newlock") - Expect(err).To(HaveOccurred()) - conErr := err.(*connector.ConnectorError) - Expect(conErr.HttpCodeOut).To(Equal(500)) - Expect(lockID).To(Equal("")) + response, err := fc.DeleteFile(ctx, "newlock") + Expect(err).To(Succeed()) + Expect(response.Status).To(Equal(500)) + Expect(response.Headers).To(BeNil()) }) It("Delete success", func() { @@ -1298,18 +1292,18 @@ var _ = Describe("FileConnector", func() { Status: status.NewOK(ctx), }, nil) - lockID, err := fc.DeleteFile(ctx, "newlock") + response, err := fc.DeleteFile(ctx, "newlock") Expect(err).To(Succeed()) - Expect(lockID).To(Equal("")) + Expect(response.Status).To(Equal(200)) + Expect(response.Headers).To(BeNil()) }) }) Describe("RenameFile", func() { It("No valid context", func() { ctx := context.Background() - response, lockID, err := fc.RenameFile(ctx, "lockid", "newFile.doc") + response, err := fc.RenameFile(ctx, "lockid", "newFile.doc") Expect(err).To(HaveOccurred()) - Expect(lockID).To(Equal("")) Expect(response).To(BeNil()) }) @@ -1321,10 +1315,9 @@ var _ = Describe("FileConnector", func() { Status: status.NewInternal(ctx, "something failed"), }, targetErr) - response, lockID, err := fc.RenameFile(ctx, "lockid", "newFile.doc") + response, err := fc.RenameFile(ctx, "lockid", "newFile.doc") Expect(err).To(HaveOccurred()) Expect(err).To(Equal(targetErr)) - Expect(lockID).To(Equal("")) Expect(response).To(BeNil()) }) @@ -1335,12 +1328,11 @@ var _ = Describe("FileConnector", func() { Status: status.NewInternal(ctx, "something failed"), }, nil) - response, lockID, err := fc.RenameFile(ctx, "lockid", "newFile.doc") - Expect(err).To(HaveOccurred()) - conErr := err.(*connector.ConnectorError) - Expect(conErr.HttpCodeOut).To(Equal(500)) - Expect(lockID).To(Equal("")) - Expect(response).To(BeNil()) + response, err := fc.RenameFile(ctx, "lockid", "newFile.doc") + Expect(err).To(Succeed()) + Expect(response.Status).To(Equal(500)) + Expect(response.Headers).To(BeNil()) + Expect(response.Body).To(BeNil()) }) It("Rename failed", func() { @@ -1361,10 +1353,9 @@ var _ = Describe("FileConnector", func() { Status: status.NewInternal(ctx, "something failed"), }, targetErr) - response, lockID, err := fc.RenameFile(ctx, "lockid", "newFile.doc") + response, err := fc.RenameFile(ctx, "lockid", "newFile.doc") Expect(err).To(HaveOccurred()) Expect(err).To(Equal(targetErr)) - Expect(lockID).To(Equal("")) Expect(response).To(BeNil()) }) @@ -1385,12 +1376,11 @@ var _ = Describe("FileConnector", func() { Status: status.NewInternal(ctx, "something failed"), }, nil) - response, lockID, err := fc.RenameFile(ctx, "lockid", "newFile.doc") - Expect(err).To(HaveOccurred()) - conErr := err.(*connector.ConnectorError) - Expect(conErr.HttpCodeOut).To(Equal(500)) - Expect(lockID).To(Equal("")) - Expect(response).To(BeNil()) + response, err := fc.RenameFile(ctx, "lockid", "newFile.doc") + Expect(err).To(Succeed()) + Expect(response.Status).To(Equal(500)) + Expect(response.Headers).To(BeNil()) + Expect(response.Body).To(BeNil()) }) It("Rename conflict", func() { @@ -1410,12 +1400,11 @@ var _ = Describe("FileConnector", func() { Status: status.NewLocked(ctx, "lock mismatch"), }, nil) - response, lockID, err := fc.RenameFile(ctx, "lockid", "newFile.doc") - Expect(err).To(HaveOccurred()) - conErr := err.(*connector.ConnectorError) - Expect(conErr.HttpCodeOut).To(Equal(409)) - Expect(lockID).To(Equal("zzz999")) - Expect(response).To(BeNil()) + response, err := fc.RenameFile(ctx, "lockid", "newFile.doc") + Expect(err).To(Succeed()) + Expect(response.Status).To(Equal(409)) + Expect(response.Headers[connector.HeaderWopiLock]).To(Equal("zzz999")) + Expect(response.Body).To(BeNil()) }) It("Rename already exists", func() { @@ -1454,10 +1443,12 @@ var _ = Describe("FileConnector", func() { Status: status.NewOK(ctx), }, nil).Once() - response, lockID, err := fc.RenameFile(ctx, "zzz999", "newFile.doc") + response, err := fc.RenameFile(ctx, "zzz999", "newFile.doc") Expect(err).To(Succeed()) - Expect(lockID).To(Equal("")) - Expect(response.Name).To(MatchRegexp(`^[a-zA-Z0-9_-]+ newFile$`)) + Expect(response.Status).To(Equal(200)) + Expect(response.Headers).To(BeNil()) + rBody := response.Body.(map[string]interface{}) + Expect(rBody["Name"]).To(MatchRegexp(`^[a-zA-Z0-9_-]+ newFile$`)) }) It("Success", func() { @@ -1484,19 +1475,21 @@ var _ = Describe("FileConnector", func() { Status: status.NewOK(ctx), }, nil).Once() - response, lockID, err := fc.RenameFile(ctx, "zzz999", "newFile.doc") + response, err := fc.RenameFile(ctx, "zzz999", "newFile.doc") Expect(err).To(Succeed()) - Expect(lockID).To(Equal("")) - Expect(response.Name).To(Equal("newFile")) + Expect(response.Status).To(Equal(200)) + Expect(response.Headers).To(BeNil()) + rBody := response.Body.(map[string]interface{}) + Expect(rBody["Name"]).To(Equal("newFile")) }) }) Describe("CheckFileInfo", func() { It("No valid context", func() { ctx := context.Background() - newFileInfo, err := fc.CheckFileInfo(ctx) + response, err := fc.CheckFileInfo(ctx) Expect(err).To(HaveOccurred()) - Expect(newFileInfo).To(BeNil()) + Expect(response).To(BeNil()) }) It("Stat fails", func() { @@ -1507,10 +1500,10 @@ var _ = Describe("FileConnector", func() { Status: status.NewInternal(ctx, "something failed"), }, targetErr) - newFileInfo, err := fc.CheckFileInfo(ctx) + response, err := fc.CheckFileInfo(ctx) Expect(err).To(HaveOccurred()) Expect(err).To(Equal(targetErr)) - Expect(newFileInfo).To(BeNil()) + Expect(response).To(BeNil()) }) It("Stat fails status not ok", func() { @@ -1520,11 +1513,10 @@ var _ = Describe("FileConnector", func() { Status: status.NewInternal(ctx, "something failed"), }, nil) - newFileInfo, err := fc.CheckFileInfo(ctx) - Expect(err).To(HaveOccurred()) - conErr := err.(*connector.ConnectorError) - Expect(conErr.HttpCodeOut).To(Equal(500)) - Expect(newFileInfo).To(BeNil()) + response, err := fc.CheckFileInfo(ctx) + Expect(err).To(Succeed()) + Expect(response.Status).To(Equal(500)) + Expect(response.Body).To(BeNil()) }) It("Stat success", func() { @@ -1568,9 +1560,10 @@ var _ = Describe("FileConnector", func() { UserFriendlyName: "Pet Shaft", } - newFileInfo, err := fc.CheckFileInfo(ctx) + response, err := fc.CheckFileInfo(ctx) Expect(err).To(Succeed()) - Expect(newFileInfo.(*fileinfo.Microsoft)).To(Equal(expectedFileInfo)) + Expect(response.Status).To(Equal(200)) + Expect(response.Body.(*fileinfo.Microsoft)).To(Equal(expectedFileInfo)) }) It("Stat success guests", func() { @@ -1626,18 +1619,19 @@ var _ = Describe("FileConnector", func() { PostMessageOrigin: "https://ocis.example.prv", } - newFileInfo, err := fc.CheckFileInfo(ctx) + response, err := fc.CheckFileInfo(ctx) // UserID and UserFriendlyName have random Ids generated which are impossible to guess // Check both separately - Expect(newFileInfo.(*fileinfo.Collabora).UserID).To(HavePrefix(hex.EncodeToString([]byte("guest-")))) - Expect(newFileInfo.(*fileinfo.Collabora).UserFriendlyName).To(HavePrefix("Guest ")) + Expect(response.Body.(*fileinfo.Collabora).UserID).To(HavePrefix(hex.EncodeToString([]byte("guest-")))) + Expect(response.Body.(*fileinfo.Collabora).UserFriendlyName).To(HavePrefix("Guest ")) // overwrite UserID and UserFriendlyName here for easier matching - newFileInfo.(*fileinfo.Collabora).UserID = "guest-zzz000" - newFileInfo.(*fileinfo.Collabora).UserFriendlyName = "guest zzz000" + response.Body.(*fileinfo.Collabora).UserID = "guest-zzz000" + response.Body.(*fileinfo.Collabora).UserFriendlyName = "guest zzz000" Expect(err).To(Succeed()) - Expect(newFileInfo.(*fileinfo.Collabora)).To(Equal(expectedFileInfo)) + Expect(response.Status).To(Equal(200)) + Expect(response.Body.(*fileinfo.Collabora)).To(Equal(expectedFileInfo)) }) It("Stat success authenticated user", func() { @@ -1685,10 +1679,11 @@ var _ = Describe("FileConnector", func() { PostMessageOrigin: "https://ocis.example.prv", } - newFileInfo, err := fc.CheckFileInfo(ctx) + response, err := fc.CheckFileInfo(ctx) Expect(err).To(Succeed()) - Expect(newFileInfo.(*fileinfo.Collabora)).To(Equal(expectedFileInfo)) + Expect(response.Status).To(Equal(200)) + Expect(response.Body.(*fileinfo.Collabora)).To(Equal(expectedFileInfo)) }) }) }) diff --git a/services/collaboration/pkg/connector/httpadapter_test.go b/services/collaboration/pkg/connector/httpadapter_test.go index 390740e51..59d9f6fd5 100644 --- a/services/collaboration/pkg/connector/httpadapter_test.go +++ b/services/collaboration/pkg/connector/httpadapter_test.go @@ -47,7 +47,7 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("GetLock", mock.Anything).Times(1).Return("", errors.New("Something happened")) + fc.On("GetLock", mock.Anything).Times(1).Return(nil, errors.New("Something happened")) httpAdapter.GetLock(w, req) resp := w.Result() @@ -60,7 +60,7 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("GetLock", mock.Anything).Times(1).Return("", connector.NewConnectorError(404, "Couldn't get the file")) + fc.On("GetLock", mock.Anything).Times(1).Return(&connector.ConnectorResponse{Status: 404}, nil) httpAdapter.GetLock(w, req) resp := w.Result() @@ -73,7 +73,7 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("GetLock", mock.Anything).Times(1).Return("zzz111", nil) + fc.On("GetLock", mock.Anything).Times(1).Return(&connector.ConnectorResponse{Status: 200, Headers: map[string]string{connector.HeaderWopiLock: "zzz111"}}, nil) httpAdapter.GetLock(w, req) resp := w.Result() @@ -87,7 +87,7 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("GetLock", mock.Anything).Times(1).Return("", nil) + fc.On("GetLock", mock.Anything).Times(1).Return(&connector.ConnectorResponse{Status: 200, Headers: map[string]string{connector.HeaderWopiLock: ""}}, nil) httpAdapter.GetLock(w, req) resp := w.Result() @@ -105,7 +105,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(nil, errors.New("Something happened")) httpAdapter.Lock(w, req) resp := w.Result() @@ -119,7 +119,7 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("Lock", mock.Anything, "", "").Times(1).Return("", connector.NewConnectorError(400, "No lockId")) + fc.On("Lock", mock.Anything, "", "").Times(1).Return(&connector.ConnectorResponse{Status: 400}, nil) httpAdapter.Lock(w, req) resp := w.Result() @@ -133,7 +133,7 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("Lock", mock.Anything, "abc123", "").Times(1).Return("zzz111", connector.NewConnectorError(409, "Lock conflict")) + fc.On("Lock", mock.Anything, "abc123", "").Times(1).Return(&connector.ConnectorResponse{Status: 409, Headers: map[string]string{connector.HeaderWopiLock: "zzz111"}}, nil) httpAdapter.Lock(w, req) resp := w.Result() @@ -148,7 +148,7 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("Lock", mock.Anything, "abc123", "").Times(1).Return("", nil) + fc.On("Lock", mock.Anything, "abc123", "").Times(1).Return(&connector.ConnectorResponse{Status: 200}, nil) httpAdapter.Lock(w, req) resp := w.Result() @@ -165,7 +165,7 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("Lock", mock.Anything, "abc123", "qwerty").Times(1).Return("", errors.New("Something happened")) + fc.On("Lock", mock.Anything, "abc123", "qwerty").Times(1).Return(nil, errors.New("Something happened")) httpAdapter.Lock(w, req) resp := w.Result() @@ -180,7 +180,7 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("Lock", mock.Anything, "", "").Times(1).Return("", connector.NewConnectorError(400, "No lockId")) + fc.On("Lock", mock.Anything, "", "").Times(1).Return(&connector.ConnectorResponse{Status: 400}, nil) httpAdapter.Lock(w, req) resp := w.Result() @@ -195,7 +195,7 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("Lock", mock.Anything, "abc123", "qwerty").Times(1).Return("zzz111", connector.NewConnectorError(409, "Lock conflict")) + fc.On("Lock", mock.Anything, "abc123", "qwerty").Times(1).Return(&connector.ConnectorResponse{Status: 409, Headers: map[string]string{connector.HeaderWopiLock: "zzz111"}}, nil) httpAdapter.Lock(w, req) resp := w.Result() @@ -211,7 +211,7 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("Lock", mock.Anything, "abc123", "qwerty").Times(1).Return("", nil) + fc.On("Lock", mock.Anything, "abc123", "qwerty").Times(1).Return(&connector.ConnectorResponse{Status: 200}, nil) httpAdapter.Lock(w, req) resp := w.Result() @@ -228,7 +228,7 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("RefreshLock", mock.Anything, "abc123").Times(1).Return("", errors.New("Something happened")) + fc.On("RefreshLock", mock.Anything, "abc123").Times(1).Return(nil, errors.New("Something happened")) httpAdapter.RefreshLock(w, req) resp := w.Result() @@ -242,7 +242,7 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("RefreshLock", mock.Anything, "").Times(1).Return("", connector.NewConnectorError(400, "No lockId")) + fc.On("RefreshLock", mock.Anything, "").Times(1).Return(&connector.ConnectorResponse{Status: 400}, nil) httpAdapter.RefreshLock(w, req) resp := w.Result() @@ -256,7 +256,7 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("RefreshLock", mock.Anything, "abc123").Times(1).Return("zzz111", connector.NewConnectorError(409, "Lock conflict")) + fc.On("RefreshLock", mock.Anything, "abc123").Times(1).Return(&connector.ConnectorResponse{Status: 409, Headers: map[string]string{connector.HeaderWopiLock: "zzz111"}}, nil) httpAdapter.RefreshLock(w, req) resp := w.Result() @@ -271,7 +271,7 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("RefreshLock", mock.Anything, "abc123").Times(1).Return("", nil) + fc.On("RefreshLock", mock.Anything, "abc123").Times(1).Return(&connector.ConnectorResponse{Status: 200}, nil) httpAdapter.RefreshLock(w, req) resp := w.Result() @@ -287,7 +287,7 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("UnLock", mock.Anything, "abc123").Times(1).Return("", errors.New("Something happened")) + fc.On("UnLock", mock.Anything, "abc123").Times(1).Return(nil, errors.New("Something happened")) httpAdapter.UnLock(w, req) resp := w.Result() @@ -301,7 +301,7 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("UnLock", mock.Anything, "").Times(1).Return("", connector.NewConnectorError(400, "No lockId")) + fc.On("UnLock", mock.Anything, "").Times(1).Return(&connector.ConnectorResponse{Status: 400}, nil) httpAdapter.UnLock(w, req) resp := w.Result() @@ -315,7 +315,7 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("UnLock", mock.Anything, "abc123").Times(1).Return("zzz111", connector.NewConnectorError(409, "Lock conflict")) + fc.On("UnLock", mock.Anything, "abc123").Times(1).Return(&connector.ConnectorResponse{Status: 409, Headers: map[string]string{connector.HeaderWopiLock: "zzz111"}}, nil) httpAdapter.UnLock(w, req) resp := w.Result() @@ -330,7 +330,7 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("UnLock", mock.Anything, "abc123").Times(1).Return("", nil) + fc.On("UnLock", mock.Anything, "abc123").Times(1).Return(&connector.ConnectorResponse{Status: 200}, nil) httpAdapter.UnLock(w, req) resp := w.Result() @@ -344,7 +344,7 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("CheckFileInfo", mock.Anything).Times(1).Return(&fileinfo.Microsoft{}, errors.New("Something happened")) + fc.On("CheckFileInfo", mock.Anything).Times(1).Return(nil, errors.New("Something happened")) httpAdapter.CheckFileInfo(w, req) resp := w.Result() @@ -358,7 +358,7 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("CheckFileInfo", mock.Anything).Times(1).Return(&fileinfo.Microsoft{}, connector.NewConnectorError(404, "Not found")) + fc.On("CheckFileInfo", mock.Anything).Times(1).Return(&connector.ConnectorResponse{Status: 404}, nil) httpAdapter.CheckFileInfo(w, req) resp := w.Result() @@ -375,7 +375,7 @@ var _ = Describe("HttpAdapter", func() { Size: 123456789, BreadcrumbDocName: "testy.docx", } - fc.On("CheckFileInfo", mock.Anything).Times(1).Return(finfo, nil) + fc.On("CheckFileInfo", mock.Anything).Times(1).Return(&connector.ConnectorResponse{Status: 200, Body: finfo}, nil) httpAdapter.CheckFileInfo(w, req) resp := w.Result() @@ -444,7 +444,7 @@ 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.On("PutFile", mock.Anything, mock.Anything, int64(len(contentBody)), "abc123").Times(1).Return(nil, errors.New("Something happened")) httpAdapter.PutFile(w, req) resp := w.Result() @@ -458,7 +458,7 @@ 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.On("PutFile", mock.Anything, mock.Anything, int64(len(contentBody)), "abc123").Times(1).Return(&connector.ConnectorResponse{Status: 409, Headers: map[string]string{connector.HeaderWopiLock: "zzz111"}}, nil) httpAdapter.PutFile(w, req) resp := w.Result() @@ -473,7 +473,7 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - cc.On("PutFile", mock.Anything, mock.Anything, int64(len(contentBody)), "abc123").Times(1).Return("", nil) + cc.On("PutFile", mock.Anything, mock.Anything, int64(len(contentBody)), "abc123").Times(1).Return(&connector.ConnectorResponse{Status: 200}, nil) httpAdapter.PutFile(w, req) resp := w.Result() From 2d3de811176e11f71ef60e8dd480b9a5c1c8d94f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Thu, 8 Aug 2024 15:35:41 +0200 Subject: [PATCH 4/6] refactor: change test names --- services/collaboration/pkg/connector/connector_suite_test.go | 2 +- services/collaboration/pkg/helpers/helpers_suite_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/services/collaboration/pkg/connector/connector_suite_test.go b/services/collaboration/pkg/connector/connector_suite_test.go index 0781160de..db5363e10 100644 --- a/services/collaboration/pkg/connector/connector_suite_test.go +++ b/services/collaboration/pkg/connector/connector_suite_test.go @@ -7,7 +7,7 @@ import ( . "github.com/onsi/gomega" ) -func TestGraph(t *testing.T) { +func TestConnector(t *testing.T) { RegisterFailHandler(Fail) RunSpecs(t, "Connector Suite") } diff --git a/services/collaboration/pkg/helpers/helpers_suite_test.go b/services/collaboration/pkg/helpers/helpers_suite_test.go index b511d28a0..5a3d46aa5 100644 --- a/services/collaboration/pkg/helpers/helpers_suite_test.go +++ b/services/collaboration/pkg/helpers/helpers_suite_test.go @@ -7,7 +7,7 @@ import ( . "github.com/onsi/gomega" ) -func TestGraph(t *testing.T) { +func TestHelpers(t *testing.T) { RegisterFailHandler(Fail) RunSpecs(t, "Helpers Suite") } From 82a6a27d2227cb8a862e1083361c7ae84a51971f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Fri, 9 Aug 2024 13:33:23 +0200 Subject: [PATCH 5/6] refactor: simplify ConnectorResponse creation --- .../collaboration/pkg/connector/connector.go | 62 ++++++++ .../pkg/connector/contentconnector.go | 24 +-- .../pkg/connector/fileconnector.go | 143 +++++------------- .../pkg/connector/fileconnector_test.go | 16 +- .../pkg/connector/httpadapter_test.go | 38 ++--- 5 files changed, 138 insertions(+), 145 deletions(-) diff --git a/services/collaboration/pkg/connector/connector.go b/services/collaboration/pkg/connector/connector.go index 200c13105..8cccc7dfb 100644 --- a/services/collaboration/pkg/connector/connector.go +++ b/services/collaboration/pkg/connector/connector.go @@ -12,6 +12,68 @@ type ConnectorResponse struct { Body interface{} } +// NewResponse creates a new ConnectorResponse with just the specified status. +// Headers and Body will be nil +func NewResponse(status int) *ConnectorResponse { + return &ConnectorResponse{Status: status} +} + +// NewResponse creates a new ConnectorResponse with the specified status +// and the "X-WOPI-Lock" header having the value in the lockID parameter. +// +// This is usually used for conflict responses where the current lock id needs +// to be returned, although the `GetLock` method also uses this method for a +// successful response (with the lock id included) +func NewResponseWithLock(status int, lockID string) *ConnectorResponse { + return &ConnectorResponse{ + Status: status, + Headers: map[string]string{ + HeaderWopiLock: lockID, + }, + } +} + +// NewResponseSuccessBody creates a new ConnectorResponse with a fixed 200 +// (success) status and the specified body. The headers will be nil. +// +// This is used for the `CheckFileInfo` method in order to return the fileinfo +func NewResponseSuccessBody(body interface{}) *ConnectorResponse { + return &ConnectorResponse{ + Status: 200, + Body: body, + } +} + +// NewResponseSuccessBodyName creates a new ConnectorResponse with a fixed 200 +// (success) status and a "map[string]interface{}" body. The body will contain +// a "Name" key with the supplied name as value. +// +// This is used for the `RenameFile` method in order to return the final name +// of the renamed file if the operation is successful +func NewResponseSuccessBodyName(name string) *ConnectorResponse { + return &ConnectorResponse{ + Status: 200, + Body: map[string]interface{}{ + "Name": name, + }, + } +} + +// NewResponseSuccessBodyNameUrl creates a new ConnectorResponse with a fixed +// 200 (success) status and a "map[string]interface{}" body. The body will +// 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 { + return &ConnectorResponse{ + Status: 200, + Body: map[string]interface{}{ + "Name": name, + "Url": url, + }, + } +} + // ConnectorError defines an error in the connector. It contains an error code // and a message. // For convenience, the error code can be used as HTTP error code, although diff --git a/services/collaboration/pkg/connector/contentconnector.go b/services/collaboration/pkg/connector/contentconnector.go index d816f4029..9812b11c9 100644 --- a/services/collaboration/pkg/connector/contentconnector.go +++ b/services/collaboration/pkg/connector/contentconnector.go @@ -223,7 +223,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 &ConnectorResponse{Status: 500}, nil + return NewResponse(500), nil } // If there is a lock and it mismatches, return 409 @@ -232,12 +232,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 &ConnectorResponse{ - Status: 409, - Headers: map[string]string{ - HeaderWopiLock: statRes.GetInfo().GetLock().GetLockId(), - }, - }, nil + return NewResponseWithLock(409, statRes.GetInfo().GetLock().GetLockId()), nil } // only unlocked uploads can go through if the target file is empty, @@ -247,12 +242,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 &ConnectorResponse{ - Status: 409, - Headers: map[string]string{ - HeaderWopiLock: "", - }, - }, nil + return NewResponseWithLock(409, ""), nil } // Prepare the data to initiate the upload @@ -288,7 +278,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 &ConnectorResponse{Status: 500}, nil + return NewResponse(500), nil } // if the content length is greater than 0, we need to upload the content to the @@ -313,7 +303,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 &ConnectorResponse{Status: 500}, nil + return NewResponse(500), nil } httpClient := http.Client{ @@ -369,10 +359,10 @@ 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 &ConnectorResponse{Status: 500}, nil + return NewResponse(500), nil } } logger.Debug().Msg("PutFile: success") - return &ConnectorResponse{Status: 200}, nil + return NewResponse(200), nil } diff --git a/services/collaboration/pkg/connector/fileconnector.go b/services/collaboration/pkg/connector/fileconnector.go index df77254d3..316e3d136 100644 --- a/services/collaboration/pkg/connector/fileconnector.go +++ b/services/collaboration/pkg/connector/fileconnector.go @@ -139,7 +139,7 @@ func (f *FileConnector) GetLock(ctx context.Context) (*ConnectorResponse, error) Str("StatusMsg", resp.GetStatus().GetMessage()). Msg("GetLock failed with unexpected status") // TODO: Should we be more strict? There could be more causes for the failure - return &ConnectorResponse{Status: 404}, nil + return NewResponse(404), nil } lockID := "" @@ -152,12 +152,7 @@ func (f *FileConnector) GetLock(ctx context.Context) (*ConnectorResponse, error) Str("LockID", lockID). Msg("GetLock success") - return &ConnectorResponse{ - Status: 200, - Headers: map[string]string{ - HeaderWopiLock: lockID, - }, - }, nil + return NewResponseWithLock(200, lockID), nil } // Lock returns a WOPI lock or performs an unlock and relock @@ -192,7 +187,7 @@ func (f *FileConnector) Lock(ctx context.Context, lockID, oldLockID string) (*Co if lockID == "" { logger.Error().Msg("Lock failed due to empty lockID") - return &ConnectorResponse{Status: 400}, nil + return NewResponse(400), nil } var setOrRefreshStatus *rpcv1beta1.Status @@ -246,7 +241,7 @@ func (f *FileConnector) Lock(ctx context.Context, lockID, oldLockID string) (*Co switch setOrRefreshStatus.GetCode() { case rpcv1beta1.Code_CODE_OK: logger.Debug().Msg("SetLock successful") - return &ConnectorResponse{Status: 200}, nil + return NewResponse(200), nil case rpcv1beta1.Code_CODE_FAILED_PRECONDITION, rpcv1beta1.Code_CODE_ABORTED: // Code_CODE_FAILED_PRECONDITION -> Lock operation mismatched lock @@ -276,12 +271,7 @@ func (f *FileConnector) Lock(ctx context.Context, lockID, oldLockID string) (*Co logger.Warn(). Str("LockID", resp.GetLock().GetLockId()). Msg("SetLock conflict") - return &ConnectorResponse{ - Status: 409, - Headers: map[string]string{ - HeaderWopiLock: resp.GetLock().GetLockId(), - }, - }, nil + return NewResponseWithLock(409, resp.GetLock().GetLockId()), nil } // TODO: according to the spec we need to treat this as a RefreshLock @@ -292,22 +282,22 @@ func (f *FileConnector) Lock(ctx context.Context, lockID, oldLockID string) (*Co logger.Warn(). Str("LockID", resp.GetLock().GetLockId()). Msg("SetLock lock refreshed instead") - return &ConnectorResponse{Status: 200}, nil // no need to send the lockID for a 200 code + return NewResponse(200), nil // no need to send the lockID for a 200 code } logger.Error().Msg("SetLock failed and could not refresh") - return &ConnectorResponse{Status: 500}, nil + return NewResponse(500), nil case rpcv1beta1.Code_CODE_NOT_FOUND: logger.Error().Msg("SetLock failed, file not found") - return &ConnectorResponse{Status: 404}, nil + return NewResponse(404), nil default: logger.Error(). Str("StatusCode", setOrRefreshStatus.GetCode().String()). Str("StatusMsg", setOrRefreshStatus.GetMessage()). Msg("SetLock failed with unexpected status") - return &ConnectorResponse{Status: 500}, nil + return NewResponse(500), nil } } @@ -337,7 +327,7 @@ func (f *FileConnector) RefreshLock(ctx context.Context, lockID string) (*Connec if lockID == "" { logger.Error().Msg("RefreshLock failed due to empty lockID") - return &ConnectorResponse{Status: 400}, nil + return NewResponse(400), nil } req := &providerv1beta1.RefreshLockRequest{ @@ -361,14 +351,14 @@ func (f *FileConnector) RefreshLock(ctx context.Context, lockID string) (*Connec switch resp.GetStatus().GetCode() { case rpcv1beta1.Code_CODE_OK: logger.Debug().Msg("RefreshLock successful") - return &ConnectorResponse{Status: 200}, nil + return NewResponse(200), nil case rpcv1beta1.Code_CODE_NOT_FOUND: logger.Error(). Str("StatusCode", resp.GetStatus().GetCode().String()). Str("StatusMsg", resp.GetStatus().GetMessage()). Msg("RefreshLock failed, file reference not found") - return &ConnectorResponse{Status: 404}, nil + return NewResponse(404), nil case rpcv1beta1.Code_CODE_ABORTED: logger.Error(). @@ -393,7 +383,7 @@ func (f *FileConnector) RefreshLock(ctx context.Context, lockID string) (*Connec Str("StatusCode", resp.GetStatus().GetCode().String()). Str("StatusMsg", resp.GetStatus().GetMessage()). Msg("RefreshLock failed, tried to get the current lock failed with unexpected status") - return &ConnectorResponse{Status: 500}, nil + return NewResponse(500), nil } if resp.GetLock() == nil { @@ -401,12 +391,7 @@ func (f *FileConnector) RefreshLock(ctx context.Context, lockID string) (*Connec Str("StatusCode", resp.GetStatus().GetCode().String()). Str("StatusMsg", resp.GetStatus().GetMessage()). Msg("RefreshLock failed, no lock on file") - return &ConnectorResponse{ - Status: 409, - Headers: map[string]string{ - HeaderWopiLock: "", - }, - }, nil + return NewResponseWithLock(409, ""), nil } else { // lock is different than the one requested, otherwise we wouldn't reached this point logger.Error(). @@ -414,19 +399,14 @@ func (f *FileConnector) RefreshLock(ctx context.Context, lockID string) (*Connec Str("StatusCode", resp.GetStatus().GetCode().String()). Str("StatusMsg", resp.GetStatus().GetMessage()). Msg("RefreshLock failed, lock mismatch") - return &ConnectorResponse{ - Status: 409, - Headers: map[string]string{ - HeaderWopiLock: resp.GetLock().GetLockId(), - }, - }, nil + return NewResponseWithLock(409, resp.GetLock().GetLockId()), nil } default: logger.Error(). Str("StatusCode", resp.GetStatus().GetCode().String()). Str("StatusMsg", resp.GetStatus().GetMessage()). Msg("RefreshLock failed with unexpected status") - return &ConnectorResponse{Status: 500}, nil + return NewResponse(500), nil } } @@ -456,7 +436,7 @@ func (f *FileConnector) UnLock(ctx context.Context, lockID string) (*ConnectorRe if lockID == "" { logger.Error().Msg("Unlock failed due to empty lockID") - return &ConnectorResponse{Status: 400}, nil + return NewResponse(400), nil } req := &providerv1beta1.UnlockRequest{ @@ -476,16 +456,11 @@ func (f *FileConnector) UnLock(ctx context.Context, lockID string) (*ConnectorRe switch resp.GetStatus().GetCode() { case rpcv1beta1.Code_CODE_OK: logger.Debug().Msg("Unlock successful") - return &ConnectorResponse{Status: 200}, nil + return NewResponse(200), nil case rpcv1beta1.Code_CODE_ABORTED: // File isn't locked. Need to return 409 with empty lock logger.Error().Err(err).Msg("Unlock failed, file isn't locked") - return &ConnectorResponse{ - Status: 409, - Headers: map[string]string{ - HeaderWopiLock: "", - }, - }, nil + return NewResponseWithLock(409, ""), nil case rpcv1beta1.Code_CODE_LOCKED: // We need to return 409 with the current lock req := &providerv1beta1.GetLockRequest{ @@ -503,7 +478,7 @@ func (f *FileConnector) UnLock(ctx context.Context, lockID string) (*ConnectorRe Str("StatusCode", resp.GetStatus().GetCode().String()). Str("StatusMsg", resp.GetStatus().GetMessage()). Msg("Unlock failed, tried to get the current lock failed with unexpected status") - return &ConnectorResponse{Status: 500}, nil + return NewResponse(500), nil } var outLockId string @@ -522,18 +497,13 @@ func (f *FileConnector) UnLock(ctx context.Context, lockID string) (*ConnectorRe Msg("Unlock failed, lock mismatch") outLockId = resp.GetLock().GetLockId() } - return &ConnectorResponse{ - Status: 409, - Headers: map[string]string{ - HeaderWopiLock: outLockId, - }, - }, nil + return NewResponseWithLock(409, outLockId), nil default: logger.Error(). Str("StatusCode", resp.GetStatus().GetCode().String()). Str("StatusMsg", resp.GetStatus().GetMessage()). Msg("Unlock failed with unexpected status") - return &ConnectorResponse{Status: 500}, nil + return NewResponse(500), nil } } @@ -588,7 +558,7 @@ func (f *FileConnector) PutRelativeFileSuggested(ctx context.Context, ccs Conten Str("StatusCode", oldStatRes.GetStatus().GetCode().String()). Str("StatusMsg", oldStatRes.GetStatus().GetMessage()). Msg("PutRelativeFileSuggested: stat failed with unexpected status") - return &ConnectorResponse{Status: 500}, nil + return NewResponse(500), nil } if strings.HasPrefix(target, ".") { @@ -634,7 +604,7 @@ func (f *FileConnector) PutRelativeFileSuggested(ctx context.Context, ccs Conten // TODO: code 400 might happen, what to do? // in other cases, just return the error newLogger.Error().Msg("PutRelativeFileSuggested: put file failed with unhandled status") - return &ConnectorResponse{Status: 500}, nil + return NewResponse(500), nil } } @@ -653,13 +623,7 @@ func (f *FileConnector) PutRelativeFileSuggested(ctx context.Context, ccs Conten Str("FinalReference", wopiContext.FileReference.String()). Msg("PutRelativeFileSuggested: success") - return &ConnectorResponse{ - Status: 200, - Body: map[string]interface{}{ - "Name": finalTarget, - "Url": wopiSrcURL.String(), - }, - }, nil + return NewResponseSuccessBodyNameUrl(finalTarget, wopiSrcURL.String()), nil } // PutRelativeFileRelative upload a file using the provided target name @@ -711,7 +675,7 @@ func (f *FileConnector) PutRelativeFileRelative(ctx context.Context, ccs Content Str("StatusCode", oldStatRes.GetStatus().GetCode().String()). Str("StatusMsg", oldStatRes.GetStatus().GetMessage()). Msg("PutRelativeFileRelative: stat failed with unexpected status") - return &ConnectorResponse{Status: 500}, nil + return NewResponse(500), nil } targetPath := utils.MakeRelativePath(target) @@ -761,6 +725,7 @@ func (f *FileConnector) PutRelativeFileRelative(ctx context.Context, ccs Content Str("LockID", lockID). Msg("PutRelativeFileRelative: error conflict") + // need to build the response ourselves return &ConnectorResponse{ Status: 409, Headers: map[string]string{ @@ -776,7 +741,7 @@ func (f *FileConnector) PutRelativeFileRelative(ctx context.Context, ccs Content newLogger.Error(). Str("LockID", lockID). Msg("PutRelativeFileRelative: put file failed with unhandled status") - return &ConnectorResponse{Status: 500}, nil + return NewResponse(500), nil } if err := f.adjustWopiReference(ctx, &wopiContext, newLogger); err != nil { @@ -791,13 +756,7 @@ func (f *FileConnector) PutRelativeFileRelative(ctx context.Context, ccs Content newLogger.Debug().Msg("PutRelativeFileRelative: success") - return &ConnectorResponse{ - Status: 200, - Body: map[string]interface{}{ - "Name": target, - "Url": wopiSrcURL.String(), - }, - }, nil + return NewResponseSuccessBodyNameUrl(target, wopiSrcURL.String()), nil } // DeleteFile will delete the requested file @@ -865,7 +824,7 @@ func (f *FileConnector) DeleteFile(ctx context.Context, lockID string) (*Connect if deleteRes.GetStatus().GetCode() == rpcv1beta1.Code_CODE_NOT_FOUND { // don't bother to check for locks of a missing file logger.Error().Msg("DeleteFile: tried to delete a missing file") - return &ConnectorResponse{Status: 404}, nil + return NewResponse(404), nil } // check if the file is locked to return a proper lockID @@ -884,27 +843,22 @@ func (f *FileConnector) DeleteFile(ctx context.Context, lockID string) (*Connect Str("StatusCode", resp.GetStatus().GetCode().String()). Str("StatusMsg", resp.GetStatus().GetMessage()). Msg("DeleteFile: GetLock failed with unexpected status") - return &ConnectorResponse{Status: 500}, nil + return NewResponse(500), nil } if resp.GetLock() != nil { logger.Error(). Str("LockID", resp.GetLock().GetLockId()). Msg("DeleteFile: file is locked") - return &ConnectorResponse{ - Status: 409, - Headers: map[string]string{ - HeaderWopiLock: resp.GetLock().GetLockId(), - }, - }, nil + return NewResponseWithLock(409, resp.GetLock().GetLockId()), nil } else { // return the original error since the file isn't locked logger.Error().Msg("DeleteFile: delete failed on unlocked file") - return &ConnectorResponse{Status: 500}, nil + return NewResponse(500), nil } } logger.Debug().Msg("DeleteFile: success") - return &ConnectorResponse{Status: 200}, nil + return NewResponse(200), nil } // RenameFile will rename the requested file @@ -944,13 +898,13 @@ func (f *FileConnector) RenameFile(ctx context.Context, lockID, target string) ( if oldStatRes.GetStatus().GetCode() != rpcv1beta1.Code_CODE_OK { if oldStatRes.GetStatus().GetCode() == rpcv1beta1.Code_CODE_NOT_FOUND { logger.Error().Msg("RenameFile: file not found") - return &ConnectorResponse{Status: 404}, nil + return NewResponse(404), nil } else { logger.Error(). Str("StatusCode", oldStatRes.GetStatus().GetCode().String()). Str("StatusMsg", oldStatRes.GetStatus().GetMessage()). Msg("RenameFile: stat failed with unexpected status") - return &ConnectorResponse{Status: 500}, nil + return NewResponse(500), nil } } @@ -989,12 +943,7 @@ func (f *FileConnector) RenameFile(ctx context.Context, lockID, target string) ( Str("StatusCode", moveRes.GetStatus().GetCode().String()). Str("StatusMsg", moveRes.GetStatus().GetMessage()). Msg("RenameFile: conflict") - return &ConnectorResponse{ - Status: 409, - Headers: map[string]string{ - HeaderWopiLock: currentLockID, - }, - }, nil + return NewResponseWithLock(409, currentLockID), nil } if moveRes.GetStatus().GetCode() == rpcv1beta1.Code_CODE_ALREADY_EXISTS { @@ -1009,7 +958,7 @@ func (f *FileConnector) RenameFile(ctx context.Context, lockID, target string) ( Str("StatusMsg", moveRes.GetStatus().GetMessage()). Msg("RenameFile: move failed with unexpected status") - return &ConnectorResponse{Status: 500}, nil + return NewResponse(500), nil } } else { // if the put is successful, exit the loop and move on @@ -1019,13 +968,8 @@ func (f *FileConnector) RenameFile(ctx context.Context, lockID, target string) ( } logger.Debug().Msg("RenameFile: success") - return &ConnectorResponse{ - Status: 200, - Body: map[string]interface{}{ - "Name": strings.TrimSuffix(path.Base(finalTarget), path.Ext(finalTarget)), // return the final filename without extension - }, - }, nil - + // return the final filename without extension + return NewResponseSuccessBodyName(strings.TrimSuffix(path.Base(finalTarget), path.Ext(finalTarget))), nil } // CheckFileInfo returns information about the requested file and capabilities of the wopi server @@ -1058,7 +1002,7 @@ func (f *FileConnector) CheckFileInfo(ctx context.Context) (*ConnectorResponse, Str("StatusCode", statRes.GetStatus().GetCode().String()). Str("StatusMsg", statRes.GetStatus().GetMessage()). Msg("CheckFileInfo: stat failed with unexpected status") - return &ConnectorResponse{Status: 500}, nil + return NewResponse(500), nil } // If a not known app name is used, consider "Microsoft" as default. @@ -1146,10 +1090,7 @@ func (f *FileConnector) CheckFileInfo(ctx context.Context) (*ConnectorResponse, info.SetProperties(infoMap) logger.Debug().Interface("FileInfo", info).Msg("CheckFileInfo: success") - return &ConnectorResponse{ - Status: 200, - Body: info, - }, nil + return NewResponseSuccessBody(info), nil } func (f *FileConnector) watermarkText(user *userv1beta1.User) string { diff --git a/services/collaboration/pkg/connector/fileconnector_test.go b/services/collaboration/pkg/connector/fileconnector_test.go index aeb303bc1..2f74ebb14 100644 --- a/services/collaboration/pkg/connector/fileconnector_test.go +++ b/services/collaboration/pkg/connector/fileconnector_test.go @@ -791,7 +791,7 @@ var _ = Describe("FileConnector", func() { }, }, nil) - ccs.On("PutFile", mock.Anything, stream, int64(stream.Len()), "").Times(1).Return(&connector.ConnectorResponse{Status: 200}, nil) + ccs.On("PutFile", mock.Anything, stream, int64(stream.Len()), "").Times(1).Return(connector.NewResponse(200), nil) stat2ParamMatcher := mock.MatchedBy(func(statReq *providerv1beta1.StatRequest) bool { if statReq.Ref.ResourceId.StorageId == "storageid" && @@ -847,7 +847,7 @@ var _ = Describe("FileConnector", func() { }, }, nil) - ccs.On("PutFile", mock.Anything, stream, int64(stream.Len()), "").Times(1).Return(&connector.ConnectorResponse{Status: 200}, nil) + ccs.On("PutFile", mock.Anything, stream, int64(stream.Len()), "").Times(1).Return(connector.NewResponse(200), nil) stat2ParamMatcher := mock.MatchedBy(func(statReq *providerv1beta1.StatRequest) bool { if statReq.Ref.ResourceId.StorageId == "storageid" && @@ -908,8 +908,8 @@ var _ = Describe("FileConnector", func() { // first call will fail with conflict, second call succeeds. // we're only interested on whether the file is locked or not, the actual lockID is irrelevant - ccs.On("PutFile", mock.Anything, stream, int64(stream.Len()), "").Times(1).Return(&connector.ConnectorResponse{Status: 409}, nil).Once() - ccs.On("PutFile", mock.Anything, stream, int64(stream.Len()), "").Times(1).Return(&connector.ConnectorResponse{Status: 200}, nil).Once() + ccs.On("PutFile", mock.Anything, stream, int64(stream.Len()), "").Times(1).Return(connector.NewResponse(409), nil).Once() + ccs.On("PutFile", mock.Anything, stream, int64(stream.Len()), "").Times(1).Return(connector.NewResponse(200), nil).Once() newFilePath := new(string) stat2ParamMatcher := mock.MatchedBy(func(statReq *providerv1beta1.StatRequest) bool { @@ -971,7 +971,7 @@ var _ = Describe("FileConnector", func() { }, }, nil) - ccs.On("PutFile", mock.Anything, stream, int64(stream.Len()), "").Times(1).Return(&connector.ConnectorResponse{Status: 500}, nil) + ccs.On("PutFile", mock.Anything, stream, int64(stream.Len()), "").Times(1).Return(connector.NewResponse(500), nil) response, err := fc.PutRelativeFileSuggested(ctx, ccs, stream, int64(stream.Len()), ".pdf") Expect(err).To(Succeed()) @@ -1043,7 +1043,7 @@ var _ = Describe("FileConnector", func() { }, }, nil) - ccs.On("PutFile", mock.Anything, stream, int64(stream.Len()), "").Times(1).Return(&connector.ConnectorResponse{Status: 200}, nil) + ccs.On("PutFile", mock.Anything, stream, int64(stream.Len()), "").Times(1).Return(connector.NewResponse(200), nil) stat2ParamMatcher := mock.MatchedBy(func(statReq *providerv1beta1.StatRequest) bool { if statReq.Ref.ResourceId.StorageId == "storageid" && @@ -1098,7 +1098,7 @@ var _ = Describe("FileConnector", func() { }, }, nil) - ccs.On("PutFile", mock.Anything, stream, int64(stream.Len()), "").Times(1).Return(&connector.ConnectorResponse{Status: 409, Headers: map[string]string{connector.HeaderWopiLock: "zzz999"}}, nil) + ccs.On("PutFile", mock.Anything, stream, int64(stream.Len()), "").Times(1).Return(connector.NewResponseWithLock(409, "zzz999"), nil) stat2ParamMatcher := mock.MatchedBy(func(statReq *providerv1beta1.StatRequest) bool { if statReq.Ref.ResourceId.StorageId == "storageid" && @@ -1158,7 +1158,7 @@ var _ = Describe("FileConnector", func() { }, }, nil) - ccs.On("PutFile", mock.Anything, stream, int64(stream.Len()), "").Times(1).Return(&connector.ConnectorResponse{Status: 500}, nil) + ccs.On("PutFile", mock.Anything, stream, int64(stream.Len()), "").Times(1).Return(connector.NewResponse(500), nil) response, err := fc.PutRelativeFileRelative(ctx, ccs, stream, int64(stream.Len()), "convFile.pdf") Expect(err).To(Succeed()) diff --git a/services/collaboration/pkg/connector/httpadapter_test.go b/services/collaboration/pkg/connector/httpadapter_test.go index 59d9f6fd5..64de4409b 100644 --- a/services/collaboration/pkg/connector/httpadapter_test.go +++ b/services/collaboration/pkg/connector/httpadapter_test.go @@ -60,7 +60,7 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("GetLock", mock.Anything).Times(1).Return(&connector.ConnectorResponse{Status: 404}, nil) + fc.On("GetLock", mock.Anything).Times(1).Return(connector.NewResponse(404), nil) httpAdapter.GetLock(w, req) resp := w.Result() @@ -73,7 +73,7 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("GetLock", mock.Anything).Times(1).Return(&connector.ConnectorResponse{Status: 200, Headers: map[string]string{connector.HeaderWopiLock: "zzz111"}}, nil) + fc.On("GetLock", mock.Anything).Times(1).Return(connector.NewResponseWithLock(200, "zzz111"), nil) httpAdapter.GetLock(w, req) resp := w.Result() @@ -87,7 +87,7 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("GetLock", mock.Anything).Times(1).Return(&connector.ConnectorResponse{Status: 200, Headers: map[string]string{connector.HeaderWopiLock: ""}}, nil) + fc.On("GetLock", mock.Anything).Times(1).Return(connector.NewResponseWithLock(200, ""), nil) httpAdapter.GetLock(w, req) resp := w.Result() @@ -119,7 +119,7 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("Lock", mock.Anything, "", "").Times(1).Return(&connector.ConnectorResponse{Status: 400}, nil) + fc.On("Lock", mock.Anything, "", "").Times(1).Return(connector.NewResponse(400), nil) httpAdapter.Lock(w, req) resp := w.Result() @@ -133,7 +133,7 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("Lock", mock.Anything, "abc123", "").Times(1).Return(&connector.ConnectorResponse{Status: 409, Headers: map[string]string{connector.HeaderWopiLock: "zzz111"}}, nil) + fc.On("Lock", mock.Anything, "abc123", "").Times(1).Return(connector.NewResponseWithLock(409, "zzz111"), nil) httpAdapter.Lock(w, req) resp := w.Result() @@ -148,7 +148,7 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("Lock", mock.Anything, "abc123", "").Times(1).Return(&connector.ConnectorResponse{Status: 200}, nil) + fc.On("Lock", mock.Anything, "abc123", "").Times(1).Return(connector.NewResponse(200), nil) httpAdapter.Lock(w, req) resp := w.Result() @@ -180,7 +180,7 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("Lock", mock.Anything, "", "").Times(1).Return(&connector.ConnectorResponse{Status: 400}, nil) + fc.On("Lock", mock.Anything, "", "").Times(1).Return(connector.NewResponse(400), nil) httpAdapter.Lock(w, req) resp := w.Result() @@ -195,7 +195,7 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("Lock", mock.Anything, "abc123", "qwerty").Times(1).Return(&connector.ConnectorResponse{Status: 409, Headers: map[string]string{connector.HeaderWopiLock: "zzz111"}}, nil) + fc.On("Lock", mock.Anything, "abc123", "qwerty").Times(1).Return(connector.NewResponseWithLock(409, "zzz111"), nil) httpAdapter.Lock(w, req) resp := w.Result() @@ -211,7 +211,7 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("Lock", mock.Anything, "abc123", "qwerty").Times(1).Return(&connector.ConnectorResponse{Status: 200}, nil) + fc.On("Lock", mock.Anything, "abc123", "qwerty").Times(1).Return(connector.NewResponse(200), nil) httpAdapter.Lock(w, req) resp := w.Result() @@ -242,7 +242,7 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("RefreshLock", mock.Anything, "").Times(1).Return(&connector.ConnectorResponse{Status: 400}, nil) + fc.On("RefreshLock", mock.Anything, "").Times(1).Return(connector.NewResponse(400), nil) httpAdapter.RefreshLock(w, req) resp := w.Result() @@ -256,7 +256,7 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("RefreshLock", mock.Anything, "abc123").Times(1).Return(&connector.ConnectorResponse{Status: 409, Headers: map[string]string{connector.HeaderWopiLock: "zzz111"}}, nil) + fc.On("RefreshLock", mock.Anything, "abc123").Times(1).Return(connector.NewResponseWithLock(409, "zzz111"), nil) httpAdapter.RefreshLock(w, req) resp := w.Result() @@ -271,7 +271,7 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("RefreshLock", mock.Anything, "abc123").Times(1).Return(&connector.ConnectorResponse{Status: 200}, nil) + fc.On("RefreshLock", mock.Anything, "abc123").Times(1).Return(connector.NewResponse(200), nil) httpAdapter.RefreshLock(w, req) resp := w.Result() @@ -301,7 +301,7 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("UnLock", mock.Anything, "").Times(1).Return(&connector.ConnectorResponse{Status: 400}, nil) + fc.On("UnLock", mock.Anything, "").Times(1).Return(connector.NewResponse(400), nil) httpAdapter.UnLock(w, req) resp := w.Result() @@ -315,7 +315,7 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("UnLock", mock.Anything, "abc123").Times(1).Return(&connector.ConnectorResponse{Status: 409, Headers: map[string]string{connector.HeaderWopiLock: "zzz111"}}, nil) + fc.On("UnLock", mock.Anything, "abc123").Times(1).Return(connector.NewResponseWithLock(409, "zzz111"), nil) httpAdapter.UnLock(w, req) resp := w.Result() @@ -330,7 +330,7 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("UnLock", mock.Anything, "abc123").Times(1).Return(&connector.ConnectorResponse{Status: 200}, nil) + fc.On("UnLock", mock.Anything, "abc123").Times(1).Return(connector.NewResponse(200), nil) httpAdapter.UnLock(w, req) resp := w.Result() @@ -358,7 +358,7 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - fc.On("CheckFileInfo", mock.Anything).Times(1).Return(&connector.ConnectorResponse{Status: 404}, nil) + fc.On("CheckFileInfo", mock.Anything).Times(1).Return(connector.NewResponse(404), nil) httpAdapter.CheckFileInfo(w, req) resp := w.Result() @@ -375,7 +375,7 @@ var _ = Describe("HttpAdapter", func() { Size: 123456789, BreadcrumbDocName: "testy.docx", } - fc.On("CheckFileInfo", mock.Anything).Times(1).Return(&connector.ConnectorResponse{Status: 200, Body: finfo}, nil) + fc.On("CheckFileInfo", mock.Anything).Times(1).Return(connector.NewResponseSuccessBody(finfo), nil) httpAdapter.CheckFileInfo(w, req) resp := w.Result() @@ -458,7 +458,7 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - cc.On("PutFile", mock.Anything, mock.Anything, int64(len(contentBody)), "abc123").Times(1).Return(&connector.ConnectorResponse{Status: 409, Headers: map[string]string{connector.HeaderWopiLock: "zzz111"}}, nil) + cc.On("PutFile", mock.Anything, mock.Anything, int64(len(contentBody)), "abc123").Times(1).Return(connector.NewResponseWithLock(409, "zzz111"), nil) httpAdapter.PutFile(w, req) resp := w.Result() @@ -473,7 +473,7 @@ var _ = Describe("HttpAdapter", func() { w := httptest.NewRecorder() - cc.On("PutFile", mock.Anything, mock.Anything, int64(len(contentBody)), "abc123").Times(1).Return(&connector.ConnectorResponse{Status: 200}, nil) + cc.On("PutFile", mock.Anything, mock.Anything, int64(len(contentBody)), "abc123").Times(1).Return(connector.NewResponse(200), nil) httpAdapter.PutFile(w, req) resp := w.Result() From 0a8fd53dac4296957ec34d8798ed9511f951fda8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Tue, 13 Aug 2024 11:08:15 +0200 Subject: [PATCH 6/6] chore: add changelog entry --- changelog/unreleased/collaboration-connector-refactor.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelog/unreleased/collaboration-connector-refactor.md diff --git a/changelog/unreleased/collaboration-connector-refactor.md b/changelog/unreleased/collaboration-connector-refactor.md new file mode 100644 index 000000000..d3ba1001c --- /dev/null +++ b/changelog/unreleased/collaboration-connector-refactor.md @@ -0,0 +1,5 @@ +Enhancement: Refactor the connector in the collaboration service + +This will simplify and homogenize the code around the connector + +https://github.com/owncloud/ocis/pull/9771