Store private fields on objects (take 2)

splash/lua_modules/extras.lua

@@ -6,12 +6,10 @@
 local wraputils = require("wraputils")
 
 local Extras = wraputils.create_metatable()
-local Extras_private = {}
 
 function Extras._create(py_extras)
   local extras = {}
-  wraputils.wrap_exposed_object(py_extras, extras, Extras, Extras_private, false)
-  return extras
+  return wraputils.wrap_exposed_object(py_extras, extras, Extras)
 end
 
 return Extras

splash/lua_modules/request.lua

@@ -5,7 +5,6 @@ local wraputils = require("wraputils")
 local treat = require("libs/treat")
 
 local Request = wraputils.create_metatable()
-local Request_private = {}
 
 function Request._create(py_request)
   local request = {
@@ -15,8 +14,7 @@ function Request._create(py_request)
     method=py_request.method,
   }
 
-  wraputils.wrap_exposed_object(py_request, request, Request, Request_private, false)
-  return request
+  return wraputils.wrap_exposed_object(py_request, request, Request)
 end
 
 return Request

splash/lua_modules/response.lua

@@ -6,16 +6,14 @@ local treat = require("libs/treat")
 local Request = require("request")
 
 local Response = wraputils.create_metatable()
-local Response_private = {}
 
 function Response._create(py_response)
   local response = {
     headers=treat.as_case_insensitive(py_response.headers),
     request=Request._create(py_response.request),
   }
 
-  wraputils.wrap_exposed_object(py_response, response, Response, Response_private, false)
-  return response
+  return wraputils.wrap_exposed_object(py_response, response, Response)
 end
 
 

splash/lua_modules/splash.lua

@@ -11,21 +11,19 @@ local Request = require("request")
 -- Lua wrapper for Splash Python object.
 --
 local Splash = wraputils.create_metatable()
-local Splash_private = {}
 
 
 function Splash._create(py_splash)
   local splash = { args = py_splash.args }
-  wraputils.wrap_exposed_object(py_splash, splash, Splash, Splash_private, true)
-  return splash
+  return wraputils.wrap_exposed_object(py_splash, splash, Splash)
 end
 
 --
 -- Create jsfunc method from private_jsfunc.
 -- It is required to handle errors properly.
 --
 function Splash:jsfunc(...)
-  local func = Splash_private.jsfunc(self, ...)
+  local func = self:_jsfunc(...)
   return wraputils.unwraps_python_result(func)
 end
 
@@ -36,7 +34,7 @@ function Splash:on_request(cb)
   if type(cb) ~= 'function' then
     error("splash:on_request callback is not a function", 2)
   end
-  Splash_private.on_request(self, function(py_request)
+  self:_on_request(function(py_request)
     local req = Request._create(py_request)
     return cb(req)
   end)
@@ -46,7 +44,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)
+  self:_on_response_headers(function(response)
     local res = Response._create(response)
     return cb(res)
   end)
@@ -56,7 +54,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)
+  self:_on_response(function(response)
     local res = Response._create(response)
     return cb(res)
   end)
@@ -67,16 +65,14 @@ end
 -- Timer Lua wrapper
 --
 local Timer = wraputils.create_metatable()
-local Timer_private = {}
 
 function Timer._create(py_timer)
   local timer = {}
-  wraputils.wrap_exposed_object(py_timer, timer, Timer, Timer_private, true)
-  return timer
+  return wraputils.wrap_exposed_object(py_timer, timer, Timer)
 end
 
 function Splash:call_later(cb, delay)
-  local py_timer = Splash_private.call_later(self, cb, delay)
+  local py_timer = self:_call_later(cb, delay)
   return Timer._create(py_timer)
 end
 

splash/lua_modules/wraputils.lua

@@ -84,10 +84,9 @@ local function sets_callback(func, storage)
 end
 
 
-local PRIVATE_PREFIX = "private_"
-
-local function is_private_name(key)
-  return string.find(key, "^" .. PRIVATE_PREFIX) ~= nil
+local function is_private_name(name)
+  -- Method/attribute name is private true if it starts with an underscore.
+  return name:sub(1, 1) == "_"
 end
 
 
@@ -100,7 +99,7 @@ end
 -- * Private methods are stored in `private_self`, public methods are
 --   stored in `self`.
 --
-local function setup_commands(py_object, self, private_self)
+local function setup_methods(py_object, self, cls)
   -- Create lua_object:<...> methods from py_object methods:
   for key, opts in pairs(py_object.commands) do
     local command = py_object[key]
@@ -119,12 +118,14 @@ local function setup_commands(py_object, self, private_self)
     end
     command = unwraps_python_result(command, nlevels)
 
-    if is_private_name(key) then
-      local short_key = string.sub(key, PRIVATE_PREFIX:len() + 1)
-      private_self[short_key] = command
-    else
-      -- avoid custom setter
-      rawset(self, key, command)
+    rawset(self, key, command)
+  end
+
+  for key, value in pairs(cls) do
+    if type(value) == "function" then
+      rawset(self, key, drops_self_argument(function(...)
+        return value(self, ...)
+      end))
     end
   end
 end
@@ -150,13 +151,64 @@ local function setup_property_access(py_object, self, cls)
 end
 
 
+-- This value is used to protect the metatable of an exposed object from being
+-- edited and replaced.
+local EXPOSED_OBJ_METATABLE_PLACEHOLDER = '<wrapped object>'
+
 --
 -- Create a Lua wrapper for a Python object.
 --
-local function wrap_exposed_object(py_object, self, cls, private_self)
-  setmetatable(self, cls)
-  setup_commands(py_object, self, private_self)
-  setup_property_access(py_object, self, cls)
+local function wrap_exposed_object(py_object, private_self, cls)
+  setmetatable(private_self, cls)
+  setup_methods(py_object, private_self, cls)
+  setup_property_access(py_object, private_self)
+
+  -- "Public" metatable that prevents access to private elements and to itself.
+  local public_mt = {
+    __index = function(self, key)
+      if is_private_name(key) then
+        return nil
+      end
+      return private_self[key]
+    end,
+
+    __newindex = function(self, key, value)
+      if is_private_name(key) then
+        error("Cannot set private field: " .. tostring(key), 2)
+      end
+      assertx(2, pcall(function()
+        private_self[key] = value
+      end))
+    end,
+
+    __pairs = function(self)
+      wrapper = function(t, k)
+        local v
+        repeat
+          k, v = next(private_self, k)
+        until k == nil or not is_private_name(k)
+        return k, v
+      end
+      return wrapper, self, nil
+    end,
+
+    __metatable = EXPOSED_OBJ_METATABLE_PLACEHOLDER,
+  }
+
+  -- Forward any metatable events not defined in the "public" table to the
+  -- actual class/metadatable.
+  setmetatable(public_mt, {__index = cls})
+
+  -- public_self should only contain a reference to the public metatable
+  -- forwarding all actual data to the "real" self object.
+  local public_self = {
+    -- Add a function to the "public_self" so that it doesn't serialize cleanly
+    -- by mistake.
+    is_object = function() return true end
+  }
+  setmetatable(public_self, public_mt)
+
+  return public_self
 end
 
 
@@ -168,9 +220,7 @@ end
 -- the calling code should add these fields to the table ASAP, preferably with
 -- rawset.
 local function create_metatable()
-  local cls = {
-    __wrapped = true
-  }
+  local cls = {}
 
   cls.__index = function(self, index)
     if self.__getters[index] then
@@ -197,10 +247,7 @@ end
 --
 local function is_wrapped(obj)
   local mt = getmetatable(obj)
-  if type(mt) ~= 'table' then
-    return false
-  end
-  return mt.__wrapped == true
+  return mt == EXPOSED_OBJ_METATABLE_PLACEHOLDER
 end
 
 -- Exposed API
@@ -211,8 +258,6 @@ return {
   raises_async = raises_async,
   yields_result = yields_result,
   sets_callback = sets_callback,
-  is_private_name = is_private_name,
-  setup_commands = setup_commands,
   setup_property_access = setup_property_access,
   wrap_exposed_object = wrap_exposed_object,
   create_metatable = create_metatable,

splash/lua_runtime.py

@@ -139,9 +139,6 @@ def _attr_getter(self, obj, attr_name):
         except TypeError:
             raise AttributeError("Non-string lookups are not allowed (requested: %r)" % attr_name)
 
-        if attr_name.startswith("_"):
-            raise AttributeError("Access to private attribute %r is not allowed" % attr_name)
-
         if obj not in self._allowed_object_attrs:
             raise AttributeError("Access to object %r is not allowed" % obj)
 

splash/qtrender_lua.py

@@ -751,7 +751,7 @@ def errback(msg, raise_):
         return PyResult.yield_(cmd)
 
     @command()
-    def private_jsfunc(self, func):
+    def _jsfunc(self, func):
         return _WrappedJavascriptFunction(self, func)
 
     def _http_request(self, url, headers, follow_redirects=True, body=None,
@@ -1070,7 +1070,7 @@ def get_perf_stats(self):
                 'walltime': time.time()}
 
     @command(sets_callback=True, decode_arguments=False)
-    def private_on_request(self, callback):
+    def _on_request(self, callback):
         """
         Register a Lua callback to be called when a resource is requested.
         """
@@ -1087,7 +1087,7 @@ def _callback(request, operation, outgoing_data):
         return True
 
     @command(sets_callback=True, decode_arguments=False)
-    def private_on_response_headers(self, callback):
+    def _on_response_headers(self, callback):
         def _callback(reply):
             if self.destroyed:
                 return
@@ -1101,7 +1101,7 @@ def _callback(reply):
         return True
 
     @command(sets_callback=True, decode_arguments=False)
-    def private_on_response(self, callback):
+    def _on_response(self, callback):
         def _callback(reply, har_entry, content):
             if self.destroyed:
                 return
@@ -1121,7 +1121,7 @@ def _callback(reply, har_entry, content):
         return True
 
     @command(sets_callback=True, decode_arguments=False)
-    def private_call_later(self, callback, delay=None):
+    def _call_later(self, callback, delay=None):
         if delay is None:
             delay = 0
         if not isinstance(delay, (float, int)):

splash/tests/lua_modules/emulation.lua

@@ -53,11 +53,11 @@ function Splash:go_and_wait(args)
     end
   end
 
-  assert(self:_wait_restart_on_redirects(wait, 10))
+  assert(self:wait_restart_on_redirects(wait, 10))
 end
 
 
-function Splash:_wait_restart_on_redirects(time, max_redirects)
+function Splash:wait_restart_on_redirects(time, max_redirects)
   if not time then
     return true
   end

splash/tests/test_execute.py

@@ -1208,17 +1208,17 @@ def test_jsfunc_attributes(self):
     def test_private_jsfunc_not_available(self):
         resp = self.request_lua("""
         function main(splash)
-            return {ok = splash.private_jsfunc == nil}
+            return {ok = splash._jsfunc == nil}
         end
         """)
         self.assertStatusCode(resp, 200)
         self.assertEqual(resp.json()[u'ok'], True)
 
     def test_private_jsfunc_attributes(self):
-        resp = self.request_lua("""                                      -- 1
-        function main(splash)                                            -- 2
-            local func = splash:private_jsfunc("function(){return 123}") -- 3 <-
-            return func.source                                           -- 4
+        resp = self.request_lua("""                               -- 1
+        function main(splash)                                     -- 2
+            local func = splash:_jsfunc("function(){return 123}") -- 3 <-
+            return func.source                                    -- 4
         end
         """)
         err = self.assertScriptError(resp, ScriptError.LUA_ERROR)