Skip to content

Commit

Permalink
fix: query result cache key is not stable
Browse files Browse the repository at this point in the history
  • Loading branch information
visca authored and Phu Tran committed May 9, 2022
1 parent 998a7f2 commit 0f762d8
Show file tree
Hide file tree
Showing 9 changed files with 88 additions and 51 deletions.
2 changes: 1 addition & 1 deletion caching.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ func (c *Caching) Provision(ctx caddy.Context) error {

func (c *Caching) Validate() error {
for ruleName, rule := range c.Rules {
for vary := range rule.Varies {
for _, vary := range rule.Varies {
if _, ok := c.Varies[vary]; !ok {
return fmt.Errorf("caching rule %s, configured vary: %s does not exist", ruleName, vary)
}
Expand Down
6 changes: 3 additions & 3 deletions caching_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,12 +142,12 @@ func (c *Caching) addCachingResponseHeaders(s CachingStatus, r *cachingQueryResu

uniqueVaries := make(map[string]struct{})

for name := range p.VaryNames {
for v := range c.Varies[name].Headers {
for _, name := range p.VaryNames {
for _, v := range c.Varies[name].Headers {
uniqueVaries[v] = struct{}{}
}

for v := range c.Varies[name].Cookies {
for _, v := range c.Varies[name].Cookies {
uniqueVaries[fmt.Sprintf("cookie:%s", v)] = struct{}{}
}
}
Expand Down
14 changes: 6 additions & 8 deletions caching_plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ var (
type cachingPlan struct {
MaxAge caddy.Duration
Swr caddy.Duration
VaryNames map[string]struct{}
VaryNames []string
Types map[string]struct{}
RulesHash uint64
VariesHash uint64
Expand Down Expand Up @@ -158,7 +158,7 @@ func (p *cachingPlanner) savePlan(plan *cachingPlan) error {

func (p *cachingPlanner) computePlan() (*cachingPlan, error) {
types := make(map[string]struct{})
varyNames := make(map[string]struct{})
var varyNames []string
plan := &cachingPlan{
Passthrough: true,
}
Expand Down Expand Up @@ -191,9 +191,7 @@ func (p *cachingPlanner) computePlan() (*cachingPlan, error) {
plan.Swr = rule.Swr
}

for vary := range rule.Varies {
varyNames[vary] = struct{}{}
}
varyNames = append(varyNames, rule.Varies...)

if rule.Types == nil {
types = nil
Expand Down Expand Up @@ -242,22 +240,22 @@ func (p *cachingPlanner) calcQueryResultCacheKey(plan *cachingPlan) (string, err

r := p.request.httpRequest

for name := range plan.VaryNames {
for _, name := range plan.VaryNames {
vary, ok := p.caching.Varies[name]

if !ok {
return "", fmt.Errorf("setting of vary %s does not exist in varies list given", vary)
}

for name := range vary.Headers {
for _, name := range vary.Headers {
buffString := fmt.Sprintf("header:%s=%s;", name, r.Header.Get(name))

if _, err := hash.Write([]byte(buffString)); err != nil {
return "", err
}
}

for name := range vary.Cookies {
for _, name := range vary.Cookies {
var value string
cookie, err := r.Cookie(name)

Expand Down
2 changes: 1 addition & 1 deletion caching_rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ type CachingRule struct {

// Varies name apply to query results that match the rule types.
// If not set query results will cache public.
Varies map[string]struct{} `json:"varies,omitempty"`
Varies []string `json:"varies,omitempty"`
}

type CachingRules map[string]*CachingRule
Expand Down
8 changes: 2 additions & 6 deletions caching_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,7 @@ func TestCaching_Validate(t *testing.T) {
Rules: CachingRules{
"default": &CachingRule{
MaxAge: 1,
Varies: map[string]struct{}{
"test": {},
},
Varies: []string{"test"},
},
},
},
Expand All @@ -72,9 +70,7 @@ func TestCaching_Validate(t *testing.T) {
Rules: CachingRules{
"default": &CachingRule{
MaxAge: 1,
Varies: map[string]struct{}{
"test": {},
},
Varies: []string{"test"},
},
},
},
Expand Down
4 changes: 2 additions & 2 deletions caching_vary.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ import (
// CachingVary using to compute query result cache key by http request cookies and headers.
type CachingVary struct {
// Headers names for identifier query result cache key.
Headers map[string]struct{} `json:"headers,omitempty"`
Headers []string `json:"headers,omitempty"`

// Cookies names for identifier query result cache key.
Cookies map[string]struct{} `json:"cookies,omitempty"`
Cookies []string `json:"cookies,omitempty"`
}

type CachingVaries map[string]*CachingVary
Expand Down
4 changes: 1 addition & 3 deletions caching_vary_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,7 @@ func TestCachingVariesHash(t *testing.T) {

varies = CachingVaries{
"default": &CachingVary{
Cookies: map[string]struct{}{
"session": {},
},
Cookies: []string{"session"},
},
}

Expand Down
32 changes: 5 additions & 27 deletions caddyfile_caching.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,17 +129,7 @@ func (c *Caching) unmarshalCaddyfileRules(d *caddyfile.Dispenser) error {
return d.ArgErr()
}

varies := make(map[string]struct{})

for _, arg := range args {
if _, exists := varies[arg]; exists {
return d.Errf("duplicate vary: %s", arg)
}

varies[arg] = struct{}{}
}

rule.Varies = varies
rule.Varies = args
default:
return d.Errf("unrecognized subdirective %s", d.Val())
}
Expand Down Expand Up @@ -188,8 +178,8 @@ func (c *Caching) unmarshalCaddyfileVaries(d *caddyfile.Dispenser) error {
for d.NextBlock(0) {
name := d.Val()
vary := &CachingVary{
Headers: make(map[string]struct{}),
Cookies: make(map[string]struct{}),
Headers: []string{},
Cookies: []string{},
}

for subNesting := d.Nesting(); d.NextBlock(subNesting); {
Expand All @@ -201,27 +191,15 @@ func (c *Caching) unmarshalCaddyfileVaries(d *caddyfile.Dispenser) error {
return d.ArgErr()
}

for _, arg := range args {
if _, exists := vary.Headers[arg]; exists {
return d.Errf("duplicate header: %s", arg)
}

vary.Headers[arg] = struct{}{}
}
vary.Headers = args
case "cookies":
args := d.RemainingArgs()

if len(args) == 0 {
return d.ArgErr()
}

for _, arg := range args {
if _, exists := vary.Cookies[arg]; exists {
return d.Errf("duplicate cookie: %s", arg)
}

vary.Cookies[arg] = struct{}{}
}
vary.Cookies = args
default:
return d.Errf("unrecognized subdirective %s", d.Val())
}
Expand Down
67 changes: 67 additions & 0 deletions handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -745,6 +745,73 @@ caching {
}
}

func (s *HandlerIntegrationTestSuite) TestCombinedCachingVaries() {
const payload = `{"query": "query { users { name } }"}`
const config = `
caching {
rules {
test {
max_age 60ms
swr 60ms
varies first second
}
}
varies {
first {
headers x-first-header-1 x-first-header-2
cookies x-first-cookie-1 x-first-cookie-2
}
second {
headers x-second-header-1 x-second-header-2
cookies x-second-cookie-1 x-second-cookie-2
}
}
}
`
tester := caddytest.NewTester(s.T())
tester.InitServer(pureCaddyfile, "caddyfile")
tester.InitServer(fmt.Sprintf(caddyfilePattern, config), "caddyfile")

// test cache hit for 20 times
for i := 0; i <= 20; i++ {
expectedCachingStatus := CachingStatusHit
expectedHitTimes := strconv.Itoa(i)
expectedBody := `{"data":{"users":[{"name":"A"},{"name":"B"},{"name":"C"}]}}`
if i == 0 {
expectedCachingStatus = CachingStatusMiss
expectedHitTimes = ""
}

r, _ := http.NewRequest(
"POST",
"http://localhost:9090/graphql",
strings.NewReader(payload),
)
r.Header.Add("content-type", "application/json")

r.Header.Set("x-first-header-1", "x-first-header-1")
r.Header.Set("x-first-header-2", "x-first-header-2")
r.Header.Set("x-second-header-1", "x-second-header-1")
r.Header.Set("x-second-header-2", "x-second-header-2")

r.AddCookie(&http.Cookie{Name: "x-first-cookie-1", Value: "x-first-cookie-1"})
r.AddCookie(&http.Cookie{Name: "x-first-cookie-2", Value: "x-first-cookie-2"})
r.AddCookie(&http.Cookie{Name: "x-second-cookie-1", Value: "x-second-cookie-1"})
r.AddCookie(&http.Cookie{Name: "x-second-cookie-2", Value: "x-second-cookie-2"})

resp := tester.AssertResponseCode(r, http.StatusOK)
respBody, _ := io.ReadAll(resp.Body)
actualStatus := resp.Header.Get("x-cache")
actualHitTimes := resp.Header.Get("x-cache-hits")

s.Require().Equalf(expectedBody, string(respBody), "unexpected payload")
s.Require().Equalf(string(expectedCachingStatus), actualStatus, "unexpected status")
s.Require().Equalf(expectedHitTimes, actualHitTimes, "unexpected hit times")

resp.Body.Close()
}
}

func (s *HandlerIntegrationTestSuite) TestEnabledPlaygrounds() {
tester := caddytest.NewTester(s.T())
tester.InitServer(pureCaddyfile, "caddyfile")
Expand Down

0 comments on commit 0f762d8

Please sign in to comment.