Tom, Wonseok,
So that the patch doesn't get lost in myriad of other changes of a single
checkin, I can do it separately.
I have couple of code-related questions:
1. The new method name: 'getSessionLog' - should it be 'getDefaultSessionLog'
as it's not really a session logger?
2. The original code:
session.setSessionLog(serverPlatform.getServerLog());
sets a new instance of the log into the session.
The new code:
session.setSessionLog(sessionLog);
sets the existing logger into the session. This needs to be fixed, right?
Wonseok,
Did you run entity-persistence-tests and QLook with your changes?
thanks,
-marina
Tom Ware wrote:
> Hi Wonseok,
>
> The proposal and implementation look good. How would you like us to
> proceed? Here are the options I see:
>
> 1. I can include these changes as part of the next entity-persistence
> update we do from Oracle
> 2. One of the folks at Sun can check this in when they have time
>
> -tom
>
> Wonseok Kim wrote:
>
>> According to my proposal, I implemented this feature.
>> Please review the code below(or see the attached file). Any comments
>> about the code and the proposal are welcome.
>>
>> Summary:
>> * The implementation for this property is very similar to the impl for
>> "toplink.target-server" property.
>> * Added oracle.toplink.essentials.config.LoggerType class for logical
>> values.
>> * Added a new property in TopLinkProperties and a nested
>> LoggerTypeProp class in PropertiesHandler.
>> * The logger is set in EntityManagerSetupImpl.predeploy() like
>> ServerPlatform.getServerLog().
>> * Added an exception message for instantiation failure of the logger
>> class.
>>
>> Diff:
>> ? src/java/oracle/toplink/essentials/config/LoggerType.java
>>
>> package oracle.toplink.essentials.config;
>>
>> /**
>> * Logger type persistence property values.
>> * <p/>
>> * JPA persistence property Usage:
>> * <blockquote>
>> * properties.add(TopLinkProperties.LoggerType, LoggerType.JavaLogger);
>> * </blockquote>
>> * Property values are case-insensitive.
>> *
>> * @author Wonseok Kim
>> */
>> public class LoggerType {
>> public static final String DefaultLogger = "DefaultLogger";
>> public static final String JavaLogger = "JavaLogger";
>>
>> public static final String DEFAULT = DefaultLogger;
>>
>> }
>>
>> Index: src/java/oracle/toplink/essentials/config/TopLinkProperties.java
>> ===================================================================
>> RCS file:
>> /cvs/glassfish/entity-persistence/src/java/oracle/toplink/essentials/config/TopLinkProperties.java,v
>>
>> retrieving revision 1.1
>> diff -c -r1.1 TopLinkProperties.java
>> ***
>> src/java/oracle/toplink/essentials/config/TopLinkProperties.java 11
>> May 2006 19:30:27 -0000 1.1
>> ---
>> src/java/oracle/toplink/essentials/config/TopLinkProperties.java 12
>> Sep 2006 05:48:49 -0000
>> ***************
>> *** 83,89 ****
>> public static final String CACHE_SHARED_DEFAULT = CACHE_SHARED_
>> + DEFAULT;
>>
>> // Customizations properties
>> ! // Valid values are names of levels defined in
>> java.util.logging.Level ,
>> // default value is java.util.logging.Level.CONFIG.getName()
>> public static final String LOGGING_LEVEL = "toplink.logging.level";
>> --- 83,93 ----
>> public static final String CACHE_SHARED_DEFAULT = CACHE_SHARED_
>> + DEFAULT;
>>
>> // Customizations properties
>> !
>> ! // The type of logger. By default DefaultSessionLog is used.
>> ! // Valid values are the logger class name which implements
>> oracle.toplink.essentials.logging.SessionLog
>> ! // or one of values defined in LoggerType.
>> ! public static final String LOGGING_LOGGER =
>> "toplink.logging.logger";
>> // Valid values are names of levels defined in
>> java.util.logging.Level,
>> // default value is java.util.logging.Level.CONFIG.getName()
>> public static final String LOGGING_LEVEL = "toplink.logging.level";
>> Index:
>> src/java/oracle/toplink/essentials/exceptions/EntityManagerSetupException.java
>>
>> ===================================================================
>> RCS file:
>> /cvs/glassfish/entity-persistence/src/java/oracle/toplink/essentials/exceptions/EntityManagerSetupException.java,v
>>
>> retrieving revision 1.5
>> diff -c -r1.5 EntityManagerSetupException.java
>> ***
>> src/java/oracle/toplink/essentials/exceptions/EntityManagerSetupException.java
>> 11 May 2006 19:30:21 -0000 1.5
>> ---
>> src/java/oracle/toplink/essentials/exceptions/EntityManagerSetupException.java
>> 12 Sep 2006 05:48:49 -0000
>> ***************
>> *** 38,43 ****
>> --- 38,44 ----
>> public static final int WRONG_PROPERTY_VALUE_TYPE = 28012;
>> public static final int CANNOT_DEPLOY_WITHOUT_PREDEPLOY = 28013;
>> public static final int FAILED_WHILE_PROCESSING_PROPERTY = 28014;
>> + public static final int FAILED_TO_INSTANTIATE_LOGGER = 28015;
>>
>> /**
>> ***************
>> *** 177,180 ****
>> --- 178,188 ----
>> return setupException;
>> }
>>
>> + public static EntityManagerSetupException
>> failedToInstantiateLogger(String loggerClassName, String propertyName,
>> Exception exception) {
>> + Object[] args = { loggerClassName, propertyName };
>> +
>> + EntityManagerSetupException setupException = new
>> EntityManagerSetupException(ExceptionMessageGenerator.buildMessage
>> (EntityManagerSetupException.class, FAILED_TO_INSTANTIATE_LOGGER,
>> args), exception);
>> + setupException.setErrorCode (FAILED_TO_INSTANTIATE_LOGGER);
>> + return setupException;
>> + }
>> }
>> Index:
>> src/java/oracle/toplink/essentials/exceptions/i18n/EntityManagerSetupExceptionResource.java
>>
>> ===================================================================
>> RCS file:
>> /cvs/glassfish/entity-persistence/src/java/oracle/toplink/essentials/exceptions/i18n/EntityManagerSetupExceptionResource.java,v
>>
>> retrieving revision 1.8
>> diff -c -r1.8 EntityManagerSetupExceptionResource.java
>> ***
>> src/java/oracle/toplink/essentials/exceptions/i18n/EntityManagerSetupExceptionResource.java
>> 8 Sep 2006 18:53:05 -0000 1.8
>> ---
>> src/java/oracle/toplink/essentials/exceptions/i18n/EntityManagerSetupExceptionResource.java
>> 12 Sep 2006 05:48:49 -0000
>> ***************
>> *** 44,50 ****
>> { "28011", "The session,
>> [{0}], built for a persistence unit was not available at the time it
>> was deployed. This means that somehow the session was removed from
>> the container in the middle of the deployment process." },
>> { "28012", "Value [{0}]
>> is of incorrect type for property [{2}], value type should be [{1}]." },
>> { "28013", "Attempted to
>> deploy PersistenceUnit [{0}] for which predeploy method either had not
>> called or had failed" },
>> ! { "28014", "Exception was
>> thrown while processing property [{0}] with value [{1}]." }
>> };
>>
>> /**
>> --- 44,51 ----
>> { "28011", "The session,
>> [{0}], built for a persistence unit was not available at the time it
>> was deployed. This means that somehow the session was removed from
>> the container in the middle of the deployment process." },
>> { "28012", "Value [{0}]
>> is of incorrect type for property [{2}], value type should be [{1}]." },
>> { "28013", "Attempted to
>> deploy PersistenceUnit [{0}] for which predeploy method either had not
>> called or had failed" },
>> ! { "28014", "Exception was
>> thrown while processing property [{0}] with value {[1}]." },
>> ! { "28015", "Failed to
>> instantiate SessionLog of type [{0}] specified in [{1}] property." }
>> };
>>
>> /**
>> Index:
>> 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.40
>> diff -c -r1.40 EntityManagerSetupImpl.java
>> ***
>> src/java/oracle/toplink/essentials/internal/ejb/cmp3/EntityManagerSetupImpl.java
>> 7 Sep 2006 19:54:31 -0000 1.40
>> ---
>> src/java/oracle/toplink/essentials/internal/ejb/cmp3/EntityManagerSetupImpl.java
>> 12 Sep 2006 05:48:50 -0000
>> ***************
>> *** 331,336 ****
>> --- 331,357 ----
>> return serverPlatform;
>> }
>> +
>> + /**
>> + * INTERNAL:
>> + * Build a SessionLog class instance from the logger type property.
>> + */
>> + protected SessionLog getSessionLog(Map m) {
>> + String loggerClassName =
>> (String)PropertiesHandler.getPropertyValueLogDebug(
>> TopLinkProperties.LOGGING_LOGGER, m, session);
>> + if (loggerClassName == null) {
>> + return null;
>> + }
>> +
>> + SessionLog sessionLog = null;
>> + Class cls = findClassForProperty(loggerClassName,
>> TopLinkProperties.LOGGING_LOGGER);
>> + try {
>> + sessionLog = (SessionLog)cls.newInstance();
>> + } catch (Exception ex) {
>> + throw
>> EntityManagerSetupException.failedToInstantiateLogger
>> (loggerClassName, TopLinkProperties.LOGGING_LOGGER, ex);
>> + }
>> + return sessionLog;
>> +
>> + }
>> protected static Class findClass(String className) throws
>> ClassNotFoundException, PrivilegedActionException {
>> if (PrivilegedAccessHelper.shouldUsePrivilegedAccess()){
>> ***************
>> *** 460,467 ****
>> session = new ServerSession(new Project(new DatabaseLogin()));
>>
>> ServerPlatform serverPlatform =
>> getServerPlatform(predeployProperties);
>> ! if (serverPlatform != null){
>> ! AbstractSessionLog.setLog(serverPlatform.getServerLog());
>> }
>>
>> //Bug5389828. Update the logging settings for the singleton
>> logger.
>> --- 481,494 ----
>> session = new ServerSession(new Project(new DatabaseLogin()));
>>
>> ServerPlatform serverPlatform =
>> getServerPlatform(predeployProperties);
>> ! // SessionLog can be specified by the logger property or
>> ServerPlatform.getServerLog().
>> ! // The logger property has a higher priority to
>> ServerPlatform.getServerLog().
>> ! SessionLog sessionLog = getSessionLog(predeployProperties);
>> ! if (sessionLog == null && serverPlatform != null){
>> ! sessionLog = serverPlatform.getServerLog();
>> ! }
>> ! if (sessionLog != null){
>> ! AbstractSessionLog.setLog (sessionLog);
>> }
>>
>> //Bug5389828. Update the logging settings for the singleton
>> logger.
>> ***************
>> *** 469,478 ****
>> // build a list of entities the persistence unit represented
>> by this EntityManagerSetupImpl will use
>> Collection entities = buildEntityList(persistenceUnitInfo,
>> privateClassLoader);
>>
>> if (serverPlatform != null){
>> - session.setSessionLog (serverPlatform.getServerLog());
>> session.setServerPlatform(serverPlatform);
>> ! }
>>
>> initOrUpdateLogging(true, predeployProperties,
>> session.getSessionLog());
>>
>> EntityManagerFactoryProvider.warnOldProperties(predeployProperties,
>> session);
>> --- 496,507 ----
>> // build a list of entities the persistence unit represented
>> by this EntityManagerSetupImpl will use
>> Collection entities = buildEntityList(persistenceUnitInfo,
>> privateClassLoader);
>>
>> + if (sessionLog != null){
>> + session.setSessionLog (sessionLog);
>> + }
>> if (serverPlatform != null){
>> session.setServerPlatform(serverPlatform);
>> ! }
>>
>> initOrUpdateLogging(true, predeployProperties,
>> session.getSessionLog());
>>
>> EntityManagerFactoryProvider.warnOldProperties(predeployProperties,
>> session);
>> Index:
>> src/java/oracle/toplink/essentials/internal/ejb/cmp3/base/PropertiesHandler.java
>>
>> ===================================================================
>> RCS file:
>> /cvs/glassfish/entity-persistence/src/java/oracle/toplink/essentials/internal/ejb/cmp3/base/PropertiesHandler.java,v
>>
>> retrieving revision 1.2
>> diff -c -r1.2 PropertiesHandler.java
>> ***
>> src/java/oracle/toplink/essentials/internal/ejb/cmp3/base/PropertiesHandler.java
>> 31 Aug 2006 02:58:14 -0000 1.2
>> ---
>> src/java/oracle/toplink/essentials/internal/ejb/cmp3/base/PropertiesHandler.java
>> 12 Sep 2006 05:48:50 -0000
>> ***************
>> *** 147,152 ****
>> --- 147,153 ----
>> boolean shouldReturnOriginalValueIfValueToApplyNotFound;
>> static {
>> + addProp(new LoggerTypeProp());
>> addProp(new LoggingLevelProp());
>> addProp(new TargetDatabaseProp());
>> addProp(new TargetServerProp());
>> ***************
>> *** 374,379 ****
>> --- 375,392 ----
>> static void addProp(Prop prop) {
>> prop.initialize();
>> mainMap.put(prop.name <http://prop.name>, prop);
>> + }
>> + }
>> +
>> + protected static class LoggerTypeProp extends Prop {
>> + LoggerTypeProp() {
>> + super(TopLinkProperties.LOGGING_LOGGER,
>> LoggerType.DEFAULT);
>> + this.shouldReturnOriginalValueIfValueToApplyNotFound =
>> true;
>> + String pcg = "oracle.toplink.essentials.logging.";
>> + valueArray = new Object[][] {
>> + {LoggerType.DefaultLogger , pcg + "DefaultSessionLog"},
>> + {LoggerType.JavaLogger, pcg + "JavaLog"}
>> + };
>> }
>> }
>>
>> Thanks,
>> -Wonseok
>>
>> On 9/12/06, *Marina Vatkina * <Marina.Vatkina_at_sun.com
>> <mailto:Marina.Vatkina_at_sun.com>> wrote:
>>
>> Sounds good to me.
>>
>> thanks,
>> -marina
>>
>> Wonseok Kim wrote On 09/11/06 09:19,:
>> > Modified following lines as Tom suggested.
>> >
>> >> This works similar to ServerPlatform.getSessionLog ().
>> >> This property is only used if there is no ServerPlatform.
>> >> If "toplink.target-server" property is specified, then the
>> specified
>> >> ServerPlatform's getSessionLog()
>> >> is used to change the logger type and " toplink.logging.logger"
>> will be
>> >> ignored(warning is required?).
>> >
>> > Modified:
>> > This works similar to ServerPlatform.getSessionLog(), but this
>> logger
>> > type property has a higher priority to
>> ServerPlatform.getSessionLog ().
>> > It means that if server-platform("toplink.target-server" or its
>> > deprecated one) property is specified explicitly by user or
>> impliciltly
>> > by application server such as GlassFish,
>> ServerPlatform.getSessionLog ()
>> > is not used and the specified logger type is used.
>> >
>> > One more thought. I obtain Valid values from physical SessionLog
>> class
>> > names - DefaultSessionLog, JavaLog. How about using more logical
>> value?
>> > "DefaultLogger", "JavaLogger"? Physical class name might be
>> changed in
>> > the future and logical one is easy to remember.
>> >
>> > Thanks
>> > -Wonseok
>> >
>> >
>> > On 9/11/06, *Tom Ware* < tom.ware_at_oracle.com
>> <mailto:tom.ware_at_oracle.com>
>> > <mailto:tom.ware_at_oracle.com <mailto:tom.ware_at_oracle.com>>> wrote:
>> >
>> > Hi Wonseok,
>> >
>> > I think this is a good proposal. My only suggestion is
>> that we might
>> > want to have the specified logger override the one specified
>> in the
>> > server platform.
>> >
>> > -Tom
>> >
>> > Wonseok Kim wrote:
>> >
>> > > Hi, Tom, Marina
>> > >
>> > > May I propose this feature? This is simple to solve, so I
>> have just
>> > > implemented the prototype and worked fine.
>> > >
>> > > Property name: toplink.logging.logger
>> > > Description: Specify the type of logger.
>> > > Purpose: Let's make it easy to change the logger type in
>> Java SE mode.
>> > > Valid values: logger class name which implements
>> > > oracle.toplink.essentials.logging.SessionLog
>> > > or one of the following string values.
>> > > * DefaultSessionLog (default)
>> > > * JavaLog
>> > >
>> > > Example: persistence.xml file
>> > > <property name="toplink.logging.logger" value="JavaLog"/>
>> > >
>> > > This works similar to ServerPlatform.getSessionLog ().
>> > > This property is only used if there is no ServerPlatform.
>> > > If "toplink.target-server" property is specified, then the
>> specified
>> > > ServerPlatform's getSessionLog()
>> > > is used to change the logger type and
>> "toplink.logging.logger " will be
>> > > ignored(warning is required?).
>> > >
>> > > Currently there are just two logger types - default and
>> Java logger...
>> > > Maybe another logger type can be added in the future.(such
>> as log4j)
>> > >
>> > > The property name and valid values can be changed if there
>> are better
>> > > ones.
>> > > This is just a proposal and comments are welcome... What
>> do you think?
>> > >
>> > > Thanks
>> > > - Wonseok
>> > >
>> > > On 9/6/06, *Wonseok Kim* <guruwons_at_gmail.com
>> <mailto:guruwons_at_gmail.com>
>> > <mailto: guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>>
>> > > <mailto:guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>
>> <mailto:guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>>>> wrote:
>> > >
>> > > I filed an issue for this.
>> > >
>> https://glassfish.dev.java.net/issues/show_bug.cgi?id=1096
>> > > <
>> https://glassfish.dev.java.net/issues/show_bug.cgi?id=1096>
>> > >
>> > > Also I wrote a blog with regard to the workaround Tom
>> suggested
>> > > (see issue comment).
>> > > I couldn't extend NoServerPlatform because it's final
>> class, so I
>> > > extended ServerPlatformBase.
>> > >
>> > > Thanks
>> > > - Wonseok
>> > >
>> > >
>> > > On 9/5/06, *Tom Ware* < tom.ware_at_oracle.com
>> <mailto:tom.ware_at_oracle.com>
>> > <mailto: tom.ware_at_oracle.com <mailto:tom.ware_at_oracle.com>>
>> > > <mailto: tom.ware_at_oracle.com
>> <mailto:tom.ware_at_oracle.com> <mailto:tom.ware_at_oracle.com
>> <mailto:tom.ware_at_oracle.com>>>> wrote:
>> > >
>> > > Hi Wonseok,
>> > >
>> > > You are correct, there is no current
>> configuration option
>> > > that lets
>> > > you automatically change the logger. That would
>> be a good
>> > feature
>> > > request to enter in the issue tracker.
>> > >
>> > > I believe your best bet is to subclass
>> NoServerPlatform and
>> > > override
>> > > the getServerLog() method in the same way as it is
>> > implemented in
>> > > SunAS9ServerPlatform.
>> > >
>> > > -Tom
>> > >
>> > > Wonseok Kim wrote:
>> > >
>> > >> Hi, all
>> > >>
>> > >> After I see the discussion thread about issue#988 - Logging
>> > >> integration with GF happens too late,
>> > >> I got a question about logger in Java SE mode.
>> > >> Java logger (JavaLog) can be used by configuring
>> > > ServerPlatform (like
>> > >> GlassFish), but can it be used in Java SE also?
>> > >> I can't see any configuration for changing logger - only
>> > >> DefaultSessionLog is used.
>> > >>
>> > >> If it cannot be changed in current implementation, how about
>> > > making
>> > >> the property like "
>> toplink.logging.logger=<logger_classname>"?
>> > >> I think it can be also used in Java EE environment. Then it
>> > > doesn't
>> > >> need to implement ServerPlatform class to change only
>> logger
>> > > in other
>> > >> Java EE container environment.
>> > >>
>> > >> Thanks
>> > >> --
>> > >> Wonseok Kim
>> > >> Senior Developer, TmaxSoft
>> > >
>> > >
>> > >
>> >
>> >
>>
>>
>