users@jax-rs-spec.java.net

[jax-rs-spec users] [jsr339-experts] Re: Re: Re: (Another) proposal on Configuration API changes.

From: Marek Potociar <marek.potociar_at_oracle.com>
Date: Wed, 31 Oct 2012 19:10:30 +0100

On Oct 31, 2012, at 2:28 PM, Bill Burke <bburke_at_redhat.com> wrote:

> What additional methods does RuntimeConfig have over Configured? What additional methods does Configuration have over Configured? I could not see a difference. So, why do we need two separate things?

Well, as for the methods have a look... also, I mentioned them in my last email.

As for why we need the separate things, I mentioned that too, in my last two emails... (injection, confusion among users trying to manually invoke Feature.configure)

> So, you just want to have these separate things because a user might try to configure a Feature or DynamicFeature manually? Come on, that is just totally ridiculous. You must have some really dumb users on the Jersey side. IMO, you've made it much more confusing.

I strongly request you to restrain from making such comments on Jersey users account in the future. Really.

> Initially what I thought you were doing was separating getProviderClasses() from registration. And have like a client-side APplication class. But this is not what you're doing.

Actually, it is. I just in addition to that looked into ways of supporting your use case of setting properties or features on the Invocation.Builder.

> I just don't think what you've done improves things. But, I've known you long enough to know you have your mind set. -1 from me, not that it matters...

Ok. Note taken. Btw. so far everybody else who saw the new API in action liked it more. Simply because as opposed to:

client.configuration().register(FeatureA.class);
WebTarget target = client.target(...);
target.configuration().register(FeatureB.class);
Invocation.Builder request = target.request();
request.configuration().setProperty("foo", "bar");
Response response = request.get();

You can now write:

Response response = client.register(FeatureA.class)
                          .target(...).register(FeatureB.class)
                          .request().setProperty("foo", "bar").get();

Yes, the above is a bit exaggerated use case to make the difference more apparent, still I don't see how the above is not an improvement and how it can be labeled as more confusing.

Marek

>
> On 10/31/2012 6:17 AM, Marek Potociar wrote:
>>
>> On Oct 30, 2012, at 7:18 PM, Bill Burke <bburke_at_redhat.com> wrote:
>>
>>> Ya, I had objections. I don't like how you've duplicated methods into multiple interfaces or get why we need 3-4 interfaces.
>>
>> Well, I tried to explain and never heard back from you until now when it seems I failed to explain the reasons properly, as you just repeat the same objections... So let me try again.
>>
>> Please check the code carefully. I did NOT duplicate anything! I'm extending a Configured interface and merely overriding the return values to keep the APIs fluent (ugly and less extensible alternative would involve making the Configured interface generic...). Also, I did NOT add any new classes or interfaces.
>>
>>> IMO, you just need one core Configurable interface implemented by Client and WebTarget and passed into Feature/DynamicFeature.
>>
>> Actually, that essentially is what I do... I extend the former client.Configuration interface that has been renamed to Configured by Client, WebTarget, Invocation.Builder, Invocation.
>>
>> As I explained, people were confused and tried to configure features manually instead of just registering them. Thus there was a need to decouple the Configurable and client.Configuration. Now it's decoupled. There is RuntimeConfig (Configurable) and client.Configured (client.Configuration).
>>
>> RuntimeConfig is used by Feature and DynamicFeature and is otherwise not exposed from client APIs. I'm also considering supporting injection of an immutable RuntimeConfig on server side. And to me "@Context RuntimeConfig config" looks better and more meaningful than "@Context Configurable configurable" which does not look like something read-only at all. Also, RuntimeConfig has methods targeted at intended deployment/runtime initialization use cases (isEnabled/isRegistered/register...).
>>
>> client.Configured is now an interface extended by main client API components (Client, WebTarget, Invocation.Builder, Invocation). It has methods specific for client-side component configuration. It has methods that enable sharing and copying configuration between components (updateFrom, getProviderClasses, getProviderInstances). Additionally, I order to make sure that Configured.register(...) methods do not break the fluent flow, the extending components override the return value for these methods.
>>
>> I was able to make the change above without introducing any additional interfaces to the API. And I believe I improved the API.
>>
>> Marek
>>
>>>
>>> On 10/30/2012 9:47 AM, Marek Potociar wrote:
>>>> To clarify, I have received a comment from Markus and a couple of
>>>> questions from Bill. But those did not seem to be in a form of an actual
>>>> objections. I'd like to hear from the other members of this EG too. If
>>>> you don't write anything because you're fine with the change, please
>>>> change that behavior, a support or positive feedback is welcome too. At
>>>> least I know you're following our discussions. A sleeping member is
>>>> useless member...
>>>>
>>>> Marek
>>>>
>>>> On Oct 30, 2012, at 2:43 PM, Marek Potociar <marek.potociar_at_oracle.com
>>>> <mailto:marek.potociar_at_oracle.com>> wrote:
>>>>
>>>>> Hello experts,
>>>>>
>>>>> I have not received any negative feedback so far on the proposed
>>>>> Configuration API change. Please send me your feedback by Thursday
>>>>> CoB. Unless I hear any objections, I plan to push the change to master
>>>>> repository on Friday.
>>>>>
>>>>> Thanks,
>>>>> Marek
>>>>>
>>>>>
>>>>> On Oct 27, 2012, at 6:17 PM, Marek Potociar <marek.potociar_at_oracle.com
>>>>> <mailto:marek.potociar_at_oracle.com>> wrote:
>>>>>
>>>>>> Hello experts,
>>>>>>
>>>>>> I'm starting a new thread using a text that I already replied to Bill
>>>>>> on one of our email threads, but I'm worried some of you may miss it.
>>>>>> So here it goes again:
>>>>>>
>>>>>> I am now working on updating my earlier proposal on config API
>>>>>> changes... And I'm thinking that what we could actually do is:
>>>>>>
>>>>>> * Decouple client.Configuration from core.Configurable. This is to
>>>>>> ensure both APIs can be specialized as they need as well as to
>>>>>> make sure nobody is trying to pass client-side
>>>>>> Configuration instance into a Feature instance directly.
>>>>>> * Rename core.Configurable to core.RuntimeConfig.
>>>>>> * Rename client.Configuration to client.Configured (...bear with me :))
>>>>>> * Let Client, WebTarget, Invocation and Invocation.Builder all
>>>>>> extend client.Configured and customize the return values in each
>>>>>> of these classes to retain the API fluency.
>>>>>> * Remove the configuration() getter method from all client-side APIs.
>>>>>>
>>>>>> That way we get the more API fluency even in cases when configuration
>>>>>> is involved, which seems to be particularly important for your
>>>>>> request building use case. Also, we'll still be able to easily create
>>>>>> new clients configured in a same way as any of the existing
>>>>>> client-side components.
>>>>>>
>>>>>> I took a stab on it and put it on github into my private jax-rs repo
>>>>>> under config-api branch (I figured that way I can present unapproved
>>>>>> proposals without pissing off Sergey by making commits into java.net
>>>>>> <http://java.net/> repo...) - please have a look:
>>>>>>
>>>>>> https://github.com/mpotociar/jax-rs/compare/config-api
>>>>>>
>>>>>> Comments are welcome.
>>>>>>
>>>>>> Marek
>>>>>
>>>>
>>>
>>> --
>>> Bill Burke
>>> JBoss, a division of Red Hat
>>> http://bill.burkecentral.com
>>
>
> --
> Bill Burke
> JBoss, a division of Red Hat
> http://bill.burkecentral.com