Martin Mucha has uploaded a new change for review. Change subject: core: refactor&test for HostNetworkAttachmentsPersister ......................................................................
core: refactor&test for HostNetworkAttachmentsPersister Change-Id: Ib39c3678a29a7bd8728d32b55490f8500a77d9d6 Signed-off-by: Martin Mucha <[email protected]> --- M backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HostNetworkAttachmentsPersister.java M backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HostNetworkTopologyPersisterImpl.java A backend/manager/modules/vdsbroker/src/test/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HostNetworkAttachmentsPersisterTest.java 3 files changed, 74 insertions(+), 17 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/67/36067/1 diff --git a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HostNetworkAttachmentsPersister.java b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HostNetworkAttachmentsPersister.java index e47ee0d..dff3e2e 100644 --- a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HostNetworkAttachmentsPersister.java +++ b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HostNetworkAttachmentsPersister.java @@ -1,7 +1,7 @@ package org.ovirt.engine.core.vdsbroker.vdsbroker; -import java.util.ArrayList; import java.util.HashMap; +import java.util.Iterator; import java.util.List; import java.util.Map; @@ -18,12 +18,21 @@ private final NetworkAttachmentDao networkAttachmentDao; private final Guid hostId; private final List<NetworkAttachment> userNetworkAttachments; - private List<NetworkAttachment> dbNetworkAttachments; private final Map<String, VdsNetworkInterface> nicsByName; + + /** + * map networkID-->NIC for each network assigned to one of given nics. Value of mapping is such mapped NIC. + */ private final Map<Guid, VdsNetworkInterface> reportedNicsByNetworkId; + + /** + * map networkID-->Network for each network assigned to one of given nics. + */ private final Map<Guid, Network> reportedNetworksById; private final BusinessEntityMap<Network> clusterNetworks; - private final List<NetworkAttachment> configuredNetworkAttachments; +// private final List<NetworkAttachment> configuredNetworkAttachments; //TODO MM: remove commented-out. + + private List<NetworkAttachment> dbNetworkAttachments; public HostNetworkAttachmentsPersister(final NetworkAttachmentDao networkAttachmentDao, final Guid hostId, @@ -35,13 +44,17 @@ this.userNetworkAttachments = userNetworkAttachments; this.clusterNetworks = new BusinessEntityMap<>(clusterNetworks); nicsByName = Entities.entitiesByName(nics); - configuredNetworkAttachments = new ArrayList<>(); +// configuredNetworkAttachments = new ArrayList<>(); //TODO MM: remove commented-out. reportedNetworksById = new HashMap<>(); reportedNicsByNetworkId = new HashMap<>(); + initReportedNetworksAndNics(nics); + } + + private void initReportedNetworksAndNics(List<VdsNetworkInterface> nics) { for (VdsNetworkInterface nic : nics) { if (nic.getNetworkName() != null) { - Network network = this.clusterNetworks.get(nic.getNetworkName()); + Network network = clusterNetworks.get(nic.getNetworkName()); if (network != null) { reportedNetworksById.put(network.getId(), network); reportedNicsByNetworkId.put(network.getId(), nic); @@ -50,8 +63,10 @@ } } - public void persistNetworkAttchments() { - removeNetworkAttachments(); + public void persistNetworkAttachments() { + //TODO MM: remove commented-out. + //adds all valid attachments into given list. + /*configuredNetworkAttachments.addAll(*/removeInvalidNetworkAttachmentsFromDb()/*)*/; saveUserNetworkAttachments(); addNewNetworkAttachments(); } @@ -82,7 +97,7 @@ private boolean networkAttachmentExistsForNetwork(String networkName) { Network network = clusterNetworks.get(networkName); - for (NetworkAttachment attachment : configuredNetworkAttachments) { + for (NetworkAttachment attachment : /*configuredNetworkAttachments*/getDbNetworkAttachments()) {//TODO MM: remove commented-out. if (network.getId().equals(attachment.getNetworkId())) { return true; } @@ -96,34 +111,55 @@ */ private void saveUserNetworkAttachments() { Map<Guid, NetworkAttachment> dbAttachmentsById = Entities.businessEntitiesById(getDbNetworkAttachments()); + //TODO MM: what should happen if there are more attachments referencing same network? Should it insert multiple records to db? Or should it update to last version? Or fail? + //TODO MM: what if attachment has nicID already assigned? for (NetworkAttachment attachment : userNetworkAttachments) { - if (networkConfiguredOnHost(attachment.getNetworkId())) { + boolean userAttachmentReferencesNetworkBoundToNic = networkConfiguredOnHost(attachment.getNetworkId()); + if (userAttachmentReferencesNetworkBoundToNic) { VdsNetworkInterface nic = reportedNicsByNetworkId.get(attachment.getNetworkId()); attachment.setNicId(nic.getId()); - if (dbAttachmentsById.containsKey(attachment.getId())) { + boolean alreadyExistingAttachment = dbAttachmentsById.containsKey(attachment.getId()); + + if (alreadyExistingAttachment) { networkAttachmentDao.update(attachment); } else { attachment.setId(Guid.newGuid()); networkAttachmentDao.save(attachment); - configuredNetworkAttachments.add(attachment); + + //update those handy caches. + getDbNetworkAttachments().add(attachment); + dbAttachmentsById.put(attachment.getId(), attachment); + +// configuredNetworkAttachments.add(attachment);//TODO MM: remove commented-out. } } } } /** - * Removes attachments for networks which weren't reported from host + * • Removes {@link org.ovirt.engine.core.common.businessentities.network.NetworkAttachment NetworkAttachments} + * which network aren't associated with any of given {@link #nicsByName nics} + * + * @return valid NetworkAttachments; remaining ones after completion of this method. */ - private void removeNetworkAttachments() { - for (NetworkAttachment attachment : getDbNetworkAttachments()) { + private /*List<NetworkAttachment>*/void removeInvalidNetworkAttachmentsFromDb() {//TODO MM: commented-out. + for (Iterator<NetworkAttachment> iterator = getDbNetworkAttachments().iterator(); iterator.hasNext(); ) { + NetworkAttachment attachment = iterator.next(); if (!networkConfiguredOnHost(attachment.getNetworkId())) { networkAttachmentDao.remove(attachment.getId()); - } else { - configuredNetworkAttachments.add(attachment); + //also remove it from cached + iterator.remove(); } } + +// //all remaining in list are valid +// return new ArrayList<>(getDbNetworkAttachments()); //TODO MM: return value is probably not needed. remove commented-out. } + /** + * @param networkId network ID. + * @return true if given network ID describes network bound to any given NIC. + */ private boolean networkConfiguredOnHost(Guid networkId) { return reportedNetworksById.containsKey(networkId); } diff --git a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HostNetworkTopologyPersisterImpl.java b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HostNetworkTopologyPersisterImpl.java index 7542f68..3ba5d87 100644 --- a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HostNetworkTopologyPersisterImpl.java +++ b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HostNetworkTopologyPersisterImpl.java @@ -249,7 +249,7 @@ userConfiguredData.getNetworkAttachments(), clusterNetworks); - networkAttachmentPersister.persistNetworkAttchments(); + networkAttachmentPersister.persistNetworkAttachments(); } private String getVmNetworksImplementedAsBridgeless(VDS host, List<Network> clusterNetworks) { diff --git a/backend/manager/modules/vdsbroker/src/test/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HostNetworkAttachmentsPersisterTest.java b/backend/manager/modules/vdsbroker/src/test/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HostNetworkAttachmentsPersisterTest.java new file mode 100644 index 0000000..8fa8ed6 --- /dev/null +++ b/backend/manager/modules/vdsbroker/src/test/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HostNetworkAttachmentsPersisterTest.java @@ -0,0 +1,21 @@ +package org.ovirt.engine.core.vdsbroker.vdsbroker; + +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; +import org.ovirt.engine.core.dao.network.NetworkAttachmentDao; + +@RunWith(MockitoJUnitRunner.class) +public class HostNetworkAttachmentsPersisterTest { + + @Mock + private NetworkAttachmentDao networkAttachmentDao; + + + @Test + public void testPersistNetworkAttachments() throws Exception { + Assert.fail(); + } +} -- To view, visit http://gerrit.ovirt.org/36067 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ib39c3678a29a7bd8728d32b55490f8500a77d9d6 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Mucha <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
