I just went through a few of the connector commands. Holy Cow. I can
see why Jerome developed CRUD -- these commands are all obvious
copy&pastes of each other!
For these particular commands the pattern is
@Param .... target ...
Server targetServer = domain.getServerNamed(target);
targetServer.someMethod(); // possible NPE
=============================================
I like the idea of a null server name meaning "ME!!!". Good idea Bill.
I think I'll run with that.
=============================================
I verified in a debugger -- @Param is **guaranteed** to get processed
long after @Inject.
I also agree that for readability/maintainability we should not have
this sort of order-dependency. The user reading the code would probably
not know about this order of evaluation. It isn't obvious. Making it
clear, simple and explicit inside the execute() command is the way to go.
Summary:
1. I'll change the handling of null to mean the current server name
where it make3s sense.
2. The setting of target will move into the execute command
=========
Thanks for the help architects!!
On 4/15/2010 2:24 PM, Bill Shannon wrote:
> Jerome Dochez wrote on 04/15/2010 01:58 PM:
>> On Apr 15, 2010, at 1:53 PM, Byron Nevins wrote:
>>
>>> Jerome, Bill
>>>
>>> I'm about to change a bunch of classes that assume the current
>>> server's name is "server".
>>>
>>> BEFORE:
>>> @Param(optional=true)
>>> String target =
>>> SystemPropertyConstants.DEFAULT_SERVER_INSTANCE_NAME;
>>>
>>> AFTER:
>>> // add implements PostConstruct if necessary
>>>
>>> @Inject
>>> private ServerEnvironment env;
>>>
>>> @Param(optional=true)
>>> String target;
>>>
>>> public void postConstruct() {
>>> if(target == null)
>>> target = env.getInstanceName();
>>> }
>>>
>>> ===============================
>>>
>>> I'm doing the more complicated postConstruct() technique because I
>>> don't think I can get away with doing the following with 100%
>>> certainty.
>>>
>>> @Inject
>>> private ServerEnvironment env;
>>>
>>> @Param(optional=true)
>>> String target = env.getInstanceName();
>>>
>>> =================================
>>>
>>> Is it guaranteed that the above will work (i.e. order of evaluation
>>> by HK2)?
>>> This is my injection solution. I normally would just use the
>>> old-fashioned way of setting a system-wide final String with the
>>> instance's name.
>>>
>>> What do you recommend?
>>
>> In order to do 2, you need to ensure that @Inject is injected before
>> @Param (which I think is the case), look in the AdminAdapter for the
>> code. You also need to then change the AdminCommand javadocs to state
>> the order of injection between the two annotations so it is an
>> explicit contract.
>> then you can use option 1.
>> otherwise option 2 is clunky yet safe.
>
> I'm pretty sure the order of injection will happen to give you what
> you want, but it seems dangerous to depend on. I'm not convinced that
> postConstruct will give you what you want. As I understand it, @Inject
> is handled by the habitat when it creates the object. The habitat then
> calls PostConstruct. The injection for @Param is done *after* the
> habitat returns the object, and thus after postConstruct is called.
>
> So I guess the question is whether to depend on this ordering, or to
> just handle this in the code...
>
> public void execute(AdminCommandContext context) {
> if (target == null)
> target = env.getInstanceName();
>
> or in the code that uses target...
>
> target != null ? target : env.getInstanceName()
>
> How exactly will target be used? If it's commonly passed to another
> method to get (e.g.) the Server object or something like that, maybe
> *that* method should be enhanced so that null means "use the current
> instance name"?
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: admin-unsubscribe_at_glassfish.dev.java.net
> For additional commands, e-mail: admin-help_at_glassfish.dev.java.net
>
--
Byron Nevins - Oracle Corporation
Home: 650-359-1290
Cell: 650-784-4123
Sierra: 209-295-2188