Re: Code review for Issue 1910: JavaLogger is not used if there are multiple PUs in SE mode
Looks ok to me too.
-marina
Tom Ware wrote:
> 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);
>>