Skip to content

Commit be39db3

Browse files
David Christofasbutonic
David Christofas
authored and
butonic
committed
Cleanup code (cs3org#2516)
* pre-compile the chunking regex * reduce type conversions * add changelog
1 parent 57591c1 commit be39db3

File tree

15 files changed

+66
-67
lines changed

15 files changed

+66
-67
lines changed

‎changelog/unreleased/cleanup-code.md

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
Enhancement: Cleaned up some code
2+
3+
- Reduced type conversions []byte <-> string
4+
- pre-compile chunking regex
5+
6+
https://github.com/cs3org/reva/pull/2516

‎internal/http/services/owncloud/ocdav/errors/error.go

+6-2
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
package errors
2020

2121
import (
22+
"bytes"
2223
"encoding/xml"
2324
"net/http"
2425

@@ -76,9 +77,12 @@ func Marshal(code code, message string, header string) ([]byte, error) {
7677
Header: header,
7778
})
7879
if err != nil {
79-
return []byte(""), err
80+
return nil, err
8081
}
81-
return []byte(xml.Header + string(xmlstring)), err
82+
var buf bytes.Buffer
83+
buf.WriteString(xml.Header)
84+
buf.Write(xmlstring)
85+
return buf.Bytes(), err
8286
}
8387

8488
// ErrorXML holds the xml representation of an error

‎internal/http/services/owncloud/ocdav/propfind/propfind.go

+16-13
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
package propfind
2020

2121
import (
22+
"bytes"
2223
"context"
2324
"encoding/json"
2425
"encoding/xml"
@@ -218,7 +219,7 @@ func (p *Handler) propfindResponse(ctx context.Context, w http.ResponseWriter, r
218219
}
219220

220221
w.WriteHeader(http.StatusMultiStatus)
221-
if _, err := w.Write([]byte(propRes)); err != nil {
222+
if _, err := w.Write(propRes); err != nil {
222223
log.Err(err).Msg("error writing response")
223224
}
224225
}
@@ -545,24 +546,26 @@ func ReadPropfind(r io.Reader) (pf XML, status int, err error) {
545546
}
546547

547548
// MultistatusResponse converts a list of resource infos into a multistatus response string
548-
func MultistatusResponse(ctx context.Context, pf *XML, mds []*provider.ResourceInfo, publicURL, ns string, linkshares map[string]struct{}) (string, error) {
549+
func MultistatusResponse(ctx context.Context, pf *XML, mds []*provider.ResourceInfo, publicURL, ns string, linkshares map[string]struct{}) ([]byte, error) {
549550
responses := make([]*ResponseXML, 0, len(mds))
550551
for i := range mds {
551552
res, err := mdToPropResponse(ctx, pf, mds[i], publicURL, ns, linkshares)
552553
if err != nil {
553-
return "", err
554+
return nil, err
554555
}
555556
responses = append(responses, res)
556557
}
557558
responsesXML, err := xml.Marshal(&responses)
558559
if err != nil {
559-
return "", err
560+
return nil, err
560561
}
561562

562-
msg := `<?xml version="1.0" encoding="utf-8"?><d:multistatus xmlns:d="DAV:" `
563-
msg += `xmlns:s="http://sabredav.org/ns" xmlns:oc="http://owncloud.org/ns">`
564-
msg += string(responsesXML) + `</d:multistatus>`
565-
return msg, nil
563+
var buf bytes.Buffer
564+
buf.WriteString(`<?xml version="1.0" encoding="utf-8"?><d:multistatus xmlns:d="DAV:" `)
565+
buf.WriteString(`xmlns:s="http://sabredav.org/ns" xmlns:oc="http://owncloud.org/ns">`)
566+
buf.Write(responsesXML)
567+
buf.WriteString(`</d:multistatus>`)
568+
return buf.Bytes(), nil
566569
}
567570

568571
// mdToPropResponse converts the CS3 metadata into a webdav PropResponse
@@ -590,7 +593,7 @@ func mdToPropResponse(ctx context.Context, pf *XML, md *provider.ResourceInfo, p
590593
// -2 indicates unknown (default)
591594
// -3 indicates unlimited
592595
quota := net.PropQuotaUnknown
593-
size := fmt.Sprintf("%d", md.Size)
596+
size := strconv.FormatUint(md.Size, 10)
594597
// TODO refactor helper functions: GetOpaqueJSONEncoded(opaque, key string, *struct) err, GetOpaquePlainEncoded(opaque, key) value, err
595598
// or use ok like pattern and return bool?
596599
if md.Opaque != nil && md.Opaque.Map != nil {
@@ -693,15 +696,15 @@ func mdToPropResponse(ctx context.Context, pf *XML, md *provider.ResourceInfo, p
693696
} else {
694697
checksums.WriteString(" MD5:")
695698
}
696-
checksums.WriteString(string(e.Value))
699+
checksums.Write(e.Value)
697700
}
698701
if e, ok := md.Opaque.Map["adler32"]; ok {
699702
if checksums.Len() == 0 {
700703
checksums.WriteString("<oc:checksum>ADLER32:")
701704
} else {
702705
checksums.WriteString(" ADLER32:")
703706
}
704-
checksums.WriteString(string(e.Value))
707+
checksums.Write(e.Value)
705708
}
706709
}
707710
if checksums.Len() > 0 {
@@ -861,15 +864,15 @@ func mdToPropResponse(ctx context.Context, pf *XML, md *provider.ResourceInfo, p
861864
} else {
862865
checksums.WriteString(" MD5:")
863866
}
864-
checksums.WriteString(string(e.Value))
867+
checksums.Write(e.Value)
865868
}
866869
if e, ok := md.Opaque.Map["adler32"]; ok {
867870
if checksums.Len() == 0 {
868871
checksums.WriteString("<oc:checksum>ADLER32:")
869872
} else {
870873
checksums.WriteString(" ADLER32:")
871874
}
872-
checksums.WriteString(string(e.Value))
875+
checksums.Write(e.Value)
873876
}
874877
}
875878
if checksums.Len() > 13 {

‎internal/http/services/owncloud/ocdav/proppatch.go

+10-7
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
package ocdav
2020

2121
import (
22+
"bytes"
2223
"context"
2324
"encoding/xml"
2425
"fmt"
@@ -309,12 +310,12 @@ func (s *svc) handleProppatchResponse(ctx context.Context, w http.ResponseWriter
309310
w.Header().Set(net.HeaderDav, "1, 3, extended-mkcol")
310311
w.Header().Set(net.HeaderContentType, "application/xml; charset=utf-8")
311312
w.WriteHeader(http.StatusMultiStatus)
312-
if _, err := w.Write([]byte(propRes)); err != nil {
313+
if _, err := w.Write(propRes); err != nil {
313314
log.Err(err).Msg("error writing response")
314315
}
315316
}
316317

317-
func (s *svc) formatProppatchResponse(ctx context.Context, acceptedProps []xml.Name, removedProps []xml.Name, ref string) (string, error) {
318+
func (s *svc) formatProppatchResponse(ctx context.Context, acceptedProps []xml.Name, removedProps []xml.Name, ref string) ([]byte, error) {
318319
responses := make([]propfind.ResponseXML, 0, 1)
319320
response := propfind.ResponseXML{
320321
Href: net.EncodePath(ref),
@@ -346,13 +347,15 @@ func (s *svc) formatProppatchResponse(ctx context.Context, acceptedProps []xml.N
346347
responses = append(responses, response)
347348
responsesXML, err := xml.Marshal(&responses)
348349
if err != nil {
349-
return "", err
350+
return nil, err
350351
}
351352

352-
msg := `<?xml version="1.0" encoding="utf-8"?><d:multistatus xmlns:d="DAV:" `
353-
msg += `xmlns:s="http://sabredav.org/ns" xmlns:oc="http://owncloud.org/ns">`
354-
msg += string(responsesXML) + `</d:multistatus>`
355-
return msg, nil
353+
var buf bytes.Buffer
354+
buf.WriteString(`<?xml version="1.0" encoding="utf-8"?><d:multistatus xmlns:d="DAV:" `)
355+
buf.WriteString(`xmlns:s="http://sabredav.org/ns" xmlns:oc="http://owncloud.org/ns">`)
356+
buf.Write(responsesXML)
357+
buf.WriteString(`</d:multistatus>`)
358+
return buf.Bytes(), nil
356359
}
357360

358361
func (s *svc) isBooleanProperty(prop string) bool {

‎internal/http/services/owncloud/ocdav/publicfile.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ func (s *svc) handlePropfindOnToken(w http.ResponseWriter, r *http.Request, ns s
122122
w.Header().Set(net.HeaderDav, "1, 3, extended-mkcol")
123123
w.Header().Set(net.HeaderContentType, "application/xml; charset=utf-8")
124124
w.WriteHeader(http.StatusMultiStatus)
125-
if _, err := w.Write([]byte(propRes)); err != nil {
125+
if _, err := w.Write(propRes); err != nil {
126126
sublog.Err(err).Msg("error writing response")
127127
}
128128
}

‎internal/http/services/owncloud/ocdav/put.go

+1-6
Original file line numberDiff line numberDiff line change
@@ -298,12 +298,7 @@ func (s *svc) handlePut(ctx context.Context, w http.ResponseWriter, r *http.Requ
298298
return
299299
}
300300

301-
ok, err := chunking.IsChunked(ref.Path)
302-
if err != nil {
303-
w.WriteHeader(http.StatusInternalServerError)
304-
return
305-
}
306-
if ok {
301+
if chunking.IsChunked(ref.Path) {
307302
chunk, err := chunking.GetChunkBLOBInfo(ref.Path)
308303
if err != nil {
309304
w.WriteHeader(http.StatusInternalServerError)

‎internal/http/services/owncloud/ocdav/report.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ func (s *svc) doFilterFiles(w http.ResponseWriter, r *http.Request, ff *reportFi
124124
w.Header().Set(net.HeaderDav, "1, 3, extended-mkcol")
125125
w.Header().Set(net.HeaderContentType, "application/xml; charset=utf-8")
126126
w.WriteHeader(http.StatusMultiStatus)
127-
if _, err := w.Write([]byte(responsesXML)); err != nil {
127+
if _, err := w.Write(responsesXML); err != nil {
128128
log.Err(err).Msg("error writing response")
129129
}
130130
}

‎internal/http/services/owncloud/ocdav/trashbin.go

+12-9
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
package ocdav
2020

2121
import (
22+
"bytes"
2223
"context"
2324
"encoding/xml"
2425
"fmt"
@@ -197,7 +198,7 @@ func (h *TrashbinHandler) listTrashbin(w http.ResponseWriter, r *http.Request, s
197198
w.Header().Set(net.HeaderDav, "1, 3, extended-mkcol")
198199
w.Header().Set(net.HeaderContentType, "application/xml; charset=utf-8")
199200
w.WriteHeader(http.StatusMultiStatus)
200-
_, err = w.Write([]byte(propRes))
201+
_, err = w.Write(propRes)
201202
if err != nil {
202203
sublog.Error().Err(err).Msg("error writing body")
203204
return
@@ -302,14 +303,14 @@ func (h *TrashbinHandler) listTrashbin(w http.ResponseWriter, r *http.Request, s
302303
w.Header().Set(net.HeaderDav, "1, 3, extended-mkcol")
303304
w.Header().Set(net.HeaderContentType, "application/xml; charset=utf-8")
304305
w.WriteHeader(http.StatusMultiStatus)
305-
_, err = w.Write([]byte(propRes))
306+
_, err = w.Write(propRes)
306307
if err != nil {
307308
sublog.Error().Err(err).Msg("error writing body")
308309
return
309310
}
310311
}
311312

312-
func (h *TrashbinHandler) formatTrashPropfind(ctx context.Context, s *svc, u *userpb.User, pf *propfind.XML, items []*provider.RecycleItem) (string, error) {
313+
func (h *TrashbinHandler) formatTrashPropfind(ctx context.Context, s *svc, u *userpb.User, pf *propfind.XML, items []*provider.RecycleItem) ([]byte, error) {
313314
responses := make([]*propfind.ResponseXML, 0, len(items)+1)
314315
// add trashbin dir . entry
315316
responses = append(responses, &propfind.ResponseXML{
@@ -336,19 +337,21 @@ func (h *TrashbinHandler) formatTrashPropfind(ctx context.Context, s *svc, u *us
336337
for i := range items {
337338
res, err := h.itemToPropResponse(ctx, s, u, pf, items[i])
338339
if err != nil {
339-
return "", err
340+
return nil, err
340341
}
341342
responses = append(responses, res)
342343
}
343344
responsesXML, err := xml.Marshal(&responses)
344345
if err != nil {
345-
return "", err
346+
return nil, err
346347
}
347348

348-
msg := `<?xml version="1.0" encoding="utf-8"?><d:multistatus xmlns:d="DAV:" `
349-
msg += `xmlns:s="http://sabredav.org/ns" xmlns:oc="http://owncloud.org/ns">`
350-
msg += string(responsesXML) + `</d:multistatus>`
351-
return msg, nil
349+
var buf bytes.Buffer
350+
buf.WriteString(`<?xml version="1.0" encoding="utf-8"?><d:multistatus xmlns:d="DAV:" `)
351+
buf.WriteString(`xmlns:s="http://sabredav.org/ns" xmlns:oc="http://owncloud.org/ns">`)
352+
buf.Write(responsesXML)
353+
buf.WriteString(`</d:multistatus>`)
354+
return buf.Bytes(), nil
352355
}
353356

354357
// itemToPropResponse needs to create a listing that contains a key and destination

‎internal/http/services/owncloud/ocdav/versions.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ func (h *VersionsHandler) doListVersions(w http.ResponseWriter, r *http.Request,
174174
w.Header().Set(net.HeaderDav, "1, 3, extended-mkcol")
175175
w.Header().Set(net.HeaderContentType, "application/xml; charset=utf-8")
176176
w.WriteHeader(http.StatusMultiStatus)
177-
_, err = w.Write([]byte(propRes))
177+
_, err = w.Write(propRes)
178178
if err != nil {
179179
sublog.Error().Err(err).Msg("error writing body")
180180
return

‎internal/http/services/owncloud/ocs/response/response.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ func encodeXML(res Response) ([]byte, error) {
189189
return nil, err
190190
}
191191
b := new(bytes.Buffer)
192-
b.Write([]byte(xml.Header))
192+
b.WriteString(xml.Header)
193193
b.Write(marshalled)
194194
return b.Bytes(), nil
195195
}

‎pkg/storage/fs/owncloudsql/upload.go

+1-5
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,7 @@ func (fs *owncloudsqlfs) Upload(ctx context.Context, ref *provider.Reference, r
5555
uploadInfo := upload.(*fileUpload)
5656

5757
p := uploadInfo.info.Storage["InternalDestination"]
58-
ok, err := chunking.IsChunked(p)
59-
if err != nil {
60-
return errors.Wrap(err, "owncloudsql: error checking path")
61-
}
62-
if ok {
58+
if chunking.IsChunked(p) {
6359
var assembledFile string
6460
p, assembledFile, err = fs.chunkHandler.WriteChunk(p, r)
6561
if err != nil {

‎pkg/storage/utils/chunking/chunking.go

+6-2
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,14 @@ import (
2929
"strings"
3030
)
3131

32+
var (
33+
chunkingPathRE = regexp.MustCompile(`-chunking-\w+-[0-9]+-[0-9]+$`)
34+
)
35+
3236
// IsChunked checks if a given path refers to a chunk or not
33-
func IsChunked(fn string) (bool, error) {
37+
func IsChunked(fn string) bool {
3438
// FIXME: also need to check whether the OC-Chunked header is set
35-
return regexp.MatchString(`-chunking-\w+-[0-9]+-[0-9]+$`, fn)
39+
return chunkingPathRE.MatchString(fn)
3640
}
3741

3842
// ChunkBLOBInfo stores info about a particular chunk

‎pkg/storage/utils/decomposedfs/upload.go

+2-9
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,7 @@ func (fs *Decomposedfs) Upload(ctx context.Context, ref *provider.Reference, r i
6464
uploadInfo := upload.(*fileUpload)
6565

6666
p := uploadInfo.info.Storage["NodeName"]
67-
ok, err := chunking.IsChunked(p) // check chunking v1
68-
if err != nil {
69-
return errors.Wrap(err, "Decomposedfs: error checking path")
70-
}
71-
if ok {
67+
if chunking.IsChunked(p) { // check chunking v1
7268
var assembledFile string
7369
p, assembledFile, err = fs.chunkHandler.WriteChunk(p, r)
7470
if err != nil {
@@ -360,10 +356,7 @@ func (fs *Decomposedfs) GetUpload(ctx context.Context, id string) (tusd.Upload,
360356
// This method can also handle lookups for paths which contain chunking information.
361357
func (fs *Decomposedfs) lookupNode(ctx context.Context, spaceRoot *node.Node, path string) (*node.Node, error) {
362358
p := path
363-
isChunked, err := chunking.IsChunked(path)
364-
if err != nil {
365-
return nil, err
366-
}
359+
isChunked := chunking.IsChunked(path)
367360
if isChunked {
368361
chunkInfo, err := chunking.GetChunkBLOBInfo(path)
369362
if err != nil {

‎pkg/storage/utils/eosfs/upload.go

+1-5
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,7 @@ func (fs *eosfs) Upload(ctx context.Context, ref *provider.Reference, r io.ReadC
3939
return errtypes.PermissionDenied("eos: cannot upload under the virtual share folder")
4040
}
4141

42-
ok, err := chunking.IsChunked(p)
43-
if err != nil {
44-
return errors.Wrap(err, "eos: error checking path")
45-
}
46-
if ok {
42+
if chunking.IsChunked(p) {
4743
var assembledFile string
4844
p, assembledFile, err = fs.chunkHandler.WriteChunk(p, r)
4945
if err != nil {

‎pkg/storage/utils/localfs/upload.go

+1-5
Original file line numberDiff line numberDiff line change
@@ -49,11 +49,7 @@ func (fs *localfs) Upload(ctx context.Context, ref *provider.Reference, r io.Rea
4949
uploadInfo := upload.(*fileUpload)
5050

5151
p := uploadInfo.info.Storage["InternalDestination"]
52-
ok, err := chunking.IsChunked(p)
53-
if err != nil {
54-
return errors.Wrap(err, "localfs: error checking path")
55-
}
56-
if ok {
52+
if chunking.IsChunked(p) {
5753
var assembledFile string
5854
p, assembledFile, err = fs.chunkHandler.WriteChunk(p, r)
5955
if err != nil {

0 commit comments

Comments
 (0)