Hi Marina,
In general, our calls that use the default logger
(AbstractSessionLog.getLog()) are there in methods that do not have
access to the session logger. That usually occurs because the code
occurs somewhere where there may not yet be a session, or somewhere
where it is very difficult to get a handle on the session.
Either the container or the user's application is expected to ensure
this log corresponds to what is expected. Hopefully we can make it so
that in the JPA RI, users do not have to worry about this setting and it
is set by default.
-Tom
Marina Vatkina wrote:
>Tom,
>
>While you are thinking about the answers, here is one more: why does the code
>use static calls AbstractSessionLog.getLog().log(...) in the following classes
>listed below? Keep in mind that there is no AbstractSessionLog.setLog call
>anywhere in the code.
>
>The list:
>src/java/oracle/toplink/essentials/ejb/cmp3/persistence/PersistenceUnitProcessor.java
>src/java/oracle/toplink/essentials/internal/ejb/cmp3/EntityManagerSetupImpl.java
>src/java/oracle/toplink/essentials/internal/ejb/cmp3/JavaSECMPInitializer.java
>src/java/oracle/toplink/essentials/internal/ejb/cmp3/xml/parser/PersistenceContentHandler.java
>src/java/oracle/toplink/essentials/internal/helper/ConcurrencyManager.java
>src/java/oracle/toplink/essentials/platform/server/ServerPlatformBase.java
>src/java/oracle/toplink/essentials/tools/schemaframework/DefaultTableGenerator.java
>src/java/oracle/toplink/essentials/tools/sessionmanagement/SessionManager.java
>
>thanks,
>-marina
>
>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
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>
>>>>>
>>>>>
>>>
>>>
>>>