Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(db) make dao:cache_key to fallback to primary_key if nothing from cache_key was found #8553

Merged
merged 1 commit into from
Mar 24, 2022

Conversation

bungle
Copy link
Member

@bungle bungle commented Mar 18, 2022

Summary

This makes the cache_key function more robust. For example this code may be quite common:

local route = kong.db.routes:select_by_name("my-route")
-- {
--   id = ...,
--   name = "my-route",
--   ...
--   service = {
--     id = ...
--   }
-- }

local cache_key = kong.db.services:cache_key(route.service)

Now if service schema has cache_key = { "name" } (it does not, but just as an example) you can see that the local cache_key will then be same for all the services (no matter if they are pointing to different service by id), as the route.service is not expanded by default, and it only contains the primary key, in this case id.

The change in this commit is that it will now actually fallback to primary_key = { "id" } in case it cannot find anything by the cache_key. As it can be seen in code above, it is quite easy to make this mistake, and not see the mistake. User could fix their code by calling:

local cache_key = kong.db.services:cache_key(route.service.id)

instead of:

local cache_key = kong.db.services:cache_key(route.service)

But as this can potentially be dangerous if forgotten, I think it is worth to fallback to primary_key by default on such case.

… cache_key was found

### Summary

This makes the `cache_key` function more robust. For example this code may be quite common:

```lua
local route = kong.db.routes:select_by_name("my-route")
-- {
--   id = ...,
--   name = "my-route",
--   ...
--   service = {
--     id = ...
--   }
-- }

local cache_key = kong.db.services:cache_key(route.service)
```

Now if `service` schema has `cache_key = { "name" }` you can see that the
`local cache_key` will then be same for all the `services` (no matter if
they are pointing to different service by id), as the `route.service` is
not expanded by default, and it only contains the primary key, in this
case `id`.

The change in this commit is that it will now actually fallback to
`primary_key = { "id" }` in case it cannot find anything by the
`cache_key`. As it can be seen in code above, it is quite easy to make
this mistake, and not see the mistake. User could fix their code by
calling:

```lua
local cache_key = kong.db.services:cache_key(route.service.id)
```

instead of:

```lua
local cache_key = kong.db.services:cache_key(route.service)
```

But as this can potentially be dangerous if forgotten, I think
it is worth to fallback to `primary_key` by default on such case.
@bungle bungle force-pushed the fix/db-cache-key branch from a9fabcf to 7700506 Compare March 18, 2022 16:59
kong/db/dao/init.lua Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants