Ravi Nori has posted comments on this change. Change subject: core : Add Entity API for CoCo commands ......................................................................
Patch Set 6: (11 comments) https://gerrit.ovirt.org/#/c/37463/6/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/CommandAssociatedEntity.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/CommandAssociatedEntity.java: Line 1: package org.ovirt.engine.core.common.businessentities; Line 2: Line 3: import org.ovirt.engine.core.common.VdcObjectType; Line 4: import org.ovirt.engine.core.common.utils.ObjectUtils; > please use java.utils.Objects instead. Done Line 5: import org.ovirt.engine.core.compat.Guid; Line 6: Line 7: public class CommandAssociatedEntity extends SubjectEntity { Line 8: Line 11: public CommandAssociatedEntity() { Line 12: } Line 13: Line 14: public CommandAssociatedEntity(Guid commandId, VdcObjectType entityType, Guid entityId) { Line 15: this.commandId = commandId; > this c'tor implies the entity is immutable. Please let have same assumption Done Line 16: setEntityType(entityType); Line 17: setEntityId(entityId); Line 18: } Line 19: Line 20: @Override Line 21: public int hashCode() { Line 22: final int prime = 31; Line 23: int result = super.hashCode(); Line 24: result = prime * result + ((commandId == null) ? 0 : commandId.hashCode()); > this can be simplified by using Objects.hashCode(commandId) Done Line 25: return result; Line 26: } Line 27: Line 28: @Override https://gerrit.ovirt.org/#/c/37463/6/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/SubjectEntity.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/SubjectEntity.java: Line 1: package org.ovirt.engine.core.common.businessentities; Line 2: Line 3: import org.ovirt.engine.core.common.VdcObjectType; Line 4: import org.ovirt.engine.core.common.utils.ObjectUtils; > please use java.utils.Objects instead. Done Line 5: import org.ovirt.engine.core.compat.Guid; Line 6: Line 7: public class SubjectEntity { Line 8: private Guid entityId; Line 28: public int hashCode() { Line 29: final int prime = 31; Line 30: int result = 1; Line 31: result = prime * result Line 32: + ((entityId == null) ? 0 : entityId.hashCode()); > as in previous file - consider using Objects.hashCode() Done Line 33: result = prime * result Line 34: + ((entityType == null) ? 0 : entityType.hashCode()); Line 35: return result; Line 36: } Line 42: } Line 43: if (obj == null) { Line 44: return false; Line 45: } Line 46: if (getClass() != obj.getClass()) { > this is a bit problematic, since if we compare SubjectEntity to CommandAsso Done Line 47: return false; Line 48: } Line 49: SubjectEntity other = (SubjectEntity) obj; Line 50: return ObjectUtils.objectsEqual(entityId, other.entityId) && https://gerrit.ovirt.org/#/c/37463/6/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/CommandEntityDaoDbFacadeImpl.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/CommandEntityDaoDbFacadeImpl.java: Line 203: return getCallsHandler().executeReadList("GetCommandIdsByEntityId", IdRowMapper.instance, parameterSource); Line 204: } Line 205: Line 206: public void insertCommandAssociatedEntities(Collection<CommandAssociatedEntity> cmdAssociatedEntities) { Line 207: getCallsHandler().executeStoredProcAsBatch("InsertCommandAssociatedEntities", cmdAssociatedEntities, cocoCmdEntityMapper); > please format (line too long) Done Line 208: } Line 209: Line 210: @Override Line 211: public List<CommandAssociatedEntity> getAllCommandAssociatedEntities(Guid cmdId) { https://gerrit.ovirt.org/#/c/37463/6/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/CommandEntityDaoTest.java File backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/CommandEntityDaoTest.java: Line 94: dao.insertCommandAssociatedEntities(cocoCmdEntities2); Line 95: Line 96: List<Guid> cmIds = dao.getCommandIdsByEntity(storageId); Line 97: assertNotNull(cmIds); Line 98: assertEquals(2, cmIds.size()); > can be replaced with: Done Line 99: assertTrue(cmIds.contains(cmdEntity1.getId())); Line 100: assertTrue(cmIds.contains(cmdEntity2.getId())); Line 101: } Line 102: Line 96: List<Guid> cmIds = dao.getCommandIdsByEntity(storageId); Line 97: assertNotNull(cmIds); Line 98: assertEquals(2, cmIds.size()); Line 99: assertTrue(cmIds.contains(cmdEntity1.getId())); Line 100: assertTrue(cmIds.contains(cmdEntity2.getId())); > please replace the two above with this assertion: Done Line 101: } Line 102: Line 103: @Test Line 104: public void testInsertAsyncTaskEntitities() { Line 100: assertTrue(cmIds.contains(cmdEntity2.getId())); Line 101: } Line 102: Line 103: @Test Line 104: public void testInsertAsyncTaskEntitities() { > seems that testGetAllCommandAssociatedEntities is a better match for the na Done Line 105: CommandEntity cmdEntity = generateNewEntity(); Line 106: dao.save(cmdEntity); Line 107: Set<CommandAssociatedEntity> cocoCmdEntities = new HashSet<>(); Line 108: cocoCmdEntities.add(new CommandAssociatedEntity(cmdEntity.getId(), VdcObjectType.Storage, Guid.newGuid())); Line 111: List<CommandAssociatedEntity> entities = dao.getAllCommandAssociatedEntities(cmdEntity.getId()); Line 112: assertNotNull(entities); Line 113: assertEquals(2, entities.size()); Line 114: for (CommandAssociatedEntity entity : cocoCmdEntities) { Line 115: assertTrue(entities.contains(entity)); > same as above. Done Line 116: } Line 117: Line 118: } -- To view, visit https://gerrit.ovirt.org/37463 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I96b934ffc44f87407df77c8e097164ca30f3a6e6 Gerrit-PatchSet: 6 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ravi Nori <rn...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Moti Asayag <masa...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: Ravi Nori <rn...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <wallaroo1...@gmail.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
