Re: Code review for Issue 1910: JavaLogger is not used if there are multiple PUs in SE mode
Looks ok to me.
Marina?
-Tom
Wonseok Kim wrote:
> Hi Marina, Tom
>
> Please see the following issue and the fix below.
> https://glassfish.dev.java.net/issues/show_bug.cgi?id=1910
>
> In EntityManagerSetupImpl.updateLoggers (), the first PU update both
> AbstractSessionLog's and Session's logger with JavaLogger correctly,
> but the second PU incorrectly determines current logger is JavaLogger
> (because AbstractSessionLog's logger is updated in the first PU), so
> doesn't update its Session's logger and DefaultLogger is used.
>
> This is because determining current logger is done with
> AbstractSessionLog's current logger. I fixed this to use Session's
> logger as below diff(Also I moved the line of getting current logger
> to the nearest line using it since no other code refer to it). The
> singleton logger - AbstractSessionLog - is very confusing and not
> adapted well in multiple PUs as well as in EE mode, so I think it
> should be avoided or removed in the future as Marina said before.
>
> I confirmed that JavaLogger is used correctly for both PUs with the
> test case.
>
> Please review...Thanks!
>
> Index:
> entity-persistence/src/java/oracle/toplink/essentials/internal/ejb/cmp3/EntityManagerSetupImpl.java
>
> ===================================================================
> RCS file:
> /cvs/glassfish/entity-persistence/src/java/oracle/toplink/essentials/internal/ejb/cmp3/EntityManagerSetupImpl.java,v
> retrieving revision 1.48
> diff -c -w -r1.48 EntityManagerSetupImpl.java
> ***
> entity-persistence/src/java/oracle/toplink/essentials/internal/ejb/cmp3/EntityManagerSetupImpl.java
> 4 Jan 2007 14:30:43 -0000 1.48
> ---
> entity-persistence/src/java/oracle/toplink/essentials/internal/ejb/cmp3/EntityManagerSetupImpl.java
> 5 Jan 2007 07:20:38 -0000
> ***************
> *** 347,353 ****
> * @param sessionNameChanged the boolean that denotes a
> sessionNameChanged change in the session.
> */
> protected void updateLoggers(Map m, boolean
> serverPlatformChanged, boolean sessionNameChanged) {
> - SessionLog currentLog = AbstractSessionLog.getLog();
> // Logger(SessionLog type) can be specified by the logger
> property or ServerPlatform.getServerLog().
> // The logger property has a higher priority to
> ServerPlatform.getServerLog().
> String loggerClassName =
> PropertiesHandler.getPropertyValueLogDebug(TopLinkProperties.LOGGING_LOGGER,
> m, session);
> --- 347,352 ----
> ***************
> *** 356,361 ****
> --- 355,361 ----
> // different state.
> SessionLog singletonLog = null, sessionLog = null;
> if (loggerClassName != null) {
> + SessionLog currentLog = session.getSessionLog();
> if
> (!currentLog.getClass().getName().equals(loggerClassName)) {
> // Logger class was specified and it's not what's
> already there.
> Class sessionLogClass =
> findClassForProperty(loggerClassName, TopLinkProperties.LOGGING_LOGGER);
>