Community
Participate
Working Groups
We should allow vendors to overload an ExtendedObjectSupplier by contributing a service with a higher service.ranking. Although this works currently we are relying on an implementation detail of Equinox when doing the look up. We should simply order the returned services based on their ranking value
(In reply to Thomas Schindl from comment #0) > We should allow vendors to overload an ExtendedObjectSupplier by > contributing a service with a higher service.ranking. > > Although this works currently we are relying on an implementation detail of > Equinox when doing the look up. We should simply order the returned services > based on their ranking value Out of curiosity, what is the equinox detail?
As far as I can read the JavaDoc BundleContext#getServiceReferences() does not spec that the returned list is sorted by service.ranking - IIRC asked you some time ago and you stated that it is the case for Equinox but I'm not feeling easy to depend on this. If you update your JavaDoc to explicitly state that I can rely on the order (if you look at getServiceReference() the order is guaranteed!) than I'm happy to go without my patch. The gerrit review is https://git.eclipse.org/r/#/c/22432/
(In reply to Thomas Schindl from comment #2) > As far as I can read the JavaDoc BundleContext#getServiceReferences() does > not spec that the returned list is sorted by service.ranking - IIRC asked > you some time ago and you stated that it is the case for Equinox but I'm not > feeling easy to depend on this. Yes, you did, I have a bad memory ;-) > > If you update your JavaDoc to explicitly state that I can rely on the order > (if you look at getServiceReference() the order is guaranteed!) than I'm > happy to go without my patch. You cannot depend on the order here and it is not likely we can change the javadoc. > > The gerrit review is https://git.eclipse.org/r/#/c/22432/ I put a comment in gerrit on the patch. It does not look correct to me. Here is my comment: ServiceReference is comparable already, should be no need to implement your own comparable here. You don't take into account the service.id if the rankings are equal. Also, the sorting here is going from lowest to highest. So would the 'highest' reference be at the end of the list, not [0].
Thanks Tom - i uploaded the patch in a rush ;-) Interesting enough your comment on gerrit is not showing up. I've uploaded a new review which uses simply Array.sort() and because I have you looking at it - is it safe to resort the result from the call or should I make a copy and sort that one?
(In reply to Thomas Schindl from comment #4) > Thanks Tom - i uploaded the patch in a rush ;-) Interesting enough your > comment on gerrit is not showing up. I was afraid of that, that is why I put the comment here in the bug also. I put the comment directly on the file change in gerrit instead as a review. > > I've uploaded a new review which uses simply Array.sort() and because I have > you looking at it - is it safe to resort the result from the call or should > I make a copy and sort that one? That is already a snapshot copy from the framework, no need to make another copy. But you still are sorting in ascending order which is not correct if you continue to use the first element from the sorted list: supplier = (ExtendedObjectSupplier) bundleContext.getService(refs[0]); I think you must use a reverse comparator in this case: Arrays.sort(refs, java.util.Collections.reverseOrder());
Silly me - I read the JavaDoc wrong! Pushed a new review which reverses the order
Added test and pushed as: http://git.eclipse.org/c/platform/eclipse.platform.runtime.git/commit/?id=4ba6ef106d448a2c8bcb9178d875fcdfc63bbbe2
checked in I20140305-2000
