Liron Ar has posted comments on this change. Change subject: core: adding DAO support for unregistered VMs/Templates. ......................................................................
Patch Set 11: (11 comments) http://gerrit.ovirt.org/#/c/26480/11/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 8: Line 9: public class OvfEntityData extends IVdcQueryable implements Serializable { Line 10: private static final long serialVersionUID = 3376648147702972152L; Line 11: Line 12: private Guid vmId; see comment in which the table was added, we should have 3 members here - id, ovfData and StorageDomainId Line 13: private String vmName; Line 14: private String entityType; Line 15: private String originalTemplateName; Line 16: private int origin; http://gerrit.ovirt.org/#/c/26480/11/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 5: Line 6: import org.ovirt.engine.core.common.businessentities.OvfEntityData; Line 7: import org.ovirt.engine.core.compat.Guid; Line 8: Line 9: public interface UnregisteredOVFDataDAO extends DAO { please change to extends GenericDao<OvfEntityData, Guid>, methods name will change here accordingly Line 10: Line 11: /** Line 12: * Retrieves the entity with the given VM id. Line 13: * Line 14: * @param id Line 15: * The VM Id. Line 16: * @return The entity instance, or <code>null</code> if not found. Line 17: */ Line 18: public OvfEntityData getByVmId(Guid vmId); your key is the vm id and the storage domain id, which means that you might have multiple records with this vmId and you'll return random one which might be from "wrong" storage domain than you meant. Line 19: Line 20: /** Line 21: * Retrieves all the entities of type OvfEntityData related to the storage Domain Id. Line 22: * Line 25: * @param entityType Line 26: * The entity type (VM/Template) Line 27: * @return List of all the OvfEntityData related to the storage Domain Id, or an empty list if none is found. Line 28: */ Line 29: public List<OvfEntityData> getAllForStorageDomain(Guid storageDomainId, String entityType); /s/getAllForStorageDomain/getAllForStorageDomainByEntityType Line 30: Line 31: /** Line 32: * Insert new entity to the unregistered table. Line 33: */ Line 50: * Line 51: * @param vmId Line 52: * @param storageDomainId Line 53: */ Line 54: public void removeEntity(Guid vmId, Guid storageDomainId); /s/vmId/entityId http://gerrit.ovirt.org/#/c/26480/11/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 28: Line 29: @Test Line 30: public void testGetVMsForStorageDomain() { Line 31: dao = dbFacade.getUnregisteredOVFDataDao(); Line 32: List<OvfEntityData> ovfEntityDataList = dao.getAllForStorageDomain(FixturesTool.STORAGE_DOAMIN_NFS2_1, "VM"); please use the appropiate constant instead of "VM" Line 33: assertTrue("a VM should be fetched for the specified storage domain", !ovfEntityDataList.isEmpty()); Line 34: } Line 35: Line 36: @Test Line 36: @Test Line 37: public void testGetTempaltesForStorageDomain() { Line 38: dao = dbFacade.getUnregisteredOVFDataDao(); Line 39: List<OvfEntityData> ovfEntityDataList = Line 40: dao.getAllForStorageDomain(FixturesTool.STORAGE_DOAMIN_NFS2_1, "Template"); please use the appropiate constant instead of "Template" Line 41: assertTrue("a Tempalte should not be fetched for the specified storage domain", ovfEntityDataList.isEmpty()); Line 42: } Line 43: Line 44: @Test Line 58: Line 59: @Test Line 60: public void testInsertTemplateToUnregisteredEntity() { Line 61: dao = dbFacade.getUnregisteredOVFDataDao(); Line 62: final String OVF_EXTRA_DATA = "<ovf> Some extra OVF data </ovf>"; please change the name to java conventions, also please rename if to ovfData Line 63: dao.insertOVFDataForEntities(FixturesTool.VM_TEMPLATE_RHEL5, "Stam", "TEMPLATE", null, 1, 1, 1, 1, null, 1, "3.4", FixturesTool.STORAGE_DOAMIN_NFS2_1, OVF_EXTRA_DATA); Line 64: OvfEntityData ovfEntityData = dao.getByVmId(FixturesTool.VM_TEMPLATE_RHEL5); Line 65: assertNotNull(ovfEntityData); Line 66: assertTrue("The entity type should be template", ovfEntityData.getEntityType().equals("TEMPLATE")); http://gerrit.ovirt.org/#/c/26480/11/packaging/dbscripts/unregistered_OVF_data_sp.sql File packaging/dbscripts/unregistered_OVF_data_sp.sql: Line 27: Create or replace FUNCTION RemoveEntityFromUnregistered(v_vm_guid UUID, v_storage_domain_id UUID) Line 28: RETURNS VOID Line 29: AS $procedure$ Line 30: DECLARE Line 31: v_val UUID; unneeded Line 32: BEGIN Line 33: DELETE FROM unregistered_ovf_of_entities Line 34: WHERE vm_guid = v_vm_guid Line 35: AND storage_domain_id = v_storage_domain_id; Line 36: END; $procedure$ Line 37: LANGUAGE plpgsql; Line 38: Line 39: Line 40: Create or replace FUNCTION GetAllOVFEntitiesForStorageDomain(v_storage_domain_id UUID, v_entity_type VARCHAR(20)) /s/GetAllOVFEntitiesForStorageDomain/GetAllOVFEntitiesForStorageDomainByEntityType Line 41: RETURNS SETOF unregistered_ovf_of_entities STABLE Line 42: AS $procedure$ Line 43: BEGIN Line 44: RETURN QUERY SELECT * http://gerrit.ovirt.org/#/c/26480/11/packaging/dbscripts/upgrade/03_05_0410_unregistered_ovf_of_entities.sql File packaging/dbscripts/upgrade/03_05_0410_unregistered_ovf_of_entities.sql: Line 1: CREATE TABLE unregistered_ovf_of_entities Line 2: ( this table should contain three columns- entity_id, entity_type, ovf_data and storage_domain_id there's no reason to duplicate all the columns and to maintain them in multiple separate tables, remember to make each change to column twice (for example, when it's removed from vm_static table to remove it also from here), etc - also, that way you can't maintain consistency between the data in the columns and the data within the ovf - that table better have 3 columns only. Line 3: vm_guid UUID, Line 4: vm_name character varying(255) NOT NULL, Line 5: entity_type character varying(32) NOT NULL, Line 6: original_template_name character varying(255), -- 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: 11 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk <[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
