Martin Mucha has posted comments on this change. Change subject: core: Introduce NetworkAttachment entity ......................................................................
Patch Set 19: (3 comments) https://gerrit.ovirt.org/#/c/32411/19/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/network/NetworkAttachment.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/network/NetworkAttachment.java: Line 22: private Guid networkId; Line 23: Line 24: private Guid nicId; Line 25: Line 26: // TODO: Add validation for the name for nic or bond > So it seems we agree this TODO is redundant, please remove it (if you haven this todo removed. in upcomming patch there's bean validation added. Field is allowed to be null, but if set, it's required for create and update to conform to length limitation: <1, org.ovirt.engine.core.common.businessentities.BusinessEntitiesDefinitions#HOST_NIC_NAME_LENGTH> — ie. at least one char inclusive, at most ... inclusive. Done. Line 27: private String nicName; Line 28: Line 29: private IpConfiguration ipConfiguration; Line 30: private Map<String, String> properties; Line 107: return false; Line 108: if (getClass() != obj.getClass()) Line 109: return false; Line 110: NetworkAttachment other = (NetworkAttachment) obj; Line 111: if (id == null) { > equals/hashCode should refer to business key of the entity. thanks for response Yevgeny, reading it (not that others has wrong statements) I realized that properties are not part of business key. However there are 2 proposals, what to add to equals/hashcode, what should be compared. Please lets decide it here prior to altering code. And to make it easier I propose next one. Host_id can be calculated from nicId, thus I think from db point of view, "candidate key" (business key) is only networkId and nicId. Also note this. Consider having HashSet, with entity with certain business ID in it. We create new entity with same business id and different remaining data. When we add this new instance to the set, it will not be added into the set, thus 'updated' properties won't be actually updated. I'm not sure if there isn't code which would be really sad about this. Line 112: if (other.id != null) Line 113: return false; Line 114: } else if (!id.equals(other.id)) Line 115: return false; Line 141: return true; Line 142: } Line 143: Line 144: @Override Line 145: public String toString() { > recently mperina had introduced a new class named ToStringBuilder, resides Done Line 146: return "NetworkAttachment [id=" + id Line 147: + ", networkId=" + networkId Line 148: + ", nicId=" + nicId Line 149: + ", nicName=" + nicName -- To view, visit https://gerrit.ovirt.org/32411 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ided81dc2be68dc4c7a9d491697f887cdae477a2c Gerrit-PatchSet: 19 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Moti Asayag <[email protected]> Gerrit-Reviewer: Alona Kaplan <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Mucha <[email protected]> Gerrit-Reviewer: Moti Asayag <[email protected]> Gerrit-Reviewer: Yevgeny Zaspitsky <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
