From cb4755835e7ba4fd0a5c056a8141653bb85bfb47 Mon Sep 17 00:00:00 2001
From: Elvin Efendi <elvin.efendiyev@gmail.com>
Date: Tue, 19 Jun 2018 12:46:49 +0400
Subject: [PATCH] refactor some lua code

---
 rootfs/etc/nginx/lua/balancer/chash.lua       |  3 +-
 rootfs/etc/nginx/lua/balancer/ewma.lua        |  7 +-
 rootfs/etc/nginx/lua/balancer/round_robin.lua |  3 +-
 rootfs/etc/nginx/lua/balancer/sticky.lua      |  3 +-
 rootfs/etc/nginx/lua/monitor.lua              |  2 +-
 .../nginx/lua/test/{defer => }/defer_test.lua |  2 +-
 .../lua/test/{monitor => }/monitor_test.lua   |  0
 rootfs/etc/nginx/lua/util.lua                 | 35 ----------
 rootfs/etc/nginx/lua/{ => util}/defer.lua     |  0
 rootfs/etc/nginx/lua/util/split.lua           | 70 +++++++++++++++++++
 rootfs/etc/nginx/template/nginx.tmpl          |  5 +-
 11 files changed, 83 insertions(+), 47 deletions(-)
 rename rootfs/etc/nginx/lua/test/{defer => }/defer_test.lua (94%)
 rename rootfs/etc/nginx/lua/test/{monitor => }/monitor_test.lua (100%)
 rename rootfs/etc/nginx/lua/{ => util}/defer.lua (100%)
 create mode 100644 rootfs/etc/nginx/lua/util/split.lua

diff --git a/rootfs/etc/nginx/lua/balancer/chash.lua b/rootfs/etc/nginx/lua/balancer/chash.lua
index 9590f1cd6..16dd89def 100644
--- a/rootfs/etc/nginx/lua/balancer/chash.lua
+++ b/rootfs/etc/nginx/lua/balancer/chash.lua
@@ -1,6 +1,7 @@
 local balancer_resty = require("balancer.resty")
 local resty_chash = require("resty.chash")
 local util = require("util")
+local split = require("util.split")
 
 local _M = balancer_resty:new({ factory = resty_chash, name = "chash" })
 
@@ -15,7 +16,7 @@ end
 function _M.balance(self)
   local key = util.lua_ngx_var(self.hash_by)
   local endpoint_string = self.instance:find(key)
-  return util.split_pair(endpoint_string, ":")
+  return split.split_pair(endpoint_string, ":")
 end
 
 return _M
diff --git a/rootfs/etc/nginx/lua/balancer/ewma.lua b/rootfs/etc/nginx/lua/balancer/ewma.lua
index 7327ead4a..c32709701 100644
--- a/rootfs/etc/nginx/lua/balancer/ewma.lua
+++ b/rootfs/etc/nginx/lua/balancer/ewma.lua
@@ -7,6 +7,7 @@
 
 local resty_lock = require("resty.lock")
 local util = require("util")
+local split = require("util.split")
 
 local DECAY_TIME = 10 -- this value is in seconds
 local LOCK_KEY = ":ewma_key"
@@ -131,10 +132,10 @@ function _M.balance(self)
 end
 
 function _M.after_balance(_)
-  local response_time = tonumber(util.get_first_value(ngx.var.upstream_response_time)) or 0
-  local connect_time = tonumber(util.get_first_value(ngx.var.upstream_connect_time)) or 0
+  local response_time = tonumber(split.get_first_value(ngx.var.upstream_response_time)) or 0
+  local connect_time = tonumber(split.get_first_value(ngx.var.upstream_connect_time)) or 0
   local rtt = connect_time + response_time
-  local upstream = util.get_first_value(ngx.var.upstream_addr)
+  local upstream = split.get_first_value(ngx.var.upstream_addr)
 
   if util.is_blank(upstream) then
     return
diff --git a/rootfs/etc/nginx/lua/balancer/round_robin.lua b/rootfs/etc/nginx/lua/balancer/round_robin.lua
index f8bbccb5a..d8909f2cd 100644
--- a/rootfs/etc/nginx/lua/balancer/round_robin.lua
+++ b/rootfs/etc/nginx/lua/balancer/round_robin.lua
@@ -1,6 +1,7 @@
 local balancer_resty = require("balancer.resty")
 local resty_roundrobin = require("resty.roundrobin")
 local util = require("util")
+local split = require("util.split")
 
 local _M = balancer_resty:new({ factory = resty_roundrobin, name = "round_robin" })
 
@@ -14,7 +15,7 @@ end
 
 function _M.balance(self)
   local endpoint_string = self.instance:find()
-  return util.split_pair(endpoint_string, ":")
+  return split.split_pair(endpoint_string, ":")
 end
 
 return _M
diff --git a/rootfs/etc/nginx/lua/balancer/sticky.lua b/rootfs/etc/nginx/lua/balancer/sticky.lua
index ee1b0b2fb..8af93a986 100644
--- a/rootfs/etc/nginx/lua/balancer/sticky.lua
+++ b/rootfs/etc/nginx/lua/balancer/sticky.lua
@@ -1,6 +1,7 @@
 local balancer_resty = require("balancer.resty")
 local resty_chash = require("resty.chash")
 local util = require("util")
+local split = require("util.split")
 local ck = require("resty.cookie")
 
 local _M = balancer_resty:new({ factory = resty_chash, name = "sticky" })
@@ -74,7 +75,7 @@ end
 
 function _M.balance(self)
   local endpoint_string = sticky_endpoint_string(self)
-  return util.split_pair(endpoint_string, ":")
+  return split.split_pair(endpoint_string, ":")
 end
 
 return _M
diff --git a/rootfs/etc/nginx/lua/monitor.lua b/rootfs/etc/nginx/lua/monitor.lua
index 605edc8eb..2ef609cfd 100644
--- a/rootfs/etc/nginx/lua/monitor.lua
+++ b/rootfs/etc/nginx/lua/monitor.lua
@@ -1,6 +1,6 @@
 local socket = ngx.socket.tcp
 local cjson = require('cjson')
-local defer = require('defer')
+local defer = require('util.defer')
 local assert = assert
 
 local _M = {}
diff --git a/rootfs/etc/nginx/lua/test/defer/defer_test.lua b/rootfs/etc/nginx/lua/test/defer_test.lua
similarity index 94%
rename from rootfs/etc/nginx/lua/test/defer/defer_test.lua
rename to rootfs/etc/nginx/lua/test/defer_test.lua
index 1afbf9159..6f4f94cc3 100644
--- a/rootfs/etc/nginx/lua/test/defer/defer_test.lua
+++ b/rootfs/etc/nginx/lua/test/defer_test.lua
@@ -1,6 +1,6 @@
 package.path = "./rootfs/etc/nginx/lua/?.lua;./rootfs/etc/nginx/lua/test/mocks/?.lua;" .. package.path
 _G._TEST = true
-local defer = require('defer')
+local defer = require('util.defer')
 
 local _ngx = {
     shared = {},
diff --git a/rootfs/etc/nginx/lua/test/monitor/monitor_test.lua b/rootfs/etc/nginx/lua/test/monitor_test.lua
similarity index 100%
rename from rootfs/etc/nginx/lua/test/monitor/monitor_test.lua
rename to rootfs/etc/nginx/lua/test/monitor_test.lua
diff --git a/rootfs/etc/nginx/lua/util.lua b/rootfs/etc/nginx/lua/util.lua
index c76dd3e43..449ad312f 100644
--- a/rootfs/etc/nginx/lua/util.lua
+++ b/rootfs/etc/nginx/lua/util.lua
@@ -49,17 +49,6 @@ function _M.lua_ngx_var(ngx_var)
   return ngx.var[var_name]
 end
 
-function _M.split_pair(pair, seperator)
-  local i = pair:find(seperator)
-  if i == nil then
-    return pair, nil
-  else
-    local name = pair:sub(1, i - 1)
-    local value = pair:sub(i + 1, -1)
-    return name, value
-  end
-end
-
 -- this implementation is taken from
 -- https://web.archive.org/web/20131225070434/http://snippets.luacode.org/snippets/Deep_Comparison_of_Two_Values_3
 -- and modified for use in this project
@@ -88,30 +77,6 @@ function _M.is_blank(str)
   return str == nil or string_len(str) == 0
 end
 
--- http://nginx.org/en/docs/http/ngx_http_upstream_module.html#example
--- CAVEAT: nginx is giving out : instead of , so the docs are wrong
--- 127.0.0.1:26157 : 127.0.0.1:26157 , ngx.var.upstream_addr
--- 200 : 200 , ngx.var.upstream_status
--- 0.00 : 0.00, ngx.var.upstream_response_time
-function _M.split_upstream_var(var)
-  if not var then
-    return nil, nil
-  end
-  local t = {}
-  for v in var:gmatch("[^%s|,]+") do
-    if v ~= ":" then
-      t[#t+1] = v
-    end
-  end
-  return t
-end
-
-function _M.get_first_value(var)
-  local t = _M.split_upstream_var(var) or {}
-  if #t == 0 then return nil end
-  return t[1]
-end
-
 -- this implementation is taken from:
 -- https://github.com/luafun/luafun/blob/master/fun.lua#L33
 -- SHA: 04c99f9c393e54a604adde4b25b794f48104e0d0
diff --git a/rootfs/etc/nginx/lua/defer.lua b/rootfs/etc/nginx/lua/util/defer.lua
similarity index 100%
rename from rootfs/etc/nginx/lua/defer.lua
rename to rootfs/etc/nginx/lua/util/defer.lua
diff --git a/rootfs/etc/nginx/lua/util/split.lua b/rootfs/etc/nginx/lua/util/split.lua
new file mode 100644
index 000000000..63282337f
--- /dev/null
+++ b/rootfs/etc/nginx/lua/util/split.lua
@@ -0,0 +1,70 @@
+local _M = {}
+
+-- splits strings into host and port
+local function parse_addr(addr)
+  local _, _, host, port = addr:find("([^:]+):([^:]+)")
+  if host and port then
+    return {host=host, port=port}
+  else
+    return nil, "error in parsing upstream address!"
+  end
+end
+
+function _M.get_first_value(var)
+  local t = _M.split_upstream_var(var) or {}
+  if #t == 0 then return nil end
+  return t[1]
+end
+
+function _M.split_pair(pair, seperator)
+  local i = pair:find(seperator)
+  if i == nil then
+    return pair, nil
+  else
+    local name = pair:sub(1, i - 1)
+    local value = pair:sub(i + 1, -1)
+    return name, value
+  end
+end
+
+-- http://nginx.org/en/docs/http/ngx_http_upstream_module.html#example
+-- CAVEAT: nginx is giving out : instead of , so the docs are wrong
+-- 127.0.0.1:26157 : 127.0.0.1:26157 , ngx.var.upstream_addr
+-- 200 : 200 , ngx.var.upstream_status
+-- 0.00 : 0.00, ngx.var.upstream_response_time
+function _M.split_upstream_var(var)
+  if not var then
+    return nil, nil
+  end
+  local t = {}
+  for v in var:gmatch("[^%s|,]+") do
+    if v ~= ":" then
+      t[#t+1] = v
+    end
+  end
+  return t
+end
+
+-- Splits an NGINX $upstream_addr and returns an array of tables with a `host` and `port` key-value pair.
+function _M.split_upstream_addr(addrs_str)
+  if not addrs_str then
+    return nil, nil
+  end
+
+  local addrs = _M.split_upstream_var(addrs_str)
+  local host_and_ports = {}
+
+  for _, v in ipairs(addrs) do
+    local a, err = parse_addr(v)
+    if err then
+      return nil, err
+    end
+    host_and_ports[#host_and_ports+1] = a
+  end
+  if #host_and_ports == 0 then
+    return nil, "no upstream addresses to parse!"
+  end
+  return host_and_ports
+end
+
+return _M
diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl
index 8d9444766..16488b36a 100644
--- a/rootfs/etc/nginx/template/nginx.tmpl
+++ b/rootfs/etc/nginx/template/nginx.tmpl
@@ -900,10 +900,7 @@ stream {
                 balancer.log()
                 {{ end }}
 
-                ok, res = pcall(monitor.call)
-                    if not ok then
-                    ngx.log(ngx.ERR, "request_id failed: " .. tostring(res))
-                end
+                monitor.call()
             }
             {{ end }}