Liran Zelkha has posted comments on this change.

Change subject: core: make DbFacade a CDI singleton
......................................................................


Patch Set 35:

(4 comments)

http://gerrit.ovirt.org/#/c/34814/35/backend/manager/modules/bll/pom.xml
File backend/manager/modules/bll/pom.xml:

Line 169:          <version>2.10.4</version>
Line 170:      </dependency>
Line 171:      <dependency>
Line 172:          <groupId>org.springframework.boot</groupId>
Line 173:          <artifactId>spring-boot-starter-test</artifactId>
> If that is for test only please specify the scope
Done
Line 174:          <version>1.1.4.RELEASE</version>
Line 175:      </dependency>
Line 176:      <dependency>
Line 177:          <groupId>javax.enterprise</groupId>


http://gerrit.ovirt.org/#/c/34814/35/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/CommonCodeTest.java
File 
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/CommonCodeTest.java:

> 1. In order to prevent a confusion could you rename that to *TestBase?
Done
Line 1: package org.ovirt.engine.core.bll;
Line 2: 
Line 3: import static org.mockito.Mockito.mock;
Line 4: 


Line 8: import org.ovirt.engine.core.dal.dbbroker.DbFacade;
Line 9: import org.springframework.context.annotation.Bean;
Line 10: 
Line 11: public class CommonCodeTest {
Line 12:     protected static DbFacade dbFacade;
> 1. Why is this protected? I'd rather make it private.
1. Done.
2. AFAIK each JUnit class run in its own JVM, no? In any case, I've added the 
@After method
Line 13: 
Line 14:     @BeforeClass
Line 15:     public static void initDbFacade() throws Exception {
Line 16:         dbFacade = mock(DbFacade.class);


http://gerrit.ovirt.org/#/c/34814/35/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 280:     private DbFacade() {
Line 281:     }
Line 282: 
Line 283:     /**
Line 284:      * A way to keep backward compatibility with static getInstance() 
usage pattern.
> How do you ensure that the getInstance isn't called statically before PostC
I don't. But since it works in the ovirt flows - if anyone tries doing it 
they'll get NPE.
Line 285:      * @TODO once the code is cleaned from getInstance we could 
remove it.
Line 286:      */
Line 287:     @PostConstruct
Line 288:     void init() {


-- 
To view, visit http://gerrit.ovirt.org/34814
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic9e57710ce8353af020cb5ae53e43b127276d6c6
Gerrit-PatchSet: 35
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Roy Golan <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Alona Kaplan <[email protected]>
Gerrit-Reviewer: Eli Mesika <[email protected]>
Gerrit-Reviewer: Liran Zelkha <[email protected]>
Gerrit-Reviewer: Moti Asayag <[email protected]>
Gerrit-Reviewer: Oved Ourfali <[email protected]>
Gerrit-Reviewer: Roy Golan <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: Yevgeny Zaspitsky <[email protected]>
Gerrit-Reviewer: [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

Reply via email to