refactor(router/atc): do not create new table in split_routes_and_services_by_path()

kong/router/transform.lua

@@ -1,6 +1,5 @@
 local bit = require("bit")
 local buffer = require("string.buffer")
-local tb_new = require("table.new")
 local tb_nkeys = require("table.nkeys")
 local uuid = require("resty.jit-uuid")
 local lrucache = require("resty.lrucache")
@@ -690,48 +689,64 @@ local function group_by(t, f)
   return result
 end
 
--- split routes into multiple routes, one for each prefix length and one for all
--- regular expressions
-local function split_route_by_path_info(route_and_service, routes_and_services_split)
-  local original_route = route_and_service.route
+-- split routes into multiple routes,
+-- one for each prefix length and one for all regular expressions
+local function split_routes_and_services_by_path(routes_and_services)
+  local count = #routes_and_services
+  local append_count = 1
 
-  if is_empty_field(original_route.paths) or #original_route.paths == 1 or
-     not is_null(original_route.expression) -- expression will ignore paths
-  then
-    tb_insert(routes_and_services_split, route_and_service)
-    return
-  end
+  for i = 1, count do
+    local route_and_service = routes_and_services[i]
+    local original_route = route_and_service.route
+    local original_paths = original_route.paths
 
-  -- make sure that route_and_service contains only the two expected entries, route and service
-  assert(tb_nkeys(route_and_service) == 1 or tb_nkeys(route_and_service) == 2)
+    if is_empty_field(original_paths) or #original_paths == 1 or
+       not is_null(original_route.expression) -- expression will ignore paths
+    then
+      goto continue
+    end
 
-  local grouped_paths = group_by(
-    original_route.paths, sort_by_regex_or_length
-  )
-  for index, paths in pairs(grouped_paths) do
-    local cloned_route = {
-      route = shallow_copy(original_route),
-      service = route_and_service.service,
-    }
+    -- make sure that route_and_service contains
+    -- only the two expected entries, route and service
+    local nkeys = tb_nkeys(route_and_service)
+    assert(nkeys == 1 or nkeys == 2)
 
-    cloned_route.route.original_route = original_route
-    cloned_route.route.paths = paths
-    cloned_route.route.id = uuid_generator(original_route.id .. "#" .. tostring(index))
+    local original_route_id = original_route.id
+    local original_service = route_and_service.service
 
-    tb_insert(routes_and_services_split, cloned_route)
-  end
-end
+    -- `grouped_paths` is a hash table, like {true={'regex'}, 2={'/a'}, 3={'/aa'},}
+    local grouped_paths = group_by(original_paths, sort_by_regex_or_length)
 
+    local is_first = true
+    for key, paths in pairs(grouped_paths) do
+      local route = shallow_copy(original_route)
 
-local function split_routes_and_services_by_path(routes_and_services)
-  local count = #routes_and_services
-  local routes_and_services_split = tb_new(count, 0)
+      -- create a new route from the original route
+      route.original_route = original_route
+      route.paths = paths
+      route.id = uuid_generator(original_route_id .. "#" .. tostring(key))
 
-  for i = 1, count do
-    split_route_by_path_info(routes_and_services[i], routes_and_services_split)
-  end
+      local cloned_route_and_service = {
+        route = route,
+        service = original_service,
+      }
+
+      if is_first then
+        -- the first one will replace the original route
+        routes_and_services[i] = cloned_route_and_service
+        is_first = false
+
+      else
+        -- the others will append to the original routes array
+        routes_and_services[count + append_count] = cloned_route_and_service
+        append_count = append_count + 1
+      end
+    end -- for pairs(grouped_paths)
+
+    ::continue::
+  end   -- for routes_and_services
 
-  return routes_and_services_split
+  return routes_and_services
 end
 
 

spec/01-unit/08-router_spec.lua

@@ -2,6 +2,7 @@ local Router
 local path_handling_tests = require "spec.fixtures.router_path_handling_tests"
 local uuid = require("kong.tools.utils").uuid
 local get_expression = require("kong.router.transform").get_expression
+local deep_copy = require("kong.tools.table").deep_copy
 
 local function reload_router(flavor, subsystem)
   _G.kong = {
@@ -3663,7 +3664,8 @@ for _, flavor in ipairs({ "traditional", "traditional_compatible", "expressions"
         }
 
         lazy_setup(function()
-          router = assert(new_router(use_case_routes))
+          -- deep copy use_case_routes in case it changes
+          router = assert(new_router(deep_copy(use_case_routes)))
         end)
 
         it("strips the specified paths from the given uri if matching", function()
@@ -3717,7 +3719,8 @@ for _, flavor in ipairs({ "traditional", "traditional_compatible", "expressions"
             },
           }
 
-          local router = assert(new_router(use_case_routes))
+          -- deep copy use_case_routes in case it changes
+          local router = assert(new_router(deep_copy(use_case_routes)))
 
           local _ngx = mock_ngx("POST", "/my-route/hello/world",
                                 { host = "domain.org" })
@@ -4900,7 +4903,8 @@ for _, flavor in ipairs({ "traditional", "traditional_compatible", "expressions"
           },
         }
 
-        router = assert(new_router(use_case))
+          -- deep copy use_case in case it changes
+        router = assert(new_router(deep_copy(use_case)))
       end)
 
       it("[assigns different priorities to regex and non-regex path]", function()
@@ -4948,7 +4952,8 @@ for _, flavor in ipairs({ "traditional", "traditional_compatible", "expressions"
           },
         }
 
-        router = assert(new_router(use_case))
+        -- deep copy use_case in case it changes
+        router = assert(new_router(deep_copy(use_case)))
       end)
 
       it("[assigns different priorities to each path]", function()