From 7bb20ea9e7c4d06b17f14df5a21a863fbe78f0de Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Fri, 22 Nov 2024 16:52:04 +0800 Subject: [PATCH] feat(dbless): restore the removed tags based query support (#13907) KAG-5725 --------- Co-authored-by: chronolaw Co-authored-by: xumin --- kong-3.9.0-0.rockspec | 1 + kong/db/strategies/off/init.lua | 159 +++++++++++++++++- kong/db/strategies/off/tags.lua | 19 +++ .../04-admin_api/14-tags_spec.lua | 88 +++++----- 4 files changed, 215 insertions(+), 52 deletions(-) create mode 100644 kong/db/strategies/off/tags.lua diff --git a/kong-3.9.0-0.rockspec b/kong-3.9.0-0.rockspec index cf323196e303..a3c618357312 100644 --- a/kong-3.9.0-0.rockspec +++ b/kong-3.9.0-0.rockspec @@ -301,6 +301,7 @@ build = { ["kong.db.strategies.postgres.plugins"] = "kong/db/strategies/postgres/plugins.lua", ["kong.db.strategies.off"] = "kong/db/strategies/off/init.lua", ["kong.db.strategies.off.connector"] = "kong/db/strategies/off/connector.lua", + ["kong.db.strategies.off.tags"] = "kong/db/strategies/off/tags.lua", ["kong.db.migrations.state"] = "kong/db/migrations/state.lua", ["kong.db.migrations.subsystems"] = "kong/db/migrations/subsystems.lua", diff --git a/kong/db/strategies/off/init.lua b/kong/db/strategies/off/init.lua index d8a3e3e58b28..1fab71dac502 100644 --- a/kong/db/strategies/off/init.lua +++ b/kong/db/strategies/off/init.lua @@ -112,7 +112,7 @@ local function select_by_key(schema, key, follow) end -local function page_for_prefix(self, prefix, size, offset, options, follow) +local function page_for_prefix(self, prefix, size, offset, options, follow, schema) if not size then size = self.connector:get_page_size(options) end @@ -126,7 +126,7 @@ local function page_for_prefix(self, prefix, size, offset, options, follow) local ret = {} local ret_idx = 0 - local schema = self.schema + local schema = schema or self.schema local last_key for _, kv in ipairs(res) do @@ -157,6 +157,154 @@ local function page_for_prefix(self, prefix, size, offset, options, follow) end +-- Define the filter logic +local function matches_condition(item_tags, tags, tags_cond) + local matches = {} + for _, tag in ipairs(tags) do + matches[tag] = false + end + + -- Mark matches + for _, item_tag in ipairs(item_tags) do + if matches[item_tag] ~= nil then + matches[item_tag] = true + end + end + + -- Evaluate the condition + if tags_cond == "and" then + for _, matched in pairs(matches) do + if not matched then + return false + end + end + return true + end + + if tags_cond == "or" then + for _, matched in pairs(matches) do + if matched then + return true + end + end + return false + end +end + + +-- AI generated function: +-- +-- This function filters a list of items based on a set of tags and a condition +-- ("and"/"or"). +-- +-- @items : a Lua array, where each item has a `tags` array indicating its +-- associated tags, e.g., { "admin", "private" }. +-- @tags : a Lua array containing tag names, e.g., { "admin", "public" }. +-- @tags_cond : specifies the condition for tag matching: "and" requires all tags +-- to match, while "or" requires at least one tag to match. +local function filter_tag_items(items, tags, tags_cond) + assert(tags_cond == "and" or tags_cond == "or") + + -- Filter the items + local filtered_items = {} + for _, item in ipairs(items) do + if item.tags and matches_condition(item.tags, tags, tags_cond) then + table.insert(filtered_items, item) + end + end + + return filtered_items +end + + +-- Tags are a global concept across workspaces, there we don't need to handle +-- ws_id here. +local function page_for_tags(self, size, offset, options) + -- /:entitiy?tags=:tags + -- search all key-values: |*|*| => actual item key + if self.schema.name ~= "tags" then + local prefix = item_key_prefix(self.schema.name, "*") -- "|*|*|" + local items, err, offset = page_for_prefix(self, prefix, size, offset, + options, true) + if not items then + return nil, err + end + + items = filter_tag_items(items, options.tags, options.tags_cond) + + return items, err, offset + end + + -- /tags + -- /tags/:tags + local matched_tags = nil + + if options.tags then + matched_tags = {} + for _, tag in ipairs(options.tags) do + matched_tags[tag] = true + end + end + + -- Each page operation retrieves the entities of only one DAO type. + local schema_name, offset_token, dao + + if offset then + schema_name, offset_token = offset:match("^([^|]+)|(.+)") + if not schema_name then + return nil, self.errors:invalid_offset(offset, "bad offset string") + end + + if offset_token == "nil" then + offset_token = nil + + else + offset_token = decode_base64(offset_token) + if not offset_token then + return nil, self.errors:invalid_offset(offset_token, "bad base64 encoding") + end + end + end + + if offset_token then + -- There are still some entities left from the last page operation that + -- haven't been retrieved, so we need to use the previous dao + dao = kong.db.daos[schema_name] + else + schema_name, dao = next(kong.db.daos, schema_name) + end + + local prefix = item_key_prefix(schema_name, "*") + + local rows, err + + rows, err, offset_token = page_for_prefix(self, prefix, size, offset_token, + options, true, dao.schema) + if not rows then + return nil, err + end + + local items = {} + for _, item in ipairs(rows) do + for _, tag in ipairs(item.tags or {}) do + -- TODO: Could item.id be used as entity_id precisely? + if not matched_tags or matched_tags[tag] then + local item = { tag = tag, entity_name = schema_name, entity_id = item.id } + table.insert(items, item) + end + end + end + + if not offset_token and not next(kong.db.daos, schema_name) then -- end + offset = nil + else + offset = schema_name .. "|" .. (offset_token or "nil") + end + + return items, nil, offset +end + + local function page(self, size, offset, options) local schema = self.schema local ws_id = workspace_id(schema, options) @@ -171,6 +319,11 @@ local function page(self, size, offset, options) offset = token end + -- Used by /:entity?tags=:tag endpoint + if options and options.tags then + return page_for_tags(self, size, offset, options) + end + return page_for_prefix(self, prefix, size, offset, options, need_follow(ws_id)) end @@ -246,6 +399,8 @@ do _mt.upsert_by_field = unsupported_by("create or update") _mt.delete_by_field = unsupported_by("remove") _mt.truncate = function() return true end + + _mt.page_for_tags = page_for_tags end diff --git a/kong/db/strategies/off/tags.lua b/kong/db/strategies/off/tags.lua new file mode 100644 index 000000000000..d11789bd129b --- /dev/null +++ b/kong/db/strategies/off/tags.lua @@ -0,0 +1,19 @@ +local Tags = {} + + +-- Used by /tags/:tag endpoint +-- @tparam string tag_pk the tag value +-- @treturn table|nil,err,offset +function Tags:page_by_tag(tag, size, offset, options) + options.tags = { tag, } + return self:page_for_tags(size, offset, options) +end + + +-- Used by /tags endpoint +function Tags:page(size, offset, options) + return self:page_for_tags(size, offset, options) +end + + +return Tags diff --git a/spec/02-integration/04-admin_api/14-tags_spec.lua b/spec/02-integration/04-admin_api/14-tags_spec.lua index 99bb91d9adca..ff8247a6b4cf 100644 --- a/spec/02-integration/04-admin_api/14-tags_spec.lua +++ b/spec/02-integration/04-admin_api/14-tags_spec.lua @@ -6,7 +6,7 @@ local cjson = require "cjson" -- This test we test on the correctness of the admin API response so that -- we can ensure the right function (page()) is executed. describe("Admin API - tags", function() - for _, strategy in helpers.each_strategy() do + for _, strategy in helpers.all_strategies() do describe("/entities?tags= with DB: #" .. strategy, function() local client, bp @@ -200,34 +200,42 @@ describe("Admin API - tags", function() helpers.stop_kong() end) + -- lmdb tags will output pagenated list of entities + local function pagenated_get_json_data(path) + local data = {} + + while path and path ~= ngx.null do + local res = assert(client:send { method = "GET", path = path, }) + local body = assert.res_status(200, res) + local json = cjson.decode(body) + + if strategy ~= "off" then -- off strategy (lmdb) needs pagenation + return json.data + end + + for _, v in ipairs(json.data) do + table.insert(data, v) + end + + path = json.next + end + + return data + end + it("/tags", function() - local res = assert(client:send { - method = "GET", - path = "/tags" - }) - local body = assert.res_status(200, res) - local json = cjson.decode(body) - assert.equals(4, #json.data) + local data = pagenated_get_json_data("/tags") + assert.equals(4, #data) end) it("/tags/:tags", function() - local res = assert(client:send { - method = "GET", - path = "/tags/corp_%20a" - }) - local body = assert.res_status(200, res) - local json = cjson.decode(body) - assert.equals(2, #json.data) + local data = pagenated_get_json_data("/tags/corp_%20a") + assert.equals(2, #data) end) it("/tags/:tags with a not exist tag", function() - local res = assert(client:send { - method = "GET", - path = "/tags/does-not-exist" - }) - local body = assert.res_status(200, res) - local ok = string.find(body, '"data":[]', nil, true) - assert.truthy(ok) + local data = pagenated_get_json_data("/tags/does-not-exist") + assert.equals(0, #data) end) it("/tags/:tags with invalid :tags value", function() @@ -241,39 +249,19 @@ describe("Admin API - tags", function() end) it("/tags ignores ?tags= query", function() - local res = assert(client:send { - method = "GET", - path = "/tags?tags=not_a_tag" - }) - local body = assert.res_status(200, res) - local json = cjson.decode(body) - assert.equals(4, #json.data) + local data = pagenated_get_json_data("/tags?tags=not_a_tag") + assert.equals(4, #data) - local res = assert(client:send { - method = "GET", - path = "/tags?tags=invalid@tag" - }) - local body = assert.res_status(200, res) - local json = cjson.decode(body) - assert.equals(4, #json.data) + data = pagenated_get_json_data("/tags?tags=invalid@tag") + assert.equals(4, #data) end) it("/tags/:tags ignores ?tags= query", function() - local res = assert(client:send { - method = "GET", - path = "/tags/corp_%20a?tags=not_a_tag" - }) - local body = assert.res_status(200, res) - local json = cjson.decode(body) - assert.equals(2, #json.data) + local data = pagenated_get_json_data("/tags/corp_%20a?tags=not_a_tag") + assert.equals(2, #data) - local res = assert(client:send { - method = "GET", - path = "/tags/corp_%20a?tags=invalid@tag" - }) - local body = assert.res_status(200, res) - local json = cjson.decode(body) - assert.equals(2, #json.data) + data = pagenated_get_json_data("/tags/corp_%20a?tags=invalid@tag") + assert.equals(2, #data) end) end) end