Skip to content

Commit

Permalink
chore(db) remove schema legacy attributes
Browse files Browse the repository at this point in the history
### Summary

Removes `legacy = true` support from `schemas` and from `schema fields`.
  • Loading branch information
bungle committed Jun 15, 2022
1 parent e3c3be6 commit fbae2f4
Show file tree
Hide file tree
Showing 8 changed files with 11 additions and 37 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,9 @@
[8823](https://github.com/Kong/kong/pull/8823)
- The Kong singletons module `"kong.singletons"` was removed in favor of the PDK `kong.*`.
[#8874](https://github.com/Kong/kong/pull/8874)
- The support for `legacy = true/false` attribute was removed from Kong schemas and
Kong field schemas.
[#8958](https://github.com/Kong/kong/pull/8958)

#### Admin API

Expand Down
2 changes: 1 addition & 1 deletion kong/api/endpoints.lua
Original file line number Diff line number Diff line change
Expand Up @@ -817,7 +817,7 @@ local function generate_endpoints(schema, endpoints)
generate_entity_endpoints(endpoints, schema)

for foreign_field_name, foreign_field in schema:each_field() do
if foreign_field.type == "foreign" and not foreign_field.schema.legacy then
if foreign_field.type == "foreign" then
-- e.g. /routes/:routes/service
generate_entity_endpoints(endpoints, schema, foreign_field.schema, foreign_field_name, true)

Expand Down
2 changes: 1 addition & 1 deletion kong/api/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ do

-- DAO Routes
for _, dao in pairs(kong.db.daos) do
if dao.schema.generate_admin_api ~= false and not dao.schema.legacy then
if dao.schema.generate_admin_api ~= false then
routes = Endpoints.new(dao.schema, routes)
end
end
Expand Down
9 changes: 1 addition & 8 deletions kong/db/schema/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -1663,10 +1663,6 @@ function Schema:process_auto_fields(data, context, nulls, opts)
for key, field in self:each_field(data) do
local ftype = field.type
local value = data[key]
if field.legacy and field.uuid and value == "" then
value = null
end

if not is_select and field.auto then
local is_insert_or_upsert = context == "insert" or context == "upsert"
if field.uuid then
Expand Down Expand Up @@ -1782,10 +1778,7 @@ function Schema:process_auto_fields(data, context, nulls, opts)
for key in pairs(data) do
local field = self.fields[key]
if field then
if not field.legacy
and field.type == "string"
and (field.len_min or 1) > 0
and data[key] == ""
if field.type == "string" and (field.len_min or 1) > 0 and data[key] == ""
then
data[key] = nulls and null or nil
end
Expand Down
10 changes: 0 additions & 10 deletions kong/db/schema/metaschema.lua
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ local field_schema = {
{ default = { type = "self" }, },
{ abstract = { type = "boolean" }, },
{ generate_admin_api = { type = "boolean" }, },
{ legacy = { type = "boolean" }, },
{ immutable = { type = "boolean" }, },
{ err = { type = "string" } },
{ encrypted = { type = "boolean" }, },
Expand Down Expand Up @@ -339,9 +338,6 @@ local attribute_types = {
uuid = {
["string"] = true,
},
legacy = {
["string"] = true,
},
unique = {
["string"] = true,
["number"] = true,
Expand Down Expand Up @@ -626,12 +622,6 @@ local MetaSchema = Schema.new({
nilable = true,
},
},
{
legacy = {
type = "boolean",
nilable = true,
},
},
{
fields = fields_array,
},
Expand Down
16 changes: 2 additions & 14 deletions spec/01-unit/01-db/01-schema/01-schema_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -2818,13 +2818,11 @@ describe("schema", function()
arr = { type = "array", elements = { type = "string" } }, }, {
set = { type = "set", elements = { type = "string" } }, }, {
map = { type = "map", keys = { type = "string" }, values = { type = "string" } }, }, {
est = { type = "string", len_min = 0 }, }, {
lst = { type = "string", legacy = true }, }, } } },
est = { type = "string", len_min = 0 }, }, }, }, },
{ arr = { type = "array", elements = { type = "string" } }, },
{ set = { type = "set", elements = { type = "string" } }, },
{ map = { type = "map", keys = { type = "string" }, values = { type = "string" } }, },
{ est = { type = "string", len_min = 0 }, },
{ lst = { type = "string", legacy = true }, },
}
})
local data, err = Test:process_auto_fields({
Expand All @@ -2835,13 +2833,11 @@ describe("schema", function()
set = { "", "a", "" },
map = { key = "" },
est = "",
lst = "",
},
arr = { "", "a", "" },
set = { "", "a", "" },
map = { key = "" },
est = "",
lst = "",
}, "select")

assert.is_nil(err)
Expand All @@ -2850,15 +2846,13 @@ describe("schema", function()
assert.same({"", "a" }, data.set) -- set, TODO: should we remove empty strings from sets?
assert.same({ key = "" }, data.map) -- map, TODO: should we remove empty strings from maps?
assert.equal("", data.est)
assert.equal("", data.lst)

-- record
assert.equal(nil, data.rec.str) -- string
assert.same({"", "a", ""}, data.rec.arr) -- array, TODO: should we remove empty strings from arrays?
assert.same({"", "a" }, data.rec.set) -- set, TODO: should we remove empty strings from sets?
assert.same({ key = "" }, data.rec.map) -- map, TODO: should we remove empty strings from maps?
assert.equal("", data.rec.est)
assert.equal("", data.rec.lst)
end)

it("produces ngx.null (when asked) for empty string fields with selects", function()
Expand All @@ -2870,13 +2864,11 @@ describe("schema", function()
arr = { type = "array", elements = { type = "string" } }, }, {
set = { type = "set", elements = { type = "string" } }, }, {
map = { type = "map", keys = { type = "string" }, values = { type = "string" } }, }, {
est = { type = "string", len_min = 0 }, }, {
lst = { type = "string", legacy = true }, }, } } },
est = { type = "string", len_min = 0 }, }, }, }, },
{ arr = { type = "array", elements = { type = "string" } }, },
{ set = { type = "set", elements = { type = "string" } }, },
{ map = { type = "map", keys = { type = "string" }, values = { type = "string" } }, },
{ est = { type = "string", len_min = 0 }, },
{ lst = { type = "string", legacy = true }, },
}
})
local data, err = Test:process_auto_fields({
Expand All @@ -2887,29 +2879,25 @@ describe("schema", function()
set = { "", "a", "" },
map = { key = "" },
est = "",
lst = "",
},
arr = { "", "a", "" },
set = { "", "a", "" },
map = { key = "" },
est = "",
lst = "",
}, "select", true)
assert.is_nil(err)
assert.equal(cjson.null, data.str) -- string
assert.same({"", "a", ""}, data.arr) -- array, TODO: should we set null empty strings from arrays?
assert.same({"", "a" }, data.set) -- set, TODO: should we set null empty strings from sets?
assert.same({ key = "" }, data.map) -- map, TODO: should we set null empty strings from maps?
assert.equal("", data.est)
assert.equal("", data.lst)

-- record
assert.equal(cjson.null, data.rec.str) -- string
assert.same({"", "a", ""}, data.rec.arr) -- array, TODO: should we set null empty strings from arrays?
assert.same({"", "a" }, data.rec.set) -- set, TODO: should we set null empty strings from sets?
assert.same({ key = "" }, data.rec.map) -- map, TODO: should we set null empty strings from maps?
assert.equal("", data.rec.est)
assert.equal("", data.rec.lst)
end)

it("does not produce non-required fields on 'update'", function()
Expand Down
4 changes: 2 additions & 2 deletions spec/01-unit/01-db/01-schema/02-metaschema_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -124,14 +124,14 @@ describe("metaschema", function()
assert.truthy(MetaSchema:validate(s))
end)

it("a schema can be marked as legacy", function()
it("a schema cannot be marked as legacy", function()
local s = {
name = "hello",
primary_key = { "foo" },
legacy = true,
fields = {
{ foo = { type = "number" } } } }
assert.truthy(MetaSchema:validate(s))
assert.falsy(MetaSchema:validate(s))

s = {
name = "hello",
Expand Down
2 changes: 1 addition & 1 deletion spec/helpers.lua
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ local function truncate_tables(db, tables)
end

for _, t in ipairs(tables) do
if db[t] and db[t].schema and not db[t].schema.legacy then
if db[t] and db[t].schema then
db[t]:truncate()
end
end
Expand Down

0 comments on commit fbae2f4

Please sign in to comment.