Extensibility issue - OSGi callouts being called before official
Description
Environment
Attachments
Activity
Murilo Habermann Torquato August 20, 2014 at 2:09 PM
As Gabriel described here https://groups.google.com/forum/#!topic/idempiere/OKnFUzEc9ac and Andrea here https://groups.google.com/forum/#!topic/idempiere/JBhxzUHt-Uw after this change OSGi callouts are being ignored if there's no oficial (old way) callout value defined fora a column.
Gabriel sent a simple workaroud, but I don't know if it's the better solution .
Thomas Bayen August 8, 2014 at 9:21 AM
Hmmm... after thinking about the code (based on todays irc chat) and what it does I feel like the best idea in our case (the workshop plugin) would be not to sort both calls but to abandon the original call. The best in our case would have been to delete the original callout entry in AD_Field.
That said
if you have a callout that does not influence with the standard callout it is best to call it after the standard (like your patch does).
If you have a callout that interferes somehow with the original callout then you should anyway better delete the AD_Field entry.
As a conclusion of this thoughts: I agree with patch and do not think we need something more powerful. There will be no real usecase.
Thomas Bayen August 6, 2014 at 1:43 PM
By only changing the order of the two different call approaches we may solve one and raise another issue.
My idea at the workshop was to use one system of calling (with one system of call priority). The most advanced call system we use up to now is to use OSGi services. So we should implement the standard callouts as an OSGi service. It seems we already have a service called org.adempiere.base.DefaultColumnCalloutFactory. It is used to call extensions in the "Equinox Extensions way". We could extend this factory to call the "old way" callouts too. We can copy the code part in GridTab that deals with "field.getCallout()" to DefaultColumnCalloutFactory and delete it at the old place.
After that we know that the DefaultColumnCalloutFactory has a "service.ranking" of 0 (this default is defined in ServiceRegistrationImpl). When we create new Callouts in our Plugins we can decide to set their service.ranking to -1 or 1 (or whatever).
If you think my idea is not rubbish I can do some further research. So: wdyt?
Carlos Ruiz August 6, 2014 at 12:53 PM
you're right , that's the way I solved LCO following advice from Heng Sin, but also Heng Sin adviced that approach encourage split package, so a better solution is welcome.
Just chatted with Heng Sin and he agreed the patch here is ok to commit.
Regards,
Carlos Ruiz
Hiep Lq August 6, 2014 at 11:14 AM
after call "before", we can create a flag to notify we wish total override office callout. Check this flag to don't call office callout.
it same event handle,
In the http://wiki.idempiere.org/en/IDempiere_Workshop_2014 we found an extensibility issue as we cannot modify via OSGi the behavior of official callouts, the problem is in the order of calling the callout.
The GridTab.processCallout is calling first the extension callouts (OSGi) and then the dictionary callouts (and the official callouts are registered in dictionary).
The example setup was trying to change the discovery of the tax after a product is set, it was not possible, the OSGi callout was called and found the correct tax, but then the official callout was called overwriting what the extension callout found.
Sounds like an extensibility bug as the intention of extensions is to be able to change the behavior of callouts.
Attached you can see a simple patch that just moved the OSGi callout to the end, but a possible stronger solution could be to implement "before" and "after" callouts. To preserve backward compatibility we could search for a timing property "before" or empty, and after official callouts we can search for callouts with the property set to "after".