Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: query result cache key is not stable #32

Merged
merged 1 commit into from
May 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
16 changes: 5 additions & 11 deletions caching_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,22 +140,16 @@ func (c *Caching) addCachingResponseHeaders(s CachingStatus, r *cachingQueryResu
return
}

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

for name := range p.VaryNames {
for v := range c.Varies[name].Headers {
uniqueVaries[v] = struct{}{}
for _, name := range p.VaryNames {
for _, v := range c.Varies[name].Headers {
h.Add("vary", v)
}

for v := range c.Varies[name].Cookies {
uniqueVaries[fmt.Sprintf("cookie:%s", v)] = struct{}{}
for _, v := range c.Varies[name].Cookies {
h.Add("vary", fmt.Sprintf("cookie:%s", v))
}
}

for vary := range uniqueVaries {
h.Add("vary", vary)
}

if s == CachingStatusHit {
age := int64(r.Age().Seconds())
maxAge := int64(time.Duration(r.MaxAge).Seconds())
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