Skip to content

Commit

Permalink
Fix VirtualPath caching
Browse files Browse the repository at this point in the history
Bug was introduced during refactoring, most likely as performance
optimization. `isVirtual` check never run if `CacheAllSafeRequests` was
true.

In addition test re-written to use full HTTP stack, and included case
when caching turned on.

Should fix TykTechnologies#1356
  • Loading branch information
buger committed Dec 14, 2017
1 parent 5f124f0 commit 5fa2c20
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 68 deletions.
8 changes: 4 additions & 4 deletions analytics.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ import (
// AnalyticsRecord encodes the details of a request
type AnalyticsRecord struct {
Method string
Path string
RawPath string
Path string // HTTP path, can be overriden by "track path" plugin
RawPath string // Original HTTP path
ContentLength int64
UserAgent string
Day int
Expand All @@ -34,8 +34,8 @@ type AnalyticsRecord struct {
OrgID string
OauthID string
RequestTime int64
RawRequest string
RawResponse string
RawRequest string // Base64 encoded request data (if detailed recording turned on)
RawResponse string // ^ same but for response
IPAddress string
Geo GeoData
Tags []string
Expand Down
11 changes: 6 additions & 5 deletions mw_redis_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,15 +105,16 @@ func (m *RedisCacheMiddleware) ProcessRequest(w http.ResponseWriter, r *http.Req
}

var stat RequestStatus
var isVirtual bool

_, versionPaths, _, _ := m.Spec.Version(r)
isVirtual, _ := m.Spec.CheckSpecMatchesStatus(r, versionPaths, VirtualPath)

// Lets see if we can throw a sledgehammer at this
if m.Spec.CacheOptions.CacheAllSafeRequests {
stat = StatusCached
} else {
// New request checker, more targeted, less likely to fail
_, versionPaths, _, _ := m.Spec.Version(r)
found, _ := m.Spec.CheckSpecMatchesStatus(r, versionPaths, Cached)
isVirtual, _ = m.Spec.CheckSpecMatchesStatus(r, versionPaths, VirtualPath)
if found {
stat = StatusCached
}
Expand All @@ -136,8 +137,8 @@ func (m *RedisCacheMiddleware) ProcessRequest(w http.ResponseWriter, r *http.Req
}

key := m.CreateCheckSum(r, token)
retBlob, found := m.CacheStore.GetKey(key)
if found != nil {
retBlob, err := m.CacheStore.GetKey(key)
if err != nil {
log.Debug("Cache enabled, but record not found")
// Pass through to proxy AND CACHE RESULT

Expand Down
119 changes: 60 additions & 59 deletions mw_virtual_endpoint_test.go
Original file line number Diff line number Diff line change
@@ -1,89 +1,90 @@
package main

import (
"encoding/base64"
"io/ioutil"
"net/http/httptest"
"os"
"path/filepath"
"net/http"
"testing"
)

const virtTestDef = `{
"api_id": "1",
"definition": {
"location": "header",
"key": "version"
},
"auth": {"auth_header_name": "authorization"},
"version_data": {
"not_versioned": true,
"versions": {
"v1": {
"name": "v1",
"use_extended_paths": true,
"extended_paths": {
"virtual": [{
"response_function_name": "testVirtData",
"function_source_type": "file",
"function_source_uri": "middleware/testVirtData.js",
"path": "/test-data",
"method": "GET"
}]
}
}
}
},
"proxy": {
"listen_path": "/v1",
"target_url": "` + testHttpAny + `"
},
"config_data": {
"foo": "x",
"bar": {"y": 3}
}
}`
"github.com/TykTechnologies/tyk/apidef"
"github.com/TykTechnologies/tyk/config"
)

const virtTestJS = `
function testVirtData(request, session, config) {
var resp = {
Body: request.Body + " added body",
Body: "foobar",
Headers: {
"data-foo": config.config_data.foo,
"data-bar-y": config.config_data.bar.y.toString()
},
Code: 202
}
return TykJsResponse(resp, session.meta_data)
return TykJsResponse(resp, session.meta_data)
}
`

func TestVirtualEndpoint(t *testing.T) {
mwPath := filepath.Join("middleware", "testVirtData.js")
if err := ioutil.WriteFile(mwPath, []byte(virtTestJS), 0644); err != nil {
config.Global.ListenAddress = "127.0.0.1"

ln, _ := generateListener(0)
baseURL := "http://" + ln.Addr().String()
listen(ln, nil, nil)
defer func() {
config.Global.ListenAddress = ""
ln.Close()
}()

buildAndLoadAPI(func(spec *APISpec) {
spec.Proxy.ListenPath = "/"

virtualMeta := apidef.VirtualMeta{
ResponseFunctionName: "testVirtData",
FunctionSourceType: "blob",
FunctionSourceURI: base64.StdEncoding.EncodeToString([]byte(virtTestJS)),
Path: "/virt",
Method: "GET",
}
v := spec.VersionData.Versions["v1"]
v.UseExtendedPaths = true
v.ExtendedPaths = apidef.ExtendedPathsSet{
Virtual: []apidef.VirtualMeta{virtualMeta},
}
spec.VersionData.Versions["v1"] = v

spec.ConfigData = map[string]interface{}{
"foo": "x",
"bar": map[string]interface{}{"y": 3},
}

// Address https://github.com/TykTechnologies/tyk/issues/1356
// VP should work with cache enabled
spec.CacheOptions = apidef.CacheOptions{
EnableCache: true,
CacheTimeout: 60,
CacheAllSafeRequests: true,
}
})

resp, err := http.Get(baseURL + "/virt")
if err != nil {
t.Fatal(err)
}
spec := createSpecTest(t, virtTestDef)
defer os.Remove(mwPath)

virt := &VirtualEndpoint{BaseMiddleware: BaseMiddleware{
spec, nil,
}}
virt.Init()
rec := httptest.NewRecorder()
r := testReq(t, "GET", "/v1/test-data", "initial body")
virt.ProcessRequest(rec, r, nil)
if want := 202; rec.Code != 202 {
t.Fatalf("wanted code to be %d, got %d", want, rec.Code)
if want := 202; resp.StatusCode != 202 {
t.Fatalf("wanted code to be %d, got %d", want, resp.StatusCode)
}
wantBody := "initial body added body"
gotBody := rec.Body.String()
if wantBody != gotBody {
t.Fatalf("wanted body to be %q, got %q", wantBody, gotBody)

wantBody := "foobar"
gotBody, _ := ioutil.ReadAll(resp.Body)

if wantBody != string(gotBody) {
t.Fatalf("wanted body to be %q, got %q", wantBody, string(gotBody))
}
if want, got := "x", rec.HeaderMap.Get("data-foo"); got != want {
if want, got := "x", resp.Header.Get("data-foo"); got != want {
t.Fatalf("wanted header to be %q, got %q", want, got)
}
if want, got := "3", rec.HeaderMap.Get("data-bar-y"); got != want {
if want, got := "3", resp.Header.Get("data-bar-y"); got != want {
t.Fatalf("wanted header to be %q, got %q", want, got)
}
}

0 comments on commit 5fa2c20

Please sign in to comment.