Martin Peřina has uploaded a new change for review. Change subject: core: VdsDAO.getAllWithName now returns instance ......................................................................
core: VdsDAO.getAllWithName now returns instance List<VDS> VdsDAO.getAllWithName(String) has been changed to VDS VdsDAO.get(String) because VDS name is unique Change-Id: I1a0cb37c98f1a4373028e6210e45261cd432eed3 Bug-Url: https://bugzilla.redhat.com/894382 Signed-off-by: Martin Perina <mper...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetVdsByNameQuery.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RegisterVdsQuery.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVdsCommand.java M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDAO.java M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDAODbFacadeImpl.java M backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VdsDAOTest.java 7 files changed, 44 insertions(+), 55 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/27/13827/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsCommand.java index 0c04662..154bb5c 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsCommand.java @@ -304,7 +304,7 @@ } else if (!ValidationUtils.validHostname(hostName)) { addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_INVALID_VDS_HOSTNAME); returnValue = false; - } else if (getVdsDAO().getAllWithName(vdsName).size() != 0) { + } else if (getVdsDAO().get(vdsName) != null) { addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_NAME_ALREADY_USED); returnValue = false; } else if (getVdsDAO().getAllForHostname(hostName).size() != 0) { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetVdsByNameQuery.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetVdsByNameQuery.java index acad10b..c7d0766 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetVdsByNameQuery.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetVdsByNameQuery.java @@ -1,7 +1,5 @@ package org.ovirt.engine.core.bll; -import java.util.List; - import org.ovirt.engine.core.common.businessentities.VDS; import org.ovirt.engine.core.common.queries.GetVdsByNameParameters; @@ -12,11 +10,9 @@ @Override protected void executeQueryCommand() { - List<VDS> vds = getDbFacade().getVdsDao().getAllWithName(getParameters().getName()); + VDS vds = getDbFacade().getVdsDao().get(getParameters().getName()); - if (vds.size() > 0) { - getQueryReturnValue().setReturnValue(vds.get(0)); - } + getQueryReturnValue().setReturnValue(vds); } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RegisterVdsQuery.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RegisterVdsQuery.java index c3a2edb..0b316fb 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RegisterVdsQuery.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RegisterVdsQuery.java @@ -435,50 +435,46 @@ log.debugFormat("Entering"); boolean returnValue = true; VdsDAO vdsDAO = DbFacade.getInstance().getVdsDao(); - List<VDS> hosts = vdsDAO.getAllWithName(getParameters().getVdsName()); + VDS storedHost = vdsDAO.get(getParameters().getVdsName()); List<String> allHostNames = getAllHostNames(vdsDAO.getAll()); boolean hostExistInDB = hostToRegister != null; - if (hosts.size() > 0) { + if (storedHost != null) { log.debugFormat( - "found {0} VDS(s) with the same name {1}. Will try to register with a new name", - hosts.size(), + "found VDS with the same name {0}. Will try to register with a new name", getParameters().getVdsName()); String nameToRegister = getParameters().getVdsName(); String uniqueIdToRegister = getParameters().getVdsUniqueId(); String newName; - for (VDS storedHost : hosts) { - // check different uniqueIds but same name - if (!uniqueIdToRegister.equals(storedHost.getUniqueId()) - && nameToRegister.equals(storedHost.getName())) { - if (hostExistInDB) { - // update the registered host if exist in db - allHostNames.remove(hostToRegister.getName()); - newName = generateUniqueName(nameToRegister, allHostNames); - hostToRegister.setVdsName(newName); - UpdateVdsActionParameters parameters = - new UpdateVdsActionParameters(hostToRegister.getStaticData(), "", false); - VdcReturnValueBase ret = Backend.getInstance().runInternalAction(VdcActionType.UpdateVds, parameters); - if (ret == null || !ret.getSucceeded()) { - error = AuditLogType.VDS_REGISTER_ERROR_UPDATING_NAME; - logable.addCustomValue("VdsName2", newName); - log.errorFormat("could not update VDS {0}", nameToRegister); - CaptureCommandErrorsToLogger(ret, "RegisterVdsQuery::HandleOldVdssWithSameName"); - return false; - } else { - log.infoFormat( - "Another VDS was using this name with IP {0}. Changed to {1}", - nameToRegister, - newName); - } + // check different uniqueIds but same name + if (!uniqueIdToRegister.equals(storedHost.getUniqueId()) + && nameToRegister.equals(storedHost.getName())) { + if (hostExistInDB) { + // update the registered host if exist in db + allHostNames.remove(hostToRegister.getName()); + newName = generateUniqueName(nameToRegister, allHostNames); + hostToRegister.setVdsName(newName); + UpdateVdsActionParameters parameters = + new UpdateVdsActionParameters(hostToRegister.getStaticData(), "", false); + VdcReturnValueBase ret = Backend.getInstance().runInternalAction(VdcActionType.UpdateVds, parameters); + if (ret == null || !ret.getSucceeded()) { + error = AuditLogType.VDS_REGISTER_ERROR_UPDATING_NAME; + logable.addCustomValue("VdsName2", newName); + log.errorFormat("could not update VDS {0}", nameToRegister); + CaptureCommandErrorsToLogger(ret, "RegisterVdsQuery::HandleOldVdssWithSameName"); + return false; } else { - // host doesn't exist in db yet. not persisting changes just object values. - newName = generateUniqueName(nameToRegister, allHostNames); - getParameters().setVdsName(newName); + log.infoFormat( + "Another VDS was using this name with IP {0}. Changed to {1}", + nameToRegister, + newName); } - break; + } else { + // host doesn't exist in db yet. not persisting changes just object values. + newName = generateUniqueName(nameToRegister, allHostNames); + getParameters().setVdsName(newName); } } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVdsCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVdsCommand.java index c6d247d..7755f2e 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVdsCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVdsCommand.java @@ -76,7 +76,7 @@ // check if a name is updated to an existing vds name else if (!StringUtils.equalsIgnoreCase(_oldVds.getName(), getParameters().getVdsStaticData() .getName()) - && getVdsDAO().getAllWithName(vdsName).size() != 0) { + && getVdsDAO().get(vdsName) != null) { addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_NAME_ALREADY_USED); } else if (!StringUtils.equalsIgnoreCase(_oldVds.getHostName(), getParameters().getVdsStaticData() .getHostName()) diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDAO.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDAO.java index e1cb3d2..1d77461 100644 --- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDAO.java +++ b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDAO.java @@ -37,13 +37,13 @@ VDS get(NGuid id, Guid userID, boolean isFiltered); /** - * Finds all instances with the given name. + * Finds an instance with the given name. * * @param name * the name - * @return the list of instances + * @return the found instance or {@code null} */ - List<VDS> getAllWithName(String name); + VDS get(String name); /** * Finds all instances for the given host. diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDAODbFacadeImpl.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDAODbFacadeImpl.java index c22891e..9c21921 100644 --- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDAODbFacadeImpl.java +++ b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDAODbFacadeImpl.java @@ -42,8 +42,8 @@ } @Override - public List<VDS> getAllWithName(String name) { - return getCallsHandler().executeReadList("GetVdsByName", + public VDS get(String name) { + return getCallsHandler().executeRead("GetVdsByName", VdsRowMapper.instance, getCustomMapSqlParameterSource() .addValue("vds_name", name)); diff --git a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VdsDAOTest.java b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VdsDAOTest.java index 29b4651..6d4366b 100644 --- a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VdsDAOTest.java +++ b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VdsDAOTest.java @@ -81,13 +81,13 @@ } /** - * Ensures that an empty collection is returned. + * Ensures that {@code null} is returned. */ @Test - public void testGetAllWithNameUsingInvalidName() { - List<VDS> result = dao.getAllWithName("farkle"); + public void testGetUsingInvalidName() { + VDS result = dao.get("farkle"); - assertIncorrectGetResult(result); + assertNull(result); } /** @@ -113,14 +113,11 @@ * Ensures the right set of objects are returned with the given name. */ @Test - public void testGetAllWithName() { - List<VDS> result = dao.getAllWithName(existingVds.getName()); + public void testGetWithName() { + VDS result = dao.get(existingVds.getName()); assertNotNull(result); - assertFalse(result.isEmpty()); - for (VDS vds : result) { - assertEquals(existingVds.getName(), vds.getName()); - } + assertEquals(existingVds.getName(), result.getName()); } /** -- To view, visit http://gerrit.ovirt.org/13827 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I1a0cb37c98f1a4373028e6210e45261cd432eed3 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Peřina <mper...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
