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> 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
>
>