Hi Marina,
I think there still may be a couple of issues:
1. Doesn't updateServerPlatform() replace the logger with the server
one? If so, don't we lose the settings from the initOfUpdateLogging()
calls that occur before update updateServerPlatform()? We cannot do
that because it will reopen the bug that initially causes us to change
where our logging settings are set.
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?
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.
-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
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>
>>>>
>>>>
>>
>>
--
Tom Ware
Principal Software Engineer
Oracle Canada Inc.
Direct: (613) 783-4598
Email: tom.ware_at_oracle.com