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
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>
>>>>
>>>>
>>
>>
>>