Add ipopo decorator for dynamic validation

[scottslewis]

Here is dialog from issue 60

scottslewis

Here's one thought: The @Validate decorator currently passes in the bundle context to the method implementation...e.g

@Validate
def _my_validate(self, bundle_context):
   # do stuff to validate

Would it be possible to enhance this validate notification so that the following signature could be supported?

@Validate
def _my_validate(self, bundle_context, props):
   # do stuff to validate

where props is a Map (the instance properties)

If that's difficult or impossible then perhaps an alternative would be

@Validate(component_context=True)
def _my_validate(self, component_context):
   # do stuff to validate

where the ComponentContext is passed in as the first arg rather than the bundle context...which can remain the default for the @Validate decorator...i.e. @Validate

With the component context, then the _my_validate method can/could also get at the instance properties (in the ComponentContext)

One reason I'm thinking this way is because of the @activate annotation for DS, which optionally allows the bundle context, component context, and/or the Map of the instance properties (although the representation of these types is slightly different).

wdyt?

tcalmant

Hi Scott,

This seems to be a good idea, especially as it would mimic the behavior of DS. Maybe this would require to open a new issue to have a specific discussion about this feature.

I think it would be better to use arguments on @Validate as it allows to be explicit on the arguments the method expects. Note that it is complicated to have the @Validate decorator working both with and without arguments (without parenthesis). There is an article talking about the problem (last part).

Maybe it could be more interesting to have a new decorator, e.g. @ValidateExt(), based on the same underlying configuration, but accepting arguments. This would keep compatibility with the current code base and would be cleaner to implement and to test.

For reference: OSGi R7 Declarative Service: Activate Method -- end comments--

Description

This bug is to follow up on this proposed addition.

In response to tcalmant's point: I understand about optional arguments for decorators. Too bad as it makes things very nice/simple via DS Activate in Java (reflection).

I'm ok with new decorator to keep backward compatibility. Maybe @Activate ? ;-) @ValidateExt is fine with me...if it works for you. Maybe even @ValidateComponent

My main desires for ValidateExt: ComponentContext rather than BundleContext in Validate. I'm only really interested in the properties in this case...however I think it makes sense to use ComponentContext rather than support (optionally) both like DS Activate does...for simplicity and Pythonicism (?). And...the call to the method should be before addition to service registry and if exception is thrown in decorated method should prevent the component's registration (and/or instantiation).

Unfortunately I'm going to have to ask you to implement this decorator, as this week I have ECF commitments for Photon and I think my knowledge of ipopo internals might limit me.

[tcalmant]

Hi Scott,

I took the liberty to update the formatting of the issue, to keep track of some links I put in the previous discussion :)

Unfortunately I'm going to have to ask you to implement this decorator, as this week I have ECF commitments for Photon and I think my knowledge of ipopo internals might limit me.

No problem. I'm also under a constant load of work, so it won't be implemented before this weekend.

The most tricky part will be to choose when the decorated method will be called (before or after @Validate).

[scottslewis]

Hi @tcalmant ,

Formatting....good idea :+1:

I appreciate the effort...lmk if/how I can help. I have a use case (RSA impl) where I can try it out right away.

[tcalmant]

Hi @scottslewis,

I've just pushed an early version of @ValidateComponent on branch validate_component.

It's an early version, so:

• the signature of the callback depends on the order of the list of arguments given to the decorator. I think this will stay
• the properties given to the callback are a copy of the component properties, to avoid messing with requirements based on properties. It would be possible to change that, but I don't consider this a priority.
• the bundle and component contexts given as parameters are the real ones.

Here is a snippet on how to use the new decorator:

from pelix.ipopo.decorators import ComponentFactory, \
    Instantiate, Validate, Invalidate, ValidateComponent
from pelix.ipopo.constants import ARG_BUNDLE_CONTEXT, \
    ARG_COMPONENT_CONTEXT, ARG_PROPERTIES

@ComponentFactory("sample-factory")
@Instantiate("sample")
class Sample(object):
    @ValidateComponent(ARG_COMPONENT_CONTEXT, ARG_PROPERTIES)
    def validate_ext(self, component_ctx, props):
        print("Component Context:", component_ctx)
        print("Component Props:", props)

    @Validate
    def validate(self, context):
        print("Validated")

    @Invalidate
    def invalidate(self, context):
        print("Invalidated")

You can mix the number and order of the authorized arguments and use any name for them. The only constraint is them to be in the same order as given in the @ValidateComponent arguments

[tcalmant]

Hi Scott, I've added the tests for this decorator and it works as intended. Let me know if you need it to be tweaked.

Once it suits your needs, I'll merge it into the master branch.

[scottslewis]

Hi Thomas. I've tried it out for my use case and it seems to work fine so I'm +1 for merging to master. Thank you for adding this so quickly!

[scottslewis]

Hi Thomas. One thing I noticed with further testing: If the ValidateComponent method raises an exception it does not prevent the component from being registered as a service in the service registry. Would it be possible for the ValidateComponent exception to prevent the component from being registered in the service registry?

[tcalmant]

Hi Scott, I'll take a look tomorrow. I missed it in the tests

[tcalmant]

Hi Scott, I've fixed the issue, all should work well now.

Note that when @ValidateComponent raises an exception, the component will be in ERRONEOUS state (you can call retry_erroneous to try new properties or kill it). During the change of state @Invalidate will be called (this is the standard error handling), so be careful with the variables you use in this callback.

[scottslewis]

Hi Thomas.

I see the behavior you describe wrt the component instances, but still see the service in the service registry (e.g. sl console command). To get the component instantiation failure to result in removal from the service registry (e.g. ValidateComponent raises) is it necessary to manually unregister it (and kill it from the component factory?) Can such clean up be safely done in Invalidate handler or elsewhere?

I guess the component/service registry relationship in ipopo is a little different than DS.

Thanks for any info.

[tcalmant]

Hi Scott, I'll take a look asap, but it looks like a bug. A component must be in valid state for the "provides" handler to be called, which shouldn't happen when the component goes into erroneous state.

[tcalmant]

Hi Scott, I can't reproduce the issue, the snippets I tried seems to behave correctly. Do you have a sample class to reproduce the issue ?

[scottslewis]

Hi Thomas. Sorry for the slow response. I've been working on the RSA impl.

This s behaving as expected now (i.e. exception in ValidateComponent leads to no component and no associated service), so if you want to merge to master (perhaps you already have) then feel free to close this issue. Thanks again for your effort here, and I think ValidateComponent will be useful to others as well.

I may need to open a new issue for exception handling of the eventlistenerhook that I added, however. I'll clarify the necessary behavior and then open a new issue.

[tcalmant]

Hi Scott, That's great!

I'm just wondering if it is necessary to keep @Validate as a whole or if it could be transformed into an alias of @ValidateComponent(ARG_BUNDLE_CONTEXT). We lose the fact that one method is called before the other, but it would seem more logical. What do you think ?

Also, it might be interesting to have the behaviour with @InvalidateComponent.

[scottslewis]

Hi Thomas,

I've already gotten into ValidateComponent/Invalidate and that's a little clumsy, so I think InvalidateComponent would be a good idea. I think the Validate/Invalidate aliasing would also make sense.

[tcalmant]

Hi Scott,

The validate_component now works as described before:

• @Validate is now an alias of @ValidateComponent(ARG_BUNDLE_CONTEXT)
• @InvalidateComponent mimics the behaviour for the invalidation callback. Exceptions raised by the decorated callback are ignored.
• @Invalidate is now an alias of @InvalidateComponent(ARG_BUNDLE_CONTEXT)

I've added the tests for the invalidation callback and all works as intented.

Note that now, you can't use both @Validate and @ValidateComponent in the same class (as it would duplicate validation callbacks).

[tcalmant]

The branch has been merged into master and deleted. The issue can now be closed as it has been fully handled.

[scottslewis]

Sounds great, thanks.