Liran Zelkha has posted comments on this change. Change subject: core: CDI - Replace EJBs with CDI Singletons ......................................................................
Patch Set 5: Code-Review+1 (5 comments) http://gerrit.ovirt.org/#/c/19887/5/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 253: /** Line 254: * just convenience so we don't refactor old code Line 255: */ Line 256: public static DbFacade getInstance() { Line 257: return instance; Throw an UnsupportedOperationException here. At least as a TODO: Line 258: } Line 259: Line 260: private CustomMapSqlParameterSource getCustomMapSqlParameterSource() { Line 261: return new CustomMapSqlParameterSource(dbEngineDialect); http://gerrit.ovirt.org/#/c/19887/5/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/TimeoutBase.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/TimeoutBase.java: Line 12: CacheManager protected? http://gerrit.ovirt.org/#/c/19887/5/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/BaseDAOTestCase.java File backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/BaseDAOTestCase.java: Line 62: dataSource = createDataSource(); Line 63: ejbRule.mockResource(ContainerManagedResourceType.DATA_SOURCE, dataSource); Line 64: Line 65: dataset = initDataSet(); Line 66: dbFacade = new DbFacade(); Really? Why not inject dbFacade? Line 67: dbFacade.setDbEngineDialect(DbFacadeLocator.loadDbEngineDialect()); Line 68: // load data from fixtures to DB Line 69: DatabaseOperation.CLEAN_INSERT.execute(getConnection(), dataset); Line 70: } http://gerrit.ovirt.org/#/c/19887/5/backend/manager/modules/scheduler/src/main/java/org/ovirt/engine/core/utils/timer/SchedulerUtilQuartzImpl.java File backend/manager/modules/scheduler/src/main/java/org/ovirt/engine/core/utils/timer/SchedulerUtilQuartzImpl.java: Line 44: Line 45: @Inject Line 46: private Log log; Line 47: private Scheduler sched; Line 48: private static volatile SchedulerUtilQuartzImpl instance; If the class is a singleton, why does this needs to be static? Line 49: Line 50: private final AtomicLong sequenceNumber = new AtomicLong(Long.MIN_VALUE); Line 51: Line 52: /** http://gerrit.ovirt.org/#/c/19887/5/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ejb/EJBUtilsStrategy.java File backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ejb/EJBUtilsStrategy.java: Line 33: protected EJBUtilsStrategy() { Line 34: // Adds JNDI resources, Line 35: addJNDIResources(); Line 36: // Adds JNDI beans - the implementation is in base classes Line 37: addJNDIBeans(); Can't this class be removed altogether? Line 38: Line 39: } Line 40: Line 41: protected abstract void addJNDIBeans(); -- To view, visit http://gerrit.ovirt.org/19887 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I126fa3f4240e81814ec9e902cb2e93ce364589ff Gerrit-PatchSet: 5 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Roy Golan <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Itamar Heim <[email protected]> Gerrit-Reviewer: Juan Hernandez <[email protected]> Gerrit-Reviewer: Liran Zelkha <[email protected]> Gerrit-Reviewer: Maor Lipchuk <[email protected]> Gerrit-Reviewer: Martin Peřina <[email protected]> Gerrit-Reviewer: Omer Frenkel <[email protected]> Gerrit-Reviewer: Ravi Nori <[email protected]> Gerrit-Reviewer: Roy Golan <[email protected]> Gerrit-Reviewer: Sergey Gotliv <[email protected]> Gerrit-Reviewer: Tal Nisan <[email protected]> Gerrit-Reviewer: Yair Zaslavsky <[email protected]> Gerrit-Reviewer: mooli tayer <[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
