Roy Golan has uploaded a new change for review. Change subject: core: make DbFacade a CID singleton ......................................................................
core: make DbFacade a CID singleton Change-Id: Ic9e57710ce8353af020cb5ae53e43b127276d6c6 Bug-Url: https://bugzilla.redhat.com/?????? Signed-off-by: Roy Golan <rgo...@redhat.com> --- M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/BackwardCompatibilityTaskCreationTest.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/SearchQueryTest.java M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/DbFacade.java M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/DbFacadeLocator.java M backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/DbFacadeDAOTest.java 5 files changed, 70 insertions(+), 129 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/14/34814/1 diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/BackwardCompatibilityTaskCreationTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/BackwardCompatibilityTaskCreationTest.java index c39f934..d4788d6 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/BackwardCompatibilityTaskCreationTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/BackwardCompatibilityTaskCreationTest.java @@ -1,5 +1,9 @@ package org.ovirt.engine.core.bll; +import org.junit.Before; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; import org.ovirt.engine.core.bll.tasks.AsyncTaskState; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; @@ -62,6 +66,7 @@ @RunWith(Theories.class) public class BackwardCompatibilityTaskCreationTest { + @Rule public final RandomUtilsSeedingRule rusr = new RandomUtilsSeedingRule(); @@ -77,19 +82,32 @@ @ClassRule public static MockEJBStrategyRule ejbRule = new MockEJBStrategyRule(BeanType.SCHEDULER, mock(SchedulerUtil.class)); - @BeforeClass - public static void beforeClass() { - ejbRule.mockResource(ContainerManagedResourceType.DATA_SOURCE, mock(Connection.class)); - DbFacade dbFacade = spy(new DbFacade()); - DbFacadeLocator.setDbFacade(dbFacade); + @Before + public void beforeClass() { + Dbs testClass = new Dbs(); + MockitoAnnotations.initMocks(testClass); AsyncTaskDAO asyncTaskDao = mock(AsyncTaskDAO.class); when(asyncTaskDao.getAll()).thenReturn(Collections.EMPTY_LIST); - when(dbFacade.getAsyncTaskDao()).thenReturn(asyncTaskDao); + when(testClass.getDbFacade().getAsyncTaskDao()).thenReturn(asyncTaskDao); CommandEntityDao cmdEntityDao = mock(CommandEntityDao.class); - when(dbFacade.getCommandEntityDao()).thenReturn(cmdEntityDao); + when(testClass.getDbFacade().getCommandEntityDao()).thenReturn(cmdEntityDao); when(cmdEntityDao.getAll()).thenReturn(Collections.EMPTY_LIST); } + class Dbs { + @Mock DbFacadeLocator dbFacadeLocator; + + @InjectMocks DbFacade dbFacade; + + public DbFacade getDbFacade() { + return dbFacade; + } + + Dbs() { + System.out.print("3"); + } + } + @SuppressWarnings({ "unchecked", "rawtypes"}) @DataPoints public static CommandBase<? extends VdcActionParametersBase>[] data() { diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/SearchQueryTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/SearchQueryTest.java index ec6b8dc..337a4ed 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/SearchQueryTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/SearchQueryTest.java @@ -39,7 +39,9 @@ import org.ovirt.engine.core.searchbackend.SearchObjects; import org.ovirt.engine.core.utils.MockConfigRule; -public class SearchQueryTest { +import javax.inject.Inject; + +public class SearchQueryTest extends CommonTestDependencies { @ClassRule public static MockConfigRule mcr = new MockConfigRule( @@ -57,6 +59,7 @@ List<StoragePool> storagePoolResultList = new ArrayList<StoragePool>(); List<GlusterVolumeEntity> glusterVolumeList = new ArrayList<GlusterVolumeEntity>(); List<NetworkView> networkResultList = new ArrayList<NetworkView>(); + @Inject private DbFacade facadeMock; private DbFacade mockDAO() { final DiskDao diskDao = Mockito.mock(DiskDao.class); @@ -68,52 +71,6 @@ final GlusterVolumeDao glusterVolumeDao = Mockito.mock(GlusterVolumeDao.class); final NetworkViewDao networkViewDao = Mockito.mock(NetworkViewDao.class); final DbEngineDialect dbEngineDialect = Mockito.mock(DbEngineDialect.class); - final DbFacade facadeMock = new DbFacade() { - @Override - public DiskDao getDiskDao() { - return diskDao; - } - - @Override - public VmDAO getVmDao() { - return vmDAO; - } - - @Override - public VdsDAO getVdsDao() { - return vdsDAO; - } - - @Override - public VdsGroupDAO getVdsGroupDao() { - return vdsGroupDAO; - } - - @Override - public StoragePoolDAO getStoragePoolDao() { - return storagePoolDAO; - } - - @Override - public DbEngineDialect getDbEngineDialect() { - return dbEngineDialect; - } - - @Override - public GlusterVolumeDao getGlusterVolumeDao() { - return glusterVolumeDao; - } - - @Override - public QuotaDAO getQuotaDao() { - return quotaDAO; - } - - @Override - public NetworkViewDao getNetworkViewDao() { - return networkViewDao; - } - }; // mock DAOs mockDiskSao(diskDao); diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/DbFacade.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/DbFacade.java index d288a14..cb4bab5 100644 --- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/DbFacade.java +++ b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/DbFacade.java @@ -159,6 +159,11 @@ import org.springframework.jdbc.core.namedparam.MapSqlParameterSource; import org.springframework.jdbc.core.simple.SimpleJdbcCall; +import javax.annotation.PostConstruct; +import javax.inject.Inject; +import javax.inject.Singleton; + +@Singleton public class DbFacade { @SuppressWarnings("unused") @@ -213,6 +218,9 @@ } }; + @Inject + private DbFacadeLocator dbFacadeLocator; + private static DbFacade instance; private JdbcTemplate jdbcTemplate; private DbEngineDialect dbEngineDialect; @@ -261,7 +269,17 @@ return dao; } - public DbFacade() { + private DbFacade() { + } + + /** + * A way to keep backward compatibility with static getInstance() usage pattern. + * TODO once the code is cleaned from getInstance we could remote it. + */ + @PostConstruct + void init() { + dbFacadeLocator.configure(this); + instance = this; } public void setTemplate(JdbcTemplate template) { @@ -273,7 +291,7 @@ * just convenience so we don't refactor old code */ public static DbFacade getInstance() { - return DbFacadeLocator.getDbFacade(); + return instance; } private CustomMapSqlParameterSource getCustomMapSqlParameterSource() { diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/DbFacadeLocator.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/DbFacadeLocator.java index d4610f8..b876eed 100644 --- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/DbFacadeLocator.java +++ b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/DbFacadeLocator.java @@ -3,12 +3,11 @@ import java.io.IOException; import java.util.Properties; +import javax.annotation.Resource; import javax.sql.DataSource; import org.ovirt.engine.core.utils.EngineLocalConfig; import org.ovirt.engine.core.utils.ResourceUtils; -import org.ovirt.engine.core.utils.ejb.ContainerManagedResourceType; -import org.ovirt.engine.core.utils.ejb.EjbUtils; import org.ovirt.engine.core.utils.log.Log; import org.ovirt.engine.core.utils.log.LogFactory; import org.springframework.jdbc.core.JdbcTemplate; @@ -34,6 +33,12 @@ // The facade singleton: private static volatile DbFacade dbFacade; + @Resource(mappedName = "java:/ENGINEDataSource") + private DataSource ds; + + private DbFacadeLocator() { + } + /** * Lazily create and configure the facade. * @@ -44,7 +49,7 @@ if (dbFacade == null) { synchronized(DbFacadeLocator.class) { if (dbFacade == null) { - dbFacade = createDbFacade(); +// dbFacade = createDbFacade(); } } } @@ -52,87 +57,26 @@ } /** - * Create and configure the facade. + * configure the dbFacade. * - * @return the reference to the facade if it was successfully created or + * @return the reference to the dbFacade if it was successfully created or * <code>null</code> if something failed */ - private static DbFacade createDbFacade() { + protected void configure(DbFacade dbFacade) { // Load the configuration: loadDbFacadeConfig(); - - // Locate the data source: - DataSource ds = locateDataSource(); - if (ds == null) { - return null; - } // Load the dialect: DbEngineDialect dialect = loadDbEngineDialect(); - // Create and configure the facade: - DbFacade facade = new DbFacade(); - facade.setOnStartConnectionTimeout(connectionTimeout); - facade.setConnectionCheckInterval(checkInterval); - facade.setDbEngineDialect(dialect); + // configure the dbFacade: + dbFacade.setOnStartConnectionTimeout(connectionTimeout); + dbFacade.setConnectionCheckInterval(checkInterval); + dbFacade.setDbEngineDialect(dialect); JdbcTemplate template = dialect.createJdbcTemplate(ds); SQLErrorCodeSQLExceptionTranslator tr = new CustomSQLErrorCodeSQLExceptionTranslator(ds); template.setExceptionTranslator(tr); - facade.setTemplate(template); - - // Return the new facade: - return facade; - } - - /** - * Locate the data source. It will try to locate the data source repeatedly - * till it succeeds or till it takes longer than the time out. - * - * @return the data source if it was located or <code>null</code> if it - * couldn't be located - */ - private static DataSource locateDataSource() { - // We don't wait forever for the data source, at most the value of - // the connection timeout parameter, so we need to remember when we - // started to wait: - long started = System.currentTimeMillis(); - - for (;;) { - // Do the lookup of the data source: - DataSource ds = EjbUtils.findResource(ContainerManagedResourceType.DATA_SOURCE); - if (ds != null) { - return ds; - } - - // Don't continue if we have already waited too long: - long now = System.currentTimeMillis(); - long waited = now - started; - if (waited > connectionTimeout) { - log.errorFormat( - "The data source can't be located after waiting for more " + - "than {0} seconds, giving up.", connectionTimeout / 1000 - ); - return null; - } - - // It failed, so tell the user that the lookup failed but that we - // will try again in a few seconds: - log.warnFormat( - "The data source can't be located after waiting for {0} " + - "seconds, will try again.", waited / 1000 - ); - - // Wait a bit before trying again: - try { - Thread.sleep(checkInterval); - } - catch (InterruptedException exception) { - log.warnFormat( - "Interrupted while waiting for data source, will try " + - " again.", exception - ); - } - } + dbFacade.setTemplate(template); } /** diff --git a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/DbFacadeDAOTest.java b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/DbFacadeDAOTest.java index 3fc6d7c..0fd1bbd 100644 --- a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/DbFacadeDAOTest.java +++ b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/DbFacadeDAOTest.java @@ -10,9 +10,12 @@ import java.io.InputStream; import java.util.Properties; +import javax.inject.Inject; import javax.sql.DataSource; +import org.jboss.arquillian.junit.Arquillian; import org.junit.Test; +import org.junit.runner.RunWith; import org.ovirt.engine.core.common.VdcObjectType; import org.ovirt.engine.core.common.businessentities.BaseDisk; import org.ovirt.engine.core.common.businessentities.Bookmark; @@ -45,6 +48,7 @@ import org.springframework.dao.DataAccessException; import org.springframework.jdbc.datasource.SingleConnectionDataSource; +@RunWith(Arquillian.class) public class DbFacadeDAOTest extends BaseDAOTestCase { // entity IDs for testing retrieving an entity by id and type @@ -70,6 +74,7 @@ private static final Guid STORAGE_POOL_WITH_MASTER_UP = new Guid("386BFFD1-E7ED-4B08-BCE9-D7DF10F8C9A0"); private static final Guid STORAGE_POOL_WITH_MASTER_DOWN = new Guid("72B9E200-F48B-4687-83F2-62828F249A47"); private static final Guid VM_STATIC_GUID = new Guid("77296e00-0cad-4e5a-9299-008a7b6f4354"); + @Inject private DbFacade localDbFacade; /** * Ensures that the checkDBConnection method returns true when the connection is up @@ -105,7 +110,6 @@ properties.getProperty("database.password"), true ); - DbFacade localDbFacade = new DbFacade(); localDbFacade.setDbEngineDialect(DbFacadeLocator.loadDbEngineDialect()); localDbFacade.setTemplate(localDbFacade.getDbEngineDialect().createJdbcTemplate(result)); localDbFacade.checkDBConnection(); -- To view, visit http://gerrit.ovirt.org/34814 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ic9e57710ce8353af020cb5ae53e43b127276d6c6 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Roy Golan <rgo...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
