Mike Kolesnik has posted comments on this change.
Change subject: core: Add QoS member to Network and VdsNetworkInterface
......................................................................
Patch Set 15:
(4 comments)
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/network/InterfaceDaoDbFacadeImpl.java
Line 123: .addValue("bridged", nic.isBridged())
Line 124: .addValue("labels",
SerializationFactory.getSerializer().serialize(nic.getLabels()));
Line 125: }
Line 126:
Line 127: private void persistQosChanges(VdsNetworkInterface entity) {
The usage of a satellite table is fine with me.
The details of how the QoS field from the VdsInterface entity is saved to the
DB, either by setting the fields directly in the table of the NIC or in a
different table is a concern of this DAO and shouldn't be exposed in the BLL.
Line 128: NetworkQoSDao qosDao = DbFacade.getInstance().getQosDao();
Line 129: Guid id = entity.getId();
Line 130: NetworkQoS oldQos = qosDao.get(id);
Line 131: NetworkQoS qos = entity.getQos();
Line 252: entity.setId(getGuidDefaultEmpty(rs, "id"));
Line 253:
entity.setBootProtocol(NetworkBootProtocol.forValue(rs.getInt("boot_protocol")));
Line 254: entity.setMtu(rs.getInt("mtu"));
Line 255: entity.setBridged(rs.getBoolean("bridged"));
Line 256:
entity.setQos(DbFacade.getInstance().getQosDao().get(getGuid(rs, "id")));
Since the id was already set, you can use entity.getId() here
Line 257:
entity.setLabels(SerializationFactory.getDeserializer().deserialize(rs.getString("labels"),
Line 258: HashSet.class));
Line 259: return entity;
Line 260: }
....................................................
File
backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/network/InterfaceDaoTest.java
Line 97: List<VdsNetworkInterface> result =
dao.getAllInterfacesForVds(VDS_ID);
Line 98: boolean found = false;
Line 99:
Line 100: for (VdsNetworkInterface iface : result) {
Line 101: found |= iface.getName()
You should also check that the QoS was saved.
Line 102: .equals(newVdsInterface.getName());
Line 103: }
Line 104:
Line 105: assertTrue(found);
Line 161: @Test
Line 162: public void testUpdateInterfaceForVds() {
Line 163: List<VdsNetworkInterface> before =
dao.getAllInterfacesForVds(VDS_ID);
Line 164: VdsNetworkInterface iface = before.get(0);
Line 165:
You should probably check update of QoS also.
Perhaps this merits two test methods since you have a couple of states: QoS
existed/didn't exist before the update
Line 166: iface.setName(iface.getName().toUpperCase());
Line 167:
Line 168: dao.updateInterfaceForVds(iface);
Line 169:
--
To view, visit http://gerrit.ovirt.org/22599
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I63776837d41c620258fa53b9a1335b76a5971cea
Gerrit-PatchSet: 15
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Lior Vernia <[email protected]>
Gerrit-Reviewer: Eli Mesika <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Lior Vernia <[email protected]>
Gerrit-Reviewer: Mike Kolesnik <[email protected]>
Gerrit-Reviewer: Moti Asayag <[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