Moti Asayag has posted comments on this change. Change subject: core: Add JPA Java infrastructure ......................................................................
Patch Set 57: (3 comments) https://gerrit.ovirt.org/#/c/33835/57/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/HibernateFacade.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/HibernateFacade.java: Line 21: extends BaseDAODbFacade > I wanted to keep old behaviour. If someone is casting the DAO to BaseDaoDbF I'm not familiar with any cases where the DAO is being cast to BaseDaoDbFacade. Access to concrete Dao should be done only via the DAO interface and its implementors. I'd also suggest to rename the name of this class to HibernateTemplate, since it is no longer acts as a Facade. Line 18: * @param <K> Line 19: * The entity key Line 20: */ Line 21: public abstract class HibernateFacade<T, K extends Serializable> extends BaseDAODbFacade { Line 22: @PersistenceContext > True. But this way trivial methods, like save, get, update, etc - are avail By looking at our existing DAOs, there is a variety between the implemented apis and the actual required api to be exposed. For instance we have ReadDao, ModificationDao and GenericDao which are commonly used, but there are a specific DAOs such as ActionGroupDAO which doesn't extend any of them. So if we'd like to be precise and not to expose all of the functionality to all of the DAO, even when not required - Yevgeny's approach seems legit to me. The inheritance tree could look like: Dao | | extends | SampleDao | | implements | ConcreteSampleDao --------> HibernateTemplate has Line 23: protected EntityManager em; Line 24: Line 25: public void save(T entity) { Line 26: em.merge(entity); Line 59: boolean detach > We need it as protected, as some subtypes might specifically want detached IMO javadoc for this class will be useful. -- To view, visit https://gerrit.ovirt.org/33835 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ide82bf8cc647426e37dc42a113867c52699c3f0b Gerrit-PatchSet: 57 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Liran Zelkha <lzel...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Liran Zelkha <lzel...@redhat.com> Gerrit-Reviewer: Moti Asayag <masa...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: Roy Golan <rgo...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <wallaroo1...@gmail.com> Gerrit-Reviewer: Yevgeny Zaspitsky <yzasp...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
