From: Andrei Ilitchev <andrei.ilitchev_at_oracle.com>
Date: Tue, 31 Oct 2006 14:22:53 -0500

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