Hi Marina,
Your code changes look good to me. Thanks to both of you.
Other comments inline:
Wonseok Kim wrote:
> 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
> <mailto: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?
>
I agree there is an issue here, but I do not think it is particularly
serious. The majority of the logging that uses AbstractSessionLog will
occur before a session is available and most of the remaining logging
will occur through the session log. Having said that, I think it is
worth filing an issue for this problem both to make it visible to the
community and so we can address it at some point.
>
> 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?
>
I do not see any problem with changing the way logging is set up in
deploy so that the logging properties are set up before any other
logging could occur.
>
> Issues: see https://glassfish.dev.java.net/issues/show_bug.cgi?id=1107
> (which means that only PU properties currently work).
>
I took a look at this bug. Can you elaborate some more?
What is <property name="oracle.toplink.essentials" value="FINE"/>
expected to acheive? Where is it being set?
Thanks,
-Tom
>
> 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>
> >>>>><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>
> <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> <
> http://toplink.platform.class.name>"},
> >>>>>! {TopLinkProperties.TARGET_SERVER, "
> >>>>>toplink.server.platform.class.name
> <http://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>
> >>>>><mailto: 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
--
Tom Ware
Principal Software Engineer
Oracle Canada Inc.
Direct: (613) 783-4598
Email: tom.ware_at_oracle.com