nic-6443 commented on code in PR #14061:
URL: https://github.com/apache/apisix/pull/14061#discussion_r4334776823
##########
apisix/plugins/oas-validator.lua:
##########
@@ -176,6 +183,22 @@ local function fetch_and_compile(conf)
return validator
end
+local function create_spec_url_entry(conf, cache_key, ttl)
+ local entry = shared_spec_url_entries[cache_key]
+ if entry and ngx_now() - entry.fetched_at < ttl then
+ return entry
+ end
+
+ local validator, err = fetch_and_compile(conf)
+ if not validator then
+ return nil, err
+ end
+
+ entry = {validator = validator, fetched_at = ngx_now()}
+ shared_spec_url_entries[cache_key] = entry
+ return entry
+end
Review Comment:
Good catch. The lrucache only serialises per resource slot, so resources
sharing a cold URL would each fetch and keep their own copy until the next
refresh. Fetch and compile are now serialised per URL key with the same
`lrucache-lock` that `core.lrucache` uses for `serial_creating`, and the shared
table is re-checked after taking the lock. TEST 28 hits three routes on one
cold URL concurrently and asserts a single fetch (it gets 3 without the lock).
##########
t/plugin/oas-validator3.t:
##########
@@ -562,3 +573,244 @@ status: 200
}
--- response_body
passed
+
+
+
+=== TEST 24: changing spec_url on a route releases the previously compiled spec
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin")
+ local http = require("resty.http")
+ local fetches = ngx.shared.oas_spec_fetches
+
+ local function put_route(id, spec_url)
+ local code, body = t.test('/apisix/admin/routes/' .. id,
ngx.HTTP_PUT, {
+ uri = "/api/v3/pet",
+ host = "r" .. id .. ".test",
+ plugins = {["oas-validator"] = {spec_url = spec_url}},
+ upstream = {type = "roundrobin", nodes =
{["127.0.0.1:1970"] = 1}},
+ })
+ assert(code < 300, body)
+ ngx.sleep(0.5)
+ end
+
+ local function hit(id)
+ local res, err = http.new():request_uri(
+ "http://127.0.0.1:"; .. ngx.var.server_port ..
"/api/v3/pet", {
+ method = "POST",
+ body = '{"id": 10, "name": "doggie", "photoUrls":
["string"]}',
+ headers = {
+ ["Content-Type"] = "application/json",
+ ["Host"] = "r" .. id .. ".test",
+ },
+ })
+ assert(res and res.status == 200, err or (res and res.status))
+ end
+
+ local function full_gc()
+ collectgarbage()
+ collectgarbage()
+ end
+ put_route(1, "http://127.0.0.1:1979/counted/a.json";)
+ hit(1)
+ put_route(1, "http://127.0.0.1:1979/counted/b.json";)
+ hit(1)
+ full_gc()
+
+ -- nothing references a.json any more, so another route declaring
+ -- it has to fetch it again
+ put_route(2, "http://127.0.0.1:1979/counted/a.json";)
+ hit(2)
+
+ ngx.say("a: ", fetches:get("/counted/a.json"))
+ ngx.say("b: ", fetches:get("/counted/b.json"))
+
+ t.test('/apisix/admin/routes/1', ngx.HTTP_DELETE)
+ t.test('/apisix/admin/routes/2', ngx.HTTP_DELETE)
+ }
+ }
+--- response_body
+a: 2
+b: 1
+
+
+
+=== TEST 25: routes declaring the same spec_url share one compiled spec
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin")
+ local http = require("resty.http")
+ local fetches = ngx.shared.oas_spec_fetches
+
+ local function put_route(id, spec_url)
+ local code, body = t.test('/apisix/admin/routes/' .. id,
ngx.HTTP_PUT, {
+ uri = "/api/v3/pet",
+ host = "r" .. id .. ".test",
+ plugins = {["oas-validator"] = {spec_url = spec_url}},
+ upstream = {type = "roundrobin", nodes =
{["127.0.0.1:1970"] = 1}},
+ })
+ assert(code < 300, body)
+ ngx.sleep(0.5)
+ end
+
+ local function hit(id)
+ local res, err = http.new():request_uri(
+ "http://127.0.0.1:"; .. ngx.var.server_port ..
"/api/v3/pet", {
+ method = "POST",
+ body = '{"id": 10, "name": "doggie", "photoUrls":
["string"]}',
+ headers = {
+ ["Content-Type"] = "application/json",
+ ["Host"] = "r" .. id .. ".test",
+ },
+ })
+ assert(res and res.status == 200, err or (res and res.status))
+ end
+
+ local function full_gc()
+ collectgarbage()
+ collectgarbage()
+ end
Review Comment:
Leaving this as is: every test-nginx block in this file is self-contained so
each one can be read and run on its own, and the new blocks follow that
convention.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
