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