[prev in list] [next in list] [prev in thread] [next in thread] 

List:       wink-dev
Subject:    Re: [DISCUSS] priority sort key as override for Providers?
From:       Bryant Luk <bryant.luk () gmail ! com>
Date:       2009-09-24 15:16:48
Message-ID: d5d04a30909240816r260833a7tc377a56dbcda96d8 () mail ! gmail ! com
[Download RAW message or body]

Regarding the user over system change in general, I think the way it
is done now (using the MessageBodyWriter<Object> vs.
MessageBodyWriter<String>) will cause the least changes in JAX-RS 1.0
vs. 1.1 user code so that's why I wanted to promote it.  However, I'm
open to the interpretation.

For determining whether a provider is system vs. user, I think having
an explicit boolean set maybe during the application processing is
better and then adding basically a ProvidersRegistry.addProvider(OF,
double, boolean) that will be called if the WinkApplication is a
"system" application. Having the system provider determined by package
name may cause unexpected behaviors if downstream consumers re-package
Wink or add in their own providers that don't start with
org.apache.wink as system default ones.

Sorry if this gets sent more than once.  Seems Gmail is not completely
working today.

2009/9/24 Michael Elman <tarlog@gmail.com>:
>>I believe that user providers should always
> be chosen over system providers regardless of the C004 change
> The problem that spec doesn't state this explicitly, but it doesn't state
> explicitly the other way...
> Currently the change is fine with me.
>
> Another possible solution is checking provider's package name and any
> provider under org.apache.wink.* is considered as a system provider and it
> is used as a first key for sorting. So it won't mess with priority.
> Something like:
>
>  public int compareTo(OFHolder<T> o) {
>                // check if this is a system provider
>                boolean systemProvider =
> of.getInstanceClass().getPackage().getName().startsWith("org.apache.wink");
>                boolean oSystemProvider =
> o.of.getInstanceClass().getPackage().getName().startsWith("org.apache.wink");
>
>                if (systemProvider) {
>                    if (!oSystemProvider) {
>                        return -1;
>                    }
>                } else if (oSystemProvider) {
>                    return 1;
>                }
>
>                // first compare by media type
>                int compare = MediaTypeUtils.compareTo(mediaType,
> o.mediaType);
>                if (compare != 0) {
>                    return compare;
>                }
>                // second compare by generic type
>                if (genericType != o.genericType) {
>                    if (genericType.isAssignableFrom(o.genericType)) {
>                        return -1;
>                    } else {
>                        return 1;
>                    }
>                }
>                // last compare by priority
>                return Double.compare(of.priority, o.of.priority);
>            }
>
> Of course " boolean systemProvider =
> of.getInstanceClass().getPackage().getName().startsWith("org.apache.wink");
> " can be stored as OFHolder field to improve performance.
>
> On Thu, Sep 24, 2009 at 2:13 AM, Bryant Luk <bryant.luk@gmail.com> wrote:
>
>> Just to give a heads up on a few changes I'm going to make.
>>
>> For the system providers, I think I'm going to make it very explicit
>> that the system providers have a 0.1 priority.  Due to the way the
>> sorting worked, user providers were always chosen before system ones
>> but they always had the same 0.5 priority on the server I believe.  I
>> think that's why Mike added his explicit setPriority code for the
>> InnerApplication in his patch.
>>
>> The other change is that I believe that user providers should always
>> be chosen over system providers regardless of the C004 change.  I
>> believe Mike brought up a good point that a user provided
>> MessageBodyWriter<Object> versus a system MessageBodyWriter<String>
>> should call the user MessageBodyWriter<Object>.isWritable() before
>> calling the system one.  I'll "abuse" the priority system to fix this
>> for now so that anything below 0.2 priority is a "system" provider
>> that will be sorted later than "user" providers above 0.2 but perhaps
>> this should be fixed differently.
>>
>> On Wed, Sep 23, 2009 at 1:57 AM, Michael Elman <tarlog@gmail.com> wrote:
>> > Currently I've implemented that system providers and the ones located
>> using
>> > wink-application has the same low (0.1) priority.
>> > It can be changed that system providers have 0.1 priority and
>> > wink-application have 0.2 priority, while the default priority is 0.5.
>> >
>> > On Tue, Sep 22, 2009 at 9:54 PM, Bryant Luk <bryant.luk@gmail.com>
>> wrote:
>> >
>> >> I think there should be a clear distinction in Wink provider priority
>> >> from default (system) providers versus user added ones (whether
>> >> priority is defined as a tie-breaker or an override).
>> >>
>> >> For the ones added automatically via wink-providers,I would prefer
>> >> somewhere in-between default system ones and explicitly user-added
>> >> ones in the JAX-RS Application subclass and any other config
>> >> registration system.  I think this would allow users to drop in
>> >> wink-provider JARs if they want to override the defaults while still
>> >> respecting the programmatic config.
>> >>
>> >> On Tue, Sep 22, 2009 at 12:50 PM, Michael Elman <tarlog@gmail.com>
>> wrote:
>> >> > Originally it was designed as a "tie-breaker".
>> >> >
>> >> >
>> >> > On Tue, Sep 22, 2009 at 8:37 PM, Michael Rheinheimer <rott@us.ibm.com
>> >> >wrote:
>> >> >
>> >> >>
>> >> >>
>> >> >> Hi team,
>> >> >>
>> >> >> We have a double 'priority' param that can be passed to some of the
>> >> >> ProvidersRegistry.addProvider methods.  This is a non-spec key, thus
>> a
>> >> WINK
>> >> >> feature.  Given that, I've been treating it as an "override" rather
>> than
>> >> a
>> >> >> "tie-breaker" when sorting Provider priority.  For example, if a user
>> >> >> specifies ContextResolver1 that @Produces("text/*") and a
>> >> ContextResolver2
>> >> >> that @Produces("text/xml") and adds them to the ProvidersRegistry as
>> >> such:
>> >> >>
>> >> >> addProvider(ContextResolver1, 5);
>> >> >> addProvider(ContextResolver2, 0);
>> >> >>
>> >> >> Even though ContextResolver2 would be a closer match for media type
>> >> >> "text/xml", ContextResolver1 would be favored due to the higher
>> declared
>> >> >> priority.  Any thoughts on this?
>> >> >>
>> >> >> If we all agree, I think we have some work to do around supporting
>> this
>> >> >> priority feature;  making sure the user knows what the default system
>> >> >> provider priority is, perhaps issuing warnings when the priority of
>> >> their
>> >> >> provider is the same as the system default or negative, adding tests,
>> >> >> optimizing code paths in ProvidersRegistry, etc.
>> >> >>
>> >> >> Please see patch in WINK-193 (this being the more informative) and
>> >> WINK-167
>> >> >> (which is easier to see how the priority would be used as an
>> override).
>> >> >>
>> >> >> Thanks..
>> >> >> mike
>> >> >
>> >>
>> >>
>> >>
>> >> --
>> >>
>> >> - Bryant Luk
>> >>
>> >
>>
>>
>>
>> --
>>
>> - Bryant Luk
>>
>



-- 

- Bryant Luk


[prev in list] [next in list] [prev in thread] [next in thread]