Maor Lipchuk has posted comments on this change. Change subject: core: adding DAO support for unregistered VMs/Templates. ......................................................................
Patch Set 12: (12 comments) http://gerrit.ovirt.org/#/c/26480/12/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/OvfEntityData.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/OvfEntityData.java: Line 9: private static final long serialVersionUID = 3376648147702972152L; Line 10: Line 11: private Guid entityId; Line 12: private String entityName; Line 13: private String entityType; > Please use VmEntityType done Line 14: private int architecture; Line 15: private Version lowestCompVersion; Line 16: private Guid storageDomainId; Line 17: private String ovfData; Line 10: Line 11: private Guid entityId; Line 12: private String entityName; Line 13: private String entityType; Line 14: private int architecture; > The type should be ArchitectureType done Line 15: private Version lowestCompVersion; Line 16: private Guid storageDomainId; Line 17: private String ovfData; Line 18: private String ovfExtraData; http://gerrit.ovirt.org/#/c/26480/12/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/UnregisteredOVFDataDAO.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/UnregisteredOVFDataDAO.java: Line 37: String entityType, Line 38: int architecture, Line 39: String lowestCompVerson, Line 40: Guid storageDomainId, Line 41: String ovfExtraData); > This should be save(OvfEntityData), to sort-of align with the other DAOs. done Line 42: Line 43: /** Line 44: * Remove an entity from the unregistered table. Line 45: * http://gerrit.ovirt.org/#/c/26480/12/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/UnregisteredOVFDataDAODbFacadeImpl.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/UnregisteredOVFDataDAODbFacadeImpl.java: Line 62: OvfEntityData entity = new OvfEntityData(); Line 63: entity.setEntityId(getGuid(rs, "entity_guid")); Line 64: entity.setEntityName(rs.getString("entity_name")); Line 65: entity.setEntityType(rs.getString("entity_type")); Line 66: entity.setArchitecture(rs.getInt("architecture")); > ArchitectureType.forValue(rs.getInt("architecture") done Line 67: entity.setLowestCompVersion(new Version(rs.getString("lowest_comp_version"))); Line 68: entity.setStorageDomainId(getGuid(rs, "storage_domain_id")); Line 69: entity.setOvfData(rs.getString("ovf_data")); Line 70: entity.setOvfExtraData(rs.getString("ovf_extra_data")); http://gerrit.ovirt.org/#/c/26480/12/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/UnregisteredOVFDataDAOTest.java File backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/UnregisteredOVFDataDAOTest.java: Line 10: import org.ovirt.engine.core.common.businessentities.OvfEntityData; Line 11: Line 12: public class UnregisteredOVFDataDAOTest extends BaseDAOTestCase { Line 13: private UnregisteredOVFDataDAO dao; Line 14: > Missing: done Line 15: @Test Line 16: public void testGetWithEntityId() { Line 17: dao = dbFacade.getUnregisteredOVFDataDao(); Line 18: OvfEntityData ovfEntityData = Line 13: private UnregisteredOVFDataDAO dao; Line 14: Line 15: @Test Line 16: public void testGetWithEntityId() { Line 17: dao = dbFacade.getUnregisteredOVFDataDao(); > Should be done in the setup. Note this re-occurs in all your test methods done Line 18: OvfEntityData ovfEntityData = Line 19: dao.getByEntityIdAndStorageDomain(FixturesTool.VM_RHEL5_POOL_50, FixturesTool.STORAGE_DOAMIN_NFS2_1); Line 20: assertNotNull("VM should exists in the UnregisteredOVFData", ovfEntityData); Line 21: } Line 75: dao.insertOVFDataForEntities(FixturesTool.VM_TEMPLATE_RHEL5, Line 76: "Stam", Line 77: "TEMPLATE", Line 78: 1, Line 79: "3.4", > Please use Version.3_4.toString() done Line 80: FixturesTool.STORAGE_DOAMIN_NFS2_1, Line 81: ovfExtraData); Line 82: OvfEntityData ovfEntityData = Line 83: dao.getByEntityIdAndStorageDomain(FixturesTool.VM_TEMPLATE_RHEL5, FixturesTool.STORAGE_DOAMIN_NFS2_1); http://gerrit.ovirt.org/#/c/26480/12/packaging/dbscripts/unregistered_OVF_data_sp.sql File packaging/dbscripts/unregistered_OVF_data_sp.sql: Line 1: ---------------------------------------------------------------- Line 2: -- [unregistered_ovf_of_entities] Table Line 3: Line 4: Create or replace FUNCTION InsertOVFDataForEntities(v_entity_guid UUID, Line 5: v_entity_name character varying(255), > Standards: done Line 6: v_entity_type character varying(32), Line 7: v_architecture integer, Line 8: v_lowest_comp_version character varying(40), Line 9: v_storage_domain_id UUID, Line 10: v_ovf_extra_data text) Line 11: RETURNS VOID Line 12: AS $procedure$ Line 13: BEGIN Line 14: INSERT INTO unregistered_ovf_of_entities(entity_guid, entity_name, entity_type, architecture, lowest_comp_version, storage_domain_id, ovf_data, ovf_extra_data) > I'd remove ovf_data and its null treatment. done Line 15: VALUES(v_entity_guid, v_entity_name, v_entity_type, v_architecture, v_lowest_comp_version, v_storage_domain_id, null, v_ovf_extra_data); Line 16: UPDATE unregistered_ovf_of_entities SET ovf_data = (SELECT ovf_data FROM vm_ovf_generations vog WHERE vog.vm_guid = v_entity_guid) where entity_guid = v_entity_guid; Line 17: END; $procedure$ Line 18: LANGUAGE plpgsql; Line 12: AS $procedure$ Line 13: BEGIN Line 14: INSERT INTO unregistered_ovf_of_entities(entity_guid, entity_name, entity_type, architecture, lowest_comp_version, storage_domain_id, ovf_data, ovf_extra_data) Line 15: VALUES(v_entity_guid, v_entity_name, v_entity_type, v_architecture, v_lowest_comp_version, v_storage_domain_id, null, v_ovf_extra_data); Line 16: UPDATE unregistered_ovf_of_entities SET ovf_data = (SELECT ovf_data FROM vm_ovf_generations vog WHERE vog.vm_guid = v_entity_guid) where entity_guid = v_entity_guid; > Use the modern syntax: done Line 17: END; $procedure$ Line 18: LANGUAGE plpgsql; Line 19: Line 20: http://gerrit.ovirt.org/#/c/26480/12/packaging/dbscripts/upgrade/03_05_0410_unregistered_ovf_of_entities.sql File packaging/dbscripts/upgrade/03_05_0410_unregistered_ovf_of_entities.sql: Line 7: lowest_comp_version character varying(40), Line 8: storage_domain_id UUID, Line 9: ovf_data text, Line 10: ovf_extra_data text, Line 11: CONSTRAINT pk_entity_guid_storage_domain_unregistered PRIMARY KEY(entity_guid, storage_domain_id) > Standards: done Line 12: ); Line 13: Line 10: ovf_extra_data text, Line 11: CONSTRAINT pk_entity_guid_storage_domain_unregistered PRIMARY KEY(entity_guid, storage_domain_id) Line 12: ); Line 13: Line 14: ALTER TABLE unregistered_ovf_of_entities add constraint "fk_unregistered_ovf_of_entities_storage_domain" FOREIGN KEY (storage_domain_id) REFERENCES storage_domain_static(id) ON DELETE CASCADE; > Please don't use quotes for object names done -- To view, visit http://gerrit.ovirt.org/26480 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I385fac757f46131ae0c0048b6cf39b78f037e852 Gerrit-PatchSet: 12 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Eli Mesika <[email protected]> Gerrit-Reviewer: Liron Ar <[email protected]> Gerrit-Reviewer: Maor Lipchuk <[email protected]> Gerrit-Reviewer: Tal Nisan <[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
