Skip to content

Commit

Permalink
Optimize parsing of daterange
Browse files Browse the repository at this point in the history 
Additional changes:
 * make correction according to @maeb's review
 * additional tests cases
  • Loading branch information
@Avokadoen
Avokadoen committed Jul 15, 2021
1 parent c2a838f commit 4b5468f
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 73 deletions.
91 changes: 32 additions & 59 deletions pkg/server/warcserver/daterange.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package warcserver
import (
"fmt"
"math"
"strings"
"time"
)

Expand All @@ -33,11 +32,11 @@ const timeLayout = "20060102150405"
func NewDateRange(fromstr string, tostr string) (DateRange, error) {
from, err := From(fromstr)
if err != nil {
return DateRange{0, 0}, err
return DateRange{}, err
}
to, err := To(tostr)
if err != nil {
return DateRange{0, 0}, err
return DateRange{}, err
}

return DateRange{
Expand All @@ -58,89 +57,63 @@ func (d DateRange) contains(ts string) (bool, error) {
return unixTs >= d.from && unixTs <= d.to, nil
}

// Implemented according to https://pywb.readthedocs.io/en/latest/manual/cdxserver_api.html#from-to
// Implemented according to https://pywb.readthedocs.io/en/latest/manual/cdxserver_api.html#from-to:
func From(f string) (int64, error) {
fLen := len(f)
if fLen%2 != 0 {
return 0, fmt.Errorf("'from' string was not an odd number, len: %d", fLen)
return 0, fmt.Errorf("'from' string was an odd number, len: %d", fLen)
}
if fLen > 14 {
return 0, fmt.Errorf("expected from string len less than 14, len: %d", fLen)
return 0, fmt.Errorf("expected 'from' string len less than 14, len: %d", fLen)
}

// No specified from date
if fLen < 4 {
return math.MinInt64, nil
}

builder := strings.Builder{}
builder.Grow(14)
builder.WriteString(f)
if builder.Len() <= 4 {
builder.WriteString("01")
}
if builder.Len() <= 6 {
builder.WriteString("01")
}
if builder.Len() <= 8 {
builder.WriteString("00")
}
if builder.Len() <= 10 {
builder.WriteString("00")
}
if builder.Len() <= 12 {
builder.WriteString("00")
return time.Time{}.Unix(), nil
}

date := builder.String()
from, err := time.Parse(timeLayout, date)
from, err := time.Parse(timeLayout[:fLen], f)
if err != nil {
return 0, fmt.Errorf("failed to parse 'from' date %s, %w", date, err)
return 0, fmt.Errorf("failed to parse 'from' date %s, %w", f, err)
}

return from.Unix(), nil
}

// Implemented according to https://pywb.readthedocs.io/en/latest/manual/cdxserver_api.html#from-to:
func To(t string) (int64, error) {
fLen := len(t)
if fLen%2 != 0 {
return 0, fmt.Errorf("'to' string was not an odd number, len: %d", fLen)
tLen := len(t)
if tLen%2 != 0 {
return 0, fmt.Errorf("'to' string was an odd number, len: %d", tLen)
}
if fLen > 14 {
return 0, fmt.Errorf("expected from string len less than 14, len: %d", fLen)
if tLen > 14 {
return 0, fmt.Errorf("expected 'to' string len less than 14, len: %d", tLen)
}

// No specified from date
if fLen < 4 {
if tLen < 4 {
return math.MaxInt64, nil
}

builder := strings.Builder{}
builder.Grow(14)
builder.WriteString(t)
if builder.Len() <= 4 {
builder.WriteString("12")
}
// Assumption: there is no harm in having a timestamp with a month with less than 31 days
// be assigned 31 days
if builder.Len() <= 6 {
builder.WriteString("31")
}
if builder.Len() <= 8 {
builder.WriteString("23")
}
if builder.Len() <= 10 {
builder.WriteString("59")
}
if builder.Len() <= 12 {
builder.WriteString("59")
}

date := builder.String()
to, err := time.Parse(timeLayout, date)
to, err := time.Parse(timeLayout[:tLen], t)
if err != nil {
return 0, fmt.Errorf("failed to parse 'to' date %s, %w", date, err)
return 0, fmt.Errorf("failed to parse 'to' date %s, %w", t, err)
}

switch tLen {
case 4:
to = to.AddDate(0, 12, -1) // 31 days in decemeber
to = to.Add(time.Hour*23 + time.Minute*59 + time.Second*59)
case 6:
// add one month - one day, i.e: user supplies january, we add 29 - 1
to = to.AddDate(0, 1, -1)
to = to.Add(time.Hour*23 + time.Minute*59 + time.Second*59)
case 8:
to = to.Add(time.Hour*23 + time.Minute*59 + time.Second*59)
case 10:
to = to.Add(time.Minute*59 + time.Second*59)
case 12:
to = to.Add(time.Second * 59)
}

return to.Unix(), nil
Expand Down
43 changes: 29 additions & 14 deletions pkg/server/warcserver/daterange_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,20 @@ package warcserver
import (
"math"
"testing"
"time"
)

type dateRangeTestData struct {
name string
daterange DateRange
timestamp string
datestr string
expect bool
}

func TestValidDateRangeContains(t *testing.T) {
tests := []dateRangeTestData{
{
"'timestamp' in range returns true",
"date string in range returns true",
DateRange{
from: 0,
to: 60,
Expand All @@ -24,7 +25,7 @@ func TestValidDateRangeContains(t *testing.T) {
true,
},
{
"'timestamp' same as 'from' returns true",
"date string same as 'from' returns true",
DateRange{
from: 0,
to: 60,
Expand All @@ -33,7 +34,7 @@ func TestValidDateRangeContains(t *testing.T) {
true,
},
{
"'timestamp' same as 'to' returns true",
"date string same as 'to' returns true",
DateRange{
from: 0,
to: 60,
Expand All @@ -42,7 +43,7 @@ func TestValidDateRangeContains(t *testing.T) {
true,
},
{
"'timestamp' below range returns false",
"date string below range returns false",
DateRange{
from: 59,
to: 60,
Expand All @@ -51,7 +52,7 @@ func TestValidDateRangeContains(t *testing.T) {
false,
},
{
"'timestamp' above range returns false",
"date string above range returns false",
DateRange{
from: 0,
to: 1,
Expand All @@ -63,7 +64,7 @@ func TestValidDateRangeContains(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
contains, err := tt.daterange.contains(tt.timestamp)
contains, err := tt.daterange.contains(tt.datestr)
if err != nil {
t.Errorf("Unexpected error: %s", err)
}
Expand All @@ -76,7 +77,7 @@ func TestValidDateRangeContains(t *testing.T) {

func TestInvalidContainData(t *testing.T) {
test := dateRangeTestData{
"invalid 'to' value fails",
"invalid date string value fails",
DateRange{
from: 0,
to: 60,
Expand All @@ -86,7 +87,7 @@ func TestInvalidContainData(t *testing.T) {
}

t.Run(test.name, func(t *testing.T) {
contains, err := test.daterange.contains(test.timestamp)
contains, err := test.daterange.contains(test.datestr)
if err == nil {
t.Errorf("Expected error, got %v", err)
}
Expand All @@ -97,7 +98,7 @@ func TestInvalidContainData(t *testing.T) {
})
}

func TestInFromAndToParsing(t *testing.T) {
func TestFromAndToParsing(t *testing.T) {
tests := []struct {
name string
fromAndTo string
Expand All @@ -113,16 +114,30 @@ func TestInFromAndToParsing(t *testing.T) {
false,
},
{
"valid partial date string succeeds",
"missing seconds date string succeeds",
"197001010000",
0,
59,
false,
},
{
"missing minutes date string succeeds",
"1970010100",
0,
59*60 + 59,
false,
},
{
"missing hours date string succeeds",
"19700101",
0,
23*60*60 + 60*59 + 59,
false,
},
{
"empty date string succeeds",
"",
math.MinInt64,
time.Time{}.Unix(),
math.MaxInt64,
false,
},
Expand Down Expand Up @@ -156,14 +171,14 @@ func TestInFromAndToParsing(t *testing.T) {
t.Errorf("Unexpected 'from' error: %s", err)
}
if from != tt.expectedFrom {
t.Errorf("Expected %d, got %d", tt.expectedFrom, from)
t.Errorf("From expected %d, got %d", tt.expectedFrom, from)
}
to, err := To(tt.fromAndTo)
if err != nil && !tt.expectError {
t.Errorf("Unexpected 'to' error: %s", err)
}
if to != tt.expectedTo {
t.Errorf("Expected %d, got %d", tt.expectedTo, to)
t.Errorf("To expected %d, got %d", tt.expectedTo, to)
}
})
}
Expand Down

0 comments on commit 4b5468f

Please sign in to comment.