Request to modify/enhance @Provides decorator functionality

[tbr]

Hi,

I want to propose a small change to the @Provides decorator. Often (as in OSGi) service classes implement interfaces, and the interfaces define how the service can be used. Currently in iPopo, the programmer of such service is required to repeat in the specifications property the list of interfaces that are defined by the class. In order to simplify this, I made a functionality change that allows iPopo to detect the direct implemented bases and use those as specifications.

To the point, with my proposal the @Provides decorator can have specifications=None, which will trigger it to use the __bases__ of the class object as specifications. Thus the following can be written (example):

@ComponentFactory()
@Provides()
class MyServiceImpl(IMyService):
    pass

Secondly, I think it is not sufficient to use name when passing a class as specification, as this might collide with other classes having the same name. I modified this to use __module + "." + __qualname__ as specification instead. Now, 'module' is not guaranteed to contain a valid value, but afaik it's more often set than not (the code checks this).

It's just a proposal, feel free to disregard it, or comment on it. Of course, it would be great to (from my perspective) to have it accepted and merged into iPopo :)

The code is in decorators.py on https://github.com/tbr/ipopo/tree/new_provider

PS: I am influenced a lot by bndtools on Java.

[tcalmant]

Hi,

About the 1st part:

[...] iPopo to detect the direct implemented bases and use those as specifications

This would be a nice feature 👍
I think it would also be necessary to filter out object when getting the list of parent classes.

About the 2nd part:
I agree, this could lead to name clashes.
Although, this is a breaking change (I think a unit test should be updated as well).
So we'll have to notify the user of this change.

It's just a proposal, feel free to disregard it, or comment on it. Of course, it would be great to (from my perspective) to have it accepted and merged into iPopo :)

Thanks for the idea :)
I'll take a look at your code this weekend.

PS: I am influenced a lot by bndtools on Java.

I didn't use it directly, I was only doing Maven stuffs when I was working in Java :)

[tbr]

Hi,

the object filter is already in the code.

The qualname change might break existing code, indeed. So I'll leave that up to you to decide on. But on the long run, I guess it's the better way. Now, I also now it can cause issues when classes are referenced from modules that have different sys.path setups (it adds extra package names in the hierarchy). So, this should be well-documented and explained.

[tcalmant]

Hi,

As the qualname change will break some existing projects, I suggest we provide both forms of the specification (short, actual, one and the fully qualified one).
The removal of the the short name could be activated by a flag to the Provides class at startup and applied when releasing a future version of iPOPO, either 1.0 or before.

About the computation of the full name, I think it could be possible to get the folder of the __module__ and go up until we either match a patch in sys.path or can't find the __init__.py file.

[tbr]

Hi,

1. qualname might break some existing projects indeed, however in most cases name and qualname are identical.
2. calculating module w.r.t. syspath is also not a stable solution, as syspath can change between loading of bundles/packages/modules. I would not make it more complex than needed. Documenting the behavior seems the most reasonable thing to do.
3. To resolve these 2 items, I propose to make the provides decorator take an extra option to select specification generation from classes. Either a) SHORT (default if not specified), based on name, or b) FULL, based on module and qualname, using module as-is. This way, current behavior is still the default.

Thanks for considering these changes :)
Tim.

[tcalmant]

Hi,

calculating module w.r.t. syspath is also not a stable solution, as syspath can change between loading of bundles/packages/modules

I agree

propose to make the provides decorator take an extra option to select specification generation from classes

That could be a solution, I suppose we could then swap the default value in a future release then remove it.
I'd prefer a more "hidden" thing, like a new member of the Provides class, like:

# in the entry point script
from pelix.ipopo.decorators import Provides
Provides.use_qualname = True

use_qualname being a class member of Provides (not an instance member)

That way, the legacy doesn't have to be changed at all, and the use the full name can be activated/tested using a single line of code.

What do you think ?

[tbr]

I think that seems a good proposal, yes.

[tbr]

If you'd like, I can implement this and make a pull request for you? See https://github.com/tbr/ipopo/tree/new_provider branch.

[tcalmant]

Hi, I won't be able to work on iPOPO before sunday, so if you have time before that, you can do it if you want :) 2017-05-04 8:46 GMT+02:00 tbr <notifications@github.com>:

…

If you'd like, I can implement this and make a pull request for you? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#78 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABeB5h4zEygi_JfrJppPOXdUdKgqla0Qks5r2XRFgaJpZM4NLNdY> .

[tbr]

The work is done and available in the new_provider branch, I suggest you review it first (when having the time) and then I'll merge it to master and create a pull request.

[tcalmant]

Hi,

I'd just change two lines for the code style:

• decorators@753: I'd replace the == object by is object;
• decorators@820: I'd remove the spaces in specifications=None.

But apart from those style details, it's good to me 👍

[tcalmant]

Closing, as this feature is bundled in iPOPO 0.6.5.