openjpa-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Pinaki Poddar (JIRA)" <j...@apache.org>
Subject [jira] Commented: (OPENJPA-891) JPA2 LockTypeMode Support
Date Mon, 09 Mar 2009 22:59:50 GMT

    [ https://issues.apache.org/jira/browse/OPENJPA-891?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12680318#action_12680318
] 

Pinaki Poddar commented on OPENJPA-891:
---------------------------------------

I have started looking into the changes in EntityManagerImpl.java related to this issue. Here
are few comments:(the first one is critical becuase it is architectural)


1. I see JDBCConfiguration appearing in EntityManagerImpl. This is not a welcome sign. Considerable
effort has gone in throughout this codebase to maintain architectural layerering so that EntityManager/Facade
does not know the nature of the Store. In fact, I thought  maven build prohibits package import
via dependencies to enforce such layering restriction.
   Anyway, the short point is: if EntityManagerImpl has to refer to JDBCConfiguration then
something else is amiss. It also violates one of the basic architectural principles.
   
2. Going by the code further, I think many of the new processing added to EntityManagerImpl
actually belongs somewhere else. Most probabaly in appropriate FetchConfiguration implementation.

3. The setFetchConfigProperty() calls in a loop
       configuration.getPropertyKeys()
    Please note that given this is a costly operation and in the returned value is not going
to change across the loop, it makes sense to compute it once before entering the loop. 

4. I also completely missed the point that why one will require to instantiate IntValue in
such a place. The purpose there looked like to populate an instance of FetchConfiguration
from the user supplied map and then push that onto the stack. Why will one require conf.IntValue
to do that is not clear to me at all.

5. FetchConfiguration seemed to be pushed in a loop.That will amount to multiple clones in
the stack -- that is *not* what is wanted -- the user properties should poulate one single
instance of FetchConfiguration and that should be pished into the stack. What will happen
by this code (perhaps, reading always can be faulty) is user property will land up in different
fetch config instances. 
  

> JPA2 LockTypeMode Support
> -------------------------
>
>                 Key: OPENJPA-891
>                 URL: https://issues.apache.org/jira/browse/OPENJPA-891
>             Project: OpenJPA
>          Issue Type: Sub-task
>          Components: jpa
>    Affects Versions: 2.0.0
>            Reporter: Albert Lee
>            Assignee: Albert Lee
>             Fix For: 2.0.0
>
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Mime
View raw message