Mailing-List: contact dev-help@openjpa.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@openjpa.apache.org
Message-ID: <1250196049.1236639590441.JavaMail.jira@brutus>
Date: Mon, 9 Mar 2009 15:59:50 -0700 (PDT)
From: "Pinaki Poddar (JIRA)" <jira@apache.org>
To: dev@openjpa.apache.org
Subject: [jira] Commented: (OPENJPA-891) JPA2 LockTypeMode Support
In-Reply-To: <762876655.1233678239877.JavaMail.jira@brutus>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


    [ 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.