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.initializeExternalTransactionController(
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.getServerSession(
EntityManagerFactoryImpl.java:78)
at
oracle.toplink.essentials.internal.ejb.cmp3.base.EntityManagerFactoryImpl.createEntityManagerImpl(
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";
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
"},
! {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
"},
! {TopLinkProperties.TARGET_SERVER, "
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> 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
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>
> >>>
>