GNOME Bugzilla – Bug 678701
Unable to create custom service proxy objects via resource factory
Last modified: 2014-01-20 13:54:30 UTC
Registered custom service proxy types (subclasses of ServiceProxy) via gupnp_resource_factory_register_resource_proxy_type(), but (via gupnp_device_info_get_service()) created objects are instances of ServiceProxy and not of my registered subtype. Did the same for custom device proxy types - that works as expected.
I'm pretty sure I filed a bug for this already, wonder where that went...
I have a small patch for this, but I'll first see if it's easy to add a test as well (something I'm hoping we could improve on).
Created attachment 266621 [details] [review] tests: Make test-bugs naming generic
Created attachment 266622 [details] [review] tests: unref objects after running test
Created attachment 266623 [details] [review] tests: Add test for #678701 Test that proxies created by ResourceFactory have the GType set in gupnp_resource_factory_register_resource_proxy_type()
Created attachment 266624 [details] [review] Ensure ResourceFactory creates proxies with right GType
The actual fix is not optimal as it does the same element parsing that gupnp_device_info_get_service() just did a nanosecond ago, but I guess that's acceptable. I added a test. It's largely copy-paste from the first one for setup and cleanup parts -- a GTest fixture would make sense if we add any more tests. The missing unrefs were producing some interesting test failures... The test functions have very similar stacks so signal handler for objects from the previous test were successfully modifying the stack variables on the current test. Fun!
Review of attachment 266621 [details] [review]: +1
Review of attachment 266622 [details] [review]: Whoops. Ok, wasn't necessary with a single test though :) +1
Review of attachment 266624 [details] [review]: I might misunderstand the original code, but allowing a NULL service_type in the original code seems like a bug. apart from the annotation change +1 ::: libgupnp/gupnp-resource-factory.c @@ +207,3 @@ * @location: The location of the service description file * @udn: The UDN of the device the service is contained in + * @service_type: The service type, or %NULL to use service type from @element That's missing the "(allow-none)" annotation then
Review of attachment 266623 [details] [review]: +1
(In reply to comment #10) > Review of attachment 266624 [details] [review]: > > I might misunderstand the original code, but allowing a NULL service_type in > the original code seems like a bug. Yeah, maybe -- service_type being there at all is just an optimization as far as I can see. ControlPoint uses that optimization but DeviceProxy does not (DeviceInfo parses device_type but there's no way to give that to the DeviceProxy get_service() implementation). The function is in -private.h, but I'll add the annotation: it can't hurt.
The following fixes have been pushed: 639e7fd Ensure ResourceFactory creates proxies with right GType ab2a6ec tests: Add test for #678701 d1b03cb tests: unref objects after running test 7d89cbb tests: Make test-bugs naming generic
Created attachment 266730 [details] [review] Ensure ResourceFactory creates proxies with right GType
Created attachment 266731 [details] [review] tests: Add test for #678701 Test that proxies created by ResourceFactory have the GType set in gupnp_resource_factory_register_resource_proxy_type()
Created attachment 266732 [details] [review] tests: unref objects after running test
Created attachment 266733 [details] [review] tests: Make test-bugs naming generic