Skip to content

Commit 846a175

Browse files
lidelhacdias
lidel
authored and
hacdias
committed
refactor(gw): merge IPFSBackend.Get and GetRange
#240 (review)
1 parent 22c7534 commit 846a175

File tree

7 files changed

+44
-70
lines changed

7 files changed

+44
-70
lines changed

‎gateway/blocks_gateway.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ func NewBlocksGateway(blockService blockservice.BlockService, opts ...BlockGatew
139139
}, nil
140140
}
141141

142-
func (api *BlocksGateway) Get(ctx context.Context, path ImmutablePath) (ContentPathMetadata, *GetResponse, error) {
142+
func (api *BlocksGateway) Get(ctx context.Context, path ImmutablePath, ranges ...ByteRange) (ContentPathMetadata, *GetResponse, error) {
143143
md, nd, err := api.getNode(ctx, path)
144144
if err != nil {
145145
return md, nil, err
@@ -180,10 +180,6 @@ func (api *BlocksGateway) Get(ctx context.Context, path ImmutablePath) (ContentP
180180
return ContentPathMetadata{}, nil, fmt.Errorf("data was not a valid file or directory: %w", ErrInternalServerError) // TODO: should there be a gateway invalid content type to abstract over the various IPLD error types?
181181
}
182182

183-
func (api *BlocksGateway) GetRange(ctx context.Context, path ImmutablePath, ranges ...GetRange) (ContentPathMetadata, *GetResponse, error) {
184-
return api.Get(ctx, path)
185-
}
186-
187183
func (api *BlocksGateway) GetAll(ctx context.Context, path ImmutablePath) (ContentPathMetadata, files.Node, error) {
188184
md, nd, err := api.getNode(ctx, path)
189185
if err != nil {

‎gateway/gateway.go

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -54,11 +54,13 @@ type ContentPathMetadata struct {
5454
ContentType string // Only used for UnixFS requests
5555
}
5656

57-
// GetRange describes a range request within a UnixFS file. From and To mostly follow HTTP Range Request semantics.
57+
// ByteRange describes a range request within a UnixFS file. From and To mostly follow [HTTP Byte Range] Request semantics.
5858
// From >= 0 and To = nil: Get the file (From, Length)
5959
// From >= 0 and To >= 0: Get the range (From, To)
6060
// From >= 0 and To <0: Get the range (From, Length - To)
61-
type GetRange struct {
61+
//
62+
// [HTTP Byte Range]: https://httpwg.org/specs/rfc9110.html#rfc.section.14.1.2
63+
type ByteRange struct {
6264
From uint64
6365
To *int64
6466
}
@@ -86,16 +88,24 @@ func NewGetResponseFromDirectoryListing(dagSize uint64, entries <-chan unixfs.Li
8688
// There are also some existing error types that the gateway code knows how to handle (e.g. context.DeadlineExceeded
8789
// and various IPLD pathing related errors).
8890
type IPFSBackend interface {
89-
// Get returns a UnixFS file, UnixFS directory, or an IPLD block depending on what the path is that has been
90-
// requested. Directories' files.DirEntry objects do not need to contain content, but must contain Name,
91-
// Size, and Cid.
92-
Get(context.Context, ImmutablePath) (ContentPathMetadata, *GetResponse, error)
93-
94-
// GetRange returns a full UnixFS file object, or a UnixFS directory. Ranges passed in are advisory for pre-fetching
95-
// data, however consumers of this function may require extra data beyond the passed ranges (e.g. the initial bit of
96-
// the file might be used for content type sniffing even if only the end of the file is requested).
97-
// Note: there is currently no semantic meaning attached to a range request for a directory
98-
GetRange(context.Context, ImmutablePath, ...GetRange) (ContentPathMetadata, *GetResponse, error)
91+
92+
// Get returns a GetResponse with UnixFS file, directory or a block in IPLD
93+
// format e.g., (DAG-)CBOR/JSON.
94+
//
95+
// Returned Directories are preferably a minimum info required for enumeration: Name, Size, and Cid.
96+
//
97+
// Optional ranges follow [HTTP Byte Ranges] notation and can be used for
98+
// pre-fetching specific sections of a file or a block.
99+
//
100+
// Range notes:
101+
// - Generating response to a range request may require additional data
102+
// beyond the passed ranges (e.g. a single byte range from the middle of a
103+
// file will still need magic bytes from the very beginning for content
104+
// type sniffing).
105+
// - A range request for a directory currently holds no semantic meaning.
106+
//
107+
// [HTTP Byte Ranges]: https://httpwg.org/specs/rfc9110.html#rfc.section.14.1.2
108+
Get(context.Context, ImmutablePath, ...ByteRange) (ContentPathMetadata, *GetResponse, error)
99109

100110
// GetAll returns a UnixFS file or directory depending on what the path is that has been requested. Directories should
101111
// include all content recursively.

‎gateway/gateway_test.go

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -107,12 +107,8 @@ func newMockAPI(t *testing.T) (*mockAPI, cid.Cid) {
107107
}, cids[0]
108108
}
109109

110-
func (api *mockAPI) Get(ctx context.Context, immutablePath ImmutablePath) (ContentPathMetadata, *GetResponse, error) {
111-
return api.gw.Get(ctx, immutablePath)
112-
}
113-
114-
func (api *mockAPI) GetRange(ctx context.Context, immutablePath ImmutablePath, ranges ...GetRange) (ContentPathMetadata, *GetResponse, error) {
115-
return api.gw.GetRange(ctx, immutablePath, ranges...)
110+
func (api *mockAPI) Get(ctx context.Context, immutablePath ImmutablePath, ranges ...ByteRange) (ContentPathMetadata, *GetResponse, error) {
111+
return api.gw.Get(ctx, immutablePath, ranges...)
116112
}
117113

118114
func (api *mockAPI) GetAll(ctx context.Context, immutablePath ImmutablePath) (ContentPathMetadata, files.Node, error) {

‎gateway/handler_defaults.go

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ func (i *handler) serveDefaults(ctx context.Context, w http.ResponseWriter, r *h
2929
isDirectoryHeadRequest bool
3030
directoryMetadata *directoryMetadata
3131
err error
32-
ranges []GetRange
32+
ranges []ByteRange
3333
)
3434

3535
switch r.Method {
@@ -60,23 +60,19 @@ func (i *handler) serveDefaults(ctx context.Context, w http.ResponseWriter, r *h
6060
}
6161

6262
var getResp *GetResponse
63-
// TODO: passing resolved path here, instead of contentPath is harming content routing. Knowing original immutableContentPath will allow backend to find providers for parents, even when internal CIDs are not announced, and will provide better key for caching related DAGs.
64-
if len(ranges) == 0 {
65-
pathMetadata, getResp, err = i.api.Get(ctx, maybeResolvedImPath)
66-
} else {
67-
pathMetadata, getResp, err = i.api.GetRange(ctx, maybeResolvedImPath, ranges...)
68-
}
63+
// TODO: passing only resolved path here, instead of contentPath is
64+
// harming content routing. Knowing original immutableContentPath will
65+
// allow backend to find providers for parents, even when internal
66+
// CIDs are not announced, and will provide better key for caching
67+
// related DAGs.
68+
pathMetadata, getResp, err = i.api.Get(ctx, maybeResolvedImPath, ranges...)
6969
if err != nil {
7070
if isWebRequest(requestedContentType) {
7171
forwardedPath, continueProcessing := i.handleWebRequestErrors(w, r, maybeResolvedImPath, immutableContentPath, contentPath, err, logger)
7272
if !continueProcessing {
7373
return false
7474
}
75-
if len(ranges) == 0 {
76-
pathMetadata, getResp, err = i.api.Get(ctx, forwardedPath)
77-
} else {
78-
pathMetadata, getResp, err = i.api.GetRange(ctx, forwardedPath, ranges...)
79-
}
75+
pathMetadata, getResp, err = i.api.Get(ctx, forwardedPath, ranges...)
8076
if err != nil {
8177
err = fmt.Errorf("failed to resolve %s: %w", debugStr(contentPath.String()), err)
8278
webError(w, err, http.StatusInternalServerError)
@@ -100,7 +96,7 @@ func (i *handler) serveDefaults(ctx context.Context, w http.ResponseWriter, r *h
10096
return false
10197
}
10298

103-
// TODO: check if we have a bug when maybeResolvedImPath is resolved and i.setIpfsRootsHeader works with pathMetadata returned by GetRange(maybeResolvedImPath)
99+
// TODO: check if we have a bug when maybeResolvedImPath is resolved and i.setIpfsRootsHeader works with pathMetadata returned by Get(maybeResolvedImPath)
104100
if err := i.setIpfsRootsHeader(w, pathMetadata); err != nil {
105101
webRequestError(w, err)
106102
return false
@@ -138,15 +134,15 @@ func (i *handler) serveDefaults(ctx context.Context, w http.ResponseWriter, r *h
138134
}
139135

140136
// parseRange parses a Range header string as per RFC 7233.
141-
func parseRange(s string) ([]GetRange, error) {
137+
func parseRange(s string) ([]ByteRange, error) {
142138
if s == "" {
143139
return nil, nil // header not present
144140
}
145141
const b = "bytes="
146142
if !strings.HasPrefix(s, b) {
147143
return nil, errors.New("invalid range")
148144
}
149-
var ranges []GetRange
145+
var ranges []ByteRange
150146
for _, ra := range strings.Split(s[len(b):], ",") {
151147
ra = textproto.TrimString(ra)
152148
if ra == "" {
@@ -157,7 +153,7 @@ func parseRange(s string) ([]GetRange, error) {
157153
return nil, errors.New("invalid range")
158154
}
159155
start, end = textproto.TrimString(start), textproto.TrimString(end)
160-
var r GetRange
156+
var r ByteRange
161157
if start == "" {
162158
r.From = 0
163159
// If no start is specified, end specifies the

‎gateway/handler_test.go

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -45,11 +45,7 @@ type errorMockAPI struct {
4545
err error
4646
}
4747

48-
func (api *errorMockAPI) Get(ctx context.Context, path ImmutablePath) (ContentPathMetadata, *GetResponse, error) {
49-
return ContentPathMetadata{}, nil, api.err
50-
}
51-
52-
func (api *errorMockAPI) GetRange(ctx context.Context, path ImmutablePath, getRange ...GetRange) (ContentPathMetadata, *GetResponse, error) {
48+
func (api *errorMockAPI) Get(ctx context.Context, path ImmutablePath, getRange ...ByteRange) (ContentPathMetadata, *GetResponse, error) {
5349
return ContentPathMetadata{}, nil, api.err
5450
}
5551

@@ -161,11 +157,7 @@ type panicMockAPI struct {
161157
panicOnHostnameHandler bool
162158
}
163159

164-
func (api *panicMockAPI) Get(ctx context.Context, immutablePath ImmutablePath) (ContentPathMetadata, *GetResponse, error) {
165-
panic("i am panicking")
166-
}
167-
168-
func (api *panicMockAPI) GetRange(ctx context.Context, immutablePath ImmutablePath, ranges ...GetRange) (ContentPathMetadata, *GetResponse, error) {
160+
func (api *panicMockAPI) Get(ctx context.Context, immutablePath ImmutablePath, ranges ...ByteRange) (ContentPathMetadata, *GetResponse, error) {
169161
panic("i am panicking")
170162
}
171163

‎gateway/handler_unixfs_dir.go

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import (
2323
// serveDirectory returns the best representation of UnixFS directory
2424
//
2525
// It will return index.html if present, or generate directory listing otherwise.
26-
func (i *handler) serveDirectory(ctx context.Context, w http.ResponseWriter, r *http.Request, resolvedPath ipath.Resolved, contentPath ipath.Path, isHeadRequest bool, directoryMetadata *directoryMetadata, ranges []GetRange, begin time.Time, logger *zap.SugaredLogger) bool {
26+
func (i *handler) serveDirectory(ctx context.Context, w http.ResponseWriter, r *http.Request, resolvedPath ipath.Resolved, contentPath ipath.Path, isHeadRequest bool, directoryMetadata *directoryMetadata, ranges []ByteRange, begin time.Time, logger *zap.SugaredLogger) bool {
2727
ctx, span := spanTrace(ctx, "ServeDirectory", trace.WithAttributes(attribute.String("path", resolvedPath.String())))
2828
defer span.End()
2929

@@ -81,11 +81,7 @@ func (i *handler) serveDirectory(ctx context.Context, w http.ResponseWriter, r *
8181
}
8282
} else {
8383
var getResp *GetResponse
84-
if len(ranges) == 0 {
85-
_, getResp, err = i.api.Get(ctx, imIndexPath)
86-
} else {
87-
_, getResp, err = i.api.GetRange(ctx, imIndexPath, ranges...)
88-
}
84+
_, getResp, err = i.api.Get(ctx, imIndexPath, ranges...)
8985
if err == nil {
9086
if getResp.bytes == nil {
9187
webError(w, fmt.Errorf("%q could not be read: %w", imIndexPath, files.ErrNotReader), http.StatusUnprocessableEntity)

‎gateway/metrics.go

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -55,25 +55,13 @@ func (b *ipfsBackendWithMetrics) updateApiCallMetric(name string, err error, beg
5555
}
5656
}
5757

58-
func (b *ipfsBackendWithMetrics) Get(ctx context.Context, path ImmutablePath) (ContentPathMetadata, *GetResponse, error) {
58+
func (b *ipfsBackendWithMetrics) Get(ctx context.Context, path ImmutablePath, ranges ...ByteRange) (ContentPathMetadata, *GetResponse, error) {
5959
begin := time.Now()
6060
name := "IPFSBackend.Get"
61-
ctx, span := spanTrace(ctx, name, trace.WithAttributes(attribute.String("path", path.String())))
62-
defer span.End()
63-
64-
md, n, err := b.api.Get(ctx, path)
65-
66-
b.updateApiCallMetric(name, err, begin)
67-
return md, n, err
68-
}
69-
70-
func (b *ipfsBackendWithMetrics) GetRange(ctx context.Context, path ImmutablePath, ranges ...GetRange) (ContentPathMetadata, *GetResponse, error) {
71-
begin := time.Now()
72-
name := "IPFSBackend.GetRange"
73-
ctx, span := spanTrace(ctx, name, trace.WithAttributes(attribute.String("path", path.String())))
61+
ctx, span := spanTrace(ctx, name, trace.WithAttributes(attribute.String("path", path.String()), attribute.Int("ranges", len(ranges))))
7462
defer span.End()
7563

76-
md, f, err := b.api.GetRange(ctx, path, ranges...)
64+
md, f, err := b.api.Get(ctx, path, ranges...)
7765

7866
b.updateApiCallMetric(name, err, begin)
7967
return md, f, err

0 commit comments

Comments
 (0)