Marina,
Session's constructor sets serverPlatform to NoServerPlatform - don't check
for null, check for NoServerPlatform.
----- Original Message -----
From: "Marina Vatkina" <Marina.Vatkina_at_Sun.COM>
To: <persistence_at_glassfish.dev.java.net>
Sent: Tuesday, October 31, 2006 6:15 PM
Subject: Re: adding server platform and session name change into logger
integration
> Which makes it even more complicated :(.
>
> The current code in the logging (no, you can't ignore it when you touch
> the server platform or the session name) relies at 'null' as the "default"
> logger use case. Will it be safe to check 'instanceof NoServerPlatform' in
> addition to null?
>
> thanks,
> -marina
>
> Andrei Ilitchev wrote:
>> Marina,
>>
>> PropertyHandler returns default value (which is NoServerPlatform) in case
>> an property value is an empty string.
>> So to get back NoServerPlatform, the user should set the property to
>> either an empty string, or "None", or
>> "oracle.toplink.essentials.platform.server.NoServerPlatform.
>>
>>
>> ----- Original Message ----- From: "Marina Vatkina"
>> <Marina.Vatkina_at_Sun.COM>
>> To: <persistence_at_glassfish.dev.java.net>
>> Sent: Tuesday, October 31, 2006 4:00 PM
>> Subject: Re: adding server platform and session name change into logger
>> integration
>>
>>
>>> Hi Andrei,
>>>
>>> Yes, this is better. But I'm confused... You filed an issue 1333 that
>>> server
>>> platform can be unset in the map. How will it be specified by the user?
>>> If
>>> a) as an empty string - your code doesn't handle this case.
>>> b) as "NoServerPlatform" - will you use reflection to create it?
>>>
>>> thanks,
>>> -marina
>>>
>>> Andrei Ilitchev wrote:
>>>
>>>> Let's try another file type...
>>>> ----- Original Message ----- From: "Marina Vatkina"
>>>> <Marina.Vatkina_at_Sun.COM>
>>>> To: <persistence_at_glassfish.dev.java.net>
>>>> Sent: Tuesday, October 31, 2006 2:28 PM
>>>> Subject: Re: adding server platform and session name change into logger
>>>> integration
>>>>
>>>>
>>>>> Andrei,
>>>>>
>>>>> Can you please resend the file? It came as 0 size.
>>>>>
>>>>> thanks,
>>>>> -marina
>>>>>
>>>>> Andrei Ilitchev wrote:
>>>>>
>>>>>> Hi Marina, Wonseok,
>>>>>> Attached is my version of changes to EntityManagerSetupImpl class.
>>>>>> I concentrated on handling of serverPlatform property and didn't
>>>>>> look at the logging stuff.
>>>>>> Thanks,
>>>>>> Andrei
>>>>>>
>>>>>> ----- Original Message -----
>>>>>> *From:* Wonseok Kim <mailto:guruwons_at_gmail.com>
>>>>>> *To:* persistence_at_glassfish.dev.java.net
>>>>>> <mailto:persistence_at_glassfish.dev.java.net>
>>>>>> *Sent:* Tuesday, October 31, 2006 11:20 AM
>>>>>> *Subject:* Re: adding server platform and session name change
>>>>>> into
>>>>>> logger integration
>>>>>>
>>>>>> Hi Marina,
>>>>>> Overall, it looks good to me.
>>>>>>
>>>>>> Some minor comments below.
>>>>>>
>>>>>> protected void updateServerSession(Map m) {
>>>>>> if (session == null || session.isConnected()) {
>>>>>> return;
>>>>>> }
>>>>>>
>>>>>> // Session name and ServerPlatform could've changed which
>>>>>> will affect the loggers.
>>>>>> boolean serverPlatformChanged =
>>>>>> setOrUpdateServerPlatform(m);
>>>>>> boolean sessionNameChanged = updateSessionName(m);
>>>>>>
>>>>>> if (serverPlatformChanged) {
>>>>>> // Update loggers and settings for the singleton
>>>>>> logger
>>>>>> and the session logger.
>>>>>> updateLoggers(predeployProperties);
>>>>>> } else if (sessionNameChanged) {
>>>>>> // In JavaLog this will result in logger name changes,
>>>>>> // but won't affect DefaultSessionLog.
>>>>>> session.getSessionLog().setSession(session);
>>>>>> }
>>>>>>
>>>>>> -> you mean updateLoggers(m), right?
>>>>>>
>>>>>> AbstractSessionLog.setSession() need to be changed to accept new
>>>>>> session, see below.
>>>>>>
>>>>>> public void setSession(Session session) {
>>>>>> if (this.session == null) {
>>>>>> this.session = session;
>>>>>> buildSessionType();
>>>>>> buildSessionHashCode();
>>>>>> }
>>>>>> }
>>>>>>
>>>>>> initOrUpdateLogging() doesn't need 'init' parameter anymore after
>>>>>> your fix for issue 1107.
>>>>>>
>>>>>> protected void initOrUpdateLogging(boolean init, Map m,
>>>>>> SessionLog log) {
>>>>>>
>>>>>> Cheers,
>>>>>> -Wonseok
>>>>>>
>>>>>>
>>>>>> On 10/31/06, *Marina Vatkina* <Marina.Vatkina_at_sun.com
>>>>>> <mailto:Marina.Vatkina_at_sun.com>> wrote:
>>>>>>
>>>>>> Tom, Andrei, Wonseok,
>>>>>>
>>>>>> Attached is the changed file with the ideas for fixing this
>>>>>> problem. I
>>>>>> didn't test it yet, but would like to start the discussion as
>>>>>> the practice
>>>>>> shows that changing EMSetupImpl is a painful exercise.
>>>>>>
>>>>>> Probably the main change for you guys is replacing void with
>>>>>> a
>>>>>> boolean
>>>>>> as the returning type for updateSessionName(). Please do not
>>>>>> panic, but
>>>>>> suggest your ideas if you don't like it - I need to know if
>>>>>> the
>>>>>> name has
>>>>>> changed and would like to avoid saving the name, then
>>>>>> comparing
>>>>>> it again
>>>>>> after it had been replaced or not: the comparison is done
>>>>>> inside
>>>>>> the method
>>>>>> anyway. Keep in mind that if the server platform has changed,
>>>>>> I
>>>>>> need the
>>>>>> new session name before fixing the loggers.
>>>>>>
>>>>>> As you can see, in case of the server platform change, all of
>>>>>> the loggers
>>>>>> need to be replaced (we can probably go further and check the
>>>>>> actual logger
>>>>>> type change, but I don't think it's worth the effort), in
>>>>>> particular when
>>>>>> the value is unset in the Map.
>>>>>>
>>>>>> If only the session name change, it seems to be enough to
>>>>>> update
>>>>>> the session
>>>>>> in the existing logger (and do not touch the default logger
>>>>>> at all).
>>>>>>
>>>>>> Wonseok, that also meant moving initOrUpdateLogging() outside
>>>>>> the
>>>>>> updateLoggers() call. I also removed parameters from the
>>>>>> method
>>>>>> signature
>>>>>> where they are available as instance variable.
>>>>>>
>>>>>> If you all agree, I'll start running the tests. If not, I'd
>>>>>> like
>>>>>> to sort
>>>>>> everything out before going further.
>>>>>>
>>>>>> thanks,
>>>>>> -marina
>>>>>>
>>>>>>
>>>>>
>>>
>