Moti Asayag has posted comments on this change. Change subject: core: [RFE]Backup Awareness - DAO ......................................................................
Patch Set 4: (13 comments) https://gerrit.ovirt.org/#/c/39525/4/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/DbFacade.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/DbFacade.java: Line 1282: public HostNicVfsConfigDao getHostNicVfsConfigDao() { Line 1283: return getDao(HostNicVfsConfigDao.class); Line 1284: } Line 1285: Line 1286: public EngineBackupLogDAO getEngineBackupLogDAO() { please maintain camelCase naming convention and replace getEngineBackupLogDAO with getEngineBackupLogDao Line 1287: return getDao(EngineBackupLogDAO.class); Line 1288: } https://gerrit.ovirt.org/#/c/39525/4/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/EngineBackupLogDAO.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/EngineBackupLogDAO.java: Line 1: package org.ovirt.engine.core.dao; Line 2: Line 3: import org.ovirt.engine.core.common.businessentities.EngineBackupLog; Line 4: Line 5: public interface EngineBackupLogDAO extends DAO { please maintain camelCase naming convention and replace EngineBackupLogDAO with EngineBackupLogDao Line 6: Line 7: /** Line 8: * Gets the last successful engine backup record Line 9: */ https://gerrit.ovirt.org/#/c/39525/4/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/EngineBackupLogDAODbFacadeImpl.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/EngineBackupLogDAODbFacadeImpl.java: Line 13: @Named Line 14: @Singleton Line 15: public class EngineBackupLogDAODbFacadeImpl extends BaseDAODbFacade implements EngineBackupLogDAO { Line 16: Line 17: private static class EngineBackupHistoryRowMapper implements RowMapper<EngineBackupLog> { s/EngineBackupHistoryRowMapper/EngineBackupLogRowMapper Line 18: public static final EngineBackupHistoryRowMapper INSTANCE = new EngineBackupHistoryRowMapper(); Line 19: Line 20: @Override Line 21: public EngineBackupLog mapRow(ResultSet rs, int rowNum) throws SQLException { Line 29: } Line 30: Line 31: @Override Line 32: public EngineBackupLog getLastSuccessfulEngineBackup(String dbName) { Line 33: return getCallsHandler().executeRead("GetLastSuccessfulEngineBackup", EngineBackupHistoryRowMapper.INSTANCE, getCustomMapSqlParameterSource().addValue("db_name", dbName)); please format the line, as it exceeds 120 chars. Line 34: } Line 35: Line 36: Line 37: @Override https://gerrit.ovirt.org/#/c/39525/4/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/AuditLogDAOTest.java File backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/AuditLogDAOTest.java: Line 399: assertEquals(2, getAlertCount(entry, dao.getAll(null, false))); Line 400: } Line 401: Line 402: public void testDeleteBackupRelatedAlerts() { Line 403: AuditLog entry = dao.getByOriginAndCustomEventId(ORIGIN, CUSTOM_BAKUP_EVENT_ID); i'd add first assertNotNull(entry); which is a proper assumption prior to validating the entry data. Line 404: assertFalse(entry.isDeleted()); Line 405: dao.deleteBackupRelatedAlerts(); Line 406: entry = dao.getByOriginAndCustomEventId(ORIGIN, CUSTOM_BAKUP_EVENT_ID); Line 407: assertTrue(entry.isDeleted()); Line 402: public void testDeleteBackupRelatedAlerts() { Line 403: AuditLog entry = dao.getByOriginAndCustomEventId(ORIGIN, CUSTOM_BAKUP_EVENT_ID); Line 404: assertFalse(entry.isDeleted()); Line 405: dao.deleteBackupRelatedAlerts(); Line 406: entry = dao.getByOriginAndCustomEventId(ORIGIN, CUSTOM_BAKUP_EVENT_ID); same Line 407: assertTrue(entry.isDeleted()); Line 408: } https://gerrit.ovirt.org/#/c/39525/4/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/EngineBackupLogDAOTest.java File backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/EngineBackupLogDAOTest.java: Line 10: import static org.junit.Assert.assertNotEquals; Line 11: import static org.junit.Assert.assertNotNull; Line 12: import static org.junit.Assert.assertNull; Line 13: Line 14: public class EngineBackupLogDAOTest extends BaseDAOTestCase { s/EngineBackupLogDAOTest/EngineBackupLogDaoTest Line 15: Line 16: private EngineBackupLogDAO dao; Line 17: private EngineBackupLog existingEngineBackupLog; Line 18: private final static String DB_NAME = "engine"; Line 15: Line 16: private EngineBackupLogDAO dao; Line 17: private EngineBackupLog existingEngineBackupLog; Line 18: private final static String DB_NAME = "engine"; Line 19: private final static String NON_EXISTING_DB_NAME = "kuku"; :) there is an option to user RandomUtils.nextString(10) to generate a string if wishes. Line 20: Line 21: @Override Line 22: @Before Line 23: public void setUp() throws Exception { Line 38: assertNull(existingEngineBackupLog); Line 39: } Line 40: Line 41: @Test Line 42: public void testAddingNewNotSuccessfulBackupEvent() { s/NotSuccessful/Unsuccessful Line 43: existingEngineBackupLog = dao.getLastSuccessfulEngineBackup(DB_NAME); Line 44: EngineBackupLog engineBackupLog = new EngineBackupLog(); Line 45: engineBackupLog.setDbName(DB_NAME); Line 46: engineBackupLog.setDoneAt(Calendar.getInstance().getTime()); Line 46: engineBackupLog.setDoneAt(Calendar.getInstance().getTime()); Line 47: engineBackupLog.setPassed(false); Line 48: engineBackupLog.setOutputMessage("backup failed"); Line 49: dao.save(engineBackupLog); Line 50: EngineBackupLog entry = dao.getLastSuccessfulEngineBackup(DB_NAME); please assertNotNull(entry); first Line 51: assertEquals(entry.getDoneAt(), existingEngineBackupLog.getDoneAt()); Line 52: } Line 53: Line 54: @Test Line 47: engineBackupLog.setPassed(false); Line 48: engineBackupLog.setOutputMessage("backup failed"); Line 49: dao.save(engineBackupLog); Line 50: EngineBackupLog entry = dao.getLastSuccessfulEngineBackup(DB_NAME); Line 51: assertEquals(entry.getDoneAt(), existingEngineBackupLog.getDoneAt()); it would be better to compare the entire object instead of a specific field. Line 52: } Line 53: Line 54: @Test Line 55: public void testAddingNewSuccessfulBackupEvent() { Line 59: engineBackupLog.setDoneAt(Calendar.getInstance().getTime()); Line 60: engineBackupLog.setPassed(true); Line 61: engineBackupLog.setOutputMessage("backup completed successfully"); Line 62: dao.save(engineBackupLog); Line 63: EngineBackupLog entry = dao.getLastSuccessfulEngineBackup(DB_NAME); please verify assertNotNull(entry); Line 64: assertNotEquals(entry.getDoneAt(), existingEngineBackupLog.getDoneAt()); Line 65: } Line 66: Line 60: engineBackupLog.setPassed(true); Line 61: engineBackupLog.setOutputMessage("backup completed successfully"); Line 62: dao.save(engineBackupLog); Line 63: EngineBackupLog entry = dao.getLastSuccessfulEngineBackup(DB_NAME); Line 64: assertNotEquals(entry.getDoneAt(), existingEngineBackupLog.getDoneAt()); you can verify also assertTrue(entry.isPassed()); to assure it is a successful backup event. Line 65: } Line 66: -- To view, visit https://gerrit.ovirt.org/39525 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9b016e050f732eecb7f2872991d9fa9d31d9c024 Gerrit-PatchSet: 4 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Martin Peřina <mper...@redhat.com> Gerrit-Reviewer: Moti Asayag <masa...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
