Liran Zelkha has posted comments on this change. Change subject: core: WIP: Adding JPA to ovirt ......................................................................
Patch Set 15: (11 comments) http://gerrit.ovirt.org/#/c/22806/15/backend/manager/modules/common/pom.xml File backend/manager/modules/common/pom.xml: Line 55: <groupId>org.hibernate</groupId> Line 56: <artifactId>hibernate-entitymanager</artifactId> Line 57: <version>4.3.0.Final</version> Line 58: </dependency> Line 59: > please remove whitespace. Done Line 60: <dependency> Line 61: <groupId>javax.annotation</groupId> Line 62: <artifactId>javax.annotation-api</artifactId> Line 63: <version>1.2</version> Line 61: <groupId>javax.annotation</groupId> Line 62: <artifactId>javax.annotation-api</artifactId> Line 63: <version>1.2</version> Line 64: </dependency> Line 65: > No need for some jpa dependency? Based on persistence.xml (contains the JPA version) Line 66: </dependencies> Line 67: Line 68: <build> Line 69: <plugins> http://gerrit.ovirt.org/#/c/22806/15/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/Permissions.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/Permissions.java: Line 24: @Column(name = "object_id") Line 25: private Guid objectId; Line 26: @Transient Line 27: private String objectName; Line 28: @Transient > Why Transient on these ones? To simplify this patch. They come from joins on other tables. Line 29: private VdcObjectType objectType; Line 30: @Transient Line 31: private String roleName; Line 32: @Transient http://gerrit.ovirt.org/#/c/22806/15/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/Securityid.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/Securityid.java: Line 1: package org.ovirt.engine.core.common.businessentities; Line 2: Line 3: import java.io.Serializable; Line 4: Line 5: import javax.persistence.Column; > is this an embaddable class? We can use an internal class. You think it's better? Line 6: Line 7: import org.ovirt.engine.core.compat.Guid; Line 8: Line 9: public class Securityid implements Serializable { Line 5: import javax.persistence.Column; Line 6: Line 7: import org.ovirt.engine.core.compat.Guid; Line 8: Line 9: public class Securityid implements Serializable { > s/Securityid/SecurityId. Please elaborate on why you introduced this class? Have to have a class for it, as the VDSGroupUsers class, which depends on the user_vds_groups_permissions_view uses it for a composite primary key (only way to do it in JPA - have a separate class) Name was changed. Line 10: Line 11: private static final long serialVersionUID = 7893248654789260759L; Line 12: Line 13: @Column(name = "entity_id") http://gerrit.ovirt.org/#/c/22806/15/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VDSGroupUsers.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VDSGroupUsers.java: Line 11: @Table(name = "user_vds_groups_permissions_view") Line 12: public class VDSGroupUsers { Line 13: Line 14: @EmbeddedId Line 15: private Securityid id; > s/Securityid/SecurityId Done Line 16: Line 17: @Column(name = "entity_id", insertable = false, updatable = false) Line 18: private Guid entityId; Line 19: http://gerrit.ovirt.org/#/c/22806/15/backend/manager/modules/dal/pom.xml File backend/manager/modules/dal/pom.xml: Line 86: </dependency> Line 87: <dependency> Line 88: <groupId>javax.persistence</groupId> Line 89: <artifactId>persistence-api</artifactId> Line 90: <version>1.0.2</version> > if this is JPA, why not JPA 2? Done Line 91: </dependency> Line 92: </dependencies> Line 93: <build> Line 94: <filters> http://gerrit.ovirt.org/#/c/22806/15/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/HashMapMapper.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/HashMapMapper.java: Line 12: import org.hibernate.engine.spi.SessionImplementor; Line 13: import org.hibernate.usertype.UserType; Line 14: import org.ovirt.engine.core.utils.SerializationFactory; Line 15: Line 16: public class HashMapMapper implements UserType { > Why is this needed? We need to serialize the HashMap as JSON (See cluster_policies table) Line 17: Line 18: @Override Line 19: public Object assemble(Serializable cached, Object owner) throws HibernateException { Line 20: return cached; http://gerrit.ovirt.org/#/c/22806/15/backend/manager/modules/dal/src/main/resources/META-INF/persistence.xml File backend/manager/modules/dal/src/main/resources/META-INF/persistence.xml: Line 2: xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; Line 3: xsi:schemaLocation="http://java.sun.com/xml/ns/persistence http://java.sun.com/xml/ns/persistence/persistence_2_0.xsd"; Line 4: version="2.0"> Line 5: <persistence-unit name="ovirt"> Line 6: <provider>org.hibernate.jpa.HibernatePersistenceProvider</provider> > whitespace + identation issue? Done Line 7: <class>org.ovirt.engine.core.common.businessentities.VDSGroup</class> Line 8: <class>org.ovirt.engine.core.common.businessentities.StoragePool</class> Line 9: <class>org.ovirt.engine.core.common.businessentities.Permissions</class> Line 10: <class>org.ovirt.engine.core.common.scheduling.ClusterPolicy</class> http://gerrit.ovirt.org/#/c/22806/15/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/ClusterMapper.java File backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/ClusterMapper.java: Line 206: if (model.isSetPolicy()) { Line 207: SchedulingPolicyType policyType = SchedulingPolicyType.fromValue(model.getPolicy()); Line 208: if (policyType != null) { Line 209: ClusterPolicy clusterPolicy = new ClusterPolicy(); Line 210: clusterPolicy.setName(policyType.name().toLowerCase()); > Why is this needed? rebase issue? Done Line 211: clusterPolicy.setId(null); Line 212: entity.setClusterPolicy(clusterPolicy); Line 213: } Line 214: } http://gerrit.ovirt.org/#/c/22806/15/packaging/dbscripts/vds_groups_sp.sql File packaging/dbscripts/vds_groups_sp.sql: Line 63 Line 64 Line 65 Line 66 Line 67 > I would suggest a different approach here - That would be impossible to manage... -- To view, visit http://gerrit.ovirt.org/22806 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3cd0bbf9f0913955cb3e1facfa9a4bdc1f1ab24d Gerrit-PatchSet: 15 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Liran Zelkha <[email protected]> Gerrit-Reviewer: Eli Mesika <[email protected]> Gerrit-Reviewer: Gustavo Frederico Temple Pedrosa <[email protected]> Gerrit-Reviewer: Itamar Heim <[email protected]> Gerrit-Reviewer: Liran Zelkha <[email protected]> Gerrit-Reviewer: Mike Kolesnik <[email protected]> Gerrit-Reviewer: Tal Nisan <[email protected]> Gerrit-Reviewer: Yair Zaslavsky <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
