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?
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?
Issues: see
https://glassfish.dev.java.net/issues/show_bug.cgi?id=1107
(which means that only PU properties currently work).
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>";
>>>>> 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>"},
>>>>>! {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>"},
>>>>>! {TopLinkProperties.TARGET_SERVER, "
>>>>>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>> 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
>>>>> >>>>>>>>>>
>>>>> >>>>>>>>>>
>>>>> >>>>>>>>>>
>>>>> >>>>>>>
>>>>> >>>>>>>
>>>>> >>>>>>>
>>>>> >>>>>
>>>>> >>>>>
>>>>> >>>>>
>>>>> >>>
>>>>> >>>
>>>>>
>>>>>
>>>>>
>>>>>