After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 678701 - Unable to create custom service proxy objects via resource factory
Unable to create custom service proxy objects via resource factory
Status: RESOLVED FIXED
Product: GUPnP
Classification: Other
Component: gupnp
0.18.x
Other Linux
: Normal normal
: ---
Assigned To: GUPnP Maintainers
GUPnP Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-06-24 12:04 UTC by Christoph Jaeger
Modified: 2014-01-20 13:54 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
tests: Make test-bugs naming generic (4.73 KB, patch)
2014-01-18 19:07 UTC, Jussi Kukkonen
accepted-commit_now Details | Review
tests: unref objects after running test (807 bytes, patch)
2014-01-18 19:07 UTC, Jussi Kukkonen
accepted-commit_now Details | Review
tests: Add test for #678701 (6.10 KB, patch)
2014-01-18 19:07 UTC, Jussi Kukkonen
accepted-commit_now Details | Review
Ensure ResourceFactory creates proxies with right GType (2.13 KB, patch)
2014-01-18 19:07 UTC, Jussi Kukkonen
needs-work Details | Review
Ensure ResourceFactory creates proxies with right GType (2.15 KB, patch)
2014-01-20 13:54 UTC, Jussi Kukkonen
committed Details | Review
tests: Add test for #678701 (6.10 KB, patch)
2014-01-20 13:54 UTC, Jussi Kukkonen
committed Details | Review
tests: unref objects after running test (857 bytes, patch)
2014-01-20 13:54 UTC, Jussi Kukkonen
committed Details | Review
tests: Make test-bugs naming generic (4.73 KB, patch)
2014-01-20 13:54 UTC, Jussi Kukkonen
committed Details | Review

Description Christoph Jaeger 2012-06-24 12:04:19 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.
Comment 1 Jens Georg 2012-06-24 14:05:03 UTC
I'm pretty sure I filed a bug for this already, wonder where that went...
Comment 2 Jussi Kukkonen 2014-01-17 16:31:59 UTC
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).
Comment 3 Jussi Kukkonen 2014-01-18 19:07:08 UTC
Created attachment 266621 [details] [review]
tests: Make test-bugs naming generic
Comment 4 Jussi Kukkonen 2014-01-18 19:07:13 UTC
Created attachment 266622 [details] [review]
tests: unref objects after running test
Comment 5 Jussi Kukkonen 2014-01-18 19:07:16 UTC
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()
Comment 6 Jussi Kukkonen 2014-01-18 19:07:20 UTC
Created attachment 266624 [details] [review]
Ensure ResourceFactory creates proxies with right GType
Comment 7 Jussi Kukkonen 2014-01-18 19:13:07 UTC
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!
Comment 8 Jens Georg 2014-01-19 12:35:20 UTC
Review of attachment 266621 [details] [review]:

+1
Comment 9 Jens Georg 2014-01-19 12:35:48 UTC
Review of attachment 266622 [details] [review]:

Whoops. Ok, wasn't necessary with a single test though :) +1
Comment 10 Jens Georg 2014-01-19 13:11:24 UTC
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
Comment 11 Jens Georg 2014-01-19 13:11:41 UTC
Review of attachment 266623 [details] [review]:

+1
Comment 12 Jussi Kukkonen 2014-01-20 09:54:14 UTC
(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.
Comment 13 Jussi Kukkonen 2014-01-20 13:53:56 UTC
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
Comment 14 Jussi Kukkonen 2014-01-20 13:54:00 UTC
Created attachment 266730 [details] [review]
Ensure ResourceFactory creates proxies with right GType
Comment 15 Jussi Kukkonen 2014-01-20 13:54:05 UTC
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()
Comment 16 Jussi Kukkonen 2014-01-20 13:54:26 UTC
Created attachment 266732 [details] [review]
tests: unref objects after running test
Comment 17 Jussi Kukkonen 2014-01-20 13:54:30 UTC
Created attachment 266733 [details] [review]
tests: Make test-bugs naming generic