Skip to content

Commit

Permalink
start testing ocdav delete
Browse files Browse the repository at this point in the history
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
  • Loading branch information
butonic committed Nov 9, 2022
1 parent ed6f03a commit a91cc7a
Show file tree
Hide file tree
Showing 6 changed files with 187 additions and 11 deletions.
6 changes: 6 additions & 0 deletions changelog/unreleased/dav-unit-tests.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Enhancement: Cover ocdav with more unit tests

We added unit tests to cover more ocdav handlers:
- delete

https://github.com/cs3org/reva/pull/3441
22 changes: 15 additions & 7 deletions internal/http/services/owncloud/ocdav/ocdav.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (
"time"

gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
gatewayv1beta1 "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
Expand Down Expand Up @@ -156,6 +155,7 @@ type svc struct {
davHandler *DavHandler
favoritesManager favorite.Manager
client *http.Client
gwClient gateway.GatewayAPIClient
// LockSystem is the lock management system.
LockSystem LockSystem
userIdentifierCache *ttlcache.Cache
Expand Down Expand Up @@ -199,11 +199,11 @@ func New(m map[string]interface{}, log *zerolog.Logger) (global.Service, error)
return nil, err
}

return NewWith(conf, fm, ls, log, rtrace.DefaultProvider())
return NewWith(conf, fm, ls, log, rtrace.DefaultProvider(), nil)
}

// NewWith returns a new ocdav service
func NewWith(conf *Config, fm favorite.Manager, ls LockSystem, _ *zerolog.Logger, tp trace.TracerProvider) (global.Service, error) {
func NewWith(conf *Config, fm favorite.Manager, ls LockSystem, _ *zerolog.Logger, tp trace.TracerProvider, gwc gateway.GatewayAPIClient) (global.Service, error) {
s := &svc{
c: conf,
webDavHandler: new(WebDavHandler),
Expand All @@ -212,6 +212,7 @@ func NewWith(conf *Config, fm favorite.Manager, ls LockSystem, _ *zerolog.Logger
rhttp.Timeout(time.Duration(conf.Timeout*int64(time.Second))),
rhttp.Insecure(conf.Insecure),
),
gwClient: gwc,
favoritesManager: fm,
LockSystem: ls,
userIdentifierCache: ttlcache.NewCache(),
Expand All @@ -226,6 +227,13 @@ func NewWith(conf *Config, fm favorite.Manager, ls LockSystem, _ *zerolog.Logger
if err := s.davHandler.init(conf); err != nil {
return nil, err
}
if gwc == nil {
var err error
s.gwClient, err = pool.GetGatewayServiceClient(s.c.GatewaySvc)
if err != nil {
return nil, err
}
}
return s, nil
}

Expand Down Expand Up @@ -315,7 +323,7 @@ func (s *svc) Handler() http.Handler {
}

func (s *svc) getClient() (gateway.GatewayAPIClient, error) {
return pool.GetGatewayServiceClient(s.c.GatewaySvc)
return s.gwClient, nil
}

func (s *svc) ApplyLayout(ctx context.Context, ns string, useLoggedInUserNS bool, requestPath string) (string, string, error) {
Expand Down Expand Up @@ -393,7 +401,7 @@ func addAccessHeaders(w http.ResponseWriter, r *http.Request) {
}
}

func authContextForUser(client gatewayv1beta1.GatewayAPIClient, userID *userpb.UserId, machineAuthAPIKey string) (context.Context, error) {
func authContextForUser(client gateway.GatewayAPIClient, userID *userpb.UserId, machineAuthAPIKey string) (context.Context, error) {
if machineAuthAPIKey == "" {
return nil, errtypes.NotSupported("machine auth not configured")
}
Expand All @@ -415,7 +423,7 @@ func authContextForUser(client gatewayv1beta1.GatewayAPIClient, userID *userpb.U
return granteeCtx, nil
}

func (s *svc) sspReferenceIsChildOf(ctx context.Context, client gatewayv1beta1.GatewayAPIClient, child, parent *provider.Reference) (bool, error) {
func (s *svc) sspReferenceIsChildOf(ctx context.Context, client gateway.GatewayAPIClient, child, parent *provider.Reference) (bool, error) {
parentStatRes, err := client.Stat(ctx, &provider.StatRequest{Ref: parent})
if err != nil {
return false, err
Expand Down Expand Up @@ -457,7 +465,7 @@ func (s *svc) sspReferenceIsChildOf(ctx context.Context, client gatewayv1beta1.G
return strings.HasPrefix(cp, pp), nil
}

func (s *svc) referenceIsChildOf(ctx context.Context, client gatewayv1beta1.GatewayAPIClient, child, parent *provider.Reference) (bool, error) {
func (s *svc) referenceIsChildOf(ctx context.Context, client gateway.GatewayAPIClient, child, parent *provider.Reference) (bool, error) {
if child.ResourceId.SpaceId != parent.ResourceId.SpaceId {
return false, nil // Not on the same storage -> not a child
}
Expand Down
154 changes: 154 additions & 0 deletions internal/http/services/owncloud/ocdav/ocdav_blackbox_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,154 @@
// Copyright 2021 CERN
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//
// In applying this license, CERN does not waive the privileges and immunities
// granted to it by virtue of its status as an Intergovernmental Organization
// or submit itself to any jurisdiction.
package ocdav_test

import (
"context"
"net/http"
"net/http/httptest"
"strings"

cs3user "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
cs3storageprovider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
typesv1beta1 "github.com/cs3org/go-cs3apis/cs3/types/v1beta1"
"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav"
"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/net"
"github.com/cs3org/reva/v2/pkg/rgrpc/status"
"github.com/cs3org/reva/v2/pkg/rhttp/global"
"github.com/cs3org/reva/v2/pkg/storagespace"
"github.com/cs3org/reva/v2/pkg/utils"
"github.com/cs3org/reva/v2/tests/cs3mocks/mocks"
"github.com/stretchr/testify/mock"
"go.opentelemetry.io/otel/trace"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)

// TODO for now we have to test all of ocdav. when this testsuite is complete we can move
// the handlers to dedicated packages to reduce the amount of complexity to get a test environment up
var _ = Describe("ocdav", func() {
var (
handler global.Service
client *mocks.GatewayAPIClient
ctx context.Context

foospace *cs3storageprovider.StorageSpace
)

JustBeforeEach(func() {
ctx = context.WithValue(context.Background(), net.CtxKeyBaseURI, "http://127.0.0.1:3000")
client = &mocks.GatewayAPIClient{}

var err error
handler, err = ocdav.NewWith(&ocdav.Config{}, nil, ocdav.NewCS3LS(client), nil, trace.NewNoopTracerProvider(), client)
Expect(err).ToNot(HaveOccurred())

foospace = &cs3storageprovider.StorageSpace{
Opaque: &typesv1beta1.Opaque{
Map: map[string]*typesv1beta1.OpaqueEntry{
"path": {
Decoder: "plain",
Value: []byte("/foospace/"),
},
},
},
Id: &cs3storageprovider.StorageSpaceId{OpaqueId: storagespace.FormatResourceID(cs3storageprovider.ResourceId{StorageId: "provider-1", SpaceId: "foospace", OpaqueId: "root"})},
Root: &cs3storageprovider.ResourceId{StorageId: "provider-1", SpaceId: "foospace", OpaqueId: "root"},
Name: "foospace",
}

client.On("GetUser", mock.Anything, mock.Anything).Return(&cs3user.GetUserResponse{
Status: status.NewNotFound(ctx, "not found"),
}, nil)
client.On("GetUserByClaim", mock.Anything, mock.Anything).Return(&cs3user.GetUserByClaimResponse{
Status: status.NewNotFound(ctx, "not found"),
}, nil)
})

Describe("NewHandler", func() {
It("returns a handler", func() {
Expect(handler).ToNot(BeNil())
})
})

Describe("HandlePathDelete", func() {
It("deletes the given path", func() {
client.On("ListStorageSpaces", mock.Anything, mock.MatchedBy(func(req *cs3storageprovider.ListStorageSpacesRequest) bool {
p := string(req.Opaque.Map["path"].Value)
return p == "/" || strings.HasPrefix(p, "/foospace")
})).Return(&cs3storageprovider.ListStorageSpacesResponse{
Status: status.NewOK(ctx),
StorageSpaces: []*cs3storageprovider.StorageSpace{foospace},
}, nil)

ref := cs3storageprovider.Reference{
ResourceId: foospace.Root,
Path: "./foo",
}

client.On("Delete", mock.Anything, mock.MatchedBy(func(req *cs3storageprovider.DeleteRequest) bool {
return utils.ResourceEqual(req.Ref, &ref)
})).Return(&cs3storageprovider.DeleteResponse{
Status: status.NewOK(ctx),
}, nil)

rr := httptest.NewRecorder()
req, err := http.NewRequest("DELETE", "/dav/files/username/foospace/foo", strings.NewReader(""))
Expect(err).ToNot(HaveOccurred())
req = req.WithContext(ctx)

handler.Handler().ServeHTTP(rr, req)
Expect(rr).To(HaveHTTPStatus(http.StatusNoContent), "WebDAV DELETE must return 204")
Expect(rr).To(HaveHTTPBody(""), "Body must be empty")
})
})

Describe("HandleSpacesDelete", func() {
It("deletes the given path", func() {
client.On("ListStorageSpaces", mock.Anything, mock.MatchedBy(func(req *cs3storageprovider.ListStorageSpacesRequest) bool {
p := string(req.Opaque.Map["path"].Value)
return p == "/" || strings.HasPrefix(p, "/foospace")
})).Return(&cs3storageprovider.ListStorageSpacesResponse{
Status: status.NewOK(ctx),
StorageSpaces: []*cs3storageprovider.StorageSpace{foospace},
}, nil)

ref := cs3storageprovider.Reference{
ResourceId: foospace.Root,
Path: "./foo",
}

client.On("Delete", mock.Anything, mock.MatchedBy(func(req *cs3storageprovider.DeleteRequest) bool {
return utils.ResourceEqual(req.Ref, &ref)
})).Return(&cs3storageprovider.DeleteResponse{
Status: status.NewOK(ctx),
}, nil)

rr := httptest.NewRecorder()
req, err := http.NewRequest("DELETE", "/dav/spaces/provider-1$foospace!root/foo", strings.NewReader(""))
Expect(err).ToNot(HaveOccurred())
req = req.WithContext(ctx)

handler.Handler().ServeHTTP(rr, req)
Expect(rr).To(HaveHTTPStatus(http.StatusNoContent), "WebDAV DELETE must return 204")
Expect(rr).To(HaveHTTPBody(""), "Body must be empty")
})

})
})
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@ package ocdav
import (
"testing"

providerv1beta1 "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
sprovider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
"github.com/cs3org/reva/v2/pkg/storagespace"
)

func TestWrapResourceID(t *testing.T) {
expected := "storageid" + "$" + "spaceid" + "!" + "opaqueid"
wrapped := storagespace.FormatResourceID(providerv1beta1.ResourceId{StorageId: "storageid", SpaceId: "spaceid", OpaqueId: "opaqueid"})
wrapped := storagespace.FormatResourceID(sprovider.ResourceId{StorageId: "storageid", SpaceId: "spaceid", OpaqueId: "opaqueid"})

if wrapped != expected {
t.Errorf("wrapped id doesn't have the expected format: got %s expected %s", wrapped, expected)
Expand Down Expand Up @@ -83,5 +83,4 @@ func TestNameDoesNotContainRule(t *testing.T) {
}
}
}

}
9 changes: 9 additions & 0 deletions pkg/micro/ocdav/option.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"context"
"crypto/tls"

gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav"
"github.com/cs3org/reva/v2/pkg/storage/favorite"
"github.com/rs/zerolog"
Expand All @@ -44,6 +45,7 @@ type Options struct {
JWTSecret string

FavoriteManager favorite.Manager
GatewayClient gateway.GatewayAPIClient

TracingEnabled bool
TracingCollector string
Expand Down Expand Up @@ -188,6 +190,13 @@ func FavoriteManager(val favorite.Manager) Option {
}
}

// GatewayClient provides a function to set the GatewayClient option.
func GatewayClient(val gateway.GatewayAPIClient) Option {
return func(o *Options) {
o.GatewayClient = val
}
}

// LockSystem provides a function to set the LockSystem option.
func LockSystem(val ocdav.LockSystem) Option {
return func(o *Options) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/micro/ocdav/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func Service(opts ...Option) (micro.Service, error) {
)

tp := rtrace.GetTracerProvider(sopts.TracingEnabled, sopts.TracingCollector, sopts.TracingEndpoint, sopts.Name)
revaService, err := ocdav.NewWith(&sopts.config, sopts.FavoriteManager, sopts.lockSystem, &sopts.Logger, tp)
revaService, err := ocdav.NewWith(&sopts.config, sopts.FavoriteManager, sopts.lockSystem, &sopts.Logger, tp, sopts.GatewayClient)
if err != nil {
return nil, err
}
Expand Down

0 comments on commit a91cc7a

Please sign in to comment.