Hi Tom,
I decided not to make any further changes and filed 2 issues -
an issue for #1:
https://glassfish.dev.java.net/issues/show_bug.cgi?id=1113
a task for #2:
https://glassfish.dev.java.net/issues/show_bug.cgi?id=1114
(should the flexibility of options to change everything so many times be
reduced or eliminated?).
Now about <property name="oracle.toplink.essentials" value="FINE"/>:
See
https://glassfish.dev.java.net/javaee5/persistence/entity-persistence-support.html#Logging
for options to change TL logging in GF. The global change results in an extra
property added to the <module-log-levels ...> in domain.xml.
thanks,
-marina
Tom Ware wrote:
> Hi Marina,
>
> Your code changes look good to me. Thanks to both of you.
>
> Other comments inline:
>
> Wonseok Kim wrote:
>
>> Marina,
>> Your patch looks good - I confirmed that it translates messages well.
>> It's my pleasure if you check in the code I contribute - actually most
>> of code comes from Tom's suggestion.
>>
>> Thanks
>> - Wonseok
>>
>> On 9/7/06, *Marina Vatkina* <Marina.Vatkina_at_sun.com
>> <mailto:Marina.Vatkina_at_sun.com>> wrote:
>>
>> Tom, Wonseok,
>>
>> I think we are ready to go, though some questions still exist
>> (see below). Wonseok, do you see a problem if I check in your
>> changes?
>>
>> The changes are attached. Here are the notes:
>>
>> a) I slightly modified Wonseok's code to init session log before
>> calling warnOldProperties.
>>
>> b) JavaLog is modified to build the message via formatMessage(entry).
>> This is done because the LogRecord's resource bundle is not being
>> used by the handler down the road, but rather accessed from its
>> logger.
>> Now there is no method Logger.setResourceBundle() as the logger is
>> being constructed with the bundle, and as such can never have it
>> changed.
>> But in TopLink code such resource is actually dynamic
>> (TraceLocalizationResource
>> vs. LoggingLocalizationResource), so the only option (other than
>> create
>> 2 loggers for each category) is preformatting the message ourselves.
>>
>> c) Fixed typo in words that include "C/categories" in JavaLog and
>> SessionLog.
>>
>> Questions (The current fix doesn't affect these problems):
>>
>> 1. We are changing AbstractSessionLog all the time, reseting it to
>> each
>> and every PU settings. Not only there can be a threading issue,
>> but even
>> for a single app with 2 PUs, the second PU wins. Is it the
>> intended behavior?
>>
> I agree there is an issue here, but I do not think it is particularly
> serious. The majority of the logging that uses AbstractSessionLog will
> occur before a session is available and most of the remaining logging
> will occur through the session log. Having said that, I think it is
> worth filing an issue for this problem both to make it visible to the
> community and so we can address it at some point.
>
>>
>> 2. The loggers properties (both, the AbstractSessionLog and the
>> session
>> logger) are reset again in updateServerSession that happens in the
>> middle
>> of deploy() processing. I.e. if there is an assumption that
>> logging setting
>> can change in between predeploy and deploy, why aren't they reset
>> the first
>> thing in deploy(), before any other step that would require a logger?
>>
> I do not see any problem with changing the way logging is set up in
> deploy so that the logging properties are set up before any other
> logging could occur.
>
>>
>> Issues: see
>> https://glassfish.dev.java.net/issues/show_bug.cgi?id=1107
>> (which means that only PU properties currently work).
>>
> I took a look at this bug. Can you elaborate some more?
> What is <property name="oracle.toplink.essentials" value="FINE"/>
> expected to acheive? Where is it being set?
>
> Thanks,
> -Tom
>
>>
>> thanks,
>> -marina
>>
>>
>> Marina Vatkina wrote On 09/06/06 10:28,:
>> > Hi Tom,
>> >
>> > I'm using ear file from Java Persistence Example page
>> > (
>>
>> https://glassfish.dev.java.net/javaee5/persistence/persistence-example.html).
>>
>> >
>> > If you apply Wonseok's changes, and deploy the app, you'll see
>> some messages
>> > displayed correctly, but in system.out (those come from java2db
>> processing -
>> > there is a separate bug on that), and the rest (from load into
>> appserver)
>> > show up as "oracle.toplink.essentials.defaultsessionname" but in
>> that strange
>> > format that I described below.
>> >
>> > thanks,
>> > -marina
>> >
>> > Tom Ware wrote:
>> >
>> >>Hi Marina,
>> >>
>> >> How do I recreate this issue?
>> >>
>> >>-Tom
>> >>
>> >>Marina Vatkina wrote:
>> >>
>> >>
>> >>>Tom, Wonseok,
>> >>>
>> >>>Yes, it solves the 1st part of problem (and I like the
>> solution), but
>> >>>unfortunately there is more to it (unless it happens only in my
>> >>>environment):
>> >>>the logging when integrated, can't resolve message keys and the
>> log
>> >>>messages
>> >>>are printed as a list of arguments, and the keys instead.
>> >>>
>> >>>Wonseok, do you see the same problem?
>> >>>
>> >>>Tom, any idea?
>> >>>
>> >>>thanks,
>> >>>-marina
>> >>>
>> >>>Tom Ware wrote:
>> >>>
>> >>>
>> >>>
>> >>>>Hi Wonseok,
>> >>>>
>> >>>>Thanks for taking a deeper look at this issue. This solution
>> seems
>> >>>>good to me. It appears to resolve all the issues.
>> >>>>Marina, what do you think?
>> >>>>
>> >>>>-Tom
>> >>>>
>> >>>>Wonseok Kim wrote:
>> >>>>
>> >>>>
>> >>>>
>> >>>>
>> >>>>>Hi Tom, Marina
>> >>>>>
>> >>>>>I'm investigating this issue too. Tom's patch has another
>> problem
>> >>>>>besides Marina's comment.
>> >>>>>
>> >>>>>session is not initialized before getServerPlatform(). So the
>> >>>>>serverPlatform is created with null session. This causes
>> following
>> >>>>>exception when creating EntityManager(in ServerSession's
>> deploy phase).
>> >>>>>
>> >>>>>-----
>> >>>>>Caused by: java.lang.NullPointerException
>> >>>>> at
>>
>> >>>>>oracle.toplink.essentials.platform.server.ServerPlatformBase.initializeExternalTransactio
>>
>>
>> >>>>>nController(ServerPlatformBase.java:213)
>> >>>>> at
>>
>> >>>>>oracle.toplink.essentials.internal.sessions.DatabaseSessionImpl.preConnectDatasource(Data
>>
>> >>>>> baseSessionImpl.java:664)
>> >>>>> at
>>
>> >>>>>oracle.toplink.essentials.internal.sessions.DatabaseSessionImpl.loginAndDetectDatasource(
>>
>> >>>>>DatabaseSessionImpl.java:534)
>> >>>>> at
>>
>> >>>>>oracle.toplink.essentials.ejb.cmp3.EntityManagerFactoryProvider.login(EntityManagerFactor
>>
>> >>>>>yProvider.java:180)
>> >>>>> at
>>
>> >>>>>oracle.toplink.essentials.internal.ejb.cmp3.EntityManagerSetupImpl.deploy(EntityManagerSe
>>
>> >>>>>tupImpl.java:230)
>> >>>>> at
>>
>> >>>>>oracle.toplink.essentials.internal.ejb.cmp3.base.EntityManagerFactoryImpl.getServerSessio
>>
>>
>> >>>>>n(EntityManagerFactoryImpl.java:78)
>> >>>>> at
>>
>> >>>>>oracle.toplink.essentials.internal.ejb.cmp3.base.EntityManagerFactoryImpl.createEntityMan
>>
>> >>>>>agerImpl( EntityManagerFactoryImpl.java:113)
>> >>>>> at
>>
>> >>>>>oracle.toplink.essentials.internal.ejb.cmp3.EntityManagerFactoryImpl.createEntityManager(
>>
>> >>>>>EntityManagerFactoryImpl.java :84)
>> >>>>> at
>>
>> >>>>>com.sun.enterprise.util.EntityManagerWrapper._getDelegate(EntityManagerWrapper.java:166)
>>
>> >>>>>
>> >>>>> at
>> com.sun.enterprise.util.EntityManagerWrapper.createQuery
>> >>>>>(EntityManagerWrapper.java:271)
>> >>>>> at
>>
>> >>>>>hellojpa.ProductManagerBean.removeAllProducts(ProductManagerBean.java:58)
>>
>> >>>>>
>> >>>>> at sun.reflect.NativeMethodAccessorImpl.invoke0(Native
>> Method)
>> >>>>> at
>>
>> >>>>>sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
>>
>> >>>>>
>> >>>>> at
>>
>> >>>>>sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
>>
>> >>>>>
>> >>>>> at java.lang.reflect.Method.invoke (Method.java:585)
>> >>>>> at
>>
>> >>>>>com.sun.enterprise.security.application.EJBSecurityManager.runMethod(EJBSecurityManager.j
>>
>> >>>>>ava:1050)
>> >>>>> at
>>
>> >>>>>com.sun.enterprise.security.SecurityUtil.invoke(SecurityUtil.java:165)
>>
>> >>>>> at
>>
>> >>>>>com.sun.ejb.containers.BaseContainer.invokeTargetBeanMethod(BaseContainer.java
>>
>> :2788)
>> >>>>>
>> >>>>> at
>>
>> >>>>>com.sun.ejb.containers.BaseContainer.intercept(BaseContainer.java:3870)
>>
>> >>>>> at
>> >>>>>com.sun.ejb.containers.EJBLocalObjectInvocationHandler.invoke
>> >>>>>(EJBLocalObjectInvocationHan dler.java:184)
>> >>>>> ... 31 more
>> >>>>>-----
>> >>>>>
>> >>>>>I have a solution for this problem (including Marina's
>> comment). May
>> >>>>>I provide a patch for this?
>> >>>>>
>> >>>>>See below diff (from Tom's patch). I also attached full
>> source and
>> >>>>>diff from cvs.
>> >>>>>In summary, this do two things.
>> >>>>>
>> >>>>>1. translateOldProperties() is done before getServerPlatform()
>> >>>>>without logging. logging is done separately by another new
>> method
>> >>>>>warnOldProperties() after logger is set.
>> >>>>>2. session is created before getServerPlatform()
>> >>>>>
>> >>>>>EntityManagerFactoryProvider.java 2006-09-06 10:16:
>> 20.000000000
>> >>>>>+0900
>> >>>>>***************
>> >>>>>*** 77,85 ****
>> >>>>> public static final String DEFAULT_DDL_GENERATION_MODE =
>> >>>>>DDL_SQL_SCRIPT_GENERATION;
>> >>>>> // TEMPORARY - WILL BE REMOVED.
>> >>>>>! // The two variables below are used to warn users about
>> >>>>>deprecated property name and suggest the valid name.
>> >>>>> // TEMPORARY the old property names will be translated
>> to the
>> >>>>>new ones and processed.
>> >>>>>- public static final String DEPRECATED_TARGET_SERVER =
>> >>>>>"toplink.server.platform.class.name
>> <http://toplink.server.platform.class.name>
>> >>>>><http://toplink.server.platform.class.name>";
>> >>>>> protected static final String oldPropertyNames[][] = {
>> >>>>> {TopLinkProperties.JDBC_WRITE_CONNECTIONS_MAX,
>> >>>>>"toplink.max-write-connections"},
>> >>>>> {TopLinkProperties.JDBC_WRITE_CONNECTIONS_MIN , "to
>> >>>>>plink.min-write-connections"},
>> >>>>>--- 77,84 ----
>> >>>>> public static final String DEFAULT_DDL_GENERATION_MODE =
>> >>>>>DDL_SQL_SCRIPT_GENERATION;
>> >>>>> // TEMPORARY - WILL BE REMOVED.
>> >>>>>! // Used to warn users about deprecated property name and
>> >>>>>suggest the valid name.
>> >>>>> // TEMPORARY the old property names will be translated
>> to the
>> >>>>>new ones and processed.
>> >>>>> protected static final String oldPropertyNames[][] = {
>> >>>>> {TopLinkProperties.JDBC_WRITE_CONNECTIONS_MAX, "
>> >>>>> toplink.max-write-connections"},
>> >>>>> {TopLinkProperties.JDBC_WRITE_CONNECTIONS_MIN,
>> >>>>>"toplink.min-write-connections"},
>> >>>>>***************
>> >>>>>*** 87,93 ****
>> >>>>> {TopLinkProperties.JDBC_READ_CONNECTIONS_MIN ,
>> >>>>>"toplink.min-read-connections"},
>> >>>>> {TopLinkProperties.JDBC_BIND_PARAMETERS ,
>> >>>>>"toplink.bind-all-parameters"},
>> >>>>> {TopLinkProperties.TARGET_DATABASE, "
>> >>>>>toplink.platform.class.name
>> <http://toplink.platform.class.name>
>> <http://toplink.platform.class.name>"},
>> >>>>>! {TopLinkProperties.TARGET_SERVER,
>> DEPRECATED_TARGET_SERVER},
>> >>>>> { TopLinkProperties.CACHE_SIZE_DEFAULT,
>> "toplink.cache.defau
>> >>>>>lt-size"}
>> >>>>> };
>> >>>>>
>> >>>>>--- 86,92 ----
>> >>>>> { TopLinkProperties.JDBC_READ_CONNECTIONS_MIN,
>> >>>>>"toplink.min-read-connections"},
>> >>>>> {TopLinkProperties.JDBC_BIND_PARAMETERS,
>> >>>>>"toplink.bind-all-parameters "},
>> >>>>> {TopLinkProperties.TARGET_DATABASE,
>> >>>>>"toplink.platform.class.name
>> <http://toplink.platform.class.name> <
>> http://toplink.platform.class.name>"},
>> >>>>>! {TopLinkProperties.TARGET_SERVER, "
>> >>>>>toplink.server.platform.class.name
>> <http://toplink.server.platform.class.name>
>> >>>>><http://toplink.server.platform.class.name>"},
>> >>>>> {TopLinkProperties.CACHE_SIZE_DEFAULT,
>> >>>>>" toplink.cache.default-size"}
>> >>>>> };
>> >>>>>
>> >>>>>***************
>> >>>>>*** 370,378 ****
>> >>>>> for(int i=0; i < oldPropertyNames.length ; i++) {
>> >>>>> Object value =
>> >>>>>getConfigPropertyAsString(oldPropertyNames[i][1], m);
>> >>>>> if(value != null) {
>> >>>>>! session.log(SessionLog.CONFIG,
>> >>>>>SessionLog.TRANSACTION , "deprecated_property",
>> oldPropertyNames[i]);
>> >>>>> m.put(oldPropertyNames[i][0], value);
>> >>>>> }
>> >>>>> }
>> >>>>> }
>> >>>>> }
>> >>>>>--- 369,387 ----
>> >>>>> for(int i=0; i < oldPropertyNames.length; i++) {
>> >>>>> Object value = getConfigPropertyAsString(oldProper
>> >>>>>tyNames[i][1], m);
>> >>>>> if(value != null) {
>> >>>>>! if(session != null)
>> >>>>>! session.log(SessionLog.CONFIG ,
>> >>>>>SessionLog.TRANSACTION, "deprecated_property",
>> oldPropertyNames[i]);
>> >>>>> m.put(oldPropertyNames[i][0], value);
>> >>>>> }
>> >>>>> }
>> >>>>> }
>> >>>>>+
>> >>>>>+ public static void warnOldProperties(Map m,
>> AbstractSession
>> >>>>>session) {
>> >>>>>+ for(int i=0; i < oldPropertyNames.length; i++) {
>> >>>>>+ Object value = m.get(oldPropertyNames[i][1]);
>> >>>>>+ if(value != null) {
>> >>>>>+ session.log (SessionLog.CONFIG,
>> >>>>>SessionLog.TRANSACTION, "deprecated_property",
>> oldPropertyNames[i]);
>> >>>>>+ }
>> >>>>>+ }
>> >>>>>+ }
>> >>>>> }
>> >>>>>
>> >>>>>EntityManagerSetupImpl.java 2006-09-06 13:35:28.000000000
>> +0900
>> >>>>>***************
>> >>>>>*** 311,323 ****
>> >>>>> protected ServerPlatform getServerPlatform(Map m) {
>> >>>>> String serverPlatformClassName =
>>
>> >>>>>(String)PropertiesHandler.getPropertyValueLogDebug(TopLinkProperties.TARGET_SERVER
>>
>>
>> >>>>>, m, session);
>> >>>>> if (serverPlatformClassName == null) {
>> >>>>>! // TEMPORARY: Check the old platform
>> name. This will
>> >>>>>be removed when the deprecated properties are actually removed
>> >>>>>! // Note: We are not printing config message
>> here. We
>> >>>>>are relying on the warning occuring later in the code path
>> where the
>> >>>>>! // properties are translated.
>> >>>>>! serverPlatformClassName =
>> >>>>>(String)PropertiesHandler.getPropertyValueLog
>> >>>>>Debug(EntityManagerFactoryProvider.DEPRECATED_TARGET_SERVER, m,
>> >>>>>session);
>> >>>>>! if (serverPlatformClassName == null) {
>> >>>>>! return null;
>> >>>>>! }
>> >>>>> }
>> >>>>>
>> >>>>> ServerPlatform serverPlatform = null;
>> >>>>>--- 311,317 ----
>> >>>>> protected ServerPlatform getServerPlatform(Map m) {
>> >>>>> String serverPlatformClassName =
>> >>>>>(String)PropertiesHandler.getPropertyValueLogDebug(
>> >>>>>TopLinkProperties.TARGET_SERVER, m, session);
>> >>>>> if (serverPlatformClassName == null) {
>> >>>>>! return null;
>> >>>>> }
>> >>>>>
>> >>>>> ServerPlatform serverPlatform = null;
>> >>>>>***************
>> >>>>>*** 458,481 ****
>> >>>>> ClassLoader privateClassLoader =
>> >>>>>persistenceUnitInfo.getNewTempClassLoader();
>> >>>>> predeployProperties =
>> >>>>>EntityManagerFactoryProvider.mergeMaps(extendedProperties,
>> >>>>> persistenceUnitInfo.getProperties());
>> >>>>>
>> >>>>>! ServerPlatform serverPlatform =
>> >>>>>getServerPlatform(predeployProperties);
>> >>>>> if (serverPlatform != null){
>> >>>>> AbstractSessionLog.setLog (serverPlatform.get
>> >>>>>ServerLog());
>> >>>>> }
>> >>>>>! //Bug5389828. Update the logging settings
>> for
>> >>>>>the singleton logger.
>> >>>>> initOrUpdateLogging(true, predeployProperties,
>> >>>>>AbstractSessionLog.getLog());
>> >>>>> // build a list of entities the persistence unit
>> >>>>>represented by this EntityManagerSetupImpl will use
>> >>>>> Collection entities =
>> buildEntityList(persistenceUnitInfo,
>> >>>>>privateClassLoader);
>> >>>>>
>> >>>>>- session = new ServerSession(new Project(new
>> >>>>>DatabaseLogin()));
>> >>>>>-
>> >>>>> if (serverPlatform != null){
>> >>>>>
>> session.setSessionLog(serverPlatform.getServerLog());
>> >>>>> session.setServerPlatform(serverPlatform);
>> >>>>> }
>> >>>>>
>> >>>>>!
>> >>>>>
>>
>> EntityManagerFactoryProvider.translateOldProperties(predeployProperties,
>> >>>>>session);
>> >>>>> initOrUpdateLogging(true, predeployProperties,
>> >>>>>session.getSessionLog ());
>> >>>>> session.getPlatform().setConversionManager(new
>> >>>>>EJB30ConversionManager());
>> >>>>> --- 452,480 ----
>> >>>>> ClassLoader privateClassLoader =
>> >>>>>persistenceUnitInfo.getNewTempClass
>> >>>>>Loader();
>> >>>>> predeployProperties =
>> >>>>>EntityManagerFactoryProvider.mergeMaps(extendedProperties,
>> >>>>>persistenceUnitInfo.getProperties ());
>> >>>>>
>> >>>>>! // translate old properties
>> >>>>>! // this should be done before using properties ( i.e.
>> >>>>>ServerPlatform)
>> >>>>>!
>>
>> >>>>>EntityManagerFactoryProvider.translateOldProperties(predeployProperties,
>>
>> >>>>>null);
>> >>>>>!
>> >>>>>! // create server session (it should be done before
>> >>>>>initializing ServerPlatform)
>> >>>>>! 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.
>> >>>>> initOrUpdateLogging(true, predeployProperties,
>> >>>>>AbstractSessionLog.getLog());
>> >>>>> // 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);
>> >>>>> }
>> >>>>>
>> >>>>>!
>>
>> >>>>>EntityManagerFactoryProvider.warnOldProperties(predeployProperties,
>> >>>>>session);
>> >>>>> initOrUpdateLogging(true, predeployProperties,
>> >>>>>session.getSessionLog());
>> >>>>> session.getPlatform().setConversionManager(new
>> >>>>>EJB30ConversionManager());
>> >>>>> Thanks
>> >>>>>- Wonseok
>> >>>>>I
>> >>>>>
>> >>>>>On 9/6/06, *Marina Vatkina* <Marina.Vatkina_at_sun.com
>> <mailto:Marina.Vatkina_at_sun.com>
>> >>>>><mailto: Marina.Vatkina_at_sun.com
>> <mailto:Marina.Vatkina_at_sun.com>>> wrote:
>> >>>>>
>> >>>>> Hi Tom,
>> >>>>>
>> >>>>> No, this doesn't work as PropertiesHandler doesn't know
>> about the
>> >>>>>old
>> >>>>> (deprecated) name and returns null as for unknown
>> property. Should
>> >>>>> I now
>> >>>>> add the old name to the PropertiesHandler as well? This
>> seems like
>> >>>>> a wrong
>> >>>>> idea for handling something that you want to deprecate...
>> >>>>>
>> >>>>> thanks,
>> >>>>> -marina
>> >>>>>
>> >>>>> Tom Ware wrote:
>> >>>>> > Hi Marina,
>> >>>>> >
>> >>>>> > The initial bug was that the logging that occurs in
>> >>>>> > buildEntityList(persistenceUnitInfo, privateClassLoader)
>> was not
>> >>>>> > affected by the logging settings.
>> >>>>> >
>> >>>>> > I am attaching some suggested changes - most are based
>> on your
>> >>>>> initial
>> >>>>> > fix. Admittedly, there are a couple of pieces that are
>> somewhat
>> >>>>> hacky.
>> >>>>> >
>> >>>>> > 1. The way we deal with the fact that ServerPlatform
>> could come
>> >>>>> from one
>> >>>>> > of the deprecated properties
>> >>>>> > 2. Only the non-deprecated properties will affect the
>> logging that
>> >>>>> > occurs in buildEntityListt(persistenceUnitInfo,
>> >>>>>privateClassLoader)
>> >>>>> >
>> >>>>> > These changes are just a suggestion and totally open to
>> alternate
>> >>>>> > changes, and I have not done much testing, but figured I
>> pass
>> >>>>>on my
>> >>>>> > thoughts.
>> >>>>> >
>> >>>>> > -Tom
>> >>>>> >
>> >>>>> >
>> >>>>> > Marina Vatkina wrote:
>> >>>>> >
>> >>>>> >> Hi Tom,
>> >>>>> >>
>> >>>>> >> Tom Ware wrote:
>> >>>>> >>
>> >>>>> >>
>> >>>>> >>> Hi Marina,
>> >>>>> >>>
>> >>>>> >>> I think there still may be a couple of issues:
>> >>>>> >>>
>> >>>>> >>> 1. Doesn't updateServerPlatform() replace the logger
>> with the
>> >>>>> server
>> >>>>> >>> one?
>> >>>>> >>
>> >>>>> >>
>> >>>>> >> Yes.
>> >>>>> >>
>> >>>>> >> If so, don't we lose the settings from the
>> initOfUpdateLogging()
>> >>>>> >>
>> >>>>> >>
>> >>>>> >>> calls that occur before update updateServerPlatform()?
>> >>>>> >>>
>> >>>>> >>
>> >>>>> >>
>> >>>>> >> We propagate the setting in setNewLog:
>> >>>>> >>
>> >>>>> >>
>> newLog.setLevel(session.getSessionLog().getLevel());
>> >>>>> >>
>> >>>>> >> newLog.setShouldPrintDate
>> >>>>> (session.getSessionLog ().shouldPrintDate());
>> >>>>> >>
>> >>>>> >>
>> >>>>>
>>
>> >>>>>newLog.setShouldPrintThread(session.getSessionLog().shouldPrintThread());
>>
>> >>>>>
>> >>>>> >>
>> >>>>> >>
>> >>>>>
>>
>> >>>>>newLog.setShouldPrintSession(session.getSessionLog().shouldPrintSession());
>>
>> >>>>>
>> >>>>>
>> >>>>> >>
>> >>>>> >>
>> >>>>> >>
>> >>>>>
>> >>>>>newLog.setShouldLogExceptionStackTrace(session.getSessionLog
>> ().shouldLogExceptionStackTrace());
>> >>>>>
>> >>>>> >>
>> >>>>> >>
>> >>>>> >> session.setSessionLog(newLog);
>> >>>>> >>
>> >>>>> >>
>> >>>>> >> But(!) we are doing it anyway even now, just a bit later.
>> >>>>> >>
>> >>>>> >> We cannot do
>> >>>>> >>
>> >>>>> >>
>> >>>>> >>> that because it will reopen the bug that initially
>> causes us to
>> >>>>> >>> change where our logging settings are set.
>> >>>>> >>>
>> >>>>> >>
>> >>>>> >>
>> >>>>> >> What was that bug about?
>> >>>>> >>
>> >>>>> >>
>> >>>>> >>
>> >>>>> >>> 2. In my mind, this bug is not fixed until all logging
>> >>>>> messages go to
>> >>>>> >>> the correct place. If the default log is not set on
>> >>>>> >>> AbstractSessionLog, that is not done. Can we not, make a
>> >>>>>call to
>> >>>>> >>> AbstractSessionLog.setLog() with the correct log?
>> >>>>> >>>
>> >>>>> >>
>> >>>>> >>
>> >>>>> >> Here is the problem:
>> >>>>> >> EntityManagerFactoryProvider.translateOldProperties
>> needs a
>> >>>>> logger, and
>> >>>>> >> we can't get the correct server platform without prior
>> >>>>> translation (blame
>> >>>>> >> those who decided to invent new names :().
>> >>>>> >>
>> >>>>> >>
>> >>>>> >>
>> >>>>> >>
>> >>>>> >>> As a result of these two concerns, I think there is
>> some work
>> >>>>> to do.
>> >>>>> >>> Perhaps a solution could be to break up the
>> >>>>> updateServerPlatform in
>> >>>>> >>> to two methods. 1 - get the serverPlatform instance,
>> 2 - set
>> >>>>> it on
>> >>>>> >>> the session. If you do that, the first method could
>> be called
>> >>>>> before
>> >>>>> >>> the first initOrUpdateLogging call, and the instance
>> derived
>> >>>>> from it
>> >>>>> >>> could to set the correct logger both on the session
>> and on the
>> >>>>> >>> AbstractSesssionLog at the appropriate time.
>> >>>>> >>>
>> >>>>> >>
>> >>>>> >>
>> >>>>> >> OK. Let's discuss it further.
>> >>>>> >>
>> >>>>> >>
>> >>>>> >>> -Tom
>> >>>>> >>>
>> >>>>> >>> Marina Vatkina wrote:
>> >>>>> >>>
>> >>>>> >>>
>> >>>>> >>>
>> >>>>> >>>> Hi Tom,
>> >>>>> >>>>
>> >>>>> >>>> Tom Ware wrote:
>> >>>>> >>>>
>> >>>>> >>>>
>> >>>>> >>>>
>> >>>>> >>>>
>> >>>>> >>>>> Hi Marina,
>> >>>>> >>>>>
>> >>>>> >>>>> First of all, sorry for the confusion.
>> >>>>> >>>>>
>> >>>>> >>>>>
>> >>>>> >>>>
>> >>>>> >>>> No problem. Good think I asked ;)
>> >>>>> >>>>
>> >>>>> >>>>
>> >>>>> >>>>
>> >>>>> >>>>
>> >>>>> >>>>> Now a question: Are we also adding some code that
>> sets the
>> >>>>> default
>> >>>>> >>>>> log? I am surprised I do not see any code that
>> influences
>> >>>>> >>>>> AbstractSessionLog's default log. Don't we need to
>> logging
>> >>>>>from
>> >>>>> >>>>> AbstractSessionLog.getLog () to be logged to the
>> correct
>> >>>>>place?
>> >>>>> >>>>>
>> >>>>> >>>>>
>> >>>>> >>>>
>> >>>>> >>>> Yes, I'd like to understand the whole
>> initOrUpdateLogging()
>> >>>>>story
>> >>>>> >>>> myself.
>> >>>>> >>>>
>> >>>>> >>>> Do you see a problem if I file a separate issue for
>> that and
>> >>>>> check
>> >>>>> >>>> in my changes now?
>> >>>>> >>>>
>> >>>>> >>>>
>> >>>>> >>>>
>> >>>>> >>>>
>> >>>>> >>>>
>> >>>>> >>>>
>> >>>>> >>>>> Aside from that, I think moving the
>> updateServerPlatform
>> >>>>> call and
>> >>>>> >>>>> chaning the location of the JTA initialization will
>> be fine.
>> >>>>> >>>>>
>> >>>>> >>>>> ServerPlatform should not change in the middle of a
>> run, so
>> >>>>> I think
>> >>>>> >>>>> removing the call to this method in updateServerSession
>> >>>>> should be
>> >>>>> >>>>> fine.
>> >>>>> >>>>>
>> >>>>> >>>>>
>> >>>>> >>>>
>> >>>>> >>>> OK.
>> >>>>> >>>>
>> >>>>> >>>> thanks,
>> >>>>> >>>> -marina
>> >>>>> >>>>
>> >>>>> >>>>
>> >>>>> >>>>
>> >>>>> >>>>
>> >>>>> >>>>
>> >>>>> >>>>> -Tom
>> >>>>> >>>>>
>> >>>>> >>>>>
>> >>>>> >>>>> Marina Vatkina wrote:
>> >>>>> >>>>>
>> >>>>> >>>>>
>> >>>>> >>>>>
>> >>>>> >>>>>
>> >>>>> >>>>>
>> >>>>> >>>>>> Hi Tom,
>> >>>>> >>>>>>
>> >>>>> >>>>>> I think (I don't know why I didn't notice it right
>> away) that
>> >>>>> >>>>>> there is
>> >>>>> >>>>>> some disconnect here ;).
>> >>>>> >>>>>>
>> >>>>> >>>>>> I didn't change updateServerSession. I added a call to
>> >>>>> >>>>>> updateServerPlatform
>> >>>>> >>>>>> from predeploy() *and* modified that
>> (updateServerPlatform)
>> >>>>> method
>> >>>>> >>>>>> not to
>> >>>>> >>>>>> set JTA flag. Setting JTA flag is called explicitly
>> from
>> >>>>> >>>>>> updateServerSession
>> >>>>> >>>>>> after updateLogins() call.
>> >>>>> >>>>>>
>> >>>>> >>>>>> Now, all updateServerPlatform is doing, is loading the
>> >>>>> class, then
>> >>>>> >>>>>> sets
>> >>>>> >>>>>> session.setServerPlatform(serverPlatform), and sets
>> the
>> >>>>>log. I
>> >>>>> >>>>>> can't set
>> >>>>> >>>>>> the log without loading the platform class. Now,
>> >>>>> >>>>>> a) how harmful is
>> session.setServerPlatform(serverPlatform)
>> >>>>> call?
>> >>>>> >>>>>> b) can server platform change in the middle of the
>> run? If
>> >>>>> yes,
>> >>>>> >>>>>> what would
>> >>>>> >>>>>> it mean? If no, then I can make another change, and
>> >>>>>completely
>> >>>>> >>>>>> remove call
>> >>>>> >>>>>> to this method from updateServerSession.
>> >>>>> >>>>>>
>> >>>>> >>>>>> What is wrong with this solution? (Apart that I'm
>> still not
>> >>>>> sure
>> >>>>> >>>>>> we are not
>> >>>>> >>>>>> wasting time on multiple calls to
>> initOrUpdateLogging() -
>> >>>>> but that
>> >>>>> >>>>>> can wait).
>> >>>>> >>>>>>
>> >>>>> >>>>>> thanks,
>> >>>>> >>>>>> -marina
>> >>>>> >>>>>>
>> >>>>> >>>>>>
>> >>>>> >>>>>> Tom Ware wrote:
>> >>>>> >>>>>>
>> >>>>> >>>>>>
>> >>>>> >>>>>>
>> >>>>> >>>>>>
>> >>>>> >>>>>>
>> >>>>> >>>>>>> Hi Marina,
>> >>>>> >>>>>>>
>> >>>>> >>>>>>> I'll give you a partial explanation and then take an
>> >>>>>educated
>> >>>>> >>>>>>> guess at the second part of the answer.
>> >>>>> >>>>>>>
>> >>>>> >>>>>>> First, the partial explanation, initOrUpdateLogging()
>> >>>>> updates the
>> >>>>> >>>>>>> logging values for a log. There are two logs we are
>> >>>>> interested in.
>> >>>>> >>>>>>>
>> >>>>> >>>>>>> 1. The default log
>> >>>>> >>>>>>> 2. The session log
>> >>>>> >>>>>>>
>> >>>>> >>>>>>> You will seen each place initOrUpdateLogging() is
>> called,
>> >>>>> it is
>> >>>>> >>>>>>> called for each of those logs. I guess that means
>> your
>> >>>>>update
>> >>>>> >>>>>>> will not necessarily in that specific method, but
>> it will be
>> >>>>> >>>>>>> where that method is called.
>> >>>>> >>>>>>>
>> >>>>> >>>>>>> Now, the educated guess (the original implementer
>> of this
>> >>>>> feature
>> >>>>> >>>>>>> will be available next week to confirm or deny
>> what I say
>> >>>>> here):
>> >>>>> >>>>>>>
>> >>>>> >>>>>>> I believe we make calls to initOrUpdateLogging()
>> both in
>> >>>>> >>>>>>> predeploy and deploy because we have always had
>> the goal of
>> >>>>> >>>>>>> allowing application servers to do all the
>> predeployment
>> >>>>>work
>> >>>>> >>>>>>> ,serialize it in some way and then do the
>> deployment work
>> >>>>> later
>> >>>>> >>>>>>> (or over and over again). When we have implemented a
>> >>>>>feature
>> >>>>> >>>>>>> like that, making these calls in both places will
>> make
>> >>>>> logging
>> >>>>> >>>>>>> more flexible. (i.e. you will be able to
>> configure it
>> >>>>>for the
>> >>>>> >>>>>>> predeployment step, and then later, when you actually
>> >>>>> deploy you
>> >>>>> >>>>>>> will be able to change the values.)
>> >>>>> >>>>>>>
>> >>>>> >>>>>>> -Tom
>> >>>>> >>>>>>>
>> >>>>> >>>>>>>
>> >>>>> >>>>>>> Marina Vatkina wrote:
>> >>>>> >>>>>>>
>> >>>>> >>>>>>>
>> >>>>> >>>>>>>
>> >>>>> >>>>>>>
>> >>>>> >>>>>>>
>> >>>>> >>>>>>>
>> >>>>> >>>>>>>> Tom,
>> >>>>> >>>>>>>>
>> >>>>> >>>>>>>> To change initOrUpdateLogging() I need answers to
>> Q2.2.
>> >>>>> >>>>>>>>
>> >>>>> >>>>>>>> thanks,
>> >>>>> >>>>>>>> -marina
>> >>>>> >>>>>>>>
>> >>>>> >>>>>>>> Tom Ware wrote:
>> >>>>> >>>>>>>>
>> >>>>> >>>>>>>>
>> >>>>> >>>>>>>>
>> >>>>> >>>>>>>>
>> >>>>> >>>>>>>>
>> >>>>> >>>>>>>>
>> >>>>> >>>>>>>>> Hi Marina,
>> >>>>> >>>>>>>>>
>> >>>>> >>>>>>>>> I think we need to investigate solving the
>> logging issue
>> >>>>> >>>>>>>>> without moving the whole updateServerSession
>> method.
>> >>>>> >>>>>>>>>
>> >>>>> >>>>>>>>> The reason I say that is that updateServerSession()
>> >>>>> occurs in
>> >>>>> >>>>>>>>> the deploy method by design. We intentionally
>> do not
>> >>>>> create a
>> >>>>> >>>>>>>>> server session until deploy is called. (Notice
>> deploy
>> >>>>> is not
>> >>>>> >>>>>>>>> called until a ServerSession is required by the
>> >>>>> >>>>>>>>> EntityManagerFactory) Update ServerSession
>> provides a
>> >>>>> number
>> >>>>> >>>>>>>>> of important updates that should only be done
>> when the
>> >>>>> session
>> >>>>> >>>>>>>>> is created.
>> >>>>> >>>>>>>>>
>> >>>>> >>>>>>>>> Is it possible to make achange in the
>> >>>>>initOrUpdateLogging()
>> >>>>> >>>>>>>>> call to first change the default log to the
>> appropriate
>> >>>>>log
>> >>>>> >>>>>>>>> prior to all other changes? There is an
>> >>>>> >>>>>>>>> abstractSessionLog.setLog() method that can be
>> used to
>> >>>>> set the
>> >>>>> >>>>>>>>> default session log. A call to that method should
>> >>>>> result in
>> >>>>> >>>>>>>>> logging going to the specified log.
>> >>>>> >>>>>>>>>
>> >>>>> >>>>>>>>> Here are some answers to a couple of your
>> questions:
>> >>>>> >>>>>>>>>
>> >>>>> >>>>>>>>> 1. Protected Methods - We realize that in
>> traditional
>> >>>>> >>>>>>>>> applications only methods that are actually used
>> in a
>> >>>>> protected
>> >>>>> >>>>>>>>> manner should be set as protected. Since
>> TopLink is a
>> >>>>> library
>> >>>>> >>>>>>>>> that people may want to extend, we provide
>> protected
>> >>>>>(rather
>> >>>>> >>>>>>>>> than private) methods as a way of allowing them
>> to do
>> >>>>>that.
>> >>>>> >>>>>>>>> 2. getServerSession()- It looks like this method
>> can be
>> >>>>> >>>>>>>>> removed. The ServerSession aquisition logic has
>> been
>> >>>>> moved to
>> >>>>> >>>>>>>>> EntityManagerFactoryImpl
>> >>>>> >>>>>>>>>
>> >>>>> >>>>>>>>> -Tom
>> >>>>> >>>>>>>>>
>> >>>>> >>>>>>>>> Marina Vatkina wrote:
>> >>>>> >>>>>>>>>
>> >>>>> >>>>>>>>>
>> >>>>> >>>>>>>>>
>> >>>>> >>>>>>>>>
>> >>>>> >>>>>>>>>
>> >>>>> >>>>>>>>>
>> >>>>> >>>>>>>>>> Team,
>> >>>>> >>>>>>>>>>
>> >>>>> >>>>>>>>>> The following solution solves the logger
>> integration
>> >>>>> problem
>> >>>>> >>>>>>>>>> (all changes are
>> >>>>> >>>>>>>>>> made to EntityManagerSetupImpl):
>> >>>>> >>>>>>>>>>
>> >>>>> >>>>>>>>>> 1. Modify updateServerPlatform() and move the
>> lines
>> >>>>> >>>>>>>>>>
>> >>>>> >>>>>>>>>>
>> >>>>> >>>>>>>>>>
>> >>>>>
>> if(!session.getLogin().shouldUseExternalTransactionController())
>> >>>>> >>>>>>>>>> {
>> >>>>> >>>>>>>>>> serverPlatform.disableJTA();
>> >>>>> >>>>>>>>>> }
>> >>>>> >>>>>>>>>> to be after updateLogins(m) as there was a comment
>> >>>>> >>>>>>>>>> "update ServerPlatform must be called after
>> >>>>> updateLogins - to
>> >>>>> >>>>>>>>>> set correct useJTA flag"
>> >>>>> >>>>>>>>>>
>> >>>>> >>>>>>>>>> 2. Add
>> updateServerPlatform(predeployProperties) after
>> >>>>>the
>> >>>>> >>>>>>>>>> second call
>> >>>>> >>>>>>>>>> to initOrUpdateLogging() in predeploy.
>> >>>>> >>>>>>>>>> Q2.1: can it be done earlier that that?
>> >>>>> >>>>>>>>>> Q2.2: Why is this method executed at least 4 times
>> >>>>> (twice in
>> >>>>> >>>>>>>>>> predeploy()
>> >>>>> >>>>>>>>>> plus twice via updateServerSession(), which
>> comments
>> >>>>> say can
>> >>>>> >>>>>>>>>> happen
>> >>>>> >>>>>>>>>> more than once)?
>> >>>>> >>>>>>>>>>
>> >>>>> >>>>>>>>>> 3. I left call to updateServerPlatform()
>> (modified as
>> >>>>> in #1) from
>> >>>>> >>>>>>>>>> updateServerSession() as I don't fully
>> understand the
>> >>>>> reason
>> >>>>> >>>>>>>>>> of calling it from
>> >>>>> >>>>>>>>>> there.
>> >>>>> >>>>>>>>>> Q3.1: Should I just remove it?
>> >>>>> >>>>>>>>>>
>> >>>>> >>>>>>>>>> Other questions:
>> >>>>> >>>>>>>>>> a) Why do we have protected methods in this
>> class that
>> >>>>> are not
>> >>>>> >>>>>>>>>> called by any
>> >>>>> >>>>>>>>>> other class?
>> >>>>> >>>>>>>>>>
>> >>>>> >>>>>>>>>> b) What is the purpose of method
>> getServerSession(Map) in
>> >>>>> >>>>>>>>>> EntityManagerSetupImpl
>> >>>>> >>>>>>>>>> that is not called by any other code? Can I
>> remove it (I
>> >>>>> >>>>>>>>>> commented it out and
>> >>>>> >>>>>>>>>> clean compile had no complains)?
>> >>>>> >>>>>>>>>>
>> >>>>> >>>>>>>>>> The changed class is attached (with commented out
>> >>>>> >>>>>>>>>> getServerSession(Map) method).
>> >>>>> >>>>>>>>>>
>> >>>>> >>>>>>>>>> thanks,
>> >>>>> >>>>>>>>>> -marina
>> >>>>> >>>>>>>>>>
>> >>>>> >>>>>>>>>>
>> >>>>> >>>>>>>>>>
>> >>>>> >>>>>>>
>> >>>>> >>>>>>>
>> >>>>> >>>>>>>
>> >>>>> >>>>>
>> >>>>> >>>>>
>> >>>>> >>>>>
>> >>>>> >>>
>> >>>>> >>>
>> >>>>>
>> >>>>>
>> >>>>>
>> >>>>>
>>
>>
>>
>>
>>
>> --
>> Wonseok Kim
>> Senior Developer, TmaxSoft
>
>
>