From 130063fed1b46372a6dbabd2d7714f8932f706a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Tue, 28 Jun 2022 15:56:50 +0000 Subject: [PATCH 01/11] update propfind unit tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../owncloud/ocdav/propfind/propfind_test.go | 24 ++++--------------- 1 file changed, 4 insertions(+), 20 deletions(-) diff --git a/internal/http/services/owncloud/ocdav/propfind/propfind_test.go b/internal/http/services/owncloud/ocdav/propfind/propfind_test.go index 0816ed7ef6..0b0048e4ea 100644 --- a/internal/http/services/owncloud/ocdav/propfind/propfind_test.go +++ b/internal/http/services/owncloud/ocdav/propfind/propfind_test.go @@ -629,7 +629,7 @@ var _ = Describe("Propfind", func() { Expect(rr.Code).To(Equal(http.StatusOK)) }) - It("mounts embedded spaces", func() { + FIt("mounts embedded spaces", func() { rr := httptest.NewRecorder() req, err := http.NewRequest("GET", "/foo", strings.NewReader("")) Expect(err).ToNot(HaveOccurred()) @@ -640,25 +640,9 @@ var _ = Describe("Propfind", func() { res, _, err := readResponse(rr.Result().Body) Expect(err).ToNot(HaveOccurred()) - Expect(len(res.Responses)).To(Equal(5)) - - root := res.Responses[0] - Expect(root.Href).To(Equal("http:/127.0.0.1:3000/foo/")) - Expect(string(root.Propstat[0].Prop[0].InnerXML)).To(ContainSubstring("1131")) - - bar := res.Responses[1] - Expect(bar.Href).To(Equal("http:/127.0.0.1:3000/foo/bar")) - Expect(string(bar.Propstat[0].Prop[0].InnerXML)).To(ContainSubstring("100")) - - baz := res.Responses[2] - Expect(baz.Href).To(Equal("http:/127.0.0.1:3000/foo/baz")) - Expect(string(baz.Propstat[0].Prop[0].InnerXML)).To(ContainSubstring("1")) - - dir := res.Responses[3] - Expect(dir.Href).To(Equal("http:/127.0.0.1:3000/foo/dir/")) - Expect(string(dir.Propstat[0].Prop[0].InnerXML)).To(ContainSubstring("30")) + Expect(len(res.Responses)).To(Equal(1)) - qux := res.Responses[4] + qux := res.Responses[0] Expect(qux.Href).To(Equal("http:/127.0.0.1:3000/foo/qux/")) Expect(string(qux.Propstat[0].Prop[0].InnerXML)).To(ContainSubstring("1000")) }) @@ -674,7 +658,7 @@ var _ = Describe("Propfind", func() { res, _, err := readResponse(rr.Result().Body) Expect(err).ToNot(HaveOccurred()) - Expect(len(res.Responses)).To(Equal(2)) + Expect(len(res.Responses)).To(Equal(4)) qux := res.Responses[0] Expect(qux.Href).To(Equal("http:/127.0.0.1:3000/foo/qux/")) From 42c94f3fc278dd92852401b99f98c7ddd425fa94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 29 Jun 2022 07:50:47 +0000 Subject: [PATCH 02/11] fix moq propfind comparison in propfind tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../owncloud/ocdav/propfind/propfind_test.go | 24 +++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/internal/http/services/owncloud/ocdav/propfind/propfind_test.go b/internal/http/services/owncloud/ocdav/propfind/propfind_test.go index 0b0048e4ea..0816ed7ef6 100644 --- a/internal/http/services/owncloud/ocdav/propfind/propfind_test.go +++ b/internal/http/services/owncloud/ocdav/propfind/propfind_test.go @@ -629,7 +629,7 @@ var _ = Describe("Propfind", func() { Expect(rr.Code).To(Equal(http.StatusOK)) }) - FIt("mounts embedded spaces", func() { + It("mounts embedded spaces", func() { rr := httptest.NewRecorder() req, err := http.NewRequest("GET", "/foo", strings.NewReader("")) Expect(err).ToNot(HaveOccurred()) @@ -640,9 +640,25 @@ var _ = Describe("Propfind", func() { res, _, err := readResponse(rr.Result().Body) Expect(err).ToNot(HaveOccurred()) - Expect(len(res.Responses)).To(Equal(1)) + Expect(len(res.Responses)).To(Equal(5)) - qux := res.Responses[0] + root := res.Responses[0] + Expect(root.Href).To(Equal("http:/127.0.0.1:3000/foo/")) + Expect(string(root.Propstat[0].Prop[0].InnerXML)).To(ContainSubstring("1131")) + + bar := res.Responses[1] + Expect(bar.Href).To(Equal("http:/127.0.0.1:3000/foo/bar")) + Expect(string(bar.Propstat[0].Prop[0].InnerXML)).To(ContainSubstring("100")) + + baz := res.Responses[2] + Expect(baz.Href).To(Equal("http:/127.0.0.1:3000/foo/baz")) + Expect(string(baz.Propstat[0].Prop[0].InnerXML)).To(ContainSubstring("1")) + + dir := res.Responses[3] + Expect(dir.Href).To(Equal("http:/127.0.0.1:3000/foo/dir/")) + Expect(string(dir.Propstat[0].Prop[0].InnerXML)).To(ContainSubstring("30")) + + qux := res.Responses[4] Expect(qux.Href).To(Equal("http:/127.0.0.1:3000/foo/qux/")) Expect(string(qux.Propstat[0].Prop[0].InnerXML)).To(ContainSubstring("1000")) }) @@ -658,7 +674,7 @@ var _ = Describe("Propfind", func() { res, _, err := readResponse(rr.Result().Body) Expect(err).ToNot(HaveOccurred()) - Expect(len(res.Responses)).To(Equal(4)) + Expect(len(res.Responses)).To(Equal(2)) qux := res.Responses[0] Expect(qux.Href).To(Equal("http:/127.0.0.1:3000/foo/qux/")) From 2d3add913cbc119ae2d996d6bb813cf184ce8c55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 29 Jun 2022 10:18:58 +0000 Subject: [PATCH 03/11] typos, mimic oc10 not found error message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- internal/http/services/owncloud/ocdav/propfind/propfind.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/http/services/owncloud/ocdav/propfind/propfind.go b/internal/http/services/owncloud/ocdav/propfind/propfind.go index df90d295d4..acc416b897 100644 --- a/internal/http/services/owncloud/ocdav/propfind/propfind.go +++ b/internal/http/services/owncloud/ocdav/propfind/propfind.go @@ -297,6 +297,10 @@ func (p *Handler) HandleSpacesPropfind(w http.ResponseWriter, r *http.Request, s if sRes.Status.Code != rpc.Code_CODE_OK { // return not found error so we do not leak existence of a space status = http.StatusNotFound +<<<<<<< HEAD +======= + m = "Resource not found" // mimic the oc10 error message +>>>>>>> d718122d (typos, mimic oc10 not found error message) } } if status == http.StatusNotFound { From 3ba90e0d67880e7427914452789d6e1d24de9a51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 29 Jun 2022 14:40:39 +0000 Subject: [PATCH 04/11] add fieldmask to ListContainer and Stat MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- internal/http/services/owncloud/ocdav/propfind/propfind.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/internal/http/services/owncloud/ocdav/propfind/propfind.go b/internal/http/services/owncloud/ocdav/propfind/propfind.go index acc416b897..df90d295d4 100644 --- a/internal/http/services/owncloud/ocdav/propfind/propfind.go +++ b/internal/http/services/owncloud/ocdav/propfind/propfind.go @@ -297,10 +297,6 @@ func (p *Handler) HandleSpacesPropfind(w http.ResponseWriter, r *http.Request, s if sRes.Status.Code != rpc.Code_CODE_OK { // return not found error so we do not leak existence of a space status = http.StatusNotFound -<<<<<<< HEAD -======= - m = "Resource not found" // mimic the oc10 error message ->>>>>>> d718122d (typos, mimic oc10 not found error message) } } if status == http.StatusNotFound { From 90164f9eae0c84bd3c867b7a9008ac4530df4a17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Thu, 30 Jun 2022 15:30:47 +0000 Subject: [PATCH 05/11] stream xml propfind per resource MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../owncloud/ocdav/propfind/propfind.go | 86 ++++++++++++------- .../services/owncloud/ocdav/publicfile.go | 27 +++--- .../http/services/owncloud/ocdav/report.go | 28 +++--- .../http/services/owncloud/ocdav/versions.go | 32 +++---- 4 files changed, 104 insertions(+), 69 deletions(-) diff --git a/internal/http/services/owncloud/ocdav/propfind/propfind.go b/internal/http/services/owncloud/ocdav/propfind/propfind.go index df90d295d4..543487a97e 100644 --- a/internal/http/services/owncloud/ocdav/propfind/propfind.go +++ b/internal/http/services/owncloud/ocdav/propfind/propfind.go @@ -401,25 +401,28 @@ func (p *Handler) propfindResponse(ctx context.Context, w http.ResponseWriter, r } } - propRes, err := MultistatusResponse(ctx, &pf, resourceInfos, p.PublicURL, namespace, linkshares) - if err != nil { - log.Error().Err(err).Msg("error formatting propfind") - w.WriteHeader(http.StatusInternalServerError) - return - } - w.Header().Set(net.HeaderDav, "1, 3, extended-mkcol") - w.Header().Set(net.HeaderContentType, "application/xml; charset=utf-8") - if sendTusHeaders { - w.Header().Add(net.HeaderAccessControlExposeHeaders, strings.Join([]string{net.HeaderTusResumable, net.HeaderTusVersion, net.HeaderTusExtension}, ", ")) - w.Header().Set(net.HeaderTusResumable, "1.0.0") - w.Header().Set(net.HeaderTusVersion, "1.0.0") - w.Header().Set(net.HeaderTusExtension, "creation,creation-with-upload,checksum,expiration") - } + /*propRes, err :=*/ + RenderMultistatusResponse(ctx, w, &pf, resourceInfos, p.PublicURL, namespace, spaceType, linkshares, sendTusHeaders) + /* + if err != nil { + log.Error().Err(err).Msg("error formatting propfind") + w.WriteHeader(http.StatusInternalServerError) + return + } + w.Header().Set(net.HeaderDav, "1, 3, extended-mkcol") + w.Header().Set(net.HeaderContentType, "application/xml; charset=utf-8") + if sendTusHeaders { + w.Header().Add(net.HeaderAccessControlExposeHeaders, strings.Join([]string{net.HeaderTusResumable, net.HeaderTusVersion, net.HeaderTusExtension}, ", ")) + w.Header().Set(net.HeaderTusResumable, "1.0.0") + w.Header().Set(net.HeaderTusVersion, "1.0.0") + w.Header().Set(net.HeaderTusExtension, "creation,creation-with-upload,checksum,expiration") + } - w.WriteHeader(http.StatusMultiStatus) - if _, err := w.Write(propRes); err != nil { - log.Err(err).Msg("error writing response") - } + w.WriteHeader(http.StatusMultiStatus) + if _, err := w.Write(propRes); err != nil { + log.Err(err).Msg("error writing response") + } + */ } // TODO this is just a stat -> rename @@ -865,24 +868,49 @@ func ReadPropfind(r io.Reader) (pf XML, status int, err error) { return pf, 0, nil } -// MultistatusResponse converts a list of resource infos into a multistatus response string -func MultistatusResponse(ctx context.Context, pf *XML, mds []*provider.ResourceInfo, publicURL, ns string, linkshares map[string]struct{}) ([]byte, error) { - responses := make([]*ResponseXML, 0, len(mds)) +func RenderMultistatusHeader(_ context.Context, w http.ResponseWriter, sendTusHeaders bool) error { + // TODO transfer encoding chunked? + w.Header().Set(net.HeaderDav, "1, 3, extended-mkcol") + w.Header().Set(net.HeaderContentType, "application/xml; charset=utf-8") + if sendTusHeaders { + w.Header().Add(net.HeaderAccessControlExposeHeaders, strings.Join([]string{net.HeaderTusResumable, net.HeaderTusVersion, net.HeaderTusExtension}, ", ")) + w.Header().Set(net.HeaderTusResumable, "1.0.0") + w.Header().Set(net.HeaderTusVersion, "1.0.0") + w.Header().Set(net.HeaderTusExtension, "creation,creation-with-upload,checksum,expiration") + } + + w.WriteHeader(http.StatusMultiStatus) + _, err := w.Write([]byte(``)) + return err +} +func RenderMultistatusFooter(_ context.Context, w http.ResponseWriter, sendTusHeaders bool) error { + _, err := w.Write([]byte(``)) + return err +} + +// RenderMultistatusResponse converts a list of resource infos into a multistatus response string +func RenderMultistatusResponse(ctx context.Context, w http.ResponseWriter, pf *XML, mds []*provider.ResourceInfo, publicURL, ns, spaceType string, linkshares map[string]struct{}, sendTusHeaders bool) { + log := appctx.GetLogger(ctx) + if err := RenderMultistatusHeader(ctx, w, sendTusHeaders); err != nil { + log.Err(err).Msg("error writing xml header") + } + enc := xml.NewEncoder(w) for i := range mds { res, err := mdToPropResponse(ctx, pf, mds[i], publicURL, ns, linkshares) if err != nil { - return nil, err + log.Error().Err(err).Msg("error formatting resource") + return + } + err = enc.Encode(res) + if err != nil { + log.Error().Err(err).Msg("error writing xml") + return } - responses = append(responses, res) } - msr := NewMultiStatusResponseXML() - msr.Responses = responses - msg, err := xml.Marshal(msr) - if err != nil { - return nil, err + if err := RenderMultistatusFooter(ctx, w, sendTusHeaders); err != nil { + log.Err(err).Msg("error writing xml footer") } - return msg, nil } // mdToPropResponse converts the CS3 metadata into a webdav PropResponse diff --git a/internal/http/services/owncloud/ocdav/publicfile.go b/internal/http/services/owncloud/ocdav/publicfile.go index da8a341ed4..83b694b906 100644 --- a/internal/http/services/owncloud/ocdav/publicfile.go +++ b/internal/http/services/owncloud/ocdav/publicfile.go @@ -111,19 +111,22 @@ func (s *svc) handlePropfindOnToken(w http.ResponseWriter, r *http.Request, ns s infos := s.getPublicFileInfos(onContainer, depth == net.DepthZero, tokenStatInfo) - propRes, err := propfind.MultistatusResponse(ctx, &pf, infos, s.c.PublicURL, ns, nil) - if err != nil { - sublog.Error().Err(err).Msg("error formatting propfind") - w.WriteHeader(http.StatusInternalServerError) - return - } + /*propRes, err :=*/ + propfind.RenderMultistatusResponse(ctx, w, &pf, infos, s.c.PublicURL, ns, "", nil, false) // TODO why not announce TUS by sending the headers? + /* + if err != nil { + sublog.Error().Err(err).Msg("error formatting propfind") + w.WriteHeader(http.StatusInternalServerError) + return + } - w.Header().Set(net.HeaderDav, "1, 3, extended-mkcol") - w.Header().Set(net.HeaderContentType, "application/xml; charset=utf-8") - w.WriteHeader(http.StatusMultiStatus) - if _, err := w.Write(propRes); err != nil { - sublog.Err(err).Msg("error writing response") - } + w.Header().Set(net.HeaderDav, "1, 3, extended-mkcol") + w.Header().Set(net.HeaderContentType, "application/xml; charset=utf-8") + w.WriteHeader(http.StatusMultiStatus) + if _, err := w.Write(propRes); err != nil { + sublog.Err(err).Msg("error writing response") + } + */ } // there are only two possible entries diff --git a/internal/http/services/owncloud/ocdav/report.go b/internal/http/services/owncloud/ocdav/report.go index b5ad80a537..f5b5798781 100644 --- a/internal/http/services/owncloud/ocdav/report.go +++ b/internal/http/services/owncloud/ocdav/report.go @@ -26,7 +26,6 @@ import ( rpcv1beta1 "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" providerv1beta1 "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/propfind" "github.com/cs3org/reva/v2/pkg/appctx" ctxpkg "github.com/cs3org/reva/v2/pkg/ctx" @@ -110,18 +109,21 @@ func (s *svc) doFilterFiles(w http.ResponseWriter, r *http.Request, ff *reportFi infos = append(infos, statRes.Info) } - responsesXML, err := propfind.MultistatusResponse(ctx, &propfind.XML{Prop: ff.Prop}, infos, s.c.PublicURL, namespace, nil) - if err != nil { - log.Error().Err(err).Msg("error formatting propfind") - w.WriteHeader(http.StatusInternalServerError) - return - } - w.Header().Set(net.HeaderDav, "1, 3, extended-mkcol") - w.Header().Set(net.HeaderContentType, "application/xml; charset=utf-8") - w.WriteHeader(http.StatusMultiStatus) - if _, err := w.Write(responsesXML); err != nil { - log.Err(err).Msg("error writing response") - } + /*responsesXML, err :=*/ + propfind.RenderMultistatusResponse(ctx, w, &propfind.XML{Prop: ff.Prop}, infos, s.c.PublicURL, namespace, "", nil, false) // TODO why not announce TUS by sending headers? + /* + if err != nil { + log.Error().Err(err).Msg("error formatting propfind") + w.WriteHeader(http.StatusInternalServerError) + return + } + w.Header().Set(net.HeaderDav, "1, 3, extended-mkcol") + w.Header().Set(net.HeaderContentType, "application/xml; charset=utf-8") + w.WriteHeader(http.StatusMultiStatus) + if _, err := w.Write(responsesXML); err != nil { + log.Err(err).Msg("error writing response") + } + */ } } diff --git a/internal/http/services/owncloud/ocdav/versions.go b/internal/http/services/owncloud/ocdav/versions.go index 24e4696ff9..88d473c000 100644 --- a/internal/http/services/owncloud/ocdav/versions.go +++ b/internal/http/services/owncloud/ocdav/versions.go @@ -189,21 +189,23 @@ func (h *VersionsHandler) doListVersions(w http.ResponseWriter, r *http.Request, infos = append(infos, vi) } - propRes, err := propfind.MultistatusResponse(ctx, &pf, infos, s.c.PublicURL, "", nil) - if err != nil { - sublog.Error().Err(err).Msg("error formatting propfind") - w.WriteHeader(http.StatusInternalServerError) - return - } - w.Header().Set(net.HeaderDav, "1, 3, extended-mkcol") - w.Header().Set(net.HeaderContentType, "application/xml; charset=utf-8") - w.WriteHeader(http.StatusMultiStatus) - _, err = w.Write(propRes) - if err != nil { - sublog.Error().Err(err).Msg("error writing body") - return - } - + /*propRes, err :=*/ + propfind.RenderMultistatusResponse(ctx, w, &pf, infos, s.c.PublicURL, "", "", nil, false) + /* + if err != nil { + sublog.Error().Err(err).Msg("error formatting propfind") + w.WriteHeader(http.StatusInternalServerError) + return + } + w.Header().Set(net.HeaderDav, "1, 3, extended-mkcol") + w.Header().Set(net.HeaderContentType, "application/xml; charset=utf-8") + w.WriteHeader(http.StatusMultiStatus) + _, err = w.Write(propRes) + if err != nil { + sublog.Error().Err(err).Msg("error writing body") + return + } + */ } func (h *VersionsHandler) doRestore(w http.ResponseWriter, r *http.Request, s *svc, rid *provider.ResourceId, key string) { From b8870456d3450d98bf7583c767baf66b46ee7a47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Fri, 1 Jul 2022 10:23:16 +0000 Subject: [PATCH 06/11] indexer: introduce LookupCtx MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../owncloud/ocdav/propfind/propfind.go | 32 +++++------ pkg/publicshare/manager/cs3/cs3.go | 36 ++++++------- .../utils/indexer/index/autoincrement.go | 49 ++++++++++++++--- pkg/storage/utils/indexer/index/index.go | 7 ++- pkg/storage/utils/indexer/index/non_unique.go | 54 ++++++++++++++++--- pkg/storage/utils/indexer/index/unique.go | 53 +++++++++++++++--- pkg/storage/utils/indexer/indexer.go | 20 +++++-- 7 files changed, 191 insertions(+), 60 deletions(-) diff --git a/internal/http/services/owncloud/ocdav/propfind/propfind.go b/internal/http/services/owncloud/ocdav/propfind/propfind.go index 543487a97e..9f4fd07bcb 100644 --- a/internal/http/services/owncloud/ocdav/propfind/propfind.go +++ b/internal/http/services/owncloud/ocdav/propfind/propfind.go @@ -366,26 +366,28 @@ func (p *Handler) propfindResponse(ctx context.Context, w http.ResponseWriter, r ctx, span := appctx.GetTracerProvider(r.Context()).Tracer(tracerName).Start(ctx, "propfind_response") defer span.End() - filters := make([]*link.ListPublicSharesRequest_Filter, 0, len(resourceInfos)) - for i := range resourceInfos { - // the list of filters grows with every public link in a folder - filters = append(filters, publicshare.ResourceIDFilter(resourceInfos[i].Id)) - } - - client, err := p.getClient() - if err != nil { - log.Error().Err(err).Msg("error getting grpc client") - w.WriteHeader(http.StatusInternalServerError) - return - } - var linkshares map[string]struct{} // public link access does not show share-types // oc:share-type is not part of an allprops response if namespace != "/public" { // only fetch this if property was queried - for _, p := range pf.Prop { - if p.Space == net.NsOwncloud && (p.Local == "share-types" || p.Local == "permissions") { + for _, prop := range pf.Prop { + if prop.Space == net.NsOwncloud && (prop.Local == "share-types" || prop.Local == "permissions") { + filters := make([]*link.ListPublicSharesRequest_Filter, 0, len(resourceInfos)) + for i := range resourceInfos { + // FIXME this is expensive + // the filters array grow by one for every file in a folder + // TODO store public links as grants on the storage, reassembling them here is too costly + // we can then add the filter if the file has share-types=3 in the opaque, + // same as user / group shares for share indicators + filters = append(filters, publicshare.ResourceIDFilter(resourceInfos[i].Id)) + } + client, err := p.getClient() + if err != nil { + log.Error().Err(err).Msg("error getting grpc client") + w.WriteHeader(http.StatusInternalServerError) + return + } listResp, err := client.ListPublicShares(ctx, &link.ListPublicSharesRequest{Filters: filters}) if err == nil { linkshares = make(map[string]struct{}, len(listResp.Share)) diff --git a/pkg/publicshare/manager/cs3/cs3.go b/pkg/publicshare/manager/cs3/cs3.go index f021389d2f..eea63ec730 100644 --- a/pkg/publicshare/manager/cs3/cs3.go +++ b/pkg/publicshare/manager/cs3/cs3.go @@ -397,33 +397,37 @@ func (m *Manager) ListPublicShares(ctx context.Context, u *user.User, filters [] return result, nil } - tokensByResourceID := make(map[string]*provider.ResourceId) - for _, filter := range idFilter { - resourceID := filter.GetResourceId() - tokens, err := m.indexer.FindBy(&link.PublicShare{}, - indexer.NewField("ResourceId", resourceIDToIndex(resourceID)), - ) - if err != nil { - continue + tokens := []string{} + if len(idFilter) > 0 { + idFilters := []indexer.Field{} + for _, filter := range idFilter { + resourceID := filter.GetResourceId() + idFilters = append(idFilters, indexer.NewField("ResourceId", resourceIDToIndex(resourceID))) } - for _, token := range tokens { - tokensByResourceID[token] = resourceID + tokens, err = m.indexer.FindBy(&link.PublicShare{}, idFilters...) + if err != nil { + return nil, err } } // statMem is used as a local cache to prevent statting resources which // already have been checked. statMem := make(map[string]struct{}) - for token, resourceID := range tokensByResourceID { + for _, token := range tokens { if _, handled := shareMem[token]; handled { // We don't want to add a share multiple times when we added it // already. continue } - if _, checked := statMem[resourceIDToIndex(resourceID)]; !checked { + s, err := m.getByToken(ctx, token) + if err != nil { + return nil, err + } + + if _, checked := statMem[resourceIDToIndex(s.PublicShare.GetResourceId())]; !checked { sReq := &provider.StatRequest{ - Ref: &provider.Reference{ResourceId: resourceID}, + Ref: &provider.Reference{ResourceId: s.PublicShare.GetResourceId()}, } sRes, err := m.gatewayClient.Stat(ctx, sReq) if err != nil { @@ -435,13 +439,9 @@ func (m *Manager) ListPublicShares(ctx context.Context, u *user.User, filters [] if !sRes.Info.PermissionSet.ListGrants { continue } - statMem[resourceIDToIndex(resourceID)] = struct{}{} + statMem[resourceIDToIndex(s.PublicShare.GetResourceId())] = struct{}{} } - s, err := m.getByToken(ctx, token) - if err != nil { - return nil, err - } if publicshare.MatchesFilters(s.PublicShare, filters) { result = append(result, &s.PublicShare) shareMem[s.PublicShare.Token] = struct{}{} diff --git a/pkg/storage/utils/indexer/index/autoincrement.go b/pkg/storage/utils/indexer/index/autoincrement.go index b02dc3df79..300d0b05be 100644 --- a/pkg/storage/utils/indexer/index/autoincrement.go +++ b/pkg/storage/utils/indexer/index/autoincrement.go @@ -75,17 +75,52 @@ func (idx *Autoincrement) Init() error { // Lookup exact lookup by value. func (idx *Autoincrement) Lookup(v string) ([]string, error) { - searchPath := path.Join(idx.indexRootDir, v) - oldname, err := idx.storage.ResolveSymlink(context.Background(), searchPath) - if err != nil { - if os.IsNotExist(err) { - err = &idxerrs.NotFoundErr{TypeName: idx.typeName, IndexBy: idx.indexBy, Value: v} + return idx.LookupCtx(context.Background(), v) +} + +func (idx *Autoincrement) LookupCtx(ctx context.Context, values ...string) ([]string, error) { + var allValues []string + var err error + if len(values) != 1 { + allValues, err = idx.storage.ReadDir(context.Background(), path.Join("/", idx.indexRootDir)) + if err != nil { + return nil, err + } + } else { + allValues = values + } + + var matches = []string{} + for _, av := range allValues { + // TODO check if av can contain more than the base path + av = path.Base(av) + for i, v := range values { + if av == v { + oldname, err := idx.storage.ResolveSymlink(context.Background(), path.Join("/", idx.indexRootDir, av)) + if err != nil { + break + } + matches = append(matches, oldname) + values = remove(values, i) + break + } } - return nil, err + } + if len(matches) == 0 { + var v string + switch len(values) { + case 0: + v = "none" + case 1: + v = values[0] + default: + v = "multiple" + } + return nil, &idxerrs.NotFoundErr{TypeName: idx.typeName, IndexBy: idx.indexBy, Value: v} } - return []string{oldname}, nil + return matches, nil } // Add a new value to the index. diff --git a/pkg/storage/utils/indexer/index/index.go b/pkg/storage/utils/indexer/index/index.go index 6b593ff97f..15b8a94704 100644 --- a/pkg/storage/utils/indexer/index/index.go +++ b/pkg/storage/utils/indexer/index/index.go @@ -18,13 +18,18 @@ package index -import "github.com/cs3org/reva/v2/pkg/storage/utils/indexer/option" +import ( + "context" + + "github.com/cs3org/reva/v2/pkg/storage/utils/indexer/option" +) // Index can be implemented to create new indexer-strategies. See Unique for example. // Each indexer implementation is bound to one data-column (IndexBy) and a data-type (TypeName) type Index interface { Init() error Lookup(v string) ([]string, error) + LookupCtx(ctx context.Context, v ...string) ([]string, error) Add(id, v string) (string, error) Remove(id string, v string) error Update(id, oldV, newV string) error diff --git a/pkg/storage/utils/indexer/index/non_unique.go b/pkg/storage/utils/indexer/index/non_unique.go index f40e992a68..3d677bfa32 100644 --- a/pkg/storage/utils/indexer/index/non_unique.go +++ b/pkg/storage/utils/indexer/index/non_unique.go @@ -79,24 +79,62 @@ func (idx *NonUnique) Init() error { // Lookup exact lookup by value. func (idx *NonUnique) Lookup(v string) ([]string, error) { - if idx.caseInsensitive { - v = strings.ToLower(v) - } - paths, err := idx.storage.ReadDir(context.Background(), path.Join("/", idx.indexRootDir, v)) + return idx.LookupCtx(context.Background(), v) +} + +func (idx *NonUnique) LookupCtx(ctx context.Context, values ...string) ([]string, error) { + allValues, err := idx.storage.ReadDir(context.Background(), path.Join("/", idx.indexRootDir)) if err != nil { return nil, err } - var matches = make([]string, 0) - for _, p := range paths { - matches = append(matches, path.Base(p)) + if idx.caseInsensitive { + for i := range values { + values[i] = strings.ToLower(values[i]) + } + } + + var matches = map[string]struct{}{} + for _, av := range allValues { + av = path.Base(av) + for i, v := range values { + if av == v { + children, err := idx.storage.ReadDir(context.Background(), path.Join("/", idx.indexRootDir, av)) + if err != nil { + break + } + for _, c := range children { + matches[path.Base(c)] = struct{}{} + } + values = remove(values, i) + break + } + } } if len(matches) == 0 { + var v string + switch len(values) { + case 0: + v = "none" + case 1: + v = values[0] + default: + v = "multiple" + } return nil, &idxerrs.NotFoundErr{TypeName: idx.typeName, IndexBy: idx.indexBy, Value: v} } - return matches, nil + ret := make([]string, 0, len(matches)) + for m := range matches { + ret = append(ret, m) + } + return ret, nil +} + +func remove(s []string, i int) []string { + s[i] = s[len(s)-1] + return s[:len(s)-1] } // Add a new value to the index. diff --git a/pkg/storage/utils/indexer/index/unique.go b/pkg/storage/utils/indexer/index/unique.go index fccf092f3a..b1150ab8ff 100644 --- a/pkg/storage/utils/indexer/index/unique.go +++ b/pkg/storage/utils/indexer/index/unique.go @@ -74,20 +74,57 @@ func (idx *Unique) Init() error { // Lookup exact lookup by value. func (idx *Unique) Lookup(v string) ([]string, error) { + return idx.LookupCtx(context.Background(), v) +} + +func (idx *Unique) LookupCtx(ctx context.Context, values ...string) ([]string, error) { + var allValues []string + var err error + if len(values) != 1 { + allValues, err = idx.storage.ReadDir(context.Background(), path.Join("/", idx.indexRootDir)) + if err != nil { + return nil, err + } + } else { + allValues = values + } if idx.caseInsensitive { - v = strings.ToLower(v) + for i := range allValues { + allValues[i] = strings.ToLower(allValues[i]) + } } - searchPath := path.Join(idx.indexRootDir, v) - oldname, err := idx.storage.ResolveSymlink(context.Background(), searchPath) - if err != nil { - if os.IsNotExist(err) { - err = &idxerrs.NotFoundErr{TypeName: idx.typeName, IndexBy: idx.indexBy, Value: v} + + var matches = make([]string, 0) + for _, av := range allValues { + // TODO check if av can contain more than the base path + av = path.Base(av) + for i, v := range values { + if av == v { + oldname, err := idx.storage.ResolveSymlink(context.Background(), path.Join(idx.indexRootDir, av)) + if err != nil { + break + } + matches = append(matches, oldname) + values = remove(values, i) + break + } } - return nil, err + } + if len(matches) == 0 { + var v string + switch len(values) { + case 0: + v = "none" + case 1: + v = values[0] + default: + v = "multiple" + } + return nil, &idxerrs.NotFoundErr{TypeName: idx.typeName, IndexBy: idx.indexBy, Value: v} } - return []string{oldname}, nil + return matches, nil } // Add adds a value to the index, returns the path to the root-document diff --git a/pkg/storage/utils/indexer/indexer.go b/pkg/storage/utils/indexer/indexer.go index 1a9837533b..a3a7ace296 100644 --- a/pkg/storage/utils/indexer/indexer.go +++ b/pkg/storage/utils/indexer/indexer.go @@ -166,9 +166,14 @@ func (i *StorageIndexer) FindBy(t interface{}, queryFields ...Field) ([]string, resultPaths := make(map[string]struct{}) if fields, ok := i.indices[typeName]; ok { - for _, field := range queryFields { - for _, idx := range fields.IndicesByField[strcase.ToCamel(field.Name)] { - res, err := idx.Lookup(field.Value) + for fieldName, queryFields := range groupFieldsByName(queryFields) { + idxes := fields.IndicesByField[strcase.ToCamel(fieldName)] + values := make([]string, 0, len(queryFields)) + for _, f := range queryFields { + values = append(values, f.Value) + } + for _, idx := range idxes { + res, err := idx.LookupCtx(context.Background(), values...) if err != nil { if _, ok := err.(errtypes.IsNotFound); ok { continue @@ -193,6 +198,15 @@ func (i *StorageIndexer) FindBy(t interface{}, queryFields ...Field) ([]string, return result, nil } +// groupFieldsByName groups the given filters and returns a map using the filter type as the key. +func groupFieldsByName(queryFields []Field) map[string][]Field { + grouped := make(map[string][]Field) + for _, f := range queryFields { + grouped[f.Name] = append(grouped[f.Name], f) + } + return grouped +} + // Delete deletes all indexed fields of a given type t on the Indexer. func (i *StorageIndexer) Delete(t interface{}) error { typeName := getTypeFQN(t) From 9c7fe9219515325f1e298f198c64252e4e48c999 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 6 Jul 2022 19:50:55 +0000 Subject: [PATCH 07/11] fix build after rebase MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- internal/http/services/owncloud/ocdav/propfind/propfind.go | 4 ++-- internal/http/services/owncloud/ocdav/publicfile.go | 2 +- internal/http/services/owncloud/ocdav/report.go | 2 +- internal/http/services/owncloud/ocdav/versions.go | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/http/services/owncloud/ocdav/propfind/propfind.go b/internal/http/services/owncloud/ocdav/propfind/propfind.go index 9f4fd07bcb..9fb37d98a9 100644 --- a/internal/http/services/owncloud/ocdav/propfind/propfind.go +++ b/internal/http/services/owncloud/ocdav/propfind/propfind.go @@ -404,7 +404,7 @@ func (p *Handler) propfindResponse(ctx context.Context, w http.ResponseWriter, r } /*propRes, err :=*/ - RenderMultistatusResponse(ctx, w, &pf, resourceInfos, p.PublicURL, namespace, spaceType, linkshares, sendTusHeaders) + RenderMultistatusResponse(ctx, w, &pf, resourceInfos, p.PublicURL, namespace, linkshares, sendTusHeaders) /* if err != nil { log.Error().Err(err).Msg("error formatting propfind") @@ -891,7 +891,7 @@ func RenderMultistatusFooter(_ context.Context, w http.ResponseWriter, sendTusHe } // RenderMultistatusResponse converts a list of resource infos into a multistatus response string -func RenderMultistatusResponse(ctx context.Context, w http.ResponseWriter, pf *XML, mds []*provider.ResourceInfo, publicURL, ns, spaceType string, linkshares map[string]struct{}, sendTusHeaders bool) { +func RenderMultistatusResponse(ctx context.Context, w http.ResponseWriter, pf *XML, mds []*provider.ResourceInfo, publicURL, ns string, linkshares map[string]struct{}, sendTusHeaders bool) { log := appctx.GetLogger(ctx) if err := RenderMultistatusHeader(ctx, w, sendTusHeaders); err != nil { log.Err(err).Msg("error writing xml header") diff --git a/internal/http/services/owncloud/ocdav/publicfile.go b/internal/http/services/owncloud/ocdav/publicfile.go index 83b694b906..f82a2824e7 100644 --- a/internal/http/services/owncloud/ocdav/publicfile.go +++ b/internal/http/services/owncloud/ocdav/publicfile.go @@ -112,7 +112,7 @@ func (s *svc) handlePropfindOnToken(w http.ResponseWriter, r *http.Request, ns s infos := s.getPublicFileInfos(onContainer, depth == net.DepthZero, tokenStatInfo) /*propRes, err :=*/ - propfind.RenderMultistatusResponse(ctx, w, &pf, infos, s.c.PublicURL, ns, "", nil, false) // TODO why not announce TUS by sending the headers? + propfind.RenderMultistatusResponse(ctx, w, &pf, infos, s.c.PublicURL, ns, nil, false) // TODO why not announce TUS by sending the headers? /* if err != nil { sublog.Error().Err(err).Msg("error formatting propfind") diff --git a/internal/http/services/owncloud/ocdav/report.go b/internal/http/services/owncloud/ocdav/report.go index f5b5798781..27bbc3e72c 100644 --- a/internal/http/services/owncloud/ocdav/report.go +++ b/internal/http/services/owncloud/ocdav/report.go @@ -110,7 +110,7 @@ func (s *svc) doFilterFiles(w http.ResponseWriter, r *http.Request, ff *reportFi } /*responsesXML, err :=*/ - propfind.RenderMultistatusResponse(ctx, w, &propfind.XML{Prop: ff.Prop}, infos, s.c.PublicURL, namespace, "", nil, false) // TODO why not announce TUS by sending headers? + propfind.RenderMultistatusResponse(ctx, w, &propfind.XML{Prop: ff.Prop}, infos, s.c.PublicURL, namespace, nil, false) // TODO why not announce TUS by sending headers? /* if err != nil { log.Error().Err(err).Msg("error formatting propfind") diff --git a/internal/http/services/owncloud/ocdav/versions.go b/internal/http/services/owncloud/ocdav/versions.go index 88d473c000..07e4b89069 100644 --- a/internal/http/services/owncloud/ocdav/versions.go +++ b/internal/http/services/owncloud/ocdav/versions.go @@ -190,7 +190,7 @@ func (h *VersionsHandler) doListVersions(w http.ResponseWriter, r *http.Request, } /*propRes, err :=*/ - propfind.RenderMultistatusResponse(ctx, w, &pf, infos, s.c.PublicURL, "", "", nil, false) + propfind.RenderMultistatusResponse(ctx, w, &pf, infos, s.c.PublicURL, "", nil, false) /* if err != nil { sublog.Error().Err(err).Msg("error formatting propfind") From 81523f6797899d61b5109347ffd8fc8134e7dc0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 6 Jul 2022 20:05:33 +0000 Subject: [PATCH 08/11] add changelog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- changelog/unreleased/stream-propfind-responses.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelog/unreleased/stream-propfind-responses.md diff --git a/changelog/unreleased/stream-propfind-responses.md b/changelog/unreleased/stream-propfind-responses.md new file mode 100644 index 0000000000..378edb4724 --- /dev/null +++ b/changelog/unreleased/stream-propfind-responses.md @@ -0,0 +1,5 @@ +Enhancement: Stream PROPFIND responses + +We switched to streamed encoding of propfind responses to decrease time to first byte. + +https://github.com/cs3org/reva/pull/3024 From f9c9d08bca4a83bdae39af64a014203a7d9f9b32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Thu, 7 Jul 2022 14:24:19 +0000 Subject: [PATCH 09/11] lint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../http/services/owncloud/ocdav/propfind/propfind.go | 8 ++++---- pkg/storage/utils/indexer/index/autoincrement.go | 1 + pkg/storage/utils/indexer/index/non_unique.go | 1 + pkg/storage/utils/indexer/index/unique.go | 1 + 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/internal/http/services/owncloud/ocdav/propfind/propfind.go b/internal/http/services/owncloud/ocdav/propfind/propfind.go index 9fb37d98a9..2387570604 100644 --- a/internal/http/services/owncloud/ocdav/propfind/propfind.go +++ b/internal/http/services/owncloud/ocdav/propfind/propfind.go @@ -870,7 +870,7 @@ func ReadPropfind(r io.Reader) (pf XML, status int, err error) { return pf, 0, nil } -func RenderMultistatusHeader(_ context.Context, w http.ResponseWriter, sendTusHeaders bool) error { +func renderMultistatusHeader(_ context.Context, w http.ResponseWriter, sendTusHeaders bool) error { // TODO transfer encoding chunked? w.Header().Set(net.HeaderDav, "1, 3, extended-mkcol") w.Header().Set(net.HeaderContentType, "application/xml; charset=utf-8") @@ -885,7 +885,7 @@ func RenderMultistatusHeader(_ context.Context, w http.ResponseWriter, sendTusHe _, err := w.Write([]byte(``)) return err } -func RenderMultistatusFooter(_ context.Context, w http.ResponseWriter, sendTusHeaders bool) error { +func renderMultistatusFooter(_ context.Context, w http.ResponseWriter, sendTusHeaders bool) error { _, err := w.Write([]byte(``)) return err } @@ -893,7 +893,7 @@ func RenderMultistatusFooter(_ context.Context, w http.ResponseWriter, sendTusHe // RenderMultistatusResponse converts a list of resource infos into a multistatus response string func RenderMultistatusResponse(ctx context.Context, w http.ResponseWriter, pf *XML, mds []*provider.ResourceInfo, publicURL, ns string, linkshares map[string]struct{}, sendTusHeaders bool) { log := appctx.GetLogger(ctx) - if err := RenderMultistatusHeader(ctx, w, sendTusHeaders); err != nil { + if err := renderMultistatusHeader(ctx, w, sendTusHeaders); err != nil { log.Err(err).Msg("error writing xml header") } enc := xml.NewEncoder(w) @@ -910,7 +910,7 @@ func RenderMultistatusResponse(ctx context.Context, w http.ResponseWriter, pf *X } } - if err := RenderMultistatusFooter(ctx, w, sendTusHeaders); err != nil { + if err := renderMultistatusFooter(ctx, w, sendTusHeaders); err != nil { log.Err(err).Msg("error writing xml footer") } } diff --git a/pkg/storage/utils/indexer/index/autoincrement.go b/pkg/storage/utils/indexer/index/autoincrement.go index 300d0b05be..02c0077a45 100644 --- a/pkg/storage/utils/indexer/index/autoincrement.go +++ b/pkg/storage/utils/indexer/index/autoincrement.go @@ -78,6 +78,7 @@ func (idx *Autoincrement) Lookup(v string) ([]string, error) { return idx.LookupCtx(context.Background(), v) } +// LookupCtx retieves multiple exact values and allows passing in a context func (idx *Autoincrement) LookupCtx(ctx context.Context, values ...string) ([]string, error) { var allValues []string var err error diff --git a/pkg/storage/utils/indexer/index/non_unique.go b/pkg/storage/utils/indexer/index/non_unique.go index 3d677bfa32..85eca10268 100644 --- a/pkg/storage/utils/indexer/index/non_unique.go +++ b/pkg/storage/utils/indexer/index/non_unique.go @@ -82,6 +82,7 @@ func (idx *NonUnique) Lookup(v string) ([]string, error) { return idx.LookupCtx(context.Background(), v) } +// LookupCtx retieves multiple exact values and allows passing in a context func (idx *NonUnique) LookupCtx(ctx context.Context, values ...string) ([]string, error) { allValues, err := idx.storage.ReadDir(context.Background(), path.Join("/", idx.indexRootDir)) if err != nil { diff --git a/pkg/storage/utils/indexer/index/unique.go b/pkg/storage/utils/indexer/index/unique.go index b1150ab8ff..843ece137c 100644 --- a/pkg/storage/utils/indexer/index/unique.go +++ b/pkg/storage/utils/indexer/index/unique.go @@ -77,6 +77,7 @@ func (idx *Unique) Lookup(v string) ([]string, error) { return idx.LookupCtx(context.Background(), v) } +// LookupCtx retieves multiple exact values and allows passing in a context func (idx *Unique) LookupCtx(ctx context.Context, values ...string) ([]string, error) { var allValues []string var err error From 4c453be150ad2c5c04240274c27df9c81056fb6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Thu, 7 Jul 2022 15:09:46 +0000 Subject: [PATCH 10/11] Revert "indexer: introduce LookupCtx" This reverts commit b8870456d3450d98bf7583c767baf66b46ee7a47. --- .../owncloud/ocdav/propfind/propfind.go | 32 +++++------ pkg/publicshare/manager/cs3/cs3.go | 36 ++++++------ .../utils/indexer/index/autoincrement.go | 50 +++-------------- pkg/storage/utils/indexer/index/index.go | 7 +-- pkg/storage/utils/indexer/index/non_unique.go | 55 +++---------------- pkg/storage/utils/indexer/index/unique.go | 54 +++--------------- pkg/storage/utils/indexer/indexer.go | 20 +------ 7 files changed, 60 insertions(+), 194 deletions(-) diff --git a/internal/http/services/owncloud/ocdav/propfind/propfind.go b/internal/http/services/owncloud/ocdav/propfind/propfind.go index 2387570604..6fb1c32e28 100644 --- a/internal/http/services/owncloud/ocdav/propfind/propfind.go +++ b/internal/http/services/owncloud/ocdav/propfind/propfind.go @@ -366,28 +366,26 @@ func (p *Handler) propfindResponse(ctx context.Context, w http.ResponseWriter, r ctx, span := appctx.GetTracerProvider(r.Context()).Tracer(tracerName).Start(ctx, "propfind_response") defer span.End() + filters := make([]*link.ListPublicSharesRequest_Filter, 0, len(resourceInfos)) + for i := range resourceInfos { + // the list of filters grows with every public link in a folder + filters = append(filters, publicshare.ResourceIDFilter(resourceInfos[i].Id)) + } + + client, err := p.getClient() + if err != nil { + log.Error().Err(err).Msg("error getting grpc client") + w.WriteHeader(http.StatusInternalServerError) + return + } + var linkshares map[string]struct{} // public link access does not show share-types // oc:share-type is not part of an allprops response if namespace != "/public" { // only fetch this if property was queried - for _, prop := range pf.Prop { - if prop.Space == net.NsOwncloud && (prop.Local == "share-types" || prop.Local == "permissions") { - filters := make([]*link.ListPublicSharesRequest_Filter, 0, len(resourceInfos)) - for i := range resourceInfos { - // FIXME this is expensive - // the filters array grow by one for every file in a folder - // TODO store public links as grants on the storage, reassembling them here is too costly - // we can then add the filter if the file has share-types=3 in the opaque, - // same as user / group shares for share indicators - filters = append(filters, publicshare.ResourceIDFilter(resourceInfos[i].Id)) - } - client, err := p.getClient() - if err != nil { - log.Error().Err(err).Msg("error getting grpc client") - w.WriteHeader(http.StatusInternalServerError) - return - } + for _, p := range pf.Prop { + if p.Space == net.NsOwncloud && (p.Local == "share-types" || p.Local == "permissions") { listResp, err := client.ListPublicShares(ctx, &link.ListPublicSharesRequest{Filters: filters}) if err == nil { linkshares = make(map[string]struct{}, len(listResp.Share)) diff --git a/pkg/publicshare/manager/cs3/cs3.go b/pkg/publicshare/manager/cs3/cs3.go index eea63ec730..f021389d2f 100644 --- a/pkg/publicshare/manager/cs3/cs3.go +++ b/pkg/publicshare/manager/cs3/cs3.go @@ -397,37 +397,33 @@ func (m *Manager) ListPublicShares(ctx context.Context, u *user.User, filters [] return result, nil } - tokens := []string{} - if len(idFilter) > 0 { - idFilters := []indexer.Field{} - for _, filter := range idFilter { - resourceID := filter.GetResourceId() - idFilters = append(idFilters, indexer.NewField("ResourceId", resourceIDToIndex(resourceID))) - } - tokens, err = m.indexer.FindBy(&link.PublicShare{}, idFilters...) + tokensByResourceID := make(map[string]*provider.ResourceId) + for _, filter := range idFilter { + resourceID := filter.GetResourceId() + tokens, err := m.indexer.FindBy(&link.PublicShare{}, + indexer.NewField("ResourceId", resourceIDToIndex(resourceID)), + ) if err != nil { - return nil, err + continue + } + for _, token := range tokens { + tokensByResourceID[token] = resourceID } } // statMem is used as a local cache to prevent statting resources which // already have been checked. statMem := make(map[string]struct{}) - for _, token := range tokens { + for token, resourceID := range tokensByResourceID { if _, handled := shareMem[token]; handled { // We don't want to add a share multiple times when we added it // already. continue } - s, err := m.getByToken(ctx, token) - if err != nil { - return nil, err - } - - if _, checked := statMem[resourceIDToIndex(s.PublicShare.GetResourceId())]; !checked { + if _, checked := statMem[resourceIDToIndex(resourceID)]; !checked { sReq := &provider.StatRequest{ - Ref: &provider.Reference{ResourceId: s.PublicShare.GetResourceId()}, + Ref: &provider.Reference{ResourceId: resourceID}, } sRes, err := m.gatewayClient.Stat(ctx, sReq) if err != nil { @@ -439,9 +435,13 @@ func (m *Manager) ListPublicShares(ctx context.Context, u *user.User, filters [] if !sRes.Info.PermissionSet.ListGrants { continue } - statMem[resourceIDToIndex(s.PublicShare.GetResourceId())] = struct{}{} + statMem[resourceIDToIndex(resourceID)] = struct{}{} } + s, err := m.getByToken(ctx, token) + if err != nil { + return nil, err + } if publicshare.MatchesFilters(s.PublicShare, filters) { result = append(result, &s.PublicShare) shareMem[s.PublicShare.Token] = struct{}{} diff --git a/pkg/storage/utils/indexer/index/autoincrement.go b/pkg/storage/utils/indexer/index/autoincrement.go index 02c0077a45..b02dc3df79 100644 --- a/pkg/storage/utils/indexer/index/autoincrement.go +++ b/pkg/storage/utils/indexer/index/autoincrement.go @@ -75,53 +75,17 @@ func (idx *Autoincrement) Init() error { // Lookup exact lookup by value. func (idx *Autoincrement) Lookup(v string) ([]string, error) { - return idx.LookupCtx(context.Background(), v) -} - -// LookupCtx retieves multiple exact values and allows passing in a context -func (idx *Autoincrement) LookupCtx(ctx context.Context, values ...string) ([]string, error) { - var allValues []string - var err error - if len(values) != 1 { - allValues, err = idx.storage.ReadDir(context.Background(), path.Join("/", idx.indexRootDir)) - if err != nil { - return nil, err - } - } else { - allValues = values - } - - var matches = []string{} - for _, av := range allValues { - // TODO check if av can contain more than the base path - av = path.Base(av) - for i, v := range values { - if av == v { - oldname, err := idx.storage.ResolveSymlink(context.Background(), path.Join("/", idx.indexRootDir, av)) - if err != nil { - break - } - matches = append(matches, oldname) - values = remove(values, i) - break - } + searchPath := path.Join(idx.indexRootDir, v) + oldname, err := idx.storage.ResolveSymlink(context.Background(), searchPath) + if err != nil { + if os.IsNotExist(err) { + err = &idxerrs.NotFoundErr{TypeName: idx.typeName, IndexBy: idx.indexBy, Value: v} } - } - if len(matches) == 0 { - var v string - switch len(values) { - case 0: - v = "none" - case 1: - v = values[0] - default: - v = "multiple" - } - return nil, &idxerrs.NotFoundErr{TypeName: idx.typeName, IndexBy: idx.indexBy, Value: v} + return nil, err } - return matches, nil + return []string{oldname}, nil } // Add a new value to the index. diff --git a/pkg/storage/utils/indexer/index/index.go b/pkg/storage/utils/indexer/index/index.go index 15b8a94704..6b593ff97f 100644 --- a/pkg/storage/utils/indexer/index/index.go +++ b/pkg/storage/utils/indexer/index/index.go @@ -18,18 +18,13 @@ package index -import ( - "context" - - "github.com/cs3org/reva/v2/pkg/storage/utils/indexer/option" -) +import "github.com/cs3org/reva/v2/pkg/storage/utils/indexer/option" // Index can be implemented to create new indexer-strategies. See Unique for example. // Each indexer implementation is bound to one data-column (IndexBy) and a data-type (TypeName) type Index interface { Init() error Lookup(v string) ([]string, error) - LookupCtx(ctx context.Context, v ...string) ([]string, error) Add(id, v string) (string, error) Remove(id string, v string) error Update(id, oldV, newV string) error diff --git a/pkg/storage/utils/indexer/index/non_unique.go b/pkg/storage/utils/indexer/index/non_unique.go index 85eca10268..f40e992a68 100644 --- a/pkg/storage/utils/indexer/index/non_unique.go +++ b/pkg/storage/utils/indexer/index/non_unique.go @@ -79,63 +79,24 @@ func (idx *NonUnique) Init() error { // Lookup exact lookup by value. func (idx *NonUnique) Lookup(v string) ([]string, error) { - return idx.LookupCtx(context.Background(), v) -} - -// LookupCtx retieves multiple exact values and allows passing in a context -func (idx *NonUnique) LookupCtx(ctx context.Context, values ...string) ([]string, error) { - allValues, err := idx.storage.ReadDir(context.Background(), path.Join("/", idx.indexRootDir)) + if idx.caseInsensitive { + v = strings.ToLower(v) + } + paths, err := idx.storage.ReadDir(context.Background(), path.Join("/", idx.indexRootDir, v)) if err != nil { return nil, err } - if idx.caseInsensitive { - for i := range values { - values[i] = strings.ToLower(values[i]) - } - } - - var matches = map[string]struct{}{} - for _, av := range allValues { - av = path.Base(av) - for i, v := range values { - if av == v { - children, err := idx.storage.ReadDir(context.Background(), path.Join("/", idx.indexRootDir, av)) - if err != nil { - break - } - for _, c := range children { - matches[path.Base(c)] = struct{}{} - } - values = remove(values, i) - break - } - } + var matches = make([]string, 0) + for _, p := range paths { + matches = append(matches, path.Base(p)) } if len(matches) == 0 { - var v string - switch len(values) { - case 0: - v = "none" - case 1: - v = values[0] - default: - v = "multiple" - } return nil, &idxerrs.NotFoundErr{TypeName: idx.typeName, IndexBy: idx.indexBy, Value: v} } - ret := make([]string, 0, len(matches)) - for m := range matches { - ret = append(ret, m) - } - return ret, nil -} - -func remove(s []string, i int) []string { - s[i] = s[len(s)-1] - return s[:len(s)-1] + return matches, nil } // Add a new value to the index. diff --git a/pkg/storage/utils/indexer/index/unique.go b/pkg/storage/utils/indexer/index/unique.go index 843ece137c..fccf092f3a 100644 --- a/pkg/storage/utils/indexer/index/unique.go +++ b/pkg/storage/utils/indexer/index/unique.go @@ -74,58 +74,20 @@ func (idx *Unique) Init() error { // Lookup exact lookup by value. func (idx *Unique) Lookup(v string) ([]string, error) { - return idx.LookupCtx(context.Background(), v) -} - -// LookupCtx retieves multiple exact values and allows passing in a context -func (idx *Unique) LookupCtx(ctx context.Context, values ...string) ([]string, error) { - var allValues []string - var err error - if len(values) != 1 { - allValues, err = idx.storage.ReadDir(context.Background(), path.Join("/", idx.indexRootDir)) - if err != nil { - return nil, err - } - } else { - allValues = values - } if idx.caseInsensitive { - for i := range allValues { - allValues[i] = strings.ToLower(allValues[i]) - } + v = strings.ToLower(v) } - - var matches = make([]string, 0) - for _, av := range allValues { - // TODO check if av can contain more than the base path - av = path.Base(av) - for i, v := range values { - if av == v { - oldname, err := idx.storage.ResolveSymlink(context.Background(), path.Join(idx.indexRootDir, av)) - if err != nil { - break - } - matches = append(matches, oldname) - values = remove(values, i) - break - } + searchPath := path.Join(idx.indexRootDir, v) + oldname, err := idx.storage.ResolveSymlink(context.Background(), searchPath) + if err != nil { + if os.IsNotExist(err) { + err = &idxerrs.NotFoundErr{TypeName: idx.typeName, IndexBy: idx.indexBy, Value: v} } - } - if len(matches) == 0 { - var v string - switch len(values) { - case 0: - v = "none" - case 1: - v = values[0] - default: - v = "multiple" - } - return nil, &idxerrs.NotFoundErr{TypeName: idx.typeName, IndexBy: idx.indexBy, Value: v} + return nil, err } - return matches, nil + return []string{oldname}, nil } // Add adds a value to the index, returns the path to the root-document diff --git a/pkg/storage/utils/indexer/indexer.go b/pkg/storage/utils/indexer/indexer.go index a3a7ace296..1a9837533b 100644 --- a/pkg/storage/utils/indexer/indexer.go +++ b/pkg/storage/utils/indexer/indexer.go @@ -166,14 +166,9 @@ func (i *StorageIndexer) FindBy(t interface{}, queryFields ...Field) ([]string, resultPaths := make(map[string]struct{}) if fields, ok := i.indices[typeName]; ok { - for fieldName, queryFields := range groupFieldsByName(queryFields) { - idxes := fields.IndicesByField[strcase.ToCamel(fieldName)] - values := make([]string, 0, len(queryFields)) - for _, f := range queryFields { - values = append(values, f.Value) - } - for _, idx := range idxes { - res, err := idx.LookupCtx(context.Background(), values...) + for _, field := range queryFields { + for _, idx := range fields.IndicesByField[strcase.ToCamel(field.Name)] { + res, err := idx.Lookup(field.Value) if err != nil { if _, ok := err.(errtypes.IsNotFound); ok { continue @@ -198,15 +193,6 @@ func (i *StorageIndexer) FindBy(t interface{}, queryFields ...Field) ([]string, return result, nil } -// groupFieldsByName groups the given filters and returns a map using the filter type as the key. -func groupFieldsByName(queryFields []Field) map[string][]Field { - grouped := make(map[string][]Field) - for _, f := range queryFields { - grouped[f.Name] = append(grouped[f.Name], f) - } - return grouped -} - // Delete deletes all indexed fields of a given type t on the Indexer. func (i *StorageIndexer) Delete(t interface{}) error { typeName := getTypeFQN(t) From a5878d22d3d45522ed2ea7bc09298a2b2eaf7682 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Thu, 7 Jul 2022 15:13:01 +0000 Subject: [PATCH 11/11] remove commented code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../owncloud/ocdav/propfind/propfind.go | 20 ------------------- .../services/owncloud/ocdav/publicfile.go | 14 ------------- .../http/services/owncloud/ocdav/report.go | 15 +------------- .../http/services/owncloud/ocdav/versions.go | 17 +--------------- 4 files changed, 2 insertions(+), 64 deletions(-) diff --git a/internal/http/services/owncloud/ocdav/propfind/propfind.go b/internal/http/services/owncloud/ocdav/propfind/propfind.go index 6fb1c32e28..4840964d8e 100644 --- a/internal/http/services/owncloud/ocdav/propfind/propfind.go +++ b/internal/http/services/owncloud/ocdav/propfind/propfind.go @@ -401,28 +401,8 @@ func (p *Handler) propfindResponse(ctx context.Context, w http.ResponseWriter, r } } - /*propRes, err :=*/ RenderMultistatusResponse(ctx, w, &pf, resourceInfos, p.PublicURL, namespace, linkshares, sendTusHeaders) - /* - if err != nil { - log.Error().Err(err).Msg("error formatting propfind") - w.WriteHeader(http.StatusInternalServerError) - return - } - w.Header().Set(net.HeaderDav, "1, 3, extended-mkcol") - w.Header().Set(net.HeaderContentType, "application/xml; charset=utf-8") - if sendTusHeaders { - w.Header().Add(net.HeaderAccessControlExposeHeaders, strings.Join([]string{net.HeaderTusResumable, net.HeaderTusVersion, net.HeaderTusExtension}, ", ")) - w.Header().Set(net.HeaderTusResumable, "1.0.0") - w.Header().Set(net.HeaderTusVersion, "1.0.0") - w.Header().Set(net.HeaderTusExtension, "creation,creation-with-upload,checksum,expiration") - } - w.WriteHeader(http.StatusMultiStatus) - if _, err := w.Write(propRes); err != nil { - log.Err(err).Msg("error writing response") - } - */ } // TODO this is just a stat -> rename diff --git a/internal/http/services/owncloud/ocdav/publicfile.go b/internal/http/services/owncloud/ocdav/publicfile.go index f82a2824e7..c7a193f869 100644 --- a/internal/http/services/owncloud/ocdav/publicfile.go +++ b/internal/http/services/owncloud/ocdav/publicfile.go @@ -111,22 +111,8 @@ func (s *svc) handlePropfindOnToken(w http.ResponseWriter, r *http.Request, ns s infos := s.getPublicFileInfos(onContainer, depth == net.DepthZero, tokenStatInfo) - /*propRes, err :=*/ propfind.RenderMultistatusResponse(ctx, w, &pf, infos, s.c.PublicURL, ns, nil, false) // TODO why not announce TUS by sending the headers? - /* - if err != nil { - sublog.Error().Err(err).Msg("error formatting propfind") - w.WriteHeader(http.StatusInternalServerError) - return - } - w.Header().Set(net.HeaderDav, "1, 3, extended-mkcol") - w.Header().Set(net.HeaderContentType, "application/xml; charset=utf-8") - w.WriteHeader(http.StatusMultiStatus) - if _, err := w.Write(propRes); err != nil { - sublog.Err(err).Msg("error writing response") - } - */ } // there are only two possible entries diff --git a/internal/http/services/owncloud/ocdav/report.go b/internal/http/services/owncloud/ocdav/report.go index 27bbc3e72c..ad5685f9bd 100644 --- a/internal/http/services/owncloud/ocdav/report.go +++ b/internal/http/services/owncloud/ocdav/report.go @@ -109,21 +109,8 @@ func (s *svc) doFilterFiles(w http.ResponseWriter, r *http.Request, ff *reportFi infos = append(infos, statRes.Info) } - /*responsesXML, err :=*/ propfind.RenderMultistatusResponse(ctx, w, &propfind.XML{Prop: ff.Prop}, infos, s.c.PublicURL, namespace, nil, false) // TODO why not announce TUS by sending headers? - /* - if err != nil { - log.Error().Err(err).Msg("error formatting propfind") - w.WriteHeader(http.StatusInternalServerError) - return - } - w.Header().Set(net.HeaderDav, "1, 3, extended-mkcol") - w.Header().Set(net.HeaderContentType, "application/xml; charset=utf-8") - w.WriteHeader(http.StatusMultiStatus) - if _, err := w.Write(responsesXML); err != nil { - log.Err(err).Msg("error writing response") - } - */ + } } diff --git a/internal/http/services/owncloud/ocdav/versions.go b/internal/http/services/owncloud/ocdav/versions.go index 07e4b89069..e2cf7d9ee8 100644 --- a/internal/http/services/owncloud/ocdav/versions.go +++ b/internal/http/services/owncloud/ocdav/versions.go @@ -189,23 +189,8 @@ func (h *VersionsHandler) doListVersions(w http.ResponseWriter, r *http.Request, infos = append(infos, vi) } - /*propRes, err :=*/ propfind.RenderMultistatusResponse(ctx, w, &pf, infos, s.c.PublicURL, "", nil, false) - /* - if err != nil { - sublog.Error().Err(err).Msg("error formatting propfind") - w.WriteHeader(http.StatusInternalServerError) - return - } - w.Header().Set(net.HeaderDav, "1, 3, extended-mkcol") - w.Header().Set(net.HeaderContentType, "application/xml; charset=utf-8") - w.WriteHeader(http.StatusMultiStatus) - _, err = w.Write(propRes) - if err != nil { - sublog.Error().Err(err).Msg("error writing body") - return - } - */ + } func (h *VersionsHandler) doRestore(w http.ResponseWriter, r *http.Request, s *svc, rid *provider.ResourceId, key string) {