Merge pull request #3636 from fschade/fix-collaboration-checkfileinfo-fields

fix(collaboration): send LastModifiedTime and the EuroOffice file size

services/collaboration/pkg/connector/fileconnector.go

@@ -45,6 +45,10 @@ const (
 	// WOPI Locks generally have a lock duration of 30 minutes and will be refreshed before expiration if needed
 	// https://docs.microsoft.com/en-us/microsoft-365/cloud-storage-partner-program/rest/concepts#lock
 	lockDuration time.Duration = 30 * time.Minute
+
+	// LastModifiedTime has to be ISO 8601 round-trip formatted
+	// https://learn.microsoft.com/en-us/microsoft-365/cloud-storage-partner-program/rest/files/checkfileinfo/checkfileinfo-other#lastmodifiedtime
+	lastModifiedTimeFormat = "2006-01-02T15:04:05.0000000Z"
 )
 
 // FileConnectorService is the interface to implement the "Files"
@@ -1260,11 +1264,16 @@ func (f *FileConnector) CheckFileInfo(ctx context.Context) (*ConnectorResponse,
 			logger.Error().Err(err).Msg("CheckFileInfo: error getting scopes from the context")
 		}
 	}
+	lastModifiedTime := time.Now().UTC().Format(lastModifiedTimeFormat)
+	if mtime := statRes.GetInfo().GetMtime(); mtime != nil {
+		lastModifiedTime = utils.TSToTime(mtime).UTC().Format(lastModifiedTimeFormat)
+	}
+
 	// fileinfo map
 	infoMap := map[string]any{
 		fileinfo.KeyOwnerID:           hexEncodedOwnerId,
-		fileinfo.KeySize:              int64(statRes.GetInfo().GetSize()),
 		fileinfo.KeyVersion:           getVersion(statRes.GetInfo().GetMtime()),
+		fileinfo.KeyLastModifiedTime:  lastModifiedTime,
 		fileinfo.KeyBaseFileName:      path.Base(statRes.GetInfo().GetPath()),
 		fileinfo.KeyBreadcrumbDocName: path.Base(statRes.GetInfo().GetPath()),
 		// to get the folder we actually need to do a GetPath() request
@@ -1298,6 +1307,16 @@ func (f *FileConnector) CheckFileInfo(ctx context.Context) (*ConnectorResponse,
 		fileinfo.KeyUserCanNotWriteRelative: false,
 	}
 
+	// EuroOffice needs the size on an empty pdf to open the form editor, but
+	// breaks on a zero size for ods, odt and odp
+	size := int64(statRes.GetInfo().GetSize()) //nolint:gosec // a byte count never reaches the int64 ceiling
+	omitSize := size == 0 &&
+		strings.ToLower(f.cfg.App.Product) == "onlyoffice" &&
+		strings.ToLower(path.Ext(statRes.GetInfo().GetPath())) != ".pdf"
+	if !omitSize {
+		infoMap[fileinfo.KeySize] = size
+	}
+
 	switch wopiContext.ViewMode {
 	case appproviderv1beta1.ViewMode_VIEW_MODE_READ_WRITE:
 		infoMap[fileinfo.KeyUserCanWrite] = true

services/collaboration/pkg/connector/fileconnector_test.go

@@ -21,6 +21,14 @@ import (
 	"github.com/golang-jwt/jwt/v5"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	ctxpkg "github.com/opencloud-eu/reva/v2/pkg/ctx"
+	"github.com/opencloud-eu/reva/v2/pkg/rgrpc/status"
+	"github.com/opencloud-eu/reva/v2/pkg/utils"
+	cs3mocks "github.com/opencloud-eu/reva/v2/tests/cs3mocks/mocks"
+	"github.com/stretchr/testify/mock"
+	"google.golang.org/grpc"
+
+	"github.com/opencloud-eu/opencloud/pkg/conversions"
 	"github.com/opencloud-eu/opencloud/pkg/shared"
 	collabmocks "github.com/opencloud-eu/opencloud/services/collaboration/mocks"
 	"github.com/opencloud-eu/opencloud/services/collaboration/pkg/config"
@@ -28,12 +36,6 @@ import (
 	"github.com/opencloud-eu/opencloud/services/collaboration/pkg/connector/fileinfo"
 	"github.com/opencloud-eu/opencloud/services/collaboration/pkg/middleware"
 	"github.com/opencloud-eu/opencloud/services/graph/mocks"
-	ctxpkg "github.com/opencloud-eu/reva/v2/pkg/ctx"
-	"github.com/opencloud-eu/reva/v2/pkg/rgrpc/status"
-	"github.com/opencloud-eu/reva/v2/pkg/utils"
-	cs3mocks "github.com/opencloud-eu/reva/v2/tests/cs3mocks/mocks"
-	"github.com/stretchr/testify/mock"
-	"google.golang.org/grpc"
 )
 
 var _ = Describe("FileConnector", func() {
@@ -1720,6 +1722,7 @@ var _ = Describe("FileConnector", func() {
 				OwnerID:                    "61616262636340637573746f6d496470", // hex of aabbcc@customIdp
 				Size:                       int64(998877),
 				Version:                    "v162738490",
+				LastModifiedTime:           "1970-07-08T08:30:49.0000000Z",
 				BaseFileName:               "test.txt",
 				BreadcrumbDocName:          "test.txt",
 				BreadcrumbFolderName:       "/path/to",
@@ -1900,6 +1903,8 @@ var _ = Describe("FileConnector", func() {
 			expectedFileInfo := &fileinfo.OnlyOffice{
 				Version:                 "v162738490",
 				BaseFileName:            "test.txt",
+				Size:                    conversions.ToPointer(int64(998877)),
+				LastModifiedTime:        "1970-07-08T08:30:49.0000000Z",
 				BreadcrumbDocName:       "test.txt",
 				BreadcrumbFolderName:    "/path/to",
 				BreadcrumbFolderURL:     "https://cloud.opencloud.test/s/ABC123",
@@ -2082,6 +2087,7 @@ var _ = Describe("FileConnector", func() {
 			expectedFileInfo := &fileinfo.OnlyOffice{
 				Version:                 "v162738490",
 				BaseFileName:            "test.txt",
+				LastModifiedTime:        "1970-07-08T08:30:49.0000000Z",
 				BreadcrumbDocName:       "test.txt",
 				BreadcrumbFolderName:    "/path/to",
 				BreadcrumbFolderURL:     "https://cloud.opencloud.test/f/storageid$spaceid%21parentopaqueid",
@@ -2120,6 +2126,86 @@ var _ = Describe("FileConnector", func() {
 			// so we can't compare the whole url
 			Expect(templateSource).To(HavePrefix(expectedTemplateSource))
 		})
+
+		Describe("Size and LastModifiedTime", func() {
+			statFile := func(ctx context.Context, filePath string, size uint64, mtime *typesv1beta1.Timestamp) {
+				gatewayClient.On("Stat", mock.Anything, mock.Anything).Times(1).Return(&providerv1beta1.StatResponse{
+					Status: status.NewOK(ctx),
+					Info: &providerv1beta1.ResourceInfo{
+						Owner: &userv1beta1.UserId{
+							Idp:      "customIdp",
+							OpaqueId: "aabbcc",
+							Type:     userv1beta1.UserType_USER_TYPE_PRIMARY,
+						},
+						Size:  size,
+						Mtime: mtime,
+						Path:  filePath,
+						Id: &providerv1beta1.ResourceId{
+							StorageId: "storageid",
+							OpaqueId:  "opaqueid",
+							SpaceId:   "spaceid",
+						},
+						ParentId: &providerv1beta1.ResourceId{
+							StorageId: "storageid",
+							OpaqueId:  "parentopaqueid",
+							SpaceId:   "spaceid",
+						},
+					},
+				}, nil)
+			}
+
+			var ctx context.Context
+
+			BeforeEach(func() {
+				ctx = middleware.WopiContextToCtx(context.Background(), wopiCtx)
+				// an empty user takes the guest path, no CheckPermission needed
+				ctx = ctxpkg.ContextSetUser(ctx, &userv1beta1.User{})
+				cfg.App.Name = "EuroOffice"
+				cfg.App.Product = "OnlyOffice"
+			})
+
+			It("keeps the zero size of an empty pdf for EuroOffice", func() {
+				statFile(ctx, "/path/to/form.pdf", 0, &typesv1beta1.Timestamp{Seconds: uint64(16273849)})
+
+				response, err := fc.CheckFileInfo(ctx)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(response.Body.(*fileinfo.OnlyOffice).Size).To(Equal(conversions.ToPointer(int64(0))))
+			})
+
+			It("drops the zero size of an empty ods for EuroOffice", func() {
+				statFile(ctx, "/path/to/sheet.ods", 0, &typesv1beta1.Timestamp{Seconds: uint64(16273849)})
+
+				response, err := fc.CheckFileInfo(ctx)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(response.Body.(*fileinfo.OnlyOffice).Size).To(BeNil())
+			})
+
+			It("keeps the size of a filled ods for EuroOffice", func() {
+				statFile(ctx, "/path/to/sheet.ods", 998877, &typesv1beta1.Timestamp{Seconds: uint64(16273849)})
+
+				response, err := fc.CheckFileInfo(ctx)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(response.Body.(*fileinfo.OnlyOffice).Size).To(Equal(conversions.ToPointer(int64(998877))))
+			})
+
+			It("takes LastModifiedTime from the mtime", func() {
+				statFile(ctx, "/path/to/test.txt", 998877, &typesv1beta1.Timestamp{Seconds: uint64(16273849), Nanos: uint32(500)})
+
+				response, err := fc.CheckFileInfo(ctx)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(response.Body.(*fileinfo.OnlyOffice).LastModifiedTime).To(Equal("1970-07-08T08:30:49.0000005Z"))
+			})
+
+			It("falls back to now when the file has no mtime", func() {
+				statFile(ctx, "/path/to/test.txt", 998877, nil)
+
+				response, err := fc.CheckFileInfo(ctx)
+				Expect(err).ToNot(HaveOccurred())
+				lastModified, perr := time.Parse("2006-01-02T15:04:05.0000000Z", response.Body.(*fileinfo.OnlyOffice).LastModifiedTime)
+				Expect(perr).ToNot(HaveOccurred())
+				Expect(lastModified).To(BeTemporally("~", time.Now().UTC(), time.Minute))
+			})
+		})
 	})
 
 	Describe("GetAvatar", func() {

services/collaboration/pkg/connector/fileinfo/onlyoffice.go

@@ -13,6 +13,10 @@ type OnlyOffice struct {
 	BaseFileName string `json:"BaseFileName,omitempty"`
 	// copied from MS WOPI
 	Version string `json:"Version,omitempty"`
+	// The size of the file in bytes. A pointer because an empty pdf has to
+	// carry its 0, while an empty ods, odt or odp must not send the property
+	// at all, EuroOffice fails to open those when it is there.
+	Size *int64 `json:"Size,omitempty"`
 
 	//
 	// Breadcrumb properties
@@ -137,7 +141,9 @@ func (oinfo *OnlyOffice) SetProperties(props map[string]any) {
 			oinfo.BaseFileName = value.(string)
 		case KeyVersion:
 			oinfo.Version = value.(string)
-
+		case KeySize:
+			size := value.(int64)
+			oinfo.Size = &size
 		case KeyBreadcrumbBrandName:
 			oinfo.BreadcrumbBrandName = value.(string)
 		case KeyBreadcrumbBrandURL: