feat: add folder id and etag to dav mkcol responses

Adds the `OC-FileId` and the `OC-ETag` headers to dav `mkcol` responses. This is necessary for clients that work with ids and want to fetch a folder immediately after creating.
cs3org · Jul 17, 2024 · 48515a7 · 48515a7
1 parent 11ee452
commit 48515a7
Show file tree

Hide file tree

Showing 3 changed files with 83 additions and 1 deletion.
diff --git a/changelog/unreleased/add-folder-id-etag-to-mkcol-response.md b/changelog/unreleased/add-folder-id-etag-to-mkcol-response.md
@@ -0,0 +1,6 @@
+Enhancement: Folder id and etag in mkcol dav responses
+
+`mkcol` dav responses now have the `OC-FileId` and the `OC-ETag` header, containing the id and the etag of the created folder.
+
+https://github.com/cs3org/reva/pull/4767
+https://github.com/owncloud/ocis/issues/9618
diff --git a/internal/http/services/owncloud/ocdav/mkcol.go b/internal/http/services/owncloud/ocdav/mkcol.go
@@ -27,10 +27,12 @@ import (
 
 	rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
 	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
+	"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/net"
 	"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/spacelookup"
 	"github.com/cs3org/reva/v2/pkg/appctx"
 	"github.com/cs3org/reva/v2/pkg/errtypes"
 	rstatus "github.com/cs3org/reva/v2/pkg/rgrpc/status"
+	"github.com/cs3org/reva/v2/pkg/storagespace"
 	"github.com/cs3org/reva/v2/pkg/utils"
 	"github.com/rs/zerolog"
 )
@@ -123,6 +125,15 @@ func (s *svc) handleMkcol(ctx context.Context, w http.ResponseWriter, r *http.Re
 	case err != nil:
 		return http.StatusInternalServerError, err
 	case res.Status.Code == rpc.Code_CODE_OK:
+		sReq := &provider.StatRequest{
+			Ref: childRef,
+		}
+		sRes, err := client.Stat(ctx, sReq)
+		if err != nil {
+			return http.StatusInternalServerError, err
+		}
+		w.Header().Set(net.HeaderOCFileID, storagespace.FormatResourceID(sRes.GetInfo().GetId()))
+		w.Header().Set(net.HeaderOCETag, sRes.GetInfo().GetEtag())
 		w.WriteHeader(http.StatusCreated)
 		return 0, nil
 	case res.Status.Code == rpc.Code_CODE_NOT_FOUND:

diff --git a/internal/http/services/owncloud/ocdav/ocdav_blackbox_test.go b/internal/http/services/owncloud/ocdav/ocdav_blackbox_test.go
@@ -371,10 +371,21 @@ var _ = Describe("ocdav", func() {
 						Status: status.NewOK(ctx),
 					}, nil)
 
+					resourceId := &cs3storageprovider.ResourceId{StorageId: "storage", SpaceId: "provider", OpaqueId: "opaque"}
+					resourceETag := "some-etag"
+					client.On("Stat", mock.Anything, mock.Anything).Return(&cs3storageprovider.StatResponse{
+						Status: status.NewOK(ctx),
+						Info: &cs3storageprovider.ResourceInfo{
+							Id:   resourceId,
+							Etag: resourceETag,
+						},
+					}, nil)
+
 					handler.Handler().ServeHTTP(rr, req)
 					Expect(rr).To(HaveHTTPStatus(http.StatusCreated))
 					Expect(rr).To(HaveHTTPBody(BeEmpty()), "Body must be empty")
-					// TODO expect fileid and etag header?
+					Expect(rr).To(HaveHTTPHeaderWithValue(net.HeaderOCFileID, storagespace.FormatResourceID(resourceId)))
+					Expect(rr).To(HaveHTTPHeaderWithValue(net.HeaderOCETag, resourceETag))
 				})
 
 			})
@@ -1254,6 +1265,15 @@ var _ = Describe("ocdav", func() {
 					Status: status.NewOK(ctx),
 				}, nil)
 
+				if expectedStatus == http.StatusCreated {
+					client.On("Stat", mock.Anything, mock.Anything).Return(&cs3storageprovider.StatResponse{
+						Status: status.NewOK(ctx),
+						Info: &cs3storageprovider.ResourceInfo{
+							Id: mReq.Source.ResourceId,
+						},
+					}, nil)
+				}
+
 				rr := httptest.NewRecorder()
 				req, err := http.NewRequest("MKCOL", endpoint+"/foo", strings.NewReader(""))
 				Expect(err).ToNot(HaveOccurred())
@@ -1342,6 +1362,15 @@ var _ = Describe("ocdav", func() {
 					return utils.ResourceEqual(req.Ref, &ref)
 				})).Return(nil, fmt.Errorf("unexpected io error"))
 
+				if expectedStatus == http.StatusCreated {
+					client.On("Stat", mock.Anything, mock.Anything).Return(&cs3storageprovider.StatResponse{
+						Status: status.NewOK(ctx),
+						Info: &cs3storageprovider.ResourceInfo{
+							Id: mReq.Source.ResourceId,
+						},
+					}, nil)
+				}
+
 				rr := httptest.NewRecorder()
 				req, err := http.NewRequest("MKCOL", endpoint+"/foo", strings.NewReader(""))
 				Expect(err).ToNot(HaveOccurred())
@@ -1431,6 +1460,15 @@ var _ = Describe("ocdav", func() {
 					Status: status.NewOK(ctx),
 				}, nil)
 
+				if expectedStatus == http.StatusCreated {
+					client.On("Stat", mock.Anything, mock.Anything).Return(&cs3storageprovider.StatResponse{
+						Status: status.NewOK(ctx),
+						Info: &cs3storageprovider.ResourceInfo{
+							Id: mReq.Source.ResourceId,
+						},
+					}, nil)
+				}
+
 				rr := httptest.NewRecorder()
 				req, err := http.NewRequest("MKCOL", endpoint+"/foo", strings.NewReader(""))
 				Expect(err).ToNot(HaveOccurred())
@@ -1519,6 +1557,15 @@ var _ = Describe("ocdav", func() {
 					Status: status.NewNotFound(ctx, "not found"),
 				}, nil)
 
+				if expectedStatus == http.StatusCreated {
+					client.On("Stat", mock.Anything, mock.Anything).Return(&cs3storageprovider.StatResponse{
+						Status: status.NewOK(ctx),
+						Info: &cs3storageprovider.ResourceInfo{
+							Id: mReq.Source.ResourceId,
+						},
+					}, nil)
+				}
+
 				rr := httptest.NewRecorder()
 				req, err := http.NewRequest("MKCOL", endpoint+"/foo", strings.NewReader(""))
 				Expect(err).ToNot(HaveOccurred())
@@ -1672,6 +1719,15 @@ var _ = Describe("ocdav", func() {
 					}, nil)
 				}
 
+				if expectedStatus == http.StatusCreated {
+					client.On("Stat", mock.Anything, mock.Anything).Return(&cs3storageprovider.StatResponse{
+						Status: status.NewOK(ctx),
+						Info: &cs3storageprovider.ResourceInfo{
+							Id: mReq.Source.ResourceId,
+						},
+					}, nil)
+				}
+
 				parentRef := cs3storageprovider.Reference{
 					ResourceId: userspace.Root,
 					Path:       utils.MakeRelativePath(path.Dir(expectedPath)),
@@ -1808,6 +1864,15 @@ var _ = Describe("ocdav", func() {
 					Status: status.NewOK(ctx),
 				}, nil)
 
+				if expectedStatus == http.StatusCreated {
+					client.On("Stat", mock.Anything, mock.Anything).Return(&cs3storageprovider.StatResponse{
+						Status: status.NewOK(ctx),
+						Info: &cs3storageprovider.ResourceInfo{
+							Id: mReq.Source.ResourceId,
+						},
+					}, nil)
+				}
+
 				rr := httptest.NewRecorder()
 				req, err := http.NewRequest("MKCOL", endpoint+"/foo", strings.NewReader(""))
 				req.Header.Set("If", "(<urn:uuid:181d4fae-7d8c-11d0-a765-00a0c91e6bf2>)")