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

wraputils.lua setup_property_access shouldn't override class methods #482

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 6 additions & 5 deletions splash/lua_modules/extras.lua
Expand Up @@ -9,11 +9,12 @@ local Extras = {}
local Extras_private = {}

function Extras._create(py_extras)
local self = {}
setmetatable(self, Extras)
wraputils.wrap_exposed_object(py_extras, self, Extras_private, false)
wraputils.setup_property_access(py_extras, self, Extras)
return self
local extras = {}
setmetatable(extras, Extras)
Extras.__index = Extras
Extras.__newindex = rawset
wraputils.wrap_exposed_object(py_extras, extras, Extras_private, false)
return extras
end

return Extras
11 changes: 7 additions & 4 deletions splash/lua_modules/request.lua
Expand Up @@ -8,16 +8,19 @@ local Request = wraputils.create_metatable()
local Request_private = {}

function Request._create(py_request)
local self = {
local request = {
info=py_request.info,
headers=treat.as_case_insensitive(py_request.headers),
url=py_request.url,
method=py_request.method,
}
setmetatable(self, Request)

wraputils.wrap_exposed_object(py_request, self, Request, Request_private, false)
return self
setmetatable(request, Request)
Request.__index = Request
Request.__newindex = rawset

wraputils.wrap_exposed_object(py_request, request, Request, Request_private, false)
return request
end

return Request
10 changes: 6 additions & 4 deletions splash/lua_modules/response.lua
Expand Up @@ -9,14 +9,16 @@ local Response = wraputils.create_metatable()
local Response_private = {}

function Response._create(py_response)
local self = {
local response = {
headers=treat.as_case_insensitive(py_response.headers),
request=Request._create(py_response.request),
}
setmetatable(self, Response)
setmetatable(response, Response)
Response.__index = Response
Response.__newindex = rawset

wraputils.wrap_exposed_object(py_response, self, Response, Response_private, false)
return self
wraputils.wrap_exposed_object(py_response, response, Response, Response_private, false)
return response
end


Expand Down
24 changes: 14 additions & 10 deletions splash/lua_modules/splash.lua
Expand Up @@ -14,10 +14,12 @@ local Splash = wraputils.create_metatable()
local Splash_private = {}

function Splash._create(py_splash)
local self = {args=py_splash.args}
setmetatable(self, Splash)
wraputils.wrap_exposed_object(py_splash, self, Splash, Splash_private, true)
return self
local splash = { args = py_splash.args }
setmetatable(splash, Splash)
Splash.__index = Splash
Splash.__newindex = rawset
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is used to reset the result of the previous setup_property_access call.

wraputils.wrap_exposed_object(py_splash, splash, Splash, Splash_private, true)
return splash
end

--
Expand Down Expand Up @@ -46,7 +48,7 @@ function Splash:on_response_headers(cb)
if type(cb) ~= 'function' then
error("splash:on_response_headers callback is not a function", 2)
end
Splash_private.on_response_headers(self, function (response)
Splash_private.on_response_headers(self, function(response)
local res = Response._create(response)
return cb(res)
end)
Expand All @@ -56,7 +58,7 @@ function Splash:on_response(cb)
if type(cb) ~= 'function' then
error("splash:on_response callback is not a function", 2)
end
Splash_private.on_response(self, function (response)
Splash_private.on_response(self, function(response)
local res = Response._create(response)
return cb(res)
end)
Expand All @@ -70,10 +72,12 @@ local Timer = wraputils.create_metatable()
local Timer_private = {}

function Timer._create(py_timer)
local self = {}
setmetatable(self, Timer)
wraputils.wrap_exposed_object(py_timer, self, Timer, Timer_private, true)
return self
local timer = {}
setmetatable(timer, Timer)
Timer.__index = Timer
Timer.__newindex = rawset
wraputils.wrap_exposed_object(py_timer, timer, Timer, Timer_private, true)
return timer
end

function Splash:call_later(cb, delay)
Expand Down
27 changes: 14 additions & 13 deletions splash/lua_modules/wraputils.lua
Expand Up @@ -134,32 +134,33 @@ end
-- Handle @lua_property decorators.
--
local function setup_property_access(py_object, self, cls)
local setters = {}
local getters = {}
self.setters = {}
self.getters = {}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suppose that we should somehow hide this objects from self. The previous closure method didn't work.


for name, opts in pairs(py_object.lua_properties) do
getters[name] = unwraps_python_result(drops_self_argument(py_object[opts.getter]))
self.getters[name] = unwraps_python_result(drops_self_argument(py_object[opts.getter]))
if opts.setter ~= nil then
setters[name] = unwraps_python_result(drops_self_argument(py_object[opts.setter]))
self.setters[name] = unwraps_python_result(drops_self_argument(py_object[opts.setter]))
else
setters[name] = function()
self.setters[name] = function()
error("Attribute " .. name .. " is read-only.", 2)
end
end
end

function cls:__newindex(index, value)
if setters[index] then
return setters[index](self, value)
function cls:__index(index)
if self.getters[index] then
return self.getters[index](self)
else
return rawset(cls, index, value)
return rawget(cls, index)
end
end

function cls:__index(index)
if getters[index] then
return getters[index](self)
function cls:__newindex(index, value)
if self.setters[index] then
return self.setters[index](self, value)
else
return rawget(cls, index)
return rawset(self, index, value)
end
end
end
Expand Down
29 changes: 29 additions & 0 deletions splash/tests/test_execute_callbacks.py
Expand Up @@ -834,6 +834,35 @@ def test_error_handled_in_callback(self):
self.assertStatusCode(resp, 200)
self.assertEqual(resp.text, "error")

def test_multiple_calls(self):
resp = self.request_lua("""
local treat = require('treat')
function main(splash)
local o = {false, false, false, false, false, false}

local timer1 = splash:call_later(function() o[1] = true end, 0.5)
local timer2 = splash:call_later(function() o[2] = true end, 0.5)
local timer3 = splash:call_later(function() o[3] = true end, 0.5)
local timer4 = splash:call_later(function() o[4] = true end, 0.5)
local timer5 = splash:call_later(function() o[5] = true end, 0.5)
local timer6 = splash:call_later(function() o[6] = true end, 0.5)

timer1:cancel()
timer2:cancel()
timer3:cancel()
timer4:cancel()
timer5:cancel()
timer6:cancel()

splash:wait(1)

return treat.as_array(o)
end
""")

self.assertStatusCode(resp, 200)
self.assertEqual(resp.json(), [False, False, False, False, False, False])
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you run this test w/o this fix you will get [False, True, True, True, True, False]. This happens because when setup_property_access is called it sets __index and __setindex metamethods to the provided class. And that way only the first and the last created instances will work correctly, because for the class (table) of the first instance is created without any overrides and the last instance works because its class methods has right self arguments.

Copy link
Contributor

Choose a reason for hiding this comment

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

This comment belongs in the source code, not here.



class WithTimeoutTest(BaseLuaRenderTest):
def test_with_timeout(self):
Expand Down