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> 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?
>
> 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
> >>>>> >>>>>>>>>>
> >>>>> >>>>>>>>>>
> >>>>> >>>>>>>>>>
> >>>>> >>>>>>>
> >>>>> >>>>>>>
> >>>>> >>>>>>>
> >>>>> >>>>>
> >>>>> >>>>>
> >>>>> >>>>>
> >>>>> >>>
> >>>>> >>>
> >>>>>
> >>>>>
> >>>>>
> >>>>>
>
>
>
--
Wonseok Kim
Senior Developer, TmaxSoft