Skip to content

Commit

Permalink
Replace globals configuration, dao with module variables.
Browse files Browse the repository at this point in the history
  • Loading branch information
mars committed Feb 9, 2016
1 parent 917e8fa commit f59c849
Show file tree
Hide file tree
Showing 24 changed files with 111 additions and 84 deletions.
1 change: 1 addition & 0 deletions kong-0.6.1-0.rockspec
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ build = {
["resty_http"] = "kong/vendor/resty_http.lua",

["kong.constants"] = "kong/constants.lua",
["kong.singletons"] = "kong/singletons.lua",

["kong.cli.utils.logger"] = "kong/cli/utils/logger.lua",
["kong.cli.utils.luarocks"] = "kong/cli/utils/luarocks.lua",
Expand Down
7 changes: 4 additions & 3 deletions kong/api/app.lua
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
local singletons = require "kong.singletons"
local lapis = require "lapis"
local utils = require "kong.tools.utils"
local stringy = require "stringy"
Expand Down Expand Up @@ -130,7 +131,7 @@ local function attach_routes(routes)

for k, v in pairs(methods) do
local method = function(self)
return v(self, dao, handler_helpers)
return v(self, singletons.dao, handler_helpers)
end
methods[k] = parse_params(method)
end
Expand All @@ -146,8 +147,8 @@ for _, v in ipairs({"kong", "apis", "consumers", "plugins", "cache", "cluster" }
end

-- Loading plugins routes
if configuration and configuration.plugins then
for _, v in ipairs(configuration.plugins) do
if singletons.configuration and singletons.configuration.plugins then
for _, v in ipairs(singletons.configuration.plugins) do
local loaded, mod = utils.load_module_if_exists("kong.plugins."..v..".api")
if loaded then
ngx.log(ngx.DEBUG, "Loading API endpoints for plugin: "..v)
Expand Down
3 changes: 2 additions & 1 deletion kong/api/routes/apis.lua
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
local singletons = require "kong.singletons"
local crud = require "kong.api.crud_helpers"
local syslog = require "kong.tools.syslog"
local constants = require "kong.constants"
Expand Down Expand Up @@ -52,7 +53,7 @@ return {

POST = function(self, dao_factory)
crud.post(self.params, dao_factory.plugins, function(data)
if configuration.send_anonymous_reports then
if singletons.configuration.send_anonymous_reports then
data.signal = constants.SYSLOG.API
syslog.log(syslog.format_entity(data))
end
Expand Down
13 changes: 7 additions & 6 deletions kong/api/routes/cluster.lua
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
local singletons = require "kong.singletons"
local responses = require "kong.tools.responses"
local cjson = require "cjson"
local Serf = require "kong.cli.services.serf"
Expand All @@ -9,7 +10,7 @@ local string_upper = string.upper
return {
["/cluster/"] = {
GET = function(self, dao_factory, helpers)
local res, err = Serf(configuration):invoke_signal("members", {["-format"] = "json"})
local res, err = Serf(singletons.configuration):invoke_signal("members", {["-format"] = "json"})
if err then
return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
end
Expand All @@ -35,7 +36,7 @@ return {
return responses.send_HTTP_BAD_REQUEST("Missing node \"name\"")
end

local _, err = Serf(configuration):invoke_signal("force-leave", {self.params.name})
local _, err = Serf(singletons.configuration):invoke_signal("force-leave", {self.params.name})
if err then
return responses.send_HTTP_BAD_REQUEST(err)
end
Expand All @@ -48,7 +49,7 @@ return {
return responses.send_HTTP_BAD_REQUEST("Missing node \"address\"")
end

local _, err = Serf(configuration):invoke_signal("join", {self.params.address})
local _, err = Serf(singletons.configuration):invoke_signal("join", {self.params.address})
if err then
return responses.send_HTTP_BAD_REQUEST(err)
end
Expand All @@ -67,9 +68,9 @@ return {
end

-- If it's an update, load the new entity too so it's available in the hooks
if message_t.type == events.TYPES.ENTITY_UPDATED then
if message_t.type == singletons.events.TYPES.ENTITY_UPDATED then
message_t.old_entity = message_t.entity
message_t.entity = dao[message_t.collection]:find_by_primary_key({id = message_t.old_entity.id})
message_t.entity = singletons.dao[message_t.collection]:find_by_primary_key({id = message_t.old_entity.id})
if not message_t.entity then
-- This means that the entity has been deleted immediately after an update in the meanwhile that
-- the system was still processing the update. A delete invalidation will come immediately after
Expand All @@ -79,7 +80,7 @@ return {
end

-- Trigger event in the node
events:publish(message_t.type, message_t)
singletons.events:publish(message_t.type, message_t)

return responses.send_HTTP_OK()
end
Expand Down
5 changes: 3 additions & 2 deletions kong/api/routes/kong.lua
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
local singletons = require "kong.singletons"
local constants = require "kong.constants"
local route_helpers = require "kong.api.route_helpers"
local utils = require "kong.tools.utils"
Expand All @@ -15,11 +16,11 @@ return {
version = constants.VERSION,
hostname = utils.get_hostname(),
plugins = {
available_on_server = configuration.plugins,
available_on_server = singletons.configuration.plugins,
enabled_in_cluster = db_plugins
},
lua_version = jit and jit.version or _VERSION,
configuration = configuration
configuration = singletons.configuration
})
end
},
Expand Down
3 changes: 2 additions & 1 deletion kong/core/certificate.lua
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
local singletons = require "kong.singletons"
local cache = require "kong.tools.database_cache"
local stringy = require "stringy"

Expand All @@ -9,7 +10,7 @@ local function find_api(hosts)
local sanitized_host = stringy.split(host, ":")[1]

retrieved_api, err = cache.get_or_set(cache.api_key(sanitized_host), function()
local apis, err = dao.apis:find_by_keys {request_host = sanitized_host}
local apis, err = singletons.dao.apis:find_by_keys {request_host = sanitized_host}
if err then
return nil, err
elseif apis and #apis == 1 then
Expand Down
15 changes: 8 additions & 7 deletions kong/core/cluster.lua
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
local singletons = require "kong.singletons"
local cluster_utils = require "kong.tools.cluster"
local Serf = require "kong.cli.services.serf"
local cache = require "kong.tools.database_cache"
Expand Down Expand Up @@ -26,7 +27,7 @@ local function async_autojoin(premature)
-- If this node is the only node in the cluster, but other nodes are present, then try to join them
-- This usually happens when two nodes are started very fast, and the first node didn't write his
-- information into the datastore yet. When the second node starts up, there is nothing to join yet.
if not configuration.cluster["auto-join"] then return end
if not singletons.configuration.cluster["auto-join"] then return end

local lock = resty_lock:new("cluster_autojoin_locks", {
exptime = ASYNC_AUTOJOIN_INTERVAL - 0.001
Expand All @@ -35,11 +36,11 @@ local function async_autojoin(premature)
if elapsed and elapsed == 0 then
-- If the current member count on this node's cluster is 1, but there are more than 1 active nodes in
-- the DAO, then try to join them
local count, err = dao.nodes:count_by_keys()
local count, err = singletons.dao.nodes:count_by_keys()
if err then
ngx.log(ngx.ERR, tostring(err))
elseif count > 1 then
local serf = Serf(configuration)
local serf = Serf(singletons.configuration)
local res, err = serf:invoke_signal("members", {["-format"] = "json"})
if err then
ngx.log(ngx.ERR, tostring(err))
Expand All @@ -48,7 +49,7 @@ local function async_autojoin(premature)
local members = cjson.decode(res).members
if #members < 2 then
-- Trigger auto-join
local _, err = serf:_autojoin(cluster_utils.get_node_name(configuration))
local _, err = serf:_autojoin(cluster_utils.get_node_name(singletons.configuration))
if err then
ngx.log(ngx.ERR, tostring(err))
end
Expand Down Expand Up @@ -78,13 +79,13 @@ local function send_keepalive(premature)
local elapsed = lock:lock("keepalive")
if elapsed and elapsed == 0 then
-- Send keepalive
local node_name = cluster_utils.get_node_name(configuration)
local nodes, err = dao.nodes:find_by_keys({name = node_name})
local node_name = cluster_utils.get_node_name(singletons.configuration)
local nodes, err = singletons.dao.nodes:find_by_keys({name = node_name})
if err then
ngx.log(ngx.ERR, tostring(err))
elseif #nodes == 1 then
local node = table.remove(nodes, 1)
local _, err = dao.nodes:update(node)
local _, err = singletons.dao.nodes:update(node)
if err then
ngx.log(ngx.ERR, tostring(err))
end
Expand Down
19 changes: 10 additions & 9 deletions kong/core/hooks.lua
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
local singletons = require "kong.singletons"
local events = require "kong.core.events"
local cache = require "kong.tools.database_cache"
local stringy = require "stringy"
Expand All @@ -23,7 +24,7 @@ local function invalidate(message_t)
end

local function get_cluster_members()
local serf = require("kong.cli.services.serf")(configuration)
local serf = require("kong.cli.services.serf")(singletons.configuration)
local res, err = serf:invoke_signal("members", { ["-format"] = "json" })
if err then
ngx.log(ngx.ERR, err)
Expand Down Expand Up @@ -60,7 +61,7 @@ end
local function member_leave(message_t)
local member = parse_member(message_t.entity)

local _, err = dao.nodes:delete({
local _, err = singletons.dao.nodes:delete({
name = member.name
})
if err then
Expand All @@ -71,7 +72,7 @@ end
local function member_update(message_t, is_reap)
local member = parse_member(message_t.entity)

local nodes, err = dao.nodes:find_by_keys({
local nodes, err = singletons.dao.nodes:find_by_keys({
name = member.name
})
if err then
Expand All @@ -82,14 +83,14 @@ local function member_update(message_t, is_reap)
if #nodes == 1 then
local node = table.remove(nodes, 1)
node.cluster_listening_address = member.cluster_listening_address
local _, err = dao.nodes:update(node)
local _, err = singletons.dao.nodes:update(node)
if err then
ngx.log(ngx.ERR, tostring(err))
return
end
end

if is_reap and dao.nodes:count_by_keys({}) > 1 then
if is_reap and singletons.dao.nodes:count_by_keys({}) > 1 then
-- Purge the cache when a failed node re-appears
cache.delete_all()
end
Expand All @@ -98,7 +99,7 @@ end
local function member_join(message_t)
local member = parse_member(message_t.entity)

local nodes, err = dao.nodes:find_by_keys({
local nodes, err = singletons.dao.nodes:find_by_keys({
name = member.name
})
if err then
Expand All @@ -107,7 +108,7 @@ local function member_join(message_t)
end

if #nodes == 0 then -- Insert
local _, err = dao.nodes:insert({
local _, err = singletons.dao.nodes:insert({
name = stringy.strip(member.name),
cluster_listening_address = stringy.strip(member.cluster_listening_address)
})
Expand All @@ -122,7 +123,7 @@ local function member_join(message_t)
end

-- Purge the cache when a new node joins
if dao.nodes:count_by_keys({}) > 1 then -- If it's only one node, no need to delete the cache
if singletons.dao.nodes:count_by_keys({}) > 1 then -- If it's only one node, no need to delete the cache
cache.delete_all()
end
end
Expand All @@ -138,7 +139,7 @@ return {
invalidate(message_t)
end,
[events.TYPES.CLUSTER_PROPAGATE] = function(message_t)
local serf = Serf(configuration)
local serf = Serf(singletons.configuration)
local ok, err = serf:event(message_t)
if not ok then
ngx.log(ngx.ERR, err)
Expand Down
3 changes: 2 additions & 1 deletion kong/core/plugins_iterator.lua
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
local singletons = require "kong.singletons"
local cache = require "kong.tools.database_cache"
local constants = require "kong.constants"
local responses = require "kong.tools.responses"
Expand All @@ -17,7 +18,7 @@ local function load_plugin_configuration(api_id, consumer_id, plugin_name)
local cache_key = cache.plugin_key(plugin_name, api_id, consumer_id)

local plugin = cache.get_or_set(cache_key, function()
local rows, err = dao.plugins:find_by_keys {
local rows, err = singletons.dao.plugins:find_by_keys {
api_id = api_id,
consumer_id = consumer_id ~= nil and consumer_id or constants.DATABASE_NULL_ID,
name = plugin_name
Expand Down
3 changes: 2 additions & 1 deletion kong/core/resolver.lua
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
local singletons = require "kong.singletons"
local url = require "socket.url"
local cache = require "kong.tools.database_cache"
local stringy = require "stringy"
Expand Down Expand Up @@ -63,7 +64,7 @@ end
-- Load all APIs in memory.
-- Sort the data for faster lookup: dictionary per request_host and an array of wildcard request_host.
function _M.load_apis_in_memory()
local apis, err = dao.apis:find_all()
local apis, err = singletons.dao.apis:find_all()
if err then
return nil, err
end
Expand Down
Loading

2 comments on commit f59c849

@thibaultcha
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is interesting for a short-term approach.

My current, long-term idea is to leave those tables as Lua upvalues in the kong.lua "main" file, responsible for the plugins event loop. The event loop would be updated to allow more granularity in the number of hooks proposed since Kong plugins need more granularity than OpenResty modules:

  • ssl
  • before balancing
  • before auth (in access)
  • before proxy (in access too)
  • on headers
  • on first body chunk (just some random idea)
  • on last body chunk
  • etc..

The event loop would then execute those contexts for each plugin, and plugins would ultimately be built in a much more FP-oriented way (ditching out the current verbose, not-so-useful OOP pattern):

return {
  before_balancing = function(ngx, kong)
    -- just some random code:
    local api, err = kong.dao.apis:find()
    if err then
      return kong.exit(err) -- maybe some syntactic sugar
    end

    ngx.ctx = api -- this is not so FP, but we need to be able to mutate some state in ngx_lua

    -- maybe consider returning values to the event loop
  end,
  log = function(ngx, kong)
    -- in log_by_lua
  end
}

This approach would make the plugins much more unit-testable than the current ones, would allow for more granularity for developers, and one could imagine 2 plugins that are executed in the same hook could be triggered in any order, since the FP paradigm simply provides us with much more robust handlers.


Of course, on a short-term timeline, your approach has the benefit of cleaning the global namespace (and local accesses are much faster than global ones too). I would not be opposed to discuss it and include this in Kong if you want to submit it.

@mars
Copy link
Owner Author

@mars mars commented on f59c849 Feb 11, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thibaultcha The more FP (function programming) style the better, especially for these granular plugin hooks.

It's understandable that ngx is inherently mutable with the request contexts. That little identifier is so overloaded with functionality & data structures, it seems we're stuck with it. Ideally the design would avoid any other side effects, though.

Great to hear about the vision for this aspect of Kong. Let's continue discussion in the PR.

Please sign in to comment.