From 82750803d10b109cd207ced4930ded77a30905cd Mon Sep 17 00:00:00 2001
From: Aapo Talvensaari <aapo.talvensaari@gmail.com>
Date: Fri, 9 Aug 2024 16:11:03 +0300
Subject: [PATCH] fix(scheme): deprecated shorthand fields precedence

### 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 <aapo.talvensaari@gmail.com>
---
 .../fix-deprecate-shorthands-precedence.yml   |  3 ++
 kong/db/schema/init.lua                       |  6 ++-
 .../01-db/01-schema/01-schema_spec.lua        | 54 +++++++++++++++++++
 3 files changed, 61 insertions(+), 2 deletions(-)
 create mode 100644 changelog/unreleased/fix-deprecate-shorthands-precedence.yml

diff --git a/changelog/unreleased/fix-deprecate-shorthands-precedence.yml b/changelog/unreleased/fix-deprecate-shorthands-precedence.yml
new file mode 100644
index 0000000000000..5053cbca27485
--- /dev/null
+++ b/changelog/unreleased/fix-deprecate-shorthands-precedence.yml
@@ -0,0 +1,3 @@
+message: Deprecated shorthand fields don't take precedence over replacement fields when both are specified.
+type: bugfix
+scope: Core
diff --git a/kong/db/schema/init.lua b/kong/db/schema/init.lua
index 37d587c97eb49..4417a48c1c059 100644
--- a/kong/db/schema/init.lua
+++ b/kong/db/schema/init.lua
@@ -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
diff --git a/spec/01-unit/01-db/01-schema/01-schema_spec.lua b/spec/01-unit/01-db/01-schema/01-schema_spec.lua
index d8d669210ff4f..5981e1dd096e9 100644
--- a/spec/01-unit/01-db/01-schema/01-schema_spec.lua
+++ b/spec/01-unit/01-db/01-schema/01-schema_spec.lua
@@ -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",