Skip to content

Commit

Permalink
fix(scheme): deprecated shorthand fields precedence
Browse files Browse the repository at this point in the history
### Summary

The shorthand fields e.g. as used in (the url is a shorthand) take
precedence:
```
http PUT :8001/services/test url=http://test.org/ port=2345
```

That means that `port=2345` will be overwritten by http default
port of `80` that is implicit in shorthand field `url`.

This is how it has been for a long long time.

In PR #12686 we added `deprecation` field to shorthand fields definition.

Some of our automatic migrations without database migrations magic use
this functionality, but the precedence there does not good:

```
http -f PUT :8001/plugins/x name=x config.new=test config.old=should_be_ignored
```

In above the `config.old` is a shorthand field with a deprecation property.
Thus it should not overwrite the `config.new`, but in current code base it
does. This PR changes it so that it doesn't do it anymore.

KAG-5134 and https://kongstrong.slack.com/archives/C07AQH7SAF8/p1722589141558609

Signed-off-by: Aapo Talvensaari <[email protected]>
  • Loading branch information
bungle committed Aug 9, 2024
1 parent d4d73dc commit 8275080
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 2 deletions.
3 changes: 3 additions & 0 deletions changelog/unreleased/fix-deprecate-shorthands-precedence.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
message: Deprecated shorthand fields don't take precedence over replacement fields when both are specified.
type: bugfix
scope: Core
6 changes: 4 additions & 2 deletions kong/db/schema/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -1743,10 +1743,12 @@ function Schema:process_auto_fields(data, context, nulls, opts)
data[sname] = nil
local new_values = sdata.func(value)
if new_values then
local deprecation = sdata.deprecation
for k, v in pairs(new_values) do
if type(v) == "table" then
data[k] = table_merge(data[k] or {}, v)
else
data[k] = deprecation and table_merge(v, data[k])
or table_merge(data[k] or {}, v)
elseif not deprecation or data[k] == nil then
data[k] = v
end
end
Expand Down
54 changes: 54 additions & 0 deletions spec/01-unit/01-db/01-schema/01-schema_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -4110,6 +4110,60 @@ describe("schema", function()
assert.same({ name = "test1" }, output)
end)

it("takes precedence", function()
local TestSchema = Schema.new({
name = "test",
fields = {
{ name = { type = "string" } },
},
shorthand_fields = {
{
username = {
type = "string",
func = function(value)
return {
name = value
}
end,
},
},
},
})

local input = { username = "test1", name = "ignored" }
local output, _ = TestSchema:process_auto_fields(input)
assert.same({ name = "test1" }, output)
end)

it("does not take precedence if deprecated", function()
local TestSchema = Schema.new({
name = "test",
fields = {
{ name = { type = "string" } },
},
shorthand_fields = {
{
username = {
type = "string",
func = function(value)
return {
name = value
}
end,
deprecation = {
message = "username is deprecated, please use name instead",
removal_in_version = "4.0",
},
},
},
},
})

local input = { username = "ignored", name = "test1" }
local output, _ = TestSchema:process_auto_fields(input)
assert.same({ name = "test1" }, output)
end)

it("can produce multiple fields", function()
local TestSchema = Schema.new({
name = "test",
Expand Down

0 comments on commit 8275080

Please sign in to comment.