Skip to content

Commit

Permalink
refactor: apply @lidel suggestion A
Browse files Browse the repository at this point in the history
  • Loading branch information
hacdias committed Sep 25, 2023
1 parent 08f2f1e commit 29e356c
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 86 deletions.
12 changes: 6 additions & 6 deletions tests/path_gateway_dag_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ func TestPlainCodec(t *testing.T) {
RunWithSpecs(t, *tests, specs.PathGatewayDAG)

if dagFormattedResponse != nil {
rangeTests := helpers.RangeTestTransform(t,
rangeTests := helpers.OnlyRangeTests(t,
SugarTest{
Name: Fmt("GET {{name}} with format=dag-{{format}} interprets {{format}} as dag-* variant and produces expected Content-Type and body, with single range request", row.Name, row.Format),
Hint: `
Expand Down Expand Up @@ -617,7 +617,7 @@ func TestNativeDag(t *testing.T) {
),
},
}
tests.Append(helpers.RangeTestTransform(t,
tests.Append(helpers.OnlyRangeTests(t,
SugarTest{
Name: Fmt("GET {{name}} on /ipfs with no explicit header", row.Name),
Request: Request().
Expand All @@ -626,7 +626,7 @@ func TestNativeDag(t *testing.T) {
}, nil,
dagTraversal.RawData(), Fmt("application/vnd.ipld.dag-{{format}}", row.Format),
)...).Append(
helpers.RangeTestTransform(t,
helpers.OnlyRangeTests(t,
SugarTest{
Name: Fmt("GET {{name}} on /ipfs with dag content headers", row.Name),
Request: Request().
Expand All @@ -639,7 +639,7 @@ func TestNativeDag(t *testing.T) {
nil, dagTraversal.RawData(),
Fmt("application/vnd.ipld.dag-{{format}}", row.Format),
)...).Append(
helpers.RangeTestTransform(t,
helpers.OnlyRangeTests(t,
SugarTest{
Name: Fmt("GET {{name}} on /ipfs with non-dag content headers", row.Name),
Request: Request().
Expand Down Expand Up @@ -681,7 +681,7 @@ func TestNativeDag(t *testing.T) {
}, specs.PathGatewayDAG)

if dagJsonConvertedData != nil {
rangeTests := helpers.RangeTestTransform(
rangeTests := helpers.OnlyRangeTests(
t,
SugarTest{
Name: "Convert application/vnd.ipld.dag-cbor to application/vnd.ipld.dag-json with range request includes correct bytes",
Expand Down Expand Up @@ -722,7 +722,7 @@ func TestNativeDag(t *testing.T) {
}, specs.PathGatewayDAG)

if dagCborHTMLRendering != nil {
rangeTests := helpers.RangeTestTransform(t,
rangeTests := helpers.OnlyRangeTests(t,
SugarTest{
Name: "Convert application/vnd.ipld.dag-cbor to text/html with range request includes correct bytes",
Hint: "",
Expand Down
5 changes: 3 additions & 2 deletions tests/trustless_gateway_raw_test.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
package tests

import (
"github.com/ipfs/gateway-conformance/tooling/helpers"
"strconv"
"strings"
"testing"

"github.com/ipfs/gateway-conformance/tooling/helpers"

"github.com/ipfs/gateway-conformance/tooling"
"github.com/ipfs/gateway-conformance/tooling/car"
"github.com/ipfs/gateway-conformance/tooling/specs"
Expand Down Expand Up @@ -140,7 +141,7 @@ func TestTrustlessRawRanges(t *testing.T) {
// correctly.
fixture := car.MustOpenUnixfsCar("gateway-raw-block.car")

tests := helpers.RangeTestTransform(t,
tests := helpers.OnlyRangeTests(t,
SugarTest{
Name: "GET with application/vnd.ipld.raw with range request includes correct bytes",
Request: Request().
Expand Down
125 changes: 47 additions & 78 deletions tooling/helpers/range.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,104 +3,73 @@ package helpers
import (
"fmt"
"net/http"
"strconv"
"strings"
"testing"

"github.com/ipfs/gateway-conformance/tooling/check"
"github.com/ipfs/gateway-conformance/tooling/test"
)

// ByteRange describes an HTTP range request and the data it corresponds to. "From" and "To" mostly
// follow [HTTP Byte Range] Request semantics:
//
// - From >= 0 and To = nil: Get the file (From, Length)
// - From >= 0 and To >= 0: Get the range (From, To)
// - From >= 0 and To <0: Get the range (From, Length - To)
//
// [HTTP Byte Range]: https://httpwg.org/specs/rfc9110.html#rfc.section.14.1.2
type ByteRange struct {
From uint64
To *int64
}

func SimpleByteRange(from, to uint64) ByteRange {
toInt := int64(to)
return ByteRange{
From: from,
To: &toInt,
}
}

func (b ByteRange) GetRangeString(t *testing.T) string {
strWithoutPrefix := b.getRangeStringWithoutPrefix(t)
return fmt.Sprintf("bytes=%s", strWithoutPrefix)
}

func (b ByteRange) getRangeStringWithoutPrefix(t *testing.T) string {
if b.To == nil {
return fmt.Sprintf("%d-", b.From)
}

to := *b.To
if to >= 0 {
return fmt.Sprintf("%d-%d", b.From, to)
}

if to < 0 && b.From != 0 {
t.Fatalf("for a suffix request the From field must be 0")
}
return fmt.Sprintf("%d", to)
}

func (b ByteRange) getRange(t *testing.T, totalSize int64) (uint64, uint64) {
if totalSize < 0 {
t.Fatalf("total size must be greater than 0")
// parseRange parses a ranges header in the format "bytes=from-to" and returns
// x and y as uint64.
func parseRange(t *testing.T, str string) (from, to uint64) {
if !strings.HasPrefix(str, "bytes=") {
t.Fatalf("byte range %s does not start with 'bytes='", str)
}

if b.To == nil {
return b.From, uint64(totalSize)
str = strings.TrimPrefix(str, "bytes=")
ranges := strings.Split(str, ",")
if len(ranges) != 1 {
t.Fatalf("byte range %s must have one range", str)
}

to := *b.To
if to >= 0 {
return b.From, uint64(to)
rng := strings.Split(ranges[0], "-")
if len(rng) != 2 {
t.Fatalf("byte range %s is invalid", str)
}

if to < 0 && b.From != 0 {
t.Fatalf("for a suffix request the From field must be 0")
var err error
from, err = strconv.ParseUint(rng[0], 10, 0)
if err != nil {
t.Fatalf("cannot parse range %s: %s", str, err.Error())
}

start := int64(totalSize) + to
if start < 0 {
t.Fatalf("suffix request must not start before the start of the file")
to, err = strconv.ParseUint(rng[1], 10, 0)
if err != nil {
t.Fatalf("cannot parse range %s: %s", str, err.Error())
}

return uint64(start), uint64(totalSize)
return from, to
}

type ByteRanges []ByteRange
func combineRanges(t *testing.T, ranges []string) string {
str := "bytes="

func (b ByteRanges) GetRangeString(t *testing.T) string {
var rangeStrs []string
for _, r := range b {
rangeStrs = append(rangeStrs, r.getRangeStringWithoutPrefix(t))
for i, rng := range ranges {
from, to := parseRange(t, rng)
str += fmt.Sprintf("%d-%d", from, to)
if i != len(ranges)-1 {
str += ","
}
}
return fmt.Sprintf("bytes=%s", strings.Join(rangeStrs, ","))

return str
}

// SingleRangeTestTransform takes a test where there is no "Range" header set in the request, or checks on the
// StatusCode, Body, or Content-Range headers and verifies whether a valid response is given for the requested range.
//
// Note: HTTP Range requests can be validly responded with either the full data, or the requested partial data
func SingleRangeTestTransform(t *testing.T, baseTest test.SugarTest, brange ByteRange, fullData []byte) test.SugarTest {
modifiedRequest := baseTest.Request.Clone().Header("Range", brange.GetRangeString(t))
// Note: HTTP Range requests can be validly responded with either the full data, or the requested partial data.
func SingleRangeTestTransform(t *testing.T, baseTest test.SugarTest, byteRange string, fullData []byte) test.SugarTest {
modifiedRequest := baseTest.Request.Clone().Header("Range", byteRange)
if baseTest.Requests != nil {
t.Fatal("does not support multiple requests or responses")
}
modifiedResponse := baseTest.Response.Clone()

fullSize := int64(len(fullData))
start, end := brange.getRange(t, fullSize)
start, end := parseRange(t, byteRange)

rangeTest := test.SugarTest{
Name: baseTest.Name,
Expand Down Expand Up @@ -128,9 +97,9 @@ func SingleRangeTestTransform(t *testing.T, baseTest test.SugarTest, brange Byte
// If contentType is empty it is ignored.
//
// Note: HTTP Multi Range requests can be validly responded with one of the full data, the partial data from the first
// range, or the partial data from all the requested ranges
func MultiRangeTestTransform(t *testing.T, baseTest test.SugarTest, branges ByteRanges, fullData []byte, contentType string) test.SugarTest {
modifiedRequest := baseTest.Request.Clone().Header("Range", branges.GetRangeString(t))
// range, or the partial data from all the requested ranges.
func MultiRangeTestTransform(t *testing.T, baseTest test.SugarTest, byteRanges []string, fullData []byte, contentType string) test.SugarTest {
modifiedRequest := baseTest.Request.Clone().Header("Range", combineRanges(t, byteRanges))
if baseTest.Requests != nil {
t.Fatal("does not support multiple requests or responses")
}
Expand All @@ -143,8 +112,8 @@ func MultiRangeTestTransform(t *testing.T, baseTest test.SugarTest, branges Byte

var multirangeBodyChecks []check.Check[string]
var ranges []rng
for _, r := range branges {
start, end := r.getRange(t, fullSize)
for _, r := range byteRanges {
start, end := parseRange(t, r)
ranges = append(ranges, rng{start: start, end: end})
multirangeBodyChecks = append(multirangeBodyChecks,
check.Contains("Content-Range: bytes {{start}}-{{end}}/{{length}}", ranges[0].start, ranges[0].end, fullSize),
Expand Down Expand Up @@ -190,7 +159,7 @@ func MultiRangeTestTransform(t *testing.T, baseTest test.SugarTest, branges Byte
// Note: HTTP Range requests can be validly responded with either the full data, or the requested partial data
// Note: HTTP Multi Range requests can be validly responded with one of the full data, the partial data from the first
// range, or the partial data from all the requested ranges
func IncludeRangeTests(t *testing.T, baseTest test.SugarTest, branges ByteRanges, fullData []byte, contentType string) test.SugarTests {
func IncludeRangeTests(t *testing.T, baseTest test.SugarTest, branges []string, fullData []byte, contentType string) test.SugarTests {
standardBaseRequest := baseTest.Request.Clone()
if contentType != "" {
standardBaseRequest = standardBaseRequest.Header("Content-Type", contentType)
Expand All @@ -206,11 +175,11 @@ func IncludeRangeTests(t *testing.T, baseTest test.SugarTest, branges ByteRanges
),
Responses: baseTest.Responses,
}
rangeTests := RangeTestTransform(t, baseTest, branges, fullData, contentType)
rangeTests := OnlyRangeTests(t, baseTest, branges, fullData, contentType)
return append(test.SugarTests{standardBase}, rangeTests...)
}

// RangeTestTransform takes a test where there is no "Range" header set in the request, or checks on the
// OnlyRangeTests takes a test where there is no "Range" header set in the request, or checks on the
// StatusCode, Body, or Content-Range headers and verifies whether a valid response is given for the requested ranges.
// Will test both a single range request for the first passed range as well as a multi-range request for all the
// requested ranges.
Expand All @@ -223,16 +192,16 @@ func IncludeRangeTests(t *testing.T, baseTest test.SugarTest, branges ByteRanges
// Note: HTTP Range requests can be validly responded with either the full data, or the requested partial data
// Note: HTTP Multi Range requests can be validly responded with one of the full data, the partial data from the first
// range, or the partial data from all the requested ranges
func RangeTestTransform(t *testing.T, baseTest test.SugarTest, branges ByteRanges, fullData []byte, contentType string) test.SugarTests {
func OnlyRangeTests(t *testing.T, baseTest test.SugarTest, branges []string, fullData []byte, contentType string) test.SugarTests {
if len(branges) == 0 {
dataLen := len(fullData)
if dataLen < 10 {
panic("transformation not defined for data smaller than 10 bytes")
}

branges = ByteRanges{
SimpleByteRange(7, 9),
SimpleByteRange(1, 3),
branges = []string{
"bytes=7-9",
"bytes=1-3",
}
}

Expand Down

0 comments on commit 29e356c

Please sign in to comment.