Skip to content

Commit 140e6fe

Browse files
committed
changed parameter precedence, so that dsn parsed params fill in gaps of
other params, rather than the other way around
1 parent fc6c14a commit 140e6fe

File tree

2 files changed

+126
-38
lines changed

2 files changed

+126
-38
lines changed

lib/resty/redis/connector.lua

Lines changed: 37 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -130,25 +130,6 @@ local _M = {
130130
local mt = { __index = _M }
131131

132132

133-
function _M.new(config)
134-
local ok, config = pcall(tbl_copy_merge_defaults, config, DEFAULTS)
135-
if not ok then
136-
return nil, config -- err
137-
else
138-
-- In proxied Redis mode disable default commands
139-
if config.connection_is_proxied == true and
140-
not next(config.disabled_commands) then
141-
142-
config.disabled_commands = default_disabled_commands
143-
end
144-
145-
return setmetatable({
146-
config = setmetatable(config, fixed_field_metatable)
147-
}, mt)
148-
end
149-
end
150-
151-
152133
local function parse_dsn(params)
153134
local url = params.url
154135
if url and url ~= "" then
@@ -172,27 +153,56 @@ local function parse_dsn(params)
172153

173154
local roles = { m = "master", s = "slave", a = "any" }
174155

156+
local parsed_params = {}
157+
175158
for i,v in ipairs(fields) do
176-
params[v] = m[i + 1]
159+
parsed_params[v] = m[i + 1]
177160
if v == "role" then
178-
params[v] = roles[params[v]]
161+
parsed_params[v] = roles[parsed_params[v]]
179162
end
180163
end
181-
end
182164

183-
return true, nil
165+
return tbl_copy_merge_defaults(params, parsed_params)
166+
end
184167
end
185168
_M.parse_dsn = parse_dsn
186169

187170

188-
function _M.connect(self, params)
189-
local params = tbl_copy_merge_defaults(params, self.config)
171+
function _M.new(config)
172+
-- Fill out gaps in config with any dsn params
173+
if config and config.url then
174+
local err
175+
config, err = parse_dsn(config)
176+
if not ok then ngx_log(ngx_ERR, err) end
177+
end
178+
179+
local ok, config = pcall(tbl_copy_merge_defaults, config, DEFAULTS)
180+
if not ok then
181+
return nil, config -- err
182+
else
183+
-- In proxied Redis mode disable default commands
184+
if config.connection_is_proxied == true and
185+
not next(config.disabled_commands) then
190186

191-
if params.url then
192-
local ok, err = parse_dsn(params)
187+
config.disabled_commands = default_disabled_commands
188+
end
189+
190+
return setmetatable({
191+
config = setmetatable(config, fixed_field_metatable)
192+
}, mt)
193+
end
194+
end
195+
196+
197+
function _M.connect(self, params)
198+
if params and params.url then
199+
local err
200+
params, err = parse_dsn(params)
193201
if not ok then ngx_log(ngx_ERR, err) end
194202
end
195203

204+
params = tbl_copy_merge_defaults(params, self.config)
205+
196206
if #params.sentinels > 0 then
197207
return self:connect_via_sentinel(params)
198208
else

t/connector.t

Lines changed: 89 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -223,8 +223,8 @@ location /t {
223223
path = "unix://tmp/redis.sock",
224224
}):connect()
225225

226-
assert(not redis and err == "no such file or directory",
227-
"bad domain socket should fail")
226+
assert(not redis and err == "no such file or directory",
227+
"bad domain socket should fail")
228228
}
229229
}
230230
--- request
@@ -241,13 +241,13 @@ location /t {
241241
content_by_lua_block {
242242
local rc = require("resty.redis.connector")
243243

244-
local params = {
245-
url = "redis://foo@127.0.0.1:$TEST_NGINX_REDIS_PORT/4"
246-
}
244+
local user_params = {
245+
url = "redis://foo@127.0.0.1:$TEST_NGINX_REDIS_PORT/4"
246+
}
247247

248-
local ok, err = rc.parse_dsn(params)
249-
assert(ok and not err,
250-
"url should parse without error: " .. tostring(err))
248+
local params, err = rc.parse_dsn(user_params)
249+
assert(params and not err,
250+
"url should parse without error: " .. tostring(err))
251251

252252
assert(params.host == "127.0.0.1", "host should be localhost")
253253
assert(tonumber(params.port) == $TEST_NGINX_REDIS_PORT,
@@ -256,12 +256,12 @@ location /t {
256256
assert(params.password == "foo", "password should be foo")
257257

258258

259-
local params = {
259+
local user_params = {
260260
url = "sentinel://foo@foomaster:s/2"
261261
}
262262

263-
local ok, err = rc.parse_dsn(params)
264-
assert(ok and not err,
263+
local params, err = rc.parse_dsn(user_params)
264+
assert(params and not err,
265265
"url should parse without error: " .. tostring(err))
266266

267267
assert(params.master_name == "foomaster", "master_name should be foomaster")
@@ -282,3 +282,81 @@ location /t {
282282
GET /t
283283
--- no_error_log
284284
[error]
285+
286+
287+
=== TEST 9: params override dsn components
288+
--- http_config eval: $::HttpConfig
289+
--- config
290+
location /t {
291+
lua_socket_log_errors Off;
292+
content_by_lua_block {
293+
local rc = require("resty.redis.connector")
294+
295+
local user_params = {
296+
url = "redis://foo@127.0.0.1:6381/4",
297+
db = 2,
298+
password = "bar",
299+
host = "example.com",
300+
}
301+
302+
local params, err = rc.parse_dsn(user_params)
303+
assert(params and not err,
304+
"url should parse without error: " .. tostring(err))
305+
306+
assert(tonumber(params.db) == 2, "db should be 2")
307+
assert(params.password == "bar", "password should be bar")
308+
assert(params.host == "example.com", "host should be example.com")
309+
310+
assert(tonumber(params.port) == 6381, "ort should still be 6381")
311+
312+
}
313+
}
314+
--- request
315+
GET /t
316+
--- no_error_log
317+
[error]
318+
319+
320+
=== TEST 9: Integration test for parse_dsn
321+
--- http_config eval: $::HttpConfig
322+
--- config
323+
location /t {
324+
lua_socket_log_errors Off;
325+
content_by_lua_block {
326+
local user_params = {
327+
url = "redis://foo.example:$TEST_NGINX_REDIS_PORT/4",
328+
db = 2,
329+
host = "127.0.0.1",
330+
}
331+
332+
local rc, err = require("resty.redis.connector").new(user_params)
333+
assert(rc and not err, "new should return positively")
334+
335+
local redis, err = rc:connect()
336+
assert(redis and not err, "connect should return positively")
337+
assert(redis:set("cat", "dog") and redis:get("cat") == "dog")
338+
339+
local redis, err = rc:connect({
340+
url = "redis://foo.example:$TEST_NGINX_REDIS_PORT/4",
341+
db = 2,
342+
host = "127.0.0.1",
343+
})
344+
assert(redis and not err, "connect should return positively")
345+
assert(redis:set("cat", "dog") and redis:get("cat") == "dog")
346+
347+
348+
local rc2, err = require("resty.redis.connector").new()
349+
local redis, err = rc2:connect({
350+
url = "redis://foo.example:$TEST_NGINX_REDIS_PORT/4",
351+
db = 2,
352+
host = "127.0.0.1",
353+
})
354+
assert(redis and not err, "connect should return positively")
355+
assert(redis:set("cat", "dog") and redis:get("cat") == "dog")
356+
357+
}
358+
}
359+
--- request
360+
GET /t
361+
--- no_error_log
362+
[error]

0 commit comments

Comments
 (0)