Moti Asayag has posted comments on this change.

Change subject: core: using JPA for engine backup awareness
......................................................................


Patch Set 1:

(4 comments)

https://gerrit.ovirt.org/#/c/40091/1/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 18: 
Line 19:     @Override
Line 20:     public EngineBackupLog getLastSuccessfulEngineBackup(String 
dbName) {
Line 21:         List<EngineBackupLog> logs = 
multipleResults(entityManager.createNamedQuery("EngineBackupLog.getLatest", 
EngineBackupLog.class).setParameter("dbName", dbName));
Line 22:         if (logs != null && logs.size() > 0) {
there is an assumption that logs cannot be null, so you can replace the entire 
if with:

  if (logs.isEmpty()) { 
    return null;
  } else {
    return logs.get(0);
  }
Line 23:             return logs.get(0);
Line 24:         } else {
Line 25:             return null;
Line 26:         }


https://gerrit.ovirt.org/#/c/40091/1/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 28:         super.setUp();
Line 29:         dao = dbFacade.getEngineBackupLogDao();
Line 30:         existingEngineBackupLog = 
dao.getLastSuccessfulEngineBackup(DB_NAME);
Line 31:         newEntity = new EngineBackupLog();
Line 32:         newEntity.setDbName("asd");
RandomUtils.nextString(20) instead ?
Line 33:         newEntity.setOutputMessage("asd");
Line 34:         newEntity.setDoneAt(new Date());
Line 35:         newEntity.setPassed(true);
Line 36:     }


Line 29:         dao = dbFacade.getEngineBackupLogDao();
Line 30:         existingEngineBackupLog = 
dao.getLastSuccessfulEngineBackup(DB_NAME);
Line 31:         newEntity = new EngineBackupLog();
Line 32:         newEntity.setDbName("asd");
Line 33:         newEntity.setOutputMessage("asd");
same ?
Line 34:         newEntity.setDoneAt(new Date());
Line 35:         newEntity.setPassed(true);
Line 36:     }
Line 37: 


Line 90:         return 1;
Line 91:     }
Line 92: 
Line 93:     @Override protected EngineBackupLog modifyEntity(EngineBackupLog 
entity) {
Line 94:         entity.setOutputMessage("test");
if "test" assumed to be verified by the next method, please extract it into a 
constant and use it in both places.
Line 95:         return entity;
Line 96:     }
Line 97: 
Line 98:     @Override protected void verifyEntityModification(EngineBackupLog 
result) {


-- 
To view, visit https://gerrit.ovirt.org/40091
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia0aa9c4d0c8c0a5f7b40a022d7670f30c5fd5b77
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Anonymous Coward #1000793
Gerrit-Reviewer: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Liran Zelkha <lzel...@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

Reply via email to